Regression(PSON-r239182): Blank view when navigating back and forth between google...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 17:59:52 +0000 (17:59 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2019 17:59:52 +0000 (17:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193224
<rdar://problem/47097726>

Reviewed by Alex Christensen.

Source/WebCore:

Since r239182, pages get suspended in-place when we suspend the old process after a process-swap on navigation.
When we return to a suspended page, we load the current history item again and it normally properly restores
the page from PageCache, even though we load the same history item and the current one and even though the
page is suspended in-place (i.e. we did not navigate away, which is the usual case for page cache).

The issue is that if the page URL contains a fragment, FrameLoader::shouldPerformFragmentNavigation() would
return true because both the source and destination URLs (which are the same) contains a fragment. To address
the issue, update FrameLoader::shouldPerformFragmentNavigation() to return false if the current page is
suspended.

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::shouldPerformFragmentNavigation):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239726 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index 90ccc5b..daf30c9 100644 (file)
@@ -1,3 +1,24 @@
+2019-01-08  Chris Dumez  <cdumez@apple.com>
+
+        Regression(PSON-r239182): Blank view when navigating back and forth between google.com and stack overflow
+        https://bugs.webkit.org/show_bug.cgi?id=193224
+        <rdar://problem/47097726>
+
+        Reviewed by Alex Christensen.
+
+        Since r239182, pages get suspended in-place when we suspend the old process after a process-swap on navigation.
+        When we return to a suspended page, we load the current history item again and it normally properly restores
+        the page from PageCache, even though we load the same history item and the current one and even though the
+        page is suspended in-place (i.e. we did not navigate away, which is the usual case for page cache).
+
+        The issue is that if the page URL contains a fragment, FrameLoader::shouldPerformFragmentNavigation() would
+        return true because both the source and destination URLs (which are the same) contains a fragment. To address
+        the issue, update FrameLoader::shouldPerformFragmentNavigation() to return false if the current page is
+        suspended.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::shouldPerformFragmentNavigation):
+
 2019-01-08  Alex Christensen  <achristensen@webkit.org>
 
         Move Windows-specific code from NetworkStorageSessionCFNet.cpp to its own file
index 51bb948..4c26a69 100644 (file)
@@ -3134,6 +3134,7 @@ bool FrameLoader::shouldPerformFragmentNavigation(bool isFormSubmission, const S
     return (!isFormSubmission || equalLettersIgnoringASCIICase(httpMethod, "get"))
         && !isReload(loadType)
         && loadType != FrameLoadType::Same
+        && m_frame.document()->pageCacheState() != Document::InPageCache
         && !shouldReload(m_frame.document()->url(), url)
         // We don't want to just scroll if a link from within a
         // frameset is trying to reload the frameset into _top.
index ed3a4b4..530911c 100644 (file)
@@ -1,3 +1,15 @@
+2019-01-08  Chris Dumez  <cdumez@apple.com>
+
+        Regression(PSON-r239182): Blank view when navigating back and forth between google.com and stack overflow
+        https://bugs.webkit.org/show_bug.cgi?id=193224
+        <rdar://problem/47097726>
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2019-01-07  David Kilzer  <ddkilzer@apple.com>
 
         Leak of ScrollCompletionCallbackData (16 bytes) in com.apple.WebKit.WebContent running WebKit layout tests
index 5333097..f35d8ba 100644 (file)
@@ -685,6 +685,63 @@ TEST(ProcessSwap, Back)
         EXPECT_EQ(5u, seenPIDs.size());
 }
 
+static const char* pageWithFragmentTestBytes = R"PSONRESOURCE(
+<div id="foo">TEST</div>
+)PSONRESOURCE";
+
+TEST(ProcessSwap, HistoryNavigationToFragmentURL)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    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.apple.com/main.html#foo" toData:pageWithFragmentTestBytes];
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"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];
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html#foo"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto applePID = [webView _webProcessIdentifier];
+
+    [webView goBack];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
+
+    [webView goForward];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
+
+    bool finishedRunningScript = false;
+    [webView evaluateJavaScript:@"document.getElementById('foo').innerText" completionHandler: [&] (id result, NSError *error) {
+        NSString *innerText = (NSString *)result;
+        EXPECT_WK_STREQ(@"TEST", innerText);
+        finishedRunningScript = true;
+    }];
+    TestWebKitAPI::Util::run(&finishedRunningScript);
+}
+
 TEST(ProcessSwap, SuspendedPageDiesAfterBackForwardListItemIsGone)
 {
     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);