Title: [225935] trunk
Revision
225935
Author
[email protected]
Date
2017-12-14 14:57:14 -0800 (Thu, 14 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):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (225934 => 225935)


--- trunk/Source/WebKit/ChangeLog	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/ChangeLog	2017-12-14 22:57:14 UTC (rev 225935)
@@ -1,3 +1,44 @@
+2017-12-14  Brady Eidson  <[email protected]>
+
+        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-14  John Wilander  <[email protected]>
 
         Storage Access API: Implement frame-specific access in the document.cookie layer

Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.cpp (225934 => 225935)


--- trunk/Source/WebKit/StorageProcess/StorageProcess.cpp	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.cpp	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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.
@@ -424,13 +424,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 (225934 => 225935)


--- trunk/Source/WebKit/StorageProcess/StorageProcess.h	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.h	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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 (225934 => 225935)


--- trunk/Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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 (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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 (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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 (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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 (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.h	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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 (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.messages.in	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/Storage/StorageProcessProxy.messages.in	2017-12-14 22:57:14 UTC (rev 225935)
@@ -33,5 +33,6 @@
 
 #if ENABLE(SERVICE_WORKER)
     EstablishWorkerContextConnectionToStorageProcess()
+    EstablishWorkerContextConnectionToStorageProcessForExplicitSession(PAL::SessionID explicitSession)
 #endif
 }

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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::existingNonDefaultDataStoreForSessionID(*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 (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.h	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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 (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2017-12-14 22:57:14 UTC (rev 225935)
@@ -55,6 +55,13 @@
 
 namespace WebKit {
 
+static HashMap<PAL::SessionID, WebsiteDataStore*>& nonDefaultDataStores()
+{
+    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,11 @@
     , m_storageManager(StorageManager::create(m_configuration.localStorageDirectory))
     , m_queue(WorkQueue::create("com.apple.WebKit.WebsiteDataStore"))
 {
+    if (m_sessionID != PAL::SessionID::defaultSessionID()) {
+        auto result = nonDefaultDataStores().add(sessionID, this);
+        ASSERT_UNUSED(result, result.isNewEntry);
+    }
+
     platformInitialize();
 }
 
@@ -79,11 +91,21 @@
     , m_configuration()
     , m_queue(WorkQueue::create("com.apple.WebKit.WebsiteDataStore"))
 {
+    if (m_sessionID != PAL::SessionID::defaultSessionID()) {
+        auto result = nonDefaultDataStores().add(sessionID, this);
+        ASSERT_UNUSED(result, result.isNewEntry);
+    }
+    
     platformInitialize();
 }
 
 WebsiteDataStore::~WebsiteDataStore()
 {
+    if (m_sessionID != PAL::SessionID::defaultSessionID()) {
+        ASSERT(nonDefaultDataStores().get(m_sessionID) == this);
+        nonDefaultDataStores().remove(m_sessionID);
+    }
+
     platformDestroy();
 
     if (m_sessionID.isValid() && m_sessionID != PAL::SessionID::defaultSessionID()) {
@@ -92,6 +114,11 @@
     }
 }
 
+WebsiteDataStore* WebsiteDataStore::existingNonDefaultDataStoreForSessionID(PAL::SessionID sessionID)
+{
+    return nonDefaultDataStores().get(sessionID);
+}
+
 WebProcessPool* WebsiteDataStore::processPoolForCookieStorageOperations()
 {
     auto pools = processPools(1, false);

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h (225934 => 225935)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	2017-12-14 22:57:14 UTC (rev 225935)
@@ -91,6 +91,8 @@
     static Ref<WebsiteDataStore> create(Configuration, PAL::SessionID);
     virtual ~WebsiteDataStore();
 
+    static WebsiteDataStore* existingNonDefaultDataStoreForSessionID(PAL::SessionID);
+
     bool isPersistent() const { return !m_sessionID.isEphemeral(); }
     PAL::SessionID sessionID() const { return m_sessionID; }
 

Modified: trunk/Tools/ChangeLog (225934 => 225935)


--- trunk/Tools/ChangeLog	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Tools/ChangeLog	2017-12-14 22:57:14 UTC (rev 225935)
@@ -1,3 +1,13 @@
+2017-12-14  Brady Eidson  <[email protected]>
+
+        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):
+
 2017-12-14  Alex Christensen  <[email protected]>
 
         Fix Mac CMake build

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm (225934 => 225935)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm	2017-12-14 22:39:48 UTC (rev 225934)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm	2017-12-14 22:57:14 UTC (rev 225935)
@@ -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
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to