REGRESSION (PSON): Twitter link gets stuck at t.co after navigating b…

…ack in tab

https://bugs.webkit.org/show_bug.cgi?id=193932
<rdar://problem/47598947>

Reviewed by Brady Eidson.

Source/WebKit:

When doing a client side redirect from origin A to origin B, we would swap process and
create a SuspendedPageProxy and save it on the source BackForwardListItem. The issue is
that the BackForwardList is locked for such redirect so we end up updating the current
BackForwardListItem with origin B's URL while origin A's suspended page remained on
the item. When going to another URL in the same origin A, we would not create a suspended
page since no process-swap would occur. When pressing the back button, we would go back
to the previous BackForwardListItem and use its SuspendedPageProxy, which is for the
wrong URL (The pre-client redirect one).

To address the issue, we no longer create a SuspendedPageProxy for cross-site client side
redirects. There will be no way no go back to this suspended page anyway since the
back/forward list item will be updated with the redirection URL.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::suspendCurrentPageIfPossible):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Canonical link: https://commits.webkit.org/208458@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240660 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog

@@ -1,3 +1,27 @@
+2019-01-29  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (PSON): Twitter link gets stuck at t.co after navigating back in tab
+        https://bugs.webkit.org/show_bug.cgi?id=193932
+        <rdar://problem/47598947>
+
+        Reviewed by Brady Eidson.
+
+        When doing a client side redirect from origin A to origin B, we would swap process and
+        create a SuspendedPageProxy and save it on the source BackForwardListItem. The issue is
+        that the BackForwardList is locked for such redirect so we end up updating the current
+        BackForwardListItem with origin B's URL while origin A's suspended page remained on
+        the item. When going to another URL in the same origin A, we would not create a suspended
+        page since no process-swap would occur. When pressing the back button, we would go back
+        to the previous BackForwardListItem and use its SuspendedPageProxy, which is for the
+        wrong URL (The pre-client redirect one).
+
+        To address the issue, we no longer create a SuspendedPageProxy for cross-site client side
+        redirects. There will be no way no go back to this suspended page anyway since the
+        back/forward list item will be updated with the redirection URL.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::suspendCurrentPageIfPossible):
+
 2019-01-29  Chris Dumez  <cdumez@apple.com>
 
         Regression(r240046) VoiceOver is no longer working after a process swap

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -762,15 +762,20 @@ bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, Opt
     if (isPageOpenedByDOMShowingInitialEmptyDocument())
         return false;
 
-    auto* currentItem = navigation.fromItem();
-    if (!currentItem) {
+    auto* fromItem = navigation.fromItem();
+    if (!fromItem) {
         LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " unable to create suspended page for process pid %i - No current back/forward item", pageID(), m_process->processIdentifier());
         return false;
     }
 
-    auto suspendedPage = std::make_unique<SuspendedPageProxy>(*this, m_process.copyRef(), *currentItem, *mainFrameID);
+    // If the source and the destination back / forward list items are the same, then this is a client-side redirect. In this case,
+    // there is no need to suspend the previous page as there will be no way to get back to it.
+    if (fromItem == m_backForwardList->currentItem())
+        return false;
+
+    auto suspendedPage = std::make_unique<SuspendedPageProxy>(*this, m_process.copyRef(), *fromItem, *mainFrameID);
 
-    LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), suspendedPage->loggingString(), m_process->processIdentifier(), currentItem->itemID().logString());
+    LOG(ProcessSwapping, "WebPageProxy %" PRIu64 " created suspended page %s for process pid %i, back/forward item %s" PRIu64, pageID(), suspendedPage->loggingString(), m_process->processIdentifier(), fromItem->itemID().logString());
 
     m_process->processPool().addSuspendedPage(WTFMove(suspendedPage));
     return true;

Tools/ChangeLog

@@ -1,3 +1,15 @@
+2019-01-29  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (PSON): Twitter link gets stuck at t.co after navigating back in tab
+        https://bugs.webkit.org/show_bug.cgi?id=193932
+        <rdar://problem/47598947>
+
+        Reviewed by Brady Eidson.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2019-01-29  Zalan Bujtas  <zalan@apple.com>
 
         Adding new passing LFC tests.

Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -1846,11 +1846,86 @@ static void runClientSideRedirectTest(ShouldEnablePSON shouldEnablePSON)
     auto pid2 = [webView _webProcessIdentifier];
     EXPECT_NE(pid1, pid2);
 
-    EXPECT_EQ(2U, [processPool _webProcessCountIgnoringPrewarmed]);
+    EXPECT_EQ(1U, [processPool _webProcessCountIgnoringPrewarmed]);
     EXPECT_TRUE(willPerformClientRedirect);
     EXPECT_TRUE(didPerformClientRedirect);
 }
 
+TEST(ProcessSwap, NavigateBackAfterClientSideRedirect)
+{
+    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.webkit.org/main.html" toData:linkToCrossSiteClientSideRedirectBytes];
+    [handler addMappingFromURLString:@"pson://www.google.com/clientSideRedirect.html" toData:crossSiteClientSideRedirectBytes];
+    [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];
+
+    willPerformClientRedirect = false;
+    didPerformClientRedirect = false;
+
+    // Navigate to the page doing a client-side redirect to apple.com.
+    [webView evaluateJavaScript:@"testLink.click()" completionHandler:nil];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_WK_STREQ(@"pson://www.google.com/clientSideRedirect.html", [[webView URL] absoluteString]);
+    auto googlePID = [webView _webProcessIdentifier];
+    EXPECT_NE(webkitPID, googlePID);
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
+
+    auto applePID = [webView _webProcessIdentifier];
+    EXPECT_NE(webkitPID, applePID);
+    EXPECT_NE(webkitPID, googlePID);
+
+    EXPECT_TRUE(willPerformClientRedirect);
+    EXPECT_TRUE(didPerformClientRedirect);
+    EXPECT_WK_STREQ(@"pson://www.google.com/clientSideRedirect.html", [clientRedirectSourceURL absoluteString]);
+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [clientRedirectDestinationURL absoluteString]);
+
+    willPerformClientRedirect = false;
+    didPerformClientRedirect = false;
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main2.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
+
+    [webView goBack];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
+    EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
+
+    EXPECT_FALSE(willPerformClientRedirect);
+    EXPECT_FALSE(didPerformClientRedirect);
+}
+
 static void runNavigationWithLockedHistoryTest(ShouldEnablePSON shouldEnablePSON)
 {
     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);