REGRESSION (r225789): API tests WKProcessPool.InitialWarmedProcessUsed and WebKit...
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Dec 2017 22:57:14 +0000 (22:57 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Dec 2017 22:57:14 +0000 (22:57 +0000)
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):

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

15 files changed:
Source/WebKit/ChangeLog
Source/WebKit/StorageProcess/StorageProcess.cpp
Source/WebKit/StorageProcess/StorageProcess.h
Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp
Source/WebKit/UIProcess/Storage/StorageProcessProxy.h
Source/WebKit/UIProcess/Storage/StorageProcessProxy.messages.in
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/InitialWarmedProcessUsed.mm

index df4db33..bd45e2d 100644 (file)
@@ -1,3 +1,44 @@
+2017-12-14  Brady Eidson  <beidson@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-14  John Wilander  <wilander@apple.com>
 
         Storage Access API: Implement frame-specific access in the document.cookie layer
index 750805a..9e2590c 100644 (file)
@@ -111,7 +111,7 @@ void StorageProcess::connectionToContextProcessWasClosed()
         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 @@ WebSWServerToContextConnection* StorageProcess::globalServerToContextConnection(
     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)
index 86e8b95..71067a2 100644 (file)
@@ -89,7 +89,7 @@ public:
     // 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&);
index 3282a34..4670828 100644 (file)
@@ -171,7 +171,7 @@ void StorageToWebProcessConnection::establishSWServerConnection(SessionID sessio
     ASSERT_UNUSED(addResult, addResult.isNewEntry);
 
     if (!StorageProcess::singleton().globalServerToContextConnection())
-        StorageProcess::singleton().createServerToContextConnection();
+        StorageProcess::singleton().createServerToContextConnection(sessionID);
 }
 
 void StorageToWebProcessConnection::removeSWServerConnection(WebCore::SWServer::Connection::Identifier serverConnectionIdentifier)
index 7c947b0..a38cba2 100644 (file)
@@ -437,6 +437,29 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
     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);
index adde7c1..5233bc6 100644 (file)
@@ -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));
 
index 4c0ad60..5b52106 100644 (file)
@@ -223,7 +223,12 @@ void StorageProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Con
 #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
 
index 26dee87..edb0d22 100644 (file)
@@ -81,6 +81,7 @@ private:
 #endif
 #if ENABLE(SERVICE_WORKER)
     void establishWorkerContextConnectionToStorageProcess();
+    void establishWorkerContextConnectionToStorageProcessForExplicitSession(PAL::SessionID);
 #endif
 
     // ProcessLauncher::Client
index c212b22..9cb8203 100644 (file)
@@ -33,5 +33,6 @@ messages -> StorageProcessProxy LegacyReceiver {
 
 #if ENABLE(SERVICE_WORKER)
     EstablishWorkerContextConnectionToStorageProcess()
+    EstablishWorkerContextConnectionToStorageProcessForExplicitSession(PAL::SessionID explicitSession)
 #endif
 }
index cbd4548..891ab75 100644 (file)
@@ -589,20 +589,29 @@ void WebProcessPool::storageProcessCrashed(StorageProcessProxy* storageProcessPr
 }
 
 #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);
 
     if (m_serviceWorkerProcess)
         return;
 
-    if (!m_websiteDataStore)
-        m_websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
+    WebsiteDataStore* websiteDataStore = nullptr;
+    if (sessionID)
+        websiteDataStore = WebsiteDataStore::existingNonDefaultDataStoreForSessionID(*sessionID);
+
+    if (!websiteDataStore) {
+        if (!m_websiteDataStore)
+            m_websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
+        websiteDataStore = &m_websiteDataStore->websiteDataStore();
+    }
 
-    auto serviceWorkerProcessProxy = ServiceWorkerProcessProxy::create(*this, 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());
index 988c33c..fb14eee 100644 (file)
@@ -323,7 +323,7 @@ public:
     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; }
index 75ee8bf..793557c 100644 (file)
 
 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 @@ WebsiteDataStore::WebsiteDataStore(Configuration configuration, PAL::SessionID s
     , 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 @@ WebsiteDataStore::WebsiteDataStore(PAL::SessionID sessionID)
     , 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()
     }
 }
 
+WebsiteDataStore* WebsiteDataStore::existingNonDefaultDataStoreForSessionID(PAL::SessionID sessionID)
+{
+    return nonDefaultDataStores().get(sessionID);
+}
+
 WebProcessPool* WebsiteDataStore::processPoolForCookieStorageOperations()
 {
     auto pools = processPools(1, false);
index d1deec7..356d2bf 100644 (file)
@@ -91,6 +91,8 @@ public:
     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; }
 
index 275f0c1..e63f56a 100644 (file)
@@ -1,3 +1,13 @@
+2017-12-14  Brady Eidson  <beidson@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):
+
 2017-12-14  Alex Christensen  <achristensen@webkit.org>
 
         Fix Mac CMake build
index 43389be..ad37c96 100644 (file)
@@ -48,7 +48,7 @@ TEST(WKProcessPool, InitialWarmedProcessUsed)
     [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