[PSON] Make sure the WebProcessCache is leverage when relaunching a process after...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 19:11:52 +0000 (19:11 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 19:11:52 +0000 (19:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195747

Reviewed by Geoff Garen.

Source/WebKit:

Make sure the WebProcessCache and the prewarmed process are used when relaunching a process
after termination (e.g. crash).

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::reattachToWebProcess):
(WebKit::WebPageProxy::reattachToWebProcessForReload):
(WebKit::WebPageProxy::reattachToWebProcessWithItem):
(WebKit::WebPageProxy::loadRequest):
(WebKit::WebPageProxy::loadFile):
(WebKit::WebPageProxy::loadData):
(WebKit::WebPageProxy::loadAlternateHTML):
(WebKit::WebPageProxy::loadWebArchiveData):
(WebKit::WebPageProxy::navigateToPDFLinkWithSimulatedClick):
* UIProcess/WebPageProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForRegistrableDomain):
(WebKit::WebProcessPool::createWebPage):
(WebKit::WebProcessPool::processForNavigationInternal):
(WebKit::WebProcessPool::tryPrewarmWithDomainInformation):
* UIProcess/WebProcessPool.h:

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index d8ac962..305e22a 100644 (file)
@@ -1,3 +1,31 @@
+2019-03-14  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] Make sure the WebProcessCache is leverage when relaunching a process after termination
+        https://bugs.webkit.org/show_bug.cgi?id=195747
+
+        Reviewed by Geoff Garen.
+
+        Make sure the WebProcessCache and the prewarmed process are used when relaunching a process
+        after termination (e.g. crash).
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::reattachToWebProcess):
+        (WebKit::WebPageProxy::reattachToWebProcessForReload):
+        (WebKit::WebPageProxy::reattachToWebProcessWithItem):
+        (WebKit::WebPageProxy::loadRequest):
+        (WebKit::WebPageProxy::loadFile):
+        (WebKit::WebPageProxy::loadData):
+        (WebKit::WebPageProxy::loadAlternateHTML):
+        (WebKit::WebPageProxy::loadWebArchiveData):
+        (WebKit::WebPageProxy::navigateToPDFLinkWithSimulatedClick):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::processForRegistrableDomain):
+        (WebKit::WebProcessPool::createWebPage):
+        (WebKit::WebProcessPool::processForNavigationInternal):
+        (WebKit::WebProcessPool::tryPrewarmWithDomainInformation):
+        * UIProcess/WebProcessPool.h:
+
 2019-03-14  Timothy Hatcher  <timothy@apple.com>
 
         Unreviewed speculative build fix for watchOS after r242908.
index 9b335a4..bf72f00 100644 (file)
@@ -734,7 +734,7 @@ void WebPageProxy::handleSynchronousMessage(IPC::Connection& connection, const S
     returnUserData = UserData(m_process->transformObjectsToHandles(returnData.get()));
 }
 
-void WebPageProxy::reattachToWebProcess()
+void WebPageProxy::reattachToWebProcess(const RegistrableDomain& registrableDomain)
 {
     ASSERT(!m_isClosed);
     ASSERT(!isValid());
@@ -745,7 +745,7 @@ void WebPageProxy::reattachToWebProcess()
     m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
 
     auto& processPool = m_process->processPool();
-    m_process = processPool.createNewWebProcessRespectingProcessCountLimit(m_websiteDataStore.get());
+    m_process = processPool.processForRegistrableDomain(m_websiteDataStore.get(), this, registrableDomain);
     m_isValid = true;
 
     m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::Yes);
@@ -895,7 +895,8 @@ RefPtr<API::Navigation> WebPageProxy::reattachToWebProcessForReload()
     }
     
     ASSERT(!isValid());
-    reattachToWebProcess();
+    auto registrableDomain = m_backForwardList->currentItem() ? RegistrableDomain { URL(URL(), m_backForwardList->currentItem()->url()) } : RegistrableDomain { };
+    reattachToWebProcess(registrableDomain);
 
     if (!m_backForwardList->currentItem()) {
         RELEASE_LOG_IF_ALLOWED(Loading, "reattachToWebProcessForReload: no current item to reload: webPID = %i, pageID = %" PRIu64, m_process->processIdentifier(), m_pageID);
@@ -921,7 +922,7 @@ RefPtr<API::Navigation> WebPageProxy::reattachToWebProcessWithItem(WebBackForwar
     }
 
     ASSERT(!isValid());
-    reattachToWebProcess();
+    reattachToWebProcess(RegistrableDomain { URL(URL(), item.url()) });
 
     if (&item != m_backForwardList->currentItem())
         m_backForwardList->goToItem(item);
@@ -1095,7 +1096,7 @@ RefPtr<API::Navigation> WebPageProxy::loadRequest(ResourceRequest&& request, Sho
     RELEASE_LOG_IF_ALLOWED(Loading, "loadRequest: webPID = %i, pageID = %" PRIu64, m_process->processIdentifier(), m_pageID);
 
     if (!isValid())
-        reattachToWebProcess();
+        reattachToWebProcess(RegistrableDomain { request.url() });
 
     auto navigation = m_navigationState->createLoadRequestNavigation(ResourceRequest(request), m_backForwardList->currentItem());
     loadRequestWithNavigationShared(m_process.copyRef(), navigation.get(), WTFMove(request), shouldOpenExternalURLsPolicy, userData, ShouldTreatAsContinuingLoad::No);
@@ -1143,7 +1144,7 @@ RefPtr<API::Navigation> WebPageProxy::loadFile(const String& fileURLString, cons
     }
 
     if (!isValid())
-        reattachToWebProcess();
+        reattachToWebProcess({ });
 
     URL fileURL = URL(URL(), fileURLString);
     if (!fileURL.isLocalFile()) {
@@ -1195,7 +1196,7 @@ RefPtr<API::Navigation> WebPageProxy::loadData(const IPC::DataReference& data, c
     }
 
     if (!isValid())
-        reattachToWebProcess();
+        reattachToWebProcess({ });
 
     auto navigation = m_navigationState->createLoadDataNavigation(std::make_unique<API::SubstituteData>(data.vector(), MIMEType, encoding, baseURL, userData));
     loadDataWithNavigationShared(m_process.copyRef(), navigation, data, MIMEType, encoding, baseURL, userData, ShouldTreatAsContinuingLoad::No);
@@ -1244,7 +1245,7 @@ void WebPageProxy::loadAlternateHTML(const IPC::DataReference& htmlData, const S
         m_isLoadingAlternateHTMLStringForFailingProvisionalLoad = true;
 
     if (!isValid())
-        reattachToWebProcess();
+        reattachToWebProcess(RegistrableDomain { baseURL });
 
     auto transaction = m_pageLoadState.transaction();
 
@@ -1281,7 +1282,7 @@ void WebPageProxy::loadWebArchiveData(API::Data* webArchiveData, API::Object* us
     }
 
     if (!isValid())
-        reattachToWebProcess();
+        reattachToWebProcess({ });
 
     auto transaction = m_pageLoadState.transaction();
     m_pageLoadState.setPendingAPIRequestURL(transaction, WTF::blankURL().string());
@@ -1298,7 +1299,7 @@ void WebPageProxy::loadWebArchiveData(API::Data* webArchiveData, API::Object* us
     m_process->responsivenessTimer().start();
 }
 
-void WebPageProxy::navigateToPDFLinkWithSimulatedClick(const String& url, IntPoint documentPoint, IntPoint screenPoint)
+void WebPageProxy::navigateToPDFLinkWithSimulatedClick(const String& urlString, IntPoint documentPoint, IntPoint screenPoint)
 {
     RELEASE_LOG_IF_ALLOWED(Loading, "navigateToPDFLinkWithSimulatedClick: webPID = %i, pageID = %" PRIu64, m_process->processIdentifier(), m_pageID);
 
@@ -1307,13 +1308,13 @@ void WebPageProxy::navigateToPDFLinkWithSimulatedClick(const String& url, IntPoi
         return;
     }
 
-    if (WTF::protocolIsJavaScript(url))
+    if (WTF::protocolIsJavaScript(urlString))
         return;
 
     if (!isValid())
-        reattachToWebProcess();
+        reattachToWebProcess(RegistrableDomain { URL(URL(), urlString) });
 
-    m_process->send(Messages::WebPage::NavigateToPDFLinkWithSimulatedClick(url, documentPoint, screenPoint), m_pageID);
+    m_process->send(Messages::WebPage::NavigateToPDFLinkWithSimulatedClick(urlString, documentPoint, screenPoint), m_pageID);
     m_process->responsivenessTimer().start();
 }
 
index da68be7..0a6bdba 100644 (file)
@@ -1655,7 +1655,7 @@ private:
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
     void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
 
-    void reattachToWebProcess();
+    void reattachToWebProcess(const WebCore::RegistrableDomain&);
     void swapToWebProcess(Ref<WebProcessProxy>&&, std::unique_ptr<DrawingAreaProxy>&&, RefPtr<WebFrameProxy>&& mainFrame);
     void didFailToSuspendAfterProcessSwap();
     void didSuspendAfterProcessSwap();
index 8c139d4..5450f3e 100644 (file)
@@ -1142,8 +1142,26 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
 #endif
 }
 
-WebProcessProxy& WebProcessPool::createNewWebProcessRespectingProcessCountLimit(WebsiteDataStore& websiteDataStore)
+WebProcessProxy& WebProcessPool::processForRegistrableDomain(WebsiteDataStore& websiteDataStore, WebPageProxy* page, const RegistrableDomain& registrableDomain)
 {
+    if (!registrableDomain.isEmpty()) {
+        if (auto process = webProcessCache().takeProcess(registrableDomain, websiteDataStore))
+            return *process;
+
+        // Check if we have a suspended page for the given registrable domain and use its process if we do, for performance reasons.
+        if (auto process = page ? findReusableSuspendedPageProcess(registrableDomain, *page, websiteDataStore) : nullptr) {
+            RELEASE_LOG(ProcessSwapping, "Using WebProcess %i from a SuspendedPage", process->processIdentifier());
+            return *process;
+        }
+    }
+
+    if (auto process = tryTakePrewarmedProcess(websiteDataStore)) {
+        RELEASE_LOG(ProcessSwapping, "Using prewarmed process %i", process->processIdentifier());
+        if (!registrableDomain.isEmpty())
+            tryPrewarmWithDomainInformation(*process, registrableDomain);
+        return *process;
+    }
+
     if (!usesSingleWebProcess())
         return createNewWebProcess(websiteDataStore);
 
@@ -1192,10 +1210,9 @@ Ref<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API:
         // Sharing processes, e.g. when creating the page via window.open().
         process = &pageConfiguration->relatedPage()->process();
     } else {
-        process = tryTakePrewarmedProcess(pageConfiguration->websiteDataStore()->websiteDataStore());
-        if (!process)
-            process = &createNewWebProcessRespectingProcessCountLimit(pageConfiguration->websiteDataStore()->websiteDataStore());
+        process = &processForRegistrableDomain(pageConfiguration->websiteDataStore()->websiteDataStore(), nullptr, { });
     }
+    ASSERT(process);
 
     auto page = process->createWebPage(pageClient, WTFMove(pageConfiguration));
 
@@ -2214,24 +2231,8 @@ void WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API:
     auto& targetURL = navigation.currentRequest().url();
     auto targetRegistrableDomain = WebCore::RegistrableDomain { targetURL };
 
-    auto createNewProcess = [this, protectedThis = makeRef(*this), page = makeRef(page), targetURL, targetRegistrableDomain, dataStore = dataStore.copyRef()] () -> Ref<WebProcessProxy> {
-        if (auto process = webProcessCache().takeProcess(targetRegistrableDomain, dataStore))
-            return process.releaseNonNull();
-
-        // Check if we have a suspended page for the given registrable domain and use its process if we do, for performance reasons.
-        if (auto process = findReusableSuspendedPageProcess(targetRegistrableDomain, page, dataStore)) {
-            RELEASE_LOG(ProcessSwapping, "Using WebProcess %i from a SuspendedPage", process->processIdentifier());
-            return process.releaseNonNull();
-        }
-
-        if (auto process = tryTakePrewarmedProcess(dataStore)) {
-            RELEASE_LOG(ProcessSwapping, "Using prewarmed process %i", process->processIdentifier());
-            tryPrewarmWithDomainInformation(*process, targetURL);
-            return process.releaseNonNull();
-        }
-
-        RELEASE_LOG(ProcessSwapping, "Launching a new process");
-        return createNewWebProcess(dataStore);
+    auto createNewProcess = [this, protectedThis = makeRef(*this), page = makeRef(page), targetRegistrableDomain, dataStore = dataStore.copyRef()] () -> Ref<WebProcessProxy> {
+        return processForRegistrableDomain(dataStore, page.ptr(), targetRegistrableDomain);
     };
 
     if (usesSingleWebProcess())
@@ -2247,7 +2248,7 @@ void WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API:
         return completionHandler(WTFMove(sourceProcess), nullptr, "An automation session is active"_s);
 
     if (!sourceProcess->hasCommittedAnyProvisionalLoads()) {
-        tryPrewarmWithDomainInformation(sourceProcess, targetURL);
+        tryPrewarmWithDomainInformation(sourceProcess, targetRegistrableDomain);
         return completionHandler(WTFMove(sourceProcess), nullptr, "Process has not yet committed any provisional loads"_s);
     }
 
@@ -2463,9 +2464,9 @@ void WebProcessPool::didCollectPrewarmInformation(const WebCore::RegistrableDoma
     *value = prewarmInformation;
 }
 
-void WebProcessPool::tryPrewarmWithDomainInformation(WebProcessProxy& process, const URL& url)
+void WebProcessPool::tryPrewarmWithDomainInformation(WebProcessProxy& process, const RegistrableDomain& registrableDomain)
 {
-    auto* prewarmInformation = m_prewarmInformationPerRegistrableDomain.get(RegistrableDomain { url });
+    auto* prewarmInformation = m_prewarmInformationPerRegistrableDomain.get(registrableDomain);
     if (!prewarmInformation)
         return;
     process.send(Messages::WebProcess::PrewarmWithDomainInformation(*prewarmInformation), 0);
index 2bec2fc..6671412 100644 (file)
@@ -305,7 +305,7 @@ public:
 
     void allowSpecificHTTPSCertificateForHost(const WebCertificateInfo*, const String& host);
 
-    WebProcessProxy& createNewWebProcessRespectingProcessCountLimit(WebsiteDataStore&); // Will return an existing one if limit is met.
+    WebProcessProxy& processForRegistrableDomain(WebsiteDataStore&, WebPageProxy*, const WebCore::RegistrableDomain&); // Will return an existing one if limit is met or due to caching.
 
     enum class MayCreateDefaultDataStore { No, Yes };
     void prewarmProcess(WebsiteDataStore*, MayCreateDefaultDataStore);
@@ -571,7 +571,7 @@ private:
     void addProcessToOriginCacheSet(WebProcessProxy&, const URL&);
     void removeProcessFromOriginCacheSet(WebProcessProxy&);
 
-    void tryPrewarmWithDomainInformation(WebProcessProxy&, const URL&);
+    void tryPrewarmWithDomainInformation(WebProcessProxy&, const WebCore::RegistrableDomain&);
 
     void updateMaxSuspendedPageCount();
 
index ddeb5e9..b1ebde3 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-14  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] Make sure the WebProcessCache is leverage when relaunching a process after termination
+        https://bugs.webkit.org/show_bug.cgi?id=195747
+
+        Reviewed by Geoff Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2019-03-13  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Make -[_WKAttachment setFileWrapper:contentType:completion:] robust when given a nil completion handler
index b4f5784..ac0644a 100644 (file)
@@ -3206,6 +3206,70 @@ TEST(ProcessSwap, PageCacheWhenNavigatingFromJS)
     EXPECT_EQ(2u, seenPIDs.size());
 }
 
+TEST(ProcessSwap, UseWebProcessCacheAfterTermination)
+{
+    auto processPoolConfiguration = psonProcessPoolConfiguration();
+    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 navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
+    [navigationDelegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
+        done = true;
+    }];
+
+    int webkitPID = 0;
+
+    @autoreleasepool {
+        auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+        [webView setNavigationDelegate:navigationDelegate.get()];
+
+        NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+
+        [webView loadRequest:request];
+        TestWebKitAPI::Util::run(&done);
+        done = false;
+        webkitPID = [webView _webProcessIdentifier];
+    }
+
+    EXPECT_EQ(1U, [processPool _webProcessCountIgnoringPrewarmed]);
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    [webView setNavigationDelegate:navigationDelegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+    auto applePID = [webView _webProcessIdentifier];
+
+    EXPECT_NE(webkitPID, applePID);
+
+    EXPECT_EQ(2U, [processPool _webProcessCountIgnoringPrewarmed]);
+
+    __block bool webProcessTerminated = false;
+    [navigationDelegate setWebContentProcessDidTerminate:^(WKWebView *) {
+        webProcessTerminated = true;
+    }];
+
+    kill(applePID, 9);
+    TestWebKitAPI::Util::run(&webProcessTerminated);
+    webProcessTerminated = false;
+
+    EXPECT_EQ(0, [webView _webProcessIdentifier]);
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
+    EXPECT_EQ(1U, [processPool _webProcessCountIgnoringPrewarmed]);
+}
+
 TEST(ProcessSwap, NumberOfPrewarmedProcesses)
 {
     auto processPoolConfiguration = psonProcessPoolConfiguration();