Changeset 237386 in webkit

Timestamp:

Oct 24, 2018 9:39:19 AM (5 years ago)

Author:

Chris Dumez

Message:

[PSON] When navigating back and forth, 'about:blank' shows up in the back/forward list
​https://bugs.webkit.org/show_bug.cgi?id=190846
<rdar://problem/45058938>

Reviewed by Antti Koivisto.

Source/WebCore:

When a page gets suspended after a process-swap, we navigate it to about:blank from inside the navigation
policy handler, by overriding the request URL. This normally works fine because we usually process-swap
on standard navigation. However, when we would process-swap on a back/forward navigation, we would end
up using the back/forward navigation load type to do the about:blank load. This would have repercussions
because history navigations update the current history item with the new URL (in this case 'about:blank').
To avoid the issue, switch to a standard load type whenever the client asks us to suspend and we load
'about:blank' as a result.

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::continueLoadAfterNavigationPolicy):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/FrameLoader.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-10-24  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] When navigating back and forth, 'about:blank' shows up in the back/forward list
+        https://bugs.webkit.org/show_bug.cgi?id=190846
+        <rdar://problem/45058938>
+
+        Reviewed by Antti Koivisto.
+
+        When a page gets suspended after a process-swap, we navigate it to about:blank from inside the navigation
+        policy handler, by overriding the request URL. This normally works fine because we usually process-swap
+        on standard navigation. However, when we would process-swap on a back/forward navigation, we would end
+        up using the back/forward navigation load type to do the about:blank load. This would have repercussions
+        because history navigations update the current history item with the new URL (in this case 'about:blank').
+        To avoid the issue, switch to a standard load type whenever the client asks us to suspend and we load
+        'about:blank' as a result.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+
 2018-10-24  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -3364 +3364 @@
 
         // We handle suspension by navigating forward to about:blank, which leaves us setup to navigate back to resume.
-        if (shouldContinue == ShouldContinue::ForSuspension)
+        if (shouldContinue == ShouldContinue::ForSuspension) {
+            // Make sure we do a standard load to about:blank instead of reusing whatever the load type was on process swap.
+            // For example, using a Back/Forward load to load about blank would cause the current HistoryItem's url to be
+            // overwritten with 'about:blank', which we do not want.
+            m_loadType = FrameLoadType::Standard;
             m_provisionalDocumentLoader->willContinueMainResourceLoadAfterRedirect({ blankURL() });
+        }
 
         m_provisionalDocumentLoader->startLoadingMainResource(shouldContinue);

trunk/Tools/ChangeLog

@@ +1 @@
+2018-10-24  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] When navigating back and forth, 'about:blank' shows up in the back/forward list
+        https://bugs.webkit.org/show_bug.cgi?id=190846
+        <rdar://problem/45058938>
+
+        Reviewed by Antti Koivisto.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-10-24  Claudio Saavedra  <csaavedra@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -1253 +1253 @@
     EXPECT_EQ(1U, backForwardList.backList.count);
 
-    // FIXME: uncomment the following once <rdar://problem/45058938> has been fixed. When enabling PSON
-    // the backItem's URL currently becomes about:blank.
-    // backItem = backForwardList.backItem;
-    // EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backItem.URL absoluteString]);
-    // EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backItem.initialURL absoluteString]);
+    backItem = backForwardList.backItem;
+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backItem.URL absoluteString]);
+    EXPECT_WK_STREQ(@"pson://www.webkit.org/main.html", [backItem.initialURL absoluteString]);
 }
 
@@ -2283 +2281 @@
 }
 
+TEST(ProcessSwap, NavigateBackAndForth)
+{
+    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]);
+    [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;
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    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]);
+
+    [webView goBack];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    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);
+    done = false;
+
+    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]);
+}
+
 #if PLATFORM(MAC)