- Revision
- 237446
- Author
- [email protected]
- Date
- 2018-10-25 22:06:24 -0700 (Thu, 25 Oct 2018)
Log Message
[PSON] Navigating cross-site with locked history but unlocked back/forward list fails to create a new BackForwardListItem
https://bugs.webkit.org/show_bug.cgi?id=190915
<rdar://problem/45059069>
Reviewed by Geoffrey Garen.
Source/WebCore:
* history/PageCache.cpp:
(WebCore::canCacheFrame):
Make sure we do not put into PageCache a page whose main frame is showing the initial empty document.
We usually do not try to put those into PageCache because we do not have a HistoryItem to save the
PageCache entry on. However, when we process-swap on a navigation with the history locked, the new
process has a HistoryItem and is initially showing the initial empty document before continuing
the load from the previous process. Note that saving the initial empty document in PageCache would
lead to crashes later on previous the initial empty document's Window is taken and reused for the
next load.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::load):
Stop assuming that we're continuing a client-side redirect when lockHistory is Yes. It is
lockBackForwardList that is actually Yes when we're doing a client-side redirect.
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
Stop using calling the completion handler with an invalid URL when the policy decision is 'Suspend' and
use 'about:blank' instead. Without this change, FrameLoader::continueLoadAfterNavigationPolicy() would
not load 'about:blank' when its AllowNavigationToInvalidURL parameter is No.
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (237445 => 237446)
--- trunk/Source/WebCore/ChangeLog 2018-10-26 04:51:59 UTC (rev 237445)
+++ trunk/Source/WebCore/ChangeLog 2018-10-26 05:06:24 UTC (rev 237446)
@@ -1,3 +1,32 @@
+2018-10-25 Chris Dumez <[email protected]>
+
+ [PSON] Navigating cross-site with locked history but unlocked back/forward list fails to create a new BackForwardListItem
+ https://bugs.webkit.org/show_bug.cgi?id=190915
+ <rdar://problem/45059069>
+
+ Reviewed by Geoffrey Garen.
+
+ * history/PageCache.cpp:
+ (WebCore::canCacheFrame):
+ Make sure we do not put into PageCache a page whose main frame is showing the initial empty document.
+ We usually do not try to put those into PageCache because we do not have a HistoryItem to save the
+ PageCache entry on. However, when we process-swap on a navigation with the history locked, the new
+ process has a HistoryItem and is initially showing the initial empty document before continuing
+ the load from the previous process. Note that saving the initial empty document in PageCache would
+ lead to crashes later on previous the initial empty document's Window is taken and reused for the
+ next load.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::load):
+ Stop assuming that we're continuing a client-side redirect when lockHistory is Yes. It is
+ lockBackForwardList that is actually Yes when we're doing a client-side redirect.
+
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+ Stop using calling the completion handler with an invalid URL when the policy decision is 'Suspend' and
+ use 'about:blank' instead. Without this change, FrameLoader::continueLoadAfterNavigationPolicy() would
+ not load 'about:blank' when its AllowNavigationToInvalidURL parameter is No.
+
2018-10-25 Devin Rousso <[email protected]>
Fix build after r237431 for platforms that don't support FULLSCREEN_API
Modified: trunk/Source/WebCore/history/PageCache.cpp (237445 => 237446)
--- trunk/Source/WebCore/history/PageCache.cpp 2018-10-26 04:51:59 UTC (rev 237445)
+++ trunk/Source/WebCore/history/PageCache.cpp 2018-10-26 05:06:24 UTC (rev 237446)
@@ -85,6 +85,11 @@
return false;
}
+ if (frame.isMainFrame() && frameLoader.stateMachine().isDisplayingInitialEmptyDocument()) {
+ PCLOG(" -MainFrame is displaying initial empty document");
+ return false;
+ }
+
DocumentLoader* documentLoader = frameLoader.documentLoader();
if (!documentLoader) {
PCLOG(" -There is no DocumentLoader object");
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (237445 => 237446)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-10-26 04:51:59 UTC (rev 237445)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-10-26 05:06:24 UTC (rev 237446)
@@ -1461,11 +1461,12 @@
addSameSiteInfoToRequestIfNeeded(loader->request());
applyShouldOpenExternalURLsPolicyToNewDocumentLoader(m_frame, loader, request);
- if (request.shouldTreatAsContinuingLoad() && request.lockHistory() == LockHistory::Yes) {
- // The load we're continuing is a client-side redirect so set things up accordingly.
+ if (request.shouldTreatAsContinuingLoad()) {
loader->setClientRedirectSourceForHistory(request.clientRedirectSourceForHistory());
- loader->setIsClientRedirect(true);
- m_loadType = FrameLoadType::RedirectWithLockedBackForwardList;
+ if (request.lockBackForwardList() == LockBackForwardList::Yes) {
+ loader->setIsClientRedirect(true);
+ m_loadType = FrameLoadType::RedirectWithLockedBackForwardList;
+ }
}
SetForScope<bool> currentLoadShouldBeTreatedAsContinuingLoadGuard(m_currentLoadShouldBeTreatedAsContinuingLoad, request.shouldTreatAsContinuingLoad());
Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (237445 => 237446)
--- trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-10-26 04:51:59 UTC (rev 237445)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-10-26 05:06:24 UTC (rev 237446)
@@ -179,7 +179,7 @@
case PolicyAction::Ignore:
return function({ }, nullptr, ShouldContinue::No);
case PolicyAction::Suspend:
- return function({ }, nullptr, ShouldContinue::ForSuspension);
+ return function({ blankURL() }, nullptr, ShouldContinue::ForSuspension);
case PolicyAction::Use:
if (!m_frame.loader().client().canHandleRequest(request)) {
handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(request));
Modified: trunk/Tools/ChangeLog (237445 => 237446)
--- trunk/Tools/ChangeLog 2018-10-26 04:51:59 UTC (rev 237445)
+++ trunk/Tools/ChangeLog 2018-10-26 05:06:24 UTC (rev 237446)
@@ -1,3 +1,15 @@
+2018-10-25 Chris Dumez <[email protected]>
+
+ [PSON] Navigating cross-site with locked history but unlocked back/forward list fails to create a new BackForwardListItem
+ https://bugs.webkit.org/show_bug.cgi?id=190915
+ <rdar://problem/45059069>
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2018-10-25 Joseph Pecoraro <[email protected]>
Web Inspector: Add a watchlist message rule to warn about feature checking new inspector protocol changes
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (237445 => 237446)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-10-26 04:51:59 UTC (rev 237445)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-10-26 05:06:24 UTC (rev 237446)
@@ -280,6 +280,38 @@
</body>
)PSONRESOURCE";
+static const char* navigationWithLockedHistoryBytes = R"PSONRESOURCE(
+<script>
+let shouldNavigate = true;
+window.addEventListener('pageshow', function(event) {
+ if (event.persisted) {
+ window.webkit.messageHandlers.pson.postMessage("Was persisted");
+ shouldNavigate = false;
+ }
+});
+
+_onload_ = function()
+{
+ if (!shouldNavigate)
+ return;
+
+ // JS navigation via window.location
+ setTimeout(() => {
+ location = "pson://www.apple.com/main.html";
+ }, 10);
+}
+</script>
+)PSONRESOURCE";
+
+static const char* pageCache1Bytes = R"PSONRESOURCE(
+<script>
+window.addEventListener('pageshow', function(event) {
+ if (event.persisted)
+ window.webkit.messageHandlers.pson.postMessage("Was persisted");
+});
+</script>
+)PSONRESOURCE";
+
#if PLATFORM(MAC)
static const char* windowOpenCrossSiteNoOpenerTestBytes = R"PSONRESOURCE(
@@ -1267,6 +1299,87 @@
runClientSideRedirectTest(ShouldEnablePSON::Yes);
}
+static void runNavigationWithLockedHistoryTest(ShouldEnablePSON shouldEnablePSON)
+{
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ processPoolConfiguration.get().processSwapsOnNavigation = shouldEnablePSON == ShouldEnablePSON::Yes ? YES : NO;
+ 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/main.html" toData:navigationWithLockedHistoryBytes];
+ [handler addMappingFromURLString:@"pson://www.apple.com/main.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/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto webkitPID = [webView _webProcessIdentifier];
+
+ // Page redirects to apple.com.
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto applePID = [webView _webProcessIdentifier];
+ if (shouldEnablePSON == ShouldEnablePSON::Yes)
+ EXPECT_NE(webkitPID, applePID);
+ else
+ EXPECT_EQ(webkitPID, applePID);
+
+ auto* backForwardList = [webView backForwardList];
+ EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [backForwardList.currentItem.URL absoluteString]);
+ EXPECT_TRUE(!backForwardList.forwardItem);
+ EXPECT_EQ(1U, backForwardList.backList.count);
+ EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backForwardList.backItem.URL absoluteString]);
+
+ receivedMessage = false;
+ [webView goBack];
+ TestWebKitAPI::Util::run(&receivedMessage); // Should be restored from PageCache.
+ receivedMessage = false;
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
+ EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backForwardList.currentItem.URL absoluteString]);
+ EXPECT_TRUE(!backForwardList.backItem);
+ EXPECT_EQ(1U, backForwardList.forwardList.count);
+ EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [backForwardList.forwardItem.URL absoluteString]);
+
+ [webView goForward];
+ TestWebKitAPI::Util::run(&done);
+ TestWebKitAPI::Util::run(&receivedMessage); // Should be restored from PageCache.
+ receivedMessage = false;
+ done = false;
+
+ EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
+
+ EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [backForwardList.currentItem.URL absoluteString]);
+ EXPECT_TRUE(!backForwardList.forwardItem);
+ EXPECT_EQ(1U, backForwardList.backList.count);
+ EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backForwardList.backItem.URL absoluteString]);
+}
+
+TEST(ProcessSwap, NavigationWithLockedHistoryWithPSON)
+{
+ runNavigationWithLockedHistoryTest(ShouldEnablePSON::Yes);
+}
+
+TEST(ProcessSwap, NavigationWithLockedHistoryWithoutPSON)
+{
+ runNavigationWithLockedHistoryTest(ShouldEnablePSON::No);
+}
+
static const char* sessionStorageTestBytes = R"PSONRESOURCE(
<head>
<script>
@@ -1448,15 +1561,6 @@
EXPECT_EQ(4u, [processPool _webProcessCountIgnoringPrewarmed]);
}
-static const char* pageCache1Bytes = R"PSONRESOURCE(
-<script>
-window.addEventListener('pageshow', function(event) {
- if (event.persisted)
- window.webkit.messageHandlers.pson.postMessage("Was persisted");
-});
-</script>
-)PSONRESOURCE";
-
TEST(ProcessSwap, PageCache1)
{
auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
