Title: [257953] trunk/Source/WebKit
Revision
257953
Author
[email protected]
Date
2020-03-05 15:55:27 -0800 (Thu, 05 Mar 2020)

Log Message

Running a single layout test makes 28 WebProcessPools (and launches 6 Network processes)
https://bugs.webkit.org/show_bug.cgi?id=208541
<rdar://problem/60018602>

Reviewed by Youenn Fablet.

WebKitTestRunner was contructing a page / process pool and then calling into various
WebsiteDataStore APIs to reset / clear state. Because nothing had been loaded in the
view yet, the page would still be using a dummy process proxy (due to delayed process
launch optimization) at the time the WebsiteDataStore APIs are called. Because the
dummy process proxy would not register itself with the WebsiteDataStore (unlike other
processes), the WebsiteDataStore thought it had no associated process pool and would
thus construct a temporary one every time it needed one.

To address the issue, we now construct one dummy process proxy per session (instead of
one for all sessions). As a result, the dummy process proxy can now behave as a normal
WebProcessProxy and have an associated data store and register / unregister itself from
it, depending on whether or not it has pages.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::launchInitialProcessIfNecessary):
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::establishWorkerContextConnectionToNetworkProcess):
(WebKit::WebProcessPool::disconnectProcess):
(WebKit::WebProcessPool::processForRegistrableDomain):
(WebKit::WebProcessPool::createWebPage):
* UIProcess/WebProcessPool.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::isDummyProcessProxy const):
(WebKit::WebProcessProxy::updateRegistrationWithDataStore):
(WebKit::WebProcessProxy::maybeShutDown):
* UIProcess/WebProcessProxy.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (257952 => 257953)


--- trunk/Source/WebKit/ChangeLog	2020-03-05 23:43:58 UTC (rev 257952)
+++ trunk/Source/WebKit/ChangeLog	2020-03-05 23:55:27 UTC (rev 257953)
@@ -1,3 +1,38 @@
+2020-03-05  Chris Dumez  <[email protected]>
+
+        Running a single layout test makes 28 WebProcessPools (and launches 6 Network processes)
+        https://bugs.webkit.org/show_bug.cgi?id=208541
+        <rdar://problem/60018602>
+
+        Reviewed by Youenn Fablet.
+
+        WebKitTestRunner was contructing a page / process pool and then calling into various
+        WebsiteDataStore APIs to reset / clear state. Because nothing had been loaded in the
+        view yet, the page would still be using a dummy process proxy (due to delayed process
+        launch optimization) at the time the WebsiteDataStore APIs are called. Because the
+        dummy process proxy would not register itself with the WebsiteDataStore (unlike other
+        processes), the WebsiteDataStore thought it had no associated process pool and would
+        thus construct a temporary one every time it needed one.
+
+        To address the issue, we now construct one dummy process proxy per session (instead of
+        one for all sessions). As a result, the dummy process proxy can now behave as a normal
+        WebProcessProxy and have an associated data store and register / unregister itself from
+        it, depending on whether or not it has pages.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::launchInitialProcessIfNecessary):
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::establishWorkerContextConnectionToNetworkProcess):
+        (WebKit::WebProcessPool::disconnectProcess):
+        (WebKit::WebProcessPool::processForRegistrableDomain):
+        (WebKit::WebProcessPool::createWebPage):
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::isDummyProcessProxy const):
+        (WebKit::WebProcessProxy::updateRegistrationWithDataStore):
+        (WebKit::WebProcessProxy::maybeShutDown):
+        * UIProcess/WebProcessProxy.h:
+
 2020-03-05  Jer Noble  <[email protected]>
 
         Unreviewed unified build fix; forward-declare and include GPUConnectionToWebProcess where it's used.

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (257952 => 257953)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2020-03-05 23:43:58 UTC (rev 257952)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2020-03-05 23:55:27 UTC (rev 257953)
@@ -3974,7 +3974,7 @@
 
 void WebPageProxy::launchInitialProcessIfNecessary()
 {
-    if (&process() == process().processPool().dummyProcessProxy())
+    if (process().isDummyProcessProxy())
         launchProcess({ }, ProcessLaunchReason::InitialProcess);
 }
 

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (257952 => 257953)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2020-03-05 23:43:58 UTC (rev 257952)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2020-03-05 23:55:27 UTC (rev 257953)
@@ -766,7 +766,7 @@
     WebProcessProxy* serviceWorkerProcessProxy { nullptr };
     if (!m_useSeparateServiceWorkerProcess) {
         for (auto& process : m_processes) {
-            if (process == m_prewarmedProcess || process == m_dummyProcessProxy)
+            if (process == m_prewarmedProcess || process->isDummyProcessProxy())
                 continue;
             if (&process->websiteDataStore() != websiteDataStore)
                 continue;
@@ -1156,11 +1156,12 @@
     if (m_prewarmedProcess == process) {
         ASSERT(m_prewarmedProcess->isPrewarmed());
         m_prewarmedProcess = nullptr;
+    } else {
+        auto dummyProcessIterator = m_dummyProcessProxies.find(process->sessionID());
+        if (dummyProcessIterator != m_dummyProcessProxies.end() && dummyProcessIterator->value == process)
+            m_dummyProcessProxies.remove(dummyProcessIterator);
     }
 
-    if (m_dummyProcessProxy == process)
-        m_dummyProcessProxy = nullptr;
-
     // FIXME (Multi-WebProcess): <rdar://problem/12239765> Some of the invalidation calls of the other supplements are still necessary in multi-process mode, but they should only affect data structures pertaining to the process being disconnected.
     // Clearing everything causes assertion failures, so it's less trouble to skip that for now.
     RefPtr<WebProcessProxy> protect(process);
@@ -1214,7 +1215,7 @@
 #endif
 
     for (auto& process : m_processes) {
-        if (process == m_prewarmedProcess || process == m_dummyProcessProxy)
+        if (process == m_prewarmedProcess || process->isDummyProcessProxy())
             continue;
 #if ENABLE(SERVICE_WORKER)
         if (process->isRunningServiceWorkers())
@@ -1253,16 +1254,16 @@
         // Sharing processes, e.g. when creating the page via window.open().
         process = &pageConfiguration->relatedPage()->ensureRunningProcess();
         // We do not support several WebsiteDataStores sharing a single process.
-        ASSERT(process.get() == m_dummyProcessProxy || pageConfiguration->websiteDataStore() == &process->websiteDataStore());
+        ASSERT(process->isDummyProcessProxy() || pageConfiguration->websiteDataStore() == &process->websiteDataStore());
         ASSERT(&pageConfiguration->relatedPage()->websiteDataStore() == pageConfiguration->websiteDataStore());
     } else if (!m_isDelayedWebProcessLaunchDisabled) {
         // In the common case, we delay process launch until something is actually loaded in the page.
-        if (!m_dummyProcessProxy) {
-            auto dummyProcessProxy = WebProcessProxy::create(*this, nullptr, WebProcessProxy::IsPrewarmed::No, WebProcessProxy::ShouldLaunchProcess::No);
-            m_dummyProcessProxy = dummyProcessProxy.ptr();
-            m_processes.append(WTFMove(dummyProcessProxy));
+        process = dummyProcessProxy(pageConfiguration->websiteDataStore()->sessionID());
+        if (!process) {
+            process = WebProcessProxy::create(*this, pageConfiguration->websiteDataStore(), WebProcessProxy::IsPrewarmed::No, WebProcessProxy::ShouldLaunchProcess::No);
+            m_dummyProcessProxies.add(pageConfiguration->websiteDataStore()->sessionID(), makeWeakPtr(*process));
+            m_processes.append(process.copyRef());
         }
-        process = m_dummyProcessProxy;
     } else
         process = &processForRegistrableDomain(*pageConfiguration->websiteDataStore(), nullptr, { });
 

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (257952 => 257953)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.h	2020-03-05 23:43:58 UTC (rev 257952)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h	2020-03-05 23:55:27 UTC (rev 257953)
@@ -56,6 +56,7 @@
 #include <wtf/OptionSet.h>
 #include <wtf/RefCounter.h>
 #include <wtf/RefPtr.h>
+#include <wtf/WeakPtr.h>
 #include <wtf/text/StringHash.h>
 #include <wtf/text/WTFString.h>
 
@@ -186,7 +187,7 @@
 
     // WebProcessProxy object which does not have a running process which is used for convenience, to avoid
     // null checks in WebPageProxy.
-    WebProcessProxy* dummyProcessProxy() const { return m_dummyProcessProxy; }
+    WebProcessProxy* dummyProcessProxy(PAL::SessionID sessionID) const { return m_dummyProcessProxies.get(sessionID).get(); }
 
     // WebProcess or NetworkProcess as approporiate for current process model. The connection must be non-null.
     IPC::Connection* networkingProcessConnection();
@@ -624,8 +625,9 @@
 
     Vector<RefPtr<WebProcessProxy>> m_processes;
     WebProcessProxy* m_prewarmedProcess { nullptr };
-    WebProcessProxy* m_dummyProcessProxy { nullptr }; // A lightweight WebProcessProxy without backing process.
 
+    HashMap<PAL::SessionID, WeakPtr<WebProcessProxy>> m_dummyProcessProxies; // Lightweight WebProcessProxy objects without backing process.
+
 #if ENABLE(SERVICE_WORKER)
     WeakHashSet<WebProcessProxy> m_serviceWorkerProcesses;
     bool m_waitingForWorkerContextProcessConnection { false };

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (257952 => 257953)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2020-03-05 23:43:58 UTC (rev 257952)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2020-03-05 23:55:27 UTC (rev 257953)
@@ -262,12 +262,17 @@
     send(Messages::WebProcess::SetWebsiteDataStoreParameters(processPool().webProcessDataStoreParameters(*this, dataStore)), 0);
 }
 
+bool WebProcessProxy::isDummyProcessProxy() const
+{
+    return m_websiteDataStore && processPool().dummyProcessProxy(m_websiteDataStore->sessionID()) == this;
+}
+
 void WebProcessProxy::updateRegistrationWithDataStore()
 {
     if (!m_websiteDataStore)
         return;
     
-    bool shouldBeRegistered = processPool().dummyProcessProxy() != this && (pageCount() || provisionalPageCount());
+    bool shouldBeRegistered = pageCount() || provisionalPageCount();
     if (shouldBeRegistered)
         m_websiteDataStore->registerProcess(*this);
     else
@@ -1030,7 +1035,7 @@
 
 void WebProcessProxy::maybeShutDown()
 {
-    if (processPool().dummyProcessProxy() == this && m_pageMap.isEmpty()) {
+    if (isDummyProcessProxy() && m_pageMap.isEmpty()) {
         ASSERT(state() == State::Terminated);
         m_processPool->disconnectProcess(this);
         return;

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (257952 => 257953)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h	2020-03-05 23:43:58 UTC (rev 257952)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h	2020-03-05 23:55:27 UTC (rev 257953)
@@ -170,6 +170,8 @@
     bool isRunningServiceWorkers() const { return !!m_serviceWorkerInformation; }
     bool isStandaloneServiceWorkerProcess() const { return isRunningServiceWorkers() && !pageCount(); }
 
+    bool isDummyProcessProxy() const;
+
     void didCreateWebPageInProcess(WebCore::PageIdentifier);
 
     void addVisitedLinkStoreUser(VisitedLinkStore&, WebPageProxyIdentifier);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to