- Revision
- 251049
- Author
- [email protected]
- Date
- 2019-10-12 12:17:37 -0700 (Sat, 12 Oct 2019)
Log Message
Back/Forward cache does not work after doing a favorite navigation
https://bugs.webkit.org/show_bug.cgi?id=202762
<rdar://problem/56126911>
Reviewed by Antti Koivisto.
Source/WebCore:
When a subframe goes into the page cache, make sure we null out the opener
link of any windows that were opened by this frame. This matches what would
happened if this frame were closed.
Covered by the following API tests:
ProcessSwap.PageCacheAfterProcessSwapByClient
ProcessSwap.OpenerLinkAfterAPIControlledProcessSwappingOfOpener
* history/CachedFrame.cpp:
(WebCore::CachedFrame::CachedFrame):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::~FrameLoader):
(WebCore::FrameLoader::detachFromAllOpenedFrames):
* loader/FrameLoader.h:
Source/WebKit:
When a process-swap was forced by the client, we would always close the page in the previous
process after navigating. This would prevent leveraging the back/forward cache when navigating
back after such a process-swap. The reason we were doing this is that other pages may have
opener link to this page and closing the page was an easy way to break this opener link.
To address the issue, we no longer close the previous page when a process-swap is forced by the
client. Instead, we make sure to disconnect the frames' openees from their opener then the opener
enters the page cache.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::suspendCurrentPageIfPossible):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::suspendForProcessSwap):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (251048 => 251049)
--- trunk/Source/WebCore/ChangeLog 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Source/WebCore/ChangeLog 2019-10-12 19:17:37 UTC (rev 251049)
@@ -1,3 +1,26 @@
+2019-10-12 Chris Dumez <[email protected]>
+
+ Back/Forward cache does not work after doing a favorite navigation
+ https://bugs.webkit.org/show_bug.cgi?id=202762
+ <rdar://problem/56126911>
+
+ Reviewed by Antti Koivisto.
+
+ When a subframe goes into the page cache, make sure we null out the opener
+ link of any windows that were opened by this frame. This matches what would
+ happened if this frame were closed.
+
+ Covered by the following API tests:
+ ProcessSwap.PageCacheAfterProcessSwapByClient
+ ProcessSwap.OpenerLinkAfterAPIControlledProcessSwappingOfOpener
+
+ * history/CachedFrame.cpp:
+ (WebCore::CachedFrame::CachedFrame):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::~FrameLoader):
+ (WebCore::FrameLoader::detachFromAllOpenedFrames):
+ * loader/FrameLoader.h:
+
2019-10-12 Chris Fleizach <[email protected]>
AX: Make AXIsolatedTree compile again
Modified: trunk/Source/WebCore/history/CachedFrame.cpp (251048 => 251049)
--- trunk/Source/WebCore/history/CachedFrame.cpp 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Source/WebCore/history/CachedFrame.cpp 2019-10-12 19:17:37 UTC (rev 251049)
@@ -179,6 +179,10 @@
// 'DidFirstVisuallyNonEmptyLayout' callback gets called against when restoring from PageCache.
m_view->resetLayoutMilestones();
+ // The main frame is reused for the navigation and the opener link to its should thus persist.
+ if (!frame.isMainFrame())
+ frame.loader().detachFromAllOpenedFrames();
+
frame.loader().client().savePlatformDataToCachedFrame(this);
// documentWillSuspendForPageCache() can set up a layout timer on the FrameView, so clear timers after that.
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (251048 => 251049)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2019-10-12 19:17:37 UTC (rev 251049)
@@ -297,10 +297,8 @@
FrameLoader::~FrameLoader()
{
setOpener(nullptr);
+ detachFromAllOpenedFrames();
- for (auto& frame : m_openedFrames)
- frame->loader().m_opener = nullptr;
-
m_client.frameLoaderDestroyed();
if (m_networkingContext)
@@ -307,6 +305,13 @@
m_networkingContext->invalidate();
}
+void FrameLoader::detachFromAllOpenedFrames()
+{
+ for (auto& frame : m_openedFrames)
+ frame->loader().m_opener = nullptr;
+ m_openedFrames.clear();
+}
+
void FrameLoader::init()
{
// This somewhat odd set of steps gives the frame an initial empty document.
Modified: trunk/Source/WebCore/loader/FrameLoader.h (251048 => 251049)
--- trunk/Source/WebCore/loader/FrameLoader.h 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Source/WebCore/loader/FrameLoader.h 2019-10-12 19:17:37 UTC (rev 251049)
@@ -253,6 +253,7 @@
WEBCORE_EXPORT Frame* opener();
WEBCORE_EXPORT void setOpener(Frame*);
+ WEBCORE_EXPORT void detachFromAllOpenedFrames();
bool hasOpenedFrames() const { return !m_openedFrames.isEmpty(); }
void resetMultipleFormSubmissionProtection();
Modified: trunk/Source/WebKit/ChangeLog (251048 => 251049)
--- trunk/Source/WebKit/ChangeLog 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Source/WebKit/ChangeLog 2019-10-12 19:17:37 UTC (rev 251049)
@@ -1,5 +1,27 @@
2019-10-12 Chris Dumez <[email protected]>
+ Back/Forward cache does not work after doing a favorite navigation
+ https://bugs.webkit.org/show_bug.cgi?id=202762
+ <rdar://problem/56126911>
+
+ Reviewed by Antti Koivisto.
+
+ When a process-swap was forced by the client, we would always close the page in the previous
+ process after navigating. This would prevent leveraging the back/forward cache when navigating
+ back after such a process-swap. The reason we were doing this is that other pages may have
+ opener link to this page and closing the page was an easy way to break this opener link.
+
+ To address the issue, we no longer close the previous page when a process-swap is forced by the
+ client. Instead, we make sure to disconnect the frames' openees from their opener then the opener
+ enters the page cache.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::suspendCurrentPageIfPossible):
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::suspendForProcessSwap):
+
+2019-10-12 Chris Dumez <[email protected]>
+
Clearing Website data for a given session should not shut down cached processes for other sessions
https://bugs.webkit.org/show_bug.cgi?id=202865
<rdar://problem/56202912>
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (251048 => 251049)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-10-12 19:17:37 UTC (rev 251049)
@@ -796,11 +796,6 @@
LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, identifier().toUInt64(), suspendedPage->loggingString(), m_process->processIdentifier(), fromItem ? fromItem->itemID().logString() : 0);
- // If the client forced a swap then it may not be web-compatible to keep the previous page because other windows may have an opener link to it. We thus close it as soon as we
- // can do so without flashing.
- if (processSwapRequestedByClient == ProcessSwapRequestedByClient::Yes)
- suspendedPage->closeWithoutFlashing();
-
m_lastSuspendedPage = makeWeakPtr(*suspendedPage);
if (fromItem && shouldUseBackForwardCache())
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (251048 => 251049)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-10-12 19:17:37 UTC (rev 251049)
@@ -1491,6 +1491,12 @@
return;
}
+ // Page cache does not break the opener link for the main frame (only does so for the subframes) because the
+ // main frame is normally re-used for the navigation. However, in the case of process-swapping, the main frame
+ // is now hosted in another process and the one in this process is in the cache.
+ if (m_mainFrame && m_mainFrame->coreFrame())
+ m_mainFrame->coreFrame()->loader().detachFromAllOpenedFrames();
+
send(Messages::WebPageProxy::DidSuspendAfterProcessSwap());
}
Modified: trunk/Tools/ChangeLog (251048 => 251049)
--- trunk/Tools/ChangeLog 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Tools/ChangeLog 2019-10-12 19:17:37 UTC (rev 251049)
@@ -1,3 +1,15 @@
+2019-10-12 Chris Dumez <[email protected]>
+
+ Back/Forward cache does not work after doing a favorite navigation
+ https://bugs.webkit.org/show_bug.cgi?id=202762
+ <rdar://problem/56126911>
+
+ Reviewed by Antti Koivisto.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2019-10-12 Ryosuke Niwa <[email protected]>
requestIdleCallback cannot be enabled in DumpRenderTree on Windows
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (251048 => 251049)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-10-12 16:17:52 UTC (rev 251048)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-10-12 19:17:37 UTC (rev 251049)
@@ -3224,6 +3224,82 @@
EXPECT_EQ(2u, seenPIDs.size());
}
+TEST(ProcessSwap, PageCacheAfterProcessSwapByClient)
+{
+ auto processPoolConfiguration = psonProcessPoolConfiguration();
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webViewConfiguration setProcessPool:processPool.get()];
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [handler addMappingFromURLString:@"pson://www.webkit.org/main1.html" toData:pageCache1Bytes];
+ [handler addMappingFromURLString:@"pson://www.webkit.org/main2.html" toData:pageCache1Bytes];
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ auto messageHandler = adoptNS([[PSONMessageHandler alloc] init]);
+ [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"pson"];
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
+
+ [webView loadRequest:request];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pidAfterLoad1 = [webView _webProcessIdentifier];
+
+ EXPECT_EQ(1u, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
+
+ // We force a proces-swap via client API.
+ delegate->decidePolicyForNavigationAction = ^(WKNavigationAction *, void (^decisionHandler)(WKNavigationActionPolicy)) {
+ decisionHandler(_WKNavigationActionPolicyAllowInNewProcess);
+ };
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
+
+ [webView loadRequest:request];
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pidAfterLoad2 = [webView _webProcessIdentifier];
+
+ EXPECT_EQ(2u, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
+ EXPECT_NE(pidAfterLoad1, pidAfterLoad2);
+
+ delegate->decidePolicyForNavigationAction = nil;
+
+ [webView goBack];
+ TestWebKitAPI::Util::run(&receivedMessage);
+ receivedMessage = false;
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pidAfterLoad3 = [webView _webProcessIdentifier];
+
+ EXPECT_EQ(2u, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
+ EXPECT_EQ(pidAfterLoad1, pidAfterLoad3);
+ EXPECT_EQ(1u, [receivedMessages count]);
+ EXPECT_TRUE([receivedMessages.get()[0] isEqualToString:@"Was persisted" ]);
+ EXPECT_EQ(2u, seenPIDs.size());
+
+ [webView goForward];
+ TestWebKitAPI::Util::run(&receivedMessage);
+ receivedMessage = false;
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pidAfterLoad4 = [webView _webProcessIdentifier];
+
+ EXPECT_EQ(2u, [processPool _webProcessCountIgnoringPrewarmedAndCached]);
+ EXPECT_EQ(pidAfterLoad2, pidAfterLoad4);
+ EXPECT_EQ(2u, [receivedMessages count]);
+ EXPECT_TRUE([receivedMessages.get()[1] isEqualToString:@"Was persisted" ]);
+ EXPECT_EQ(2u, seenPIDs.size());
+}
+
TEST(ProcessSwap, PageCacheWhenNavigatingFromJS)
{
auto processPoolConfiguration = psonProcessPoolConfiguration();
