Changeset 236973 in webkit

Timestamp:

Oct 9, 2018, 12:04:44 PM (8 years ago)

Author:

Chris Dumez

Message:

PSON: Doing a cross-site navigation via the URL bar does not swap process on iOS
​https://bugs.webkit.org/show_bug.cgi?id=190378
<rdar://problem/45059466>

Reviewed by Geoffrey Garen.

Source/WebKit:

Process swapping was sometimes not happening via URL bar navigation on iOS due to top-hit preloading,
which would use a new WKWebView for the speculative load and rely on the _relatedWebView SPI to use
the same WebContent process as the view currently on screen.

To address the issue, if the source URL is empty and the page has a related page, use the related
page's URL as source URL when doing the process-swap decision.

• UIProcess/API/APIPageConfiguration.cpp:

(API::PageConfiguration::relatedPage const):
(API::PageConfiguration::relatedPage): Deleted.

• UIProcess/API/APIPageConfiguration.h:
• UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::processForNavigationInternal):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/APIPageConfiguration.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/API/APIPageConfiguration.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-10-09  Chris Dumez  <cdumez@apple.com>
+
+        PSON: Doing a cross-site navigation via the URL bar does not swap process on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=190378
+        <rdar://problem/45059466>
+
+        Reviewed by Geoffrey Garen.
+
+        Process swapping was sometimes not happening via URL bar navigation on iOS due to top-hit preloading,
+        which would use a new WKWebView for the speculative load and rely on the _relatedWebView SPI to use
+        the same WebContent process as the view currently on screen.
+
+        To address the issue, if the source URL is empty and the page has a related page, use the related
+        page's URL as source URL when doing the process-swap decision.
+
+        * UIProcess/API/APIPageConfiguration.cpp:
+        (API::PageConfiguration::relatedPage const):
+        (API::PageConfiguration::relatedPage): Deleted.
+        * UIProcess/API/APIPageConfiguration.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::processForNavigationInternal):
+
 2018-10-09  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebKit/UIProcess/API/APIPageConfiguration.cpp

@@ -130 +130 @@
 }
 
-WebPageProxy* PageConfiguration::relatedPage()
+WebPageProxy* PageConfiguration::relatedPage() const
 {
     return m_relatedPage.get();

trunk/Source/WebKit/UIProcess/API/APIPageConfiguration.h

@@ -73 +73 @@
     WebKit::WebPreferencesStore::ValueMap& preferenceValues() { return m_preferenceValues; }
 
-    WebKit::WebPageProxy* relatedPage();
+    WebKit::WebPageProxy* relatedPage() const;
     void setRelatedPage(WebKit::WebPageProxy*);
 

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -2139 +2139 @@
 
         bool isInitialLoadInNewWindowOpenedByDOM = page.openedByDOM() && !page.hasCommittedAnyProvisionalLoads();
-        URL url;
+        URL sourceURL;
         if (isInitialLoadInNewWindowOpenedByDOM && !navigation.requesterOrigin().isEmpty())
-            url = URL { URL(), navigation.requesterOrigin().toString() };
+            sourceURL = URL { URL(), navigation.requesterOrigin().toString() };
         else
-            url = URL { { }, page.pageLoadState().url() };
-        if (!url.isValid() || !targetURL.isValid() || url.isEmpty() || url.isBlankURL() || registrableDomainsAreEqual(url, targetURL)) {
+            sourceURL = URL { { }, page.pageLoadState().url() };
+
+        if (sourceURL.isEmpty() && page.configuration().relatedPage()) {
+            sourceURL = URL { { }, page.configuration().relatedPage()->pageLoadState().url() };
+            RELEASE_LOG(ProcessSwapping, "Using related page %p's URL as source URL for process swap decision", page.configuration().relatedPage());
+        }
+
+        if (!sourceURL.isValid() || !targetURL.isValid() || sourceURL.isEmpty() || sourceURL.isBlankURL() || registrableDomainsAreEqual(sourceURL, targetURL)) {
             reason = "Navigation is same-site"_s;
             return page.process();

trunk/Tools/ChangeLog

@@ +1 @@
+2018-10-09  Chris Dumez  <cdumez@apple.com>
+
+        PSON: Doing a cross-site navigation via the URL bar does not swap process on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=190378
+        <rdar://problem/45059466>
+
+        Reviewed by Geoffrey Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-10-09  Jer Noble  <jer.noble@apple.com>
 

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

@@ -1804 +1804 @@
 }
 
+enum class ExpectSwap { No, Yes };
+static void runProcessSwapDueToRelatedWebViewTest(NSURL* relatedViewURL, NSURL* targetURL, ExpectSwap expectSwap)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webView1Configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webView1Configuration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [webView1Configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+    auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webView1Configuration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView1 setNavigationDelegate:delegate.get()];
+
+    numberOfDecidePolicyCalls = 0;
+    NSURLRequest *request = [NSURLRequest requestWithURL:relatedViewURL];
+    [webView1 loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid1 = [webView1 _webProcessIdentifier];
+
+    auto webView2Configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webView2Configuration setProcessPool:processPool.get()];
+    [webView2Configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+    webView2Configuration.get()._relatedWebView = webView1.get(); // webView2 will be related to webView1 and webView1's URL will be used for process swap decision.
+    auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webView2Configuration.get()]);
+    [webView2 setNavigationDelegate:delegate.get()];
+
+    request = [NSURLRequest requestWithURL:targetURL];
+    [webView2 loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid2 = [webView2 _webProcessIdentifier];
+
+    if (expectSwap == ExpectSwap::No)
+        EXPECT_TRUE(pid1 == pid2);
+    else
+        EXPECT_FALSE(pid1 == pid2);
+
+    EXPECT_EQ(2, numberOfDecidePolicyCalls);
+}
+
+TEST(ProcessSwap, ProcessSwapDueToRelatedView)
+{
+    runProcessSwapDueToRelatedWebViewTest([NSURL URLWithString:@"pson://www.webkit.org/main1.html"], [NSURL URLWithString:@"pson://www.apple.com/main2.html"], ExpectSwap::Yes);
+}
+
+TEST(ProcessSwap, NoProcessSwapDueToRelatedView)
+{
+    runProcessSwapDueToRelatedWebViewTest([NSURL URLWithString:@"pson://www.webkit.org/main1.html"], [NSURL URLWithString:@"pson://www.webkit.org/main2.html"], ExpectSwap::No);
+}
+
 TEST(ProcessSwap, TerminatedSuspendedPageProcess)
 {