Attachment 349576 Details for Bug 189557 – Patch

[patch] Patch

bug-189557-20180912131738.patch (text/plain), 6.10 KB, created by Chris Dumez on 2018-09-12 13:17:38 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-09-12 13:17:38 PDT

Size: 6.10 KB

patch

obsolete

>Subversion Revision: 235900
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 7889be290b7c6f30f796ed3e4b3825193bf2c2a6..43b47831af16188150054dc109aa4c061c5107d4 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-09-12  Chris Dumez  <cdumez@apple.com>
>+
>+        PSON: No process swap on back navigation after URL bar navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=189557
>+        <rdar://problem/44353108>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Our logic in WebProcessPool::processForNavigationInternal() was wrongly using
>+        WebBackForwardList::currentItem() as source item of the navigation, instead of
>+        using Navigation::fromItem(). In case of back navigation, by the time
>+        processForNavigation() is called, the WebBackForwardList's currentItem has already
>+        been updated to be the target item, via a Sync IPC from the WebProcess. As a result,
>+        the source and target items would be the same in the following check:
>+        ` if (currentItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)`
>+
>+        This would cause us to reuse the same process incorrectly. Our existing API test coverage
>+        did not catch this because our target HistoryItem usually has a SuspendedPage and we decide
>+        to use the SuspendedPage's process a few lines above in WebProcessPool::processForNavigationInternal().
>+
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::processForNavigationInternal):
>+
> 2018-09-11  Chris Dumez  <cdumez@apple.com>
> 
>         Clean up SuspendedPageProxy
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 14b67469caeec142532c0a9e5068a45715cf803b..96ff529b5a0abedf8af5b453e604090811ff70d5 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -2159,8 +2159,8 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
> 
>         // If the target back/forward item and the current back/forward item originated
>         // in the same WebProcess then we should reuse the current WebProcess.
>-        if (auto* currentItem = page.backForwardList().currentItem()) {
>-            if (currentItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)
>+        if (auto* fromItem = navigation.fromItem()) {
>+            if (fromItem->itemID().processIdentifier == backForwardListItem->itemID().processIdentifier)
>                 return page.process();
>         }
>     }
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index e60abee4189f1442afc52c598099305a20175ed1..0a06e50aa9a4858c57812062aace989b69a088ec 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1 +1,13 @@
>+2018-09-12  Chris Dumez  <cdumez@apple.com>
>+
>+        PSON: No process swap on back navigation after URL bar navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=189557
>+        <rdar://problem/44353108>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> == Rolled over to ChangeLog-2018-09-11 ==
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index 5c47f6f927698b9c046159d308e0562a633450f3..f70d11f1fa56b2f8ef7ddbb618e577c35b984f78 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -419,6 +419,64 @@ TEST(ProcessSwap, Back)
>     EXPECT_TRUE(pid1 == pid3);
> }
> 
>+TEST(ProcessSwap, BackWithoutSuspendedPage)
>+{
>+    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:testBytes];
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    RetainPtr<PSONMessageHandler> messageHandler = adoptNS([[PSONMessageHandler alloc] init]);
>+    [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"pson"];
>+
>+    auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>+    [webView1 setNavigationDelegate:delegate.get()];
>+
>+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
>+    [webView1 loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&receivedMessage);
>+    receivedMessage = false;
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto pid1 = [webView1 _webProcessIdentifier];
>+    RetainPtr<_WKSessionState> sessionState = [webView1 _sessionState];
>+    webView1 = nullptr;
>+
>+    auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>+    delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>+    [webView2 setNavigationDelegate:delegate.get()];
>+
>+    [webView2 _restoreSessionState:sessionState.get() andNavigate:NO];
>+
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
>+    [webView2 loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto pid2 = [webView2 _webProcessIdentifier];
>+
>+    [webView2 goBack];
>+
>+    TestWebKitAPI::Util::run(&receivedMessage);
>+    receivedMessage = false;
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto pid3 = [webView2 _webProcessIdentifier];
>+
>+    EXPECT_FALSE(pid1 == pid2);
>+    EXPECT_FALSE(pid2 == pid3);
>+}
>+
> #if PLATFORM(MAC)
> 
> TEST(ProcessSwap, CrossOriginWindowOpenNoOpener)

Actions: View | Formatted Diff | Diff

Attachments on bug 189557: 349576