Title: [225864] trunk
Revision
225864
Author
beid...@apple.com
Date
2017-12-13 12:20:18 -0800 (Wed, 13 Dec 2017)

Log Message

REGRESSION (r225789): API tests WKProcessPool.InitialWarmedProcessUsed and WebKit.WebsiteDataStoreCustomPaths are failing.
https://bugs.webkit.org/show_bug.cgi?id=180722

Reviewed by Chris Dumez.

Source/WebKit:

- Add a test-only accessor to get the number of WebProcesses hosting WebPages
- Have WebsiteDataStore keep track of all instances so they can be looked up by SessionID
- When the StorageProcess needs to establish a SW context connection on behalf of a specific SessionID,
  the UI process will now prefer using the WebsiteDataStore associated with that SessionID. This allows
  us to continue deferring creation of the default data store if it's not needed.

* StorageProcess/StorageProcess.cpp:
(WebKit::StorageProcess::connectionToContextProcessWasClosed):
(WebKit::StorageProcess::createServerToContextConnection):
* StorageProcess/StorageProcess.h:

* StorageProcess/StorageToWebProcessConnection.cpp:
(WebKit::StorageToWebProcessConnection::establishSWServerConnection):

* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _webPageContentProcessCount]):
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:

* UIProcess/Storage/StorageProcessProxy.cpp:
(WebKit::StorageProcessProxy::establishWorkerContextConnectionToStorageProcess):
(WebKit::StorageProcessProxy::establishWorkerContextConnectionToStorageProcessForExplicitSession):
* UIProcess/Storage/StorageProcessProxy.h:
* UIProcess/Storage/StorageProcessProxy.messages.in:

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::establishWorkerContextConnectionToStorageProcess):
* UIProcess/WebProcessPool.h:

* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::WebsiteDataStore):
(WebKit::WebsiteDataStore::~WebsiteDataStore):
(WebKit::WebsiteDataStore::existingDataStoreForSessionID):
* UIProcess/WebsiteData/WebsiteDataStore.h:

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm:
(TEST): Call a new SPI to get the count of WebProcesses hosting WebPages.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (225863 => 225864)


--- trunk/Source/WebKit/ChangeLog	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/ChangeLog	2017-12-13 20:20:18 UTC (rev 225864)
@@ -1,3 +1,44 @@
+2017-12-13  Brady Eidson  <beid...@apple.com>
+
+        REGRESSION (r225789): API tests WKProcessPool.InitialWarmedProcessUsed and WebKit.WebsiteDataStoreCustomPaths are failing.
+        https://bugs.webkit.org/show_bug.cgi?id=180722
+
+        Reviewed by Chris Dumez.
+
+        - Add a test-only accessor to get the number of WebProcesses hosting WebPages
+        - Have WebsiteDataStore keep track of all instances so they can be looked up by SessionID
+        - When the StorageProcess needs to establish a SW context connection on behalf of a specific SessionID,
+          the UI process will now prefer using the WebsiteDataStore associated with that SessionID. This allows
+          us to continue deferring creation of the default data store if it's not needed.
+
+        * StorageProcess/StorageProcess.cpp:
+        (WebKit::StorageProcess::connectionToContextProcessWasClosed):
+        (WebKit::StorageProcess::createServerToContextConnection):
+        * StorageProcess/StorageProcess.h:
+
+        * StorageProcess/StorageToWebProcessConnection.cpp:
+        (WebKit::StorageToWebProcessConnection::establishSWServerConnection):
+
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _webPageContentProcessCount]):
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+
+        * UIProcess/Storage/StorageProcessProxy.cpp:
+        (WebKit::StorageProcessProxy::establishWorkerContextConnectionToStorageProcess):
+        (WebKit::StorageProcessProxy::establishWorkerContextConnectionToStorageProcessForExplicitSession):
+        * UIProcess/Storage/StorageProcessProxy.h:
+        * UIProcess/Storage/StorageProcessProxy.messages.in:
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::establishWorkerContextConnectionToStorageProcess):
+        * UIProcess/WebProcessPool.h:
+
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::WebsiteDataStore):
+        (WebKit::WebsiteDataStore::~WebsiteDataStore):
+        (WebKit::WebsiteDataStore::existingDataStoreForSessionID):
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+
 2017-12-13  Per Arne Vollan  <pvol...@apple.com>
 
         REGRESSION(225597): Can't select a text box or web view on a page when VO is on.

Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.cpp (225863 => 225864)


--- trunk/Source/WebKit/StorageProcess/StorageProcess.cpp	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.cpp	2017-12-13 20:20:18 UTC (rev 225864)
@@ -111,7 +111,7 @@
         swServer->markAllWorkersAsTerminated();
 
     if (shouldRelaunch)
-        createServerToContextConnection();
+        createServerToContextConnection(std::nullopt);
 }
 
 // The rule is that we need a context process (and a connection to it) as long as we have SWServerConnections to regular WebProcesses.
@@ -427,13 +427,16 @@
     return m_serverToContextConnection.get();
 }
 
-void StorageProcess::createServerToContextConnection()
+void StorageProcess::createServerToContextConnection(std::optional<PAL::SessionID> sessionID)
 {
     if (m_waitingForServerToContextProcessConnection)
         return;
     
     m_waitingForServerToContextProcessConnection = true;
-    parentProcessConnection()->send(Messages::StorageProcessProxy::EstablishWorkerContextConnectionToStorageProcess(), 0);
+    if (sessionID)
+        parentProcessConnection()->send(Messages::StorageProcessProxy::EstablishWorkerContextConnectionToStorageProcessForExplicitSession(*sessionID), 0);
+    else
+        parentProcessConnection()->send(Messages::StorageProcessProxy::EstablishWorkerContextConnectionToStorageProcess(), 0);
 }
 
 void StorageProcess::didFailFetch(SWServerConnectionIdentifier serverConnectionIdentifier, uint64_t fetchIdentifier)

Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.h (225863 => 225864)


--- trunk/Source/WebKit/StorageProcess/StorageProcess.h	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.h	2017-12-13 20:20:18 UTC (rev 225864)
@@ -89,7 +89,7 @@
     // For now we just have one global connection to service worker context processes.
     // This will change in the future.
     WebSWServerToContextConnection* globalServerToContextConnection();
-    void createServerToContextConnection();
+    void createServerToContextConnection(std::optional<PAL::SessionID>);
 
     WebCore::SWServer& swServerForSession(PAL::SessionID);
     void registerSWServerConnection(WebSWServerConnection&);

Modified: trunk/Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp (225863 => 225864)


--- trunk/Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp	2017-12-13 20:20:18 UTC (rev 225864)
@@ -171,7 +171,7 @@
     ASSERT_UNUSED(addResult, addResult.isNewEntry);
 
     if (!StorageProcess::singleton().globalServerToContextConnection())
-        StorageProcess::singleton().createServerToContextConnection();
+        StorageProcess::singleton().createServerToContextConnection(sessionID);
 }
 
 void StorageToWebProcessConnection::removeSWServerConnection(WebCore::SWServer::Connection::Identifier serverConnectionIdentifier)

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2017-12-13 20:20:18 UTC (rev 225864)
@@ -437,6 +437,29 @@
     return _processPool->processes().size();
 }
 
+- (size_t)_webPageContentProcessCount
+{
+    auto allWebProcesses = _processPool->processes();
+    auto* serviceWorkerProcess = _processPool->serviceWorkerProxy();
+    if (!serviceWorkerProcess)
+        return allWebProcesses.size();
+
+#if !ASSERT_DISABLED
+    bool serviceWorkerProcessWasFound = false;
+    for (auto& process : allWebProcesses) {
+        if (process == serviceWorkerProcess) {
+            serviceWorkerProcessWasFound = true;
+            break;
+        }
+    }
+
+    ASSERT(serviceWorkerProcessWasFound);
+    ASSERT(allWebProcesses.size() > 1);
+#endif
+
+    return allWebProcesses.size() - 1;
+}
+
 - (void)_preconnectToServer:(NSURL *)serverURL
 {
     _processPool->preconnectToServer(serverURL);

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	2017-12-13 20:20:18 UTC (rev 225864)
@@ -85,6 +85,9 @@
 - (size_t)_pluginProcessCount WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 - (void)_syncNetworkProcessCookies WK_API_AVAILABLE(macosx(10.13), ios(11.0));
 
+// Test only. Returns web processes running web pages (does not include web processes running service workers)
+- (size_t)_webPageContentProcessCount WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+
 // Test only. Should be called before any web content processes are launched.
 + (void)_forceGameControllerFramework WK_API_AVAILABLE(macosx(10.13), ios(11.0));
 

Modified: trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2017-12-13 20:20:18 UTC (rev 225864)
@@ -223,8 +223,13 @@
 #if ENABLE(SERVICE_WORKER)
 void StorageProcessProxy::establishWorkerContextConnectionToStorageProcess()
 {
-    m_processPool.establishWorkerContextConnectionToStorageProcess(*this);
+    m_processPool.establishWorkerContextConnectionToStorageProcess(*this, std::nullopt);
 }
+
+void StorageProcessProxy::establishWorkerContextConnectionToStorageProcessForExplicitSession(PAL::SessionID sessionID)
+{
+    m_processPool.establishWorkerContextConnectionToStorageProcess(*this, sessionID);
+}
 #endif
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h	2017-12-13 20:20:18 UTC (rev 225864)
@@ -81,6 +81,7 @@
 #endif
 #if ENABLE(SERVICE_WORKER)
     void establishWorkerContextConnectionToStorageProcess();
+    void establishWorkerContextConnectionToStorageProcessForExplicitSession(PAL::SessionID);
 #endif
 
     // ProcessLauncher::Client

Modified: trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.messages.in (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.messages.in	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.messages.in	2017-12-13 20:20:18 UTC (rev 225864)
@@ -33,5 +33,6 @@
 
 #if ENABLE(SERVICE_WORKER)
     EstablishWorkerContextConnectionToStorageProcess()
+    EstablishWorkerContextConnectionToStorageProcessForExplicitSession(PAL::SessionID explicitSession)
 #endif
 }

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2017-12-13 20:20:18 UTC (rev 225864)
@@ -589,7 +589,7 @@
 }
 
 #if ENABLE(SERVICE_WORKER)
-void WebProcessPool::establishWorkerContextConnectionToStorageProcess(StorageProcessProxy& proxy)
+void WebProcessPool::establishWorkerContextConnectionToStorageProcess(StorageProcessProxy& proxy, std::optional<PAL::SessionID> sessionID)
 {
     ASSERT_UNUSED(proxy, &proxy == m_storageProcess);
 
@@ -596,13 +596,22 @@
     if (m_serviceWorkerProcess)
         return;
 
-    if (!m_websiteDataStore)
-        m_websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
+    WebsiteDataStore* websiteDataStore = nullptr;
+    if (sessionID)
+        websiteDataStore = WebsiteDataStore::existingDataStoreForSessionID(*sessionID);
 
-    auto serviceWorkerProcessProxy = ServiceWorkerProcessProxy::create(*this, m_websiteDataStore->websiteDataStore());
+    if (!websiteDataStore) {
+        if (!m_websiteDataStore)
+            m_websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
+        websiteDataStore = &m_websiteDataStore->websiteDataStore();
+    }
+
+    auto serviceWorkerProcessProxy = ServiceWorkerProcessProxy::create(*this, *websiteDataStore);
     m_serviceWorkerProcess = serviceWorkerProcessProxy.ptr();
+
     updateProcessAssertions();
-    initializeNewWebProcess(serviceWorkerProcessProxy.get(), m_websiteDataStore->websiteDataStore());
+    initializeNewWebProcess(serviceWorkerProcessProxy.get(), *websiteDataStore);
+
     m_processes.append(WTFMove(serviceWorkerProcessProxy));
 
     m_serviceWorkerProcess->start(m_defaultPageGroup->preferences().store());

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.h	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h	2017-12-13 20:20:18 UTC (rev 225864)
@@ -323,7 +323,7 @@
     void getStorageProcessConnection(bool isServiceWorkerProcess, Ref<Messages::WebProcessProxy::GetStorageProcessConnection::DelayedReply>&&);
     void storageProcessCrashed(StorageProcessProxy*);
 #if ENABLE(SERVICE_WORKER)
-    void establishWorkerContextConnectionToStorageProcess(StorageProcessProxy&);
+    void establishWorkerContextConnectionToStorageProcess(StorageProcessProxy&, std::optional<PAL::SessionID>);
     bool isServiceWorker(uint64_t pageID) const { return m_serviceWorkerProcess && m_serviceWorkerProcess->pageID() == pageID; }
     ServiceWorkerProcessProxy* serviceWorkerProxy() const { return m_serviceWorkerProcess; }
     void setAllowsAnySSLCertificateForServiceWorker(bool allows) { m_allowsAnySSLCertificateForServiceWorker = allows; }

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-12-13 20:20:18 UTC (rev 225864)
@@ -55,6 +55,13 @@
 
 namespace WebKit {
 
+static HashMap<PAL::SessionID, WebsiteDataStore*>& allDataStores()
+{
+    RELEASE_ASSERT(isMainThread());
+    static NeverDestroyed<HashMap<PAL::SessionID, WebsiteDataStore*>> map;
+    return map;
+}
+
 Ref<WebsiteDataStore> WebsiteDataStore::createNonPersistent()
 {
     return adoptRef(*new WebsiteDataStore(PAL::SessionID::generateEphemeralSessionID()));
@@ -71,6 +78,9 @@
     , m_storageManager(StorageManager::create(m_configuration.localStorageDirectory))
     , m_queue(WorkQueue::create("com.apple.WebKit.WebsiteDataStore"))
 {
+    auto result = allDataStores().add(sessionID, this);
+    ASSERT_UNUSED(result, result.isNewEntry);
+
     platformInitialize();
 }
 
@@ -79,11 +89,17 @@
     , m_configuration()
     , m_queue(WorkQueue::create("com.apple.WebKit.WebsiteDataStore"))
 {
+    auto result = allDataStores().add(sessionID, this);
+    ASSERT_UNUSED(result, result.isNewEntry);
+
     platformInitialize();
 }
 
 WebsiteDataStore::~WebsiteDataStore()
 {
+    ASSERT(allDataStores().get(m_sessionID) == this);
+    allDataStores().remove(m_sessionID);
+
     platformDestroy();
 
     if (m_sessionID.isValid() && m_sessionID != PAL::SessionID::defaultSessionID()) {
@@ -92,6 +108,11 @@
     }
 }
 
+WebsiteDataStore* WebsiteDataStore::existingDataStoreForSessionID(PAL::SessionID sessionID)
+{
+    return allDataStores().get(sessionID);
+}
+
 WebProcessPool* WebsiteDataStore::processPoolForCookieStorageOperations()
 {
     auto pools = processPools(1, false);

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h (225863 => 225864)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	2017-12-13 20:20:18 UTC (rev 225864)
@@ -91,6 +91,8 @@
     static Ref<WebsiteDataStore> create(Configuration, PAL::SessionID);
     virtual ~WebsiteDataStore();
 
+    static WebsiteDataStore* existingDataStoreForSessionID(PAL::SessionID);
+
     bool isPersistent() const { return !m_sessionID.isEphemeral(); }
     PAL::SessionID sessionID() const { return m_sessionID; }
 

Modified: trunk/Tools/ChangeLog (225863 => 225864)


--- trunk/Tools/ChangeLog	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Tools/ChangeLog	2017-12-13 20:20:18 UTC (rev 225864)
@@ -1,3 +1,13 @@
+2017-12-13  Brady Eidson  <beid...@apple.com>
+
+        REGRESSION (r225789): API tests WKProcessPool.InitialWarmedProcessUsed and WebKit.WebsiteDataStoreCustomPaths are failing.
+        https://bugs.webkit.org/show_bug.cgi?id=180722
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm:
+        (TEST): Call a new SPI to get the count of WebProcesses hosting WebPages.
+
 2017-12-13  Mark Lam  <mark....@apple.com>
 
         Fill out some Poisoned APIs, fix some bugs, and add some tests.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm (225863 => 225864)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm	2017-12-13 20:19:08 UTC (rev 225863)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm	2017-12-13 20:20:18 UTC (rev 225864)
@@ -48,7 +48,7 @@
     [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:loadableURL]]];
     [webView _test_waitForDidFinishNavigation];
 
-    EXPECT_EQ([pool _webProcessCount], static_cast<size_t>(1));
+    EXPECT_EQ([pool _webPageContentProcessCount], static_cast<size_t>(1));
 }
 
 #endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to