REGRESSION (r225789): API tests WKProcessPool.InitialWarmedProcessUsed and WebKit...
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Dec 2017 20:20:18 +0000 (20:20 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Dec 2017 20:20:18 +0000 (20:20 +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): Call a new SPI to get the count of WebProcesses hosting WebPages.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225864 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 8f96a61..59bf707 100644 (file)
@@ -1,3 +1,44 @@
+2017-12-13  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-13  Per Arne Vollan  <pvollan@apple.com>
 
         REGRESSION(225597): Can't select a text box or web view on a page when VO is on.
index ed7d650..be11328 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.
@@ -427,13 +427,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 3482e74..8ec67ce 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 74f2553..d59b6ae 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::existingDataStoreForSessionID(*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..27ec260 100644 (file)
 
 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 @@ WebsiteDataStore::WebsiteDataStore(Configuration configuration, PAL::SessionID s
     , 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 @@ WebsiteDataStore::WebsiteDataStore(PAL::SessionID sessionID)
     , 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()
     }
 }
 
+WebsiteDataStore* WebsiteDataStore::existingDataStoreForSessionID(PAL::SessionID sessionID)
+{
+    return allDataStores().get(sessionID);
+}
+
 WebProcessPool* WebsiteDataStore::processPoolForCookieStorageOperations()
 {
     auto pools = processPools(1, false);
index d1deec7..f9522d1 100644 (file)
@@ -91,6 +91,8 @@ public:
     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; }
 
index 8b3eeeb..46d1797 100644 (file)
@@ -1,3 +1,13 @@
+2017-12-13  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): Call a new SPI to get the count of WebProcesses hosting WebPages.
+
 2017-12-13  Mark Lam  <mark.lam@apple.com>
 
         Fill out some Poisoned APIs, fix some bugs, and add some tests.
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