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:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251049 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 629efb5..1eb4b3f 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2019-10-12 Chris Dumez <cdumez@apple.com>
+
+ 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 <cfleizach@apple.com>
AX: Make AXIsolatedTree compile again
diff --git a/Source/WebCore/history/CachedFrame.cpp b/Source/WebCore/history/CachedFrame.cpp
index df78c03..59a26b9 100644
--- a/Source/WebCore/history/CachedFrame.cpp
+++ b/Source/WebCore/history/CachedFrame.cpp
@@ -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.
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index 61a0aeb..fd84cb1 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -297,9 +297,7 @@
FrameLoader::~FrameLoader()
{
setOpener(nullptr);
-
- for (auto& frame : m_openedFrames)
- frame->loader().m_opener = nullptr;
+ detachFromAllOpenedFrames();
m_client.frameLoaderDestroyed();
@@ -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.
diff --git a/Source/WebCore/loader/FrameLoader.h b/Source/WebCore/loader/FrameLoader.h
index 671bad4..a3716d9 100644
--- a/Source/WebCore/loader/FrameLoader.h
+++ b/Source/WebCore/loader/FrameLoader.h
@@ -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();
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index 6a3688c..3c5546b 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,5 +1,27 @@
2019-10-12 Chris Dumez <cdumez@apple.com>
+ 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 <cdumez@apple.com>
+
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>
diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
index 01876c6..82e2c4f 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -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())
diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
index b790c3c..6ca57ce 100644
--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
@@ -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());
}
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 8f8d3b8..89924d5 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,15 @@
+2019-10-12 Chris Dumez <cdumez@apple.com>
+
+ 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 <rniwa@webkit.org>
requestIdleCallback cannot be enabled in DumpRenderTree on Windows
diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
index 5d68b8d..ee40378 100644
--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
@@ -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();
