Unreviewed, rolling out r242952.
authorsroberts@apple.com <sroberts@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 18:26:23 +0000 (18:26 +0000)
committersroberts@apple.com <sroberts@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2019 18:26:23 +0000 (18:26 +0000)
Causing API failures on iOS Simulator

Reverted changeset:

"[PSON] Make sure the WebProcessCache is leverage when
relaunching a process after termination"
https://bugs.webkit.org/show_bug.cgi?id=195747
https://trac.webkit.org/changeset/242952

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243003 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 7524653..dbcd749 100644 (file)
@@ -1,3 +1,16 @@
+2019-03-15  Shawn Roberts  <sroberts@apple.com>
+
+        Unreviewed, rolling out r242952.
+
+        Causing API failures on iOS Simulator
+
+        Reverted changeset:
+
+        "[PSON] Make sure the WebProcessCache is leverage when
+        relaunching a process after termination"
+        https://bugs.webkit.org/show_bug.cgi?id=195747
+        https://trac.webkit.org/changeset/242952
+
 2019-03-15  Per Arne Vollan  <pvollan@apple.com>
 
         [macOS] Broker access to Speech Synthesis
index 4dd2214..94dc253 100644 (file)
@@ -734,7 +734,7 @@ void WebPageProxy::handleSynchronousMessage(IPC::Connection& connection, const S
     returnUserData = UserData(m_process->transformObjectsToHandles(returnData.get()));
 }
 
-void WebPageProxy::reattachToWebProcess(const RegistrableDomain& registrableDomain)
+void WebPageProxy::reattachToWebProcess()
 {
     ASSERT(!m_isClosed);
     ASSERT(!isValid());
@@ -745,7 +745,7 @@ void WebPageProxy::reattachToWebProcess(const RegistrableDomain& registrableDoma
     m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
 
     auto& processPool = m_process->processPool();
-    m_process = processPool.processForRegistrableDomain(m_websiteDataStore.get(), this, registrableDomain);
+    m_process = processPool.createNewWebProcessRespectingProcessCountLimit(m_websiteDataStore.get());
     m_isValid = true;
 
     m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::Yes);
@@ -895,8 +895,7 @@ RefPtr<API::Navigation> WebPageProxy::reattachToWebProcessForReload()
     }
     
     ASSERT(!isValid());
-    auto registrableDomain = m_backForwardList->currentItem() ? RegistrableDomain { URL(URL(), m_backForwardList->currentItem()->url()) } : RegistrableDomain { };
-    reattachToWebProcess(registrableDomain);
+    reattachToWebProcess();
 
     if (!m_backForwardList->currentItem()) {
         RELEASE_LOG_IF_ALLOWED(Loading, "reattachToWebProcessForReload: no current item to reload: webPID = %i, pageID = %" PRIu64, m_process->processIdentifier(), m_pageID);
@@ -922,7 +921,7 @@ RefPtr<API::Navigation> WebPageProxy::reattachToWebProcessWithItem(WebBackForwar
     }
 
     ASSERT(!isValid());
-    reattachToWebProcess(RegistrableDomain { URL(URL(), item.url()) });
+    reattachToWebProcess();
 
     if (&item != m_backForwardList->currentItem())
         m_backForwardList->goToItem(item);
@@ -1096,7 +1095,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(RegistrableDomain { request.url() });
+        reattachToWebProcess();
 
     auto navigation = m_navigationState->createLoadRequestNavigation(ResourceRequest(request), m_backForwardList->currentItem());
     loadRequestWithNavigationShared(m_process.copyRef(), navigation.get(), WTFMove(request), shouldOpenExternalURLsPolicy, userData, ShouldTreatAsContinuingLoad::No);
@@ -1144,7 +1143,7 @@ RefPtr<API::Navigation> WebPageProxy::loadFile(const String& fileURLString, cons
     }
 
     if (!isValid())
-        reattachToWebProcess({ });
+        reattachToWebProcess();
 
     URL fileURL = URL(URL(), fileURLString);
     if (!fileURL.isLocalFile()) {
@@ -1196,7 +1195,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);
@@ -1245,7 +1244,7 @@ void WebPageProxy::loadAlternateHTML(const IPC::DataReference& htmlData, const S
         m_isLoadingAlternateHTMLStringForFailingProvisionalLoad = true;
 
     if (!isValid())
-        reattachToWebProcess(RegistrableDomain { baseURL });
+        reattachToWebProcess();
 
     auto transaction = m_pageLoadState.transaction();
 
@@ -1282,7 +1281,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());
@@ -1299,7 +1298,7 @@ void WebPageProxy::loadWebArchiveData(API::Data* webArchiveData, API::Object* us
     m_process->responsivenessTimer().start();
 }
 
-void WebPageProxy::navigateToPDFLinkWithSimulatedClick(const String& urlString, IntPoint documentPoint, IntPoint screenPoint)
+void WebPageProxy::navigateToPDFLinkWithSimulatedClick(const String& url, IntPoint documentPoint, IntPoint screenPoint)
 {
     RELEASE_LOG_IF_ALLOWED(Loading, "navigateToPDFLinkWithSimulatedClick: webPID = %i, pageID = %" PRIu64, m_process->processIdentifier(), m_pageID);
 
@@ -1308,13 +1307,13 @@ void WebPageProxy::navigateToPDFLinkWithSimulatedClick(const String& urlString,
         return;
     }
 
-    if (WTF::protocolIsJavaScript(urlString))
+    if (WTF::protocolIsJavaScript(url))
         return;
 
     if (!isValid())
-        reattachToWebProcess(RegistrableDomain { URL(URL(), urlString) });
+        reattachToWebProcess();
 
-    m_process->send(Messages::WebPage::NavigateToPDFLinkWithSimulatedClick(urlString, documentPoint, screenPoint), m_pageID);
+    m_process->send(Messages::WebPage::NavigateToPDFLinkWithSimulatedClick(url, documentPoint, screenPoint), m_pageID);
     m_process->responsivenessTimer().start();
 }
 
index 90588d9..7f05d39 100644 (file)
@@ -1671,7 +1671,7 @@ private:
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
     void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
 
-    void reattachToWebProcess(const WebCore::RegistrableDomain&);
+    void reattachToWebProcess();
     void swapToWebProcess(Ref<WebProcessProxy>&&, std::unique_ptr<DrawingAreaProxy>&&, RefPtr<WebFrameProxy>&& mainFrame);
     void didFailToSuspendAfterProcessSwap();
     void didSuspendAfterProcessSwap();
index 5450f3e..8c139d4 100644 (file)
@@ -1142,26 +1142,8 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
 #endif
 }
 
-WebProcessProxy& WebProcessPool::processForRegistrableDomain(WebsiteDataStore& websiteDataStore, WebPageProxy* page, const RegistrableDomain& registrableDomain)
+WebProcessProxy& WebProcessPool::createNewWebProcessRespectingProcessCountLimit(WebsiteDataStore& websiteDataStore)
 {
-    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);
 
@@ -1210,9 +1192,10 @@ 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 = &processForRegistrableDomain(pageConfiguration->websiteDataStore()->websiteDataStore(), nullptr, { });
+        process = tryTakePrewarmedProcess(pageConfiguration->websiteDataStore()->websiteDataStore());
+        if (!process)
+            process = &createNewWebProcessRespectingProcessCountLimit(pageConfiguration->websiteDataStore()->websiteDataStore());
     }
-    ASSERT(process);
 
     auto page = process->createWebPage(pageClient, WTFMove(pageConfiguration));
 
@@ -2231,8 +2214,24 @@ 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), targetRegistrableDomain, dataStore = dataStore.copyRef()] () -> Ref<WebProcessProxy> {
-        return processForRegistrableDomain(dataStore, page.ptr(), targetRegistrableDomain);
+    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);
     };
 
     if (usesSingleWebProcess())
@@ -2248,7 +2247,7 @@ void WebProcessPool::processForNavigationInternal(WebPageProxy& page, const API:
         return completionHandler(WTFMove(sourceProcess), nullptr, "An automation session is active"_s);
 
     if (!sourceProcess->hasCommittedAnyProvisionalLoads()) {
-        tryPrewarmWithDomainInformation(sourceProcess, targetRegistrableDomain);
+        tryPrewarmWithDomainInformation(sourceProcess, targetURL);
         return completionHandler(WTFMove(sourceProcess), nullptr, "Process has not yet committed any provisional loads"_s);
     }
 
@@ -2464,9 +2463,9 @@ void WebProcessPool::didCollectPrewarmInformation(const WebCore::RegistrableDoma
     *value = prewarmInformation;
 }
 
-void WebProcessPool::tryPrewarmWithDomainInformation(WebProcessProxy& process, const RegistrableDomain& registrableDomain)
+void WebProcessPool::tryPrewarmWithDomainInformation(WebProcessProxy& process, const URL& url)
 {
-    auto* prewarmInformation = m_prewarmInformationPerRegistrableDomain.get(registrableDomain);
+    auto* prewarmInformation = m_prewarmInformationPerRegistrableDomain.get(RegistrableDomain { url });
     if (!prewarmInformation)
         return;
     process.send(Messages::WebProcess::PrewarmWithDomainInformation(*prewarmInformation), 0);
index 6671412..2bec2fc 100644 (file)
@@ -305,7 +305,7 @@ public:
 
     void allowSpecificHTTPSCertificateForHost(const WebCertificateInfo*, const String& host);
 
-    WebProcessProxy& processForRegistrableDomain(WebsiteDataStore&, WebPageProxy*, const WebCore::RegistrableDomain&); // Will return an existing one if limit is met or due to caching.
+    WebProcessProxy& createNewWebProcessRespectingProcessCountLimit(WebsiteDataStore&); // Will return an existing one if limit is met.
 
     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 WebCore::RegistrableDomain&);
+    void tryPrewarmWithDomainInformation(WebProcessProxy&, const URL&);
 
     void updateMaxSuspendedPageCount();
 
index d955f40..3df589b 100644 (file)
@@ -1,3 +1,16 @@
+2019-03-15  Shawn Roberts  <sroberts@apple.com>
+
+        Unreviewed, rolling out r242952.
+
+        Causing API failures on iOS Simulator
+
+        Reverted changeset:
+
+        "[PSON] Make sure the WebProcessCache is leverage when
+        relaunching a process after termination"
+        https://bugs.webkit.org/show_bug.cgi?id=195747
+        https://trac.webkit.org/changeset/242952
+
 2019-03-14  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [Win][MinBrowser][WK2] Implement createNewPage of WKPageUIClient to open a new window
index ac0644a..b4f5784 100644 (file)
@@ -3206,70 +3206,6 @@ 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();