WebKit.WebsiteDataStoreCustomPaths API test is failing when enabling process prewarming

https://bugs.webkit.org/show_bug.cgi?id=191638

Reviewed by Alex Christensen.

Source/WebKit:

WebProcessPool::prewarmProcess() should not create the default WebSite Data Store if it
does not exist yet. This is bad for memory consumption and it is what was causing this
API test to fail.

WebProcessPool::prewarmProcess() now tries to use the following data stores in this
order of preference:
1. WebProcessPool::m_websiteDataStore (aka this process pool's primary data store)
2. The data store of the last WebProcessProxy that was created
3. The default data store if it exists

If no suitable data store is found, we cancel the process prewarming and log a console
message instead.

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::prewarmProcess):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
(runWebsiteDataStoreCustomPaths):
(TEST):

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

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index f99d08d..f48b40f 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,26 @@
+2018-11-14  Chris Dumez  <cdumez@apple.com>
+
+        WebKit.WebsiteDataStoreCustomPaths API test is failing when enabling process prewarming
+        https://bugs.webkit.org/show_bug.cgi?id=191638
+
+        Reviewed by Alex Christensen.
+
+        WebProcessPool::prewarmProcess() should not create the default WebSite Data Store if it
+        does not exist yet. This is bad for memory consumption and it is what was causing this
+        API test to fail.
+
+        WebProcessPool::prewarmProcess() now tries to use the following data stores in this
+        order of preference:
+        1. WebProcessPool::m_websiteDataStore (aka this process pool's primary data store)
+        2. The data store of the last WebProcessProxy that was created
+        3. The default data store if it exists
+
+        If no suitable data store is found, we cancel the process prewarming and log a console
+        message instead.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::prewarmProcess):
+
 2018-11-14  Timothy Hatcher  <timothy@apple.com>
 
         Enabled dark mode CSS support by default.

diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
index c82afcf..d92f42e 100644 (file)
--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
@@ -955,12 +955,21 @@ void WebProcessPool::prewarmProcess()
     if (m_prewarmedProcess)
         return;
 
-    auto* websiteDataStore = m_websiteDataStore.get();
-    if (!websiteDataStore)
-        websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
+    auto* websiteDataStore = m_websiteDataStore ? &m_websiteDataStore->websiteDataStore() : nullptr;
+    if (!websiteDataStore) {
+        if (!m_processes.isEmpty())
+            websiteDataStore = &m_processes.last()->websiteDataStore();
+        else if (API::WebsiteDataStore::defaultDataStoreExists())
+            websiteDataStore = &API::WebsiteDataStore::defaultDataStore()->websiteDataStore();
+        else {
+            RELEASE_LOG(PerformanceLogging, "Unable to prewarming a WebProcess because we could not find a usable data store");
+            return;
+        }
+    }
+    ASSERT(websiteDataStore);
 
     RELEASE_LOG(PerformanceLogging, "Prewarming a WebProcess for performance");
-    createNewWebProcess(websiteDataStore->websiteDataStore(), WebProcessProxy::IsPrewarmed::Yes);
+    createNewWebProcess(*websiteDataStore, WebProcessProxy::IsPrewarmed::Yes);
 }
 
 void WebProcessPool::enableProcessTermination()

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 8c535c7..b764caa 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,16 @@
+2018-11-14  Chris Dumez  <cdumez@apple.com>
+
+        WebKit.WebsiteDataStoreCustomPaths API test is failing when enabling process prewarming
+        https://bugs.webkit.org/show_bug.cgi?id=191638
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
+        (runWebsiteDataStoreCustomPaths):
+        (TEST):
+
 2018-11-14  Alex Christensen  <achristensen@webkit.org>
 
         Add SPI to show a safe browsing warning

diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm
index 9975ca7..f1d8aa5 100644 (file)
--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm
+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm
@@ -73,8 +73,14 @@ static WKScriptMessage *getNextMessage()
     return [[scriptMessages.takeFirst() retain] autorelease];
 }
 
-TEST(WebKit, WebsiteDataStoreCustomPaths)
+enum class ShouldEnableProcessPrewarming { No, Yes };
+
+static void runWebsiteDataStoreCustomPaths(ShouldEnableProcessPrewarming shouldEnableProcessPrewarming)
 {
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = shouldEnableProcessPrewarming == ShouldEnableProcessPrewarming::Yes ? YES : NO;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
     RetainPtr<WebsiteDataStoreCustomPathsMessageHandler> handler = adoptNS([[WebsiteDataStoreCustomPathsMessageHandler alloc] init]);
     RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     [[configuration userContentController] addScriptMessageHandler:handler.get() name:@"testHandler"];
@@ -118,6 +124,7 @@ TEST(WebKit, WebsiteDataStoreCustomPaths)
     websiteDataStoreConfiguration.get()._resourceLoadStatisticsDirectory = resourceLoadStatisticsPath;
 
     configuration.get().websiteDataStore = [[[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()] autorelease];
+    configuration.get().processPool = processPool.get();
 
     RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
     [webView setNavigationDelegate:handler.get()];
@@ -177,8 +184,6 @@ TEST(WebKit, WebsiteDataStoreCustomPaths)
     RetainPtr<NSURL> fileIDBPath = [idbPath URLByAppendingPathComponent:@"file__0"];
     EXPECT_TRUE([[NSFileManager defaultManager] fileExistsAtPath:fileIDBPath.get().path]);
 
-    // Data stores can't delete anything unless a WKProcessPool exists, so make sure the shared data store exists.
-    auto *processPool = [WKProcessPool _sharedProcessPool];
     RetainPtr<WKWebsiteDataStore> dataStore = [[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()];
     RetainPtr<NSSet> types = adoptNS([[NSSet alloc] initWithObjects:WKWebsiteDataTypeIndexedDBDatabases, nil]);
 
@@ -186,17 +191,17 @@ TEST(WebKit, WebsiteDataStoreCustomPaths)
     RetainPtr<NSURL> url1 = [[NSBundle mainBundle] URLForResource:@"IndexedDB" withExtension:@"sqlite3" subdirectory:@"TestWebKitAPI.resources"];
     RetainPtr<NSURL> url2 = [[NSBundle mainBundle] URLForResource:@"IndexedDB" withExtension:@"sqlite3-shm" subdirectory:@"TestWebKitAPI.resources"];
     RetainPtr<NSURL> url3 = [[NSBundle mainBundle] URLForResource:@"IndexedDB" withExtension:@"sqlite3-wal" subdirectory:@"TestWebKitAPI.resources"];
-    
+
     RetainPtr<NSURL> frameIDBPath = [[fileIDBPath URLByAppendingPathComponent:@"https_apple.com_0"] URLByAppendingPathComponent:@"WebsiteDataStoreCustomPaths"];
     [[NSFileManager defaultManager] createDirectoryAtURL:frameIDBPath.get() withIntermediateDirectories:YES attributes:nil error:nil];
-    
+
     [[NSFileManager defaultManager] copyItemAtURL:url1.get() toURL:[frameIDBPath.get() URLByAppendingPathComponent:@"IndexedDB.sqlite3"] error:nil];
     [[NSFileManager defaultManager] copyItemAtURL:url2.get() toURL:[frameIDBPath.get() URLByAppendingPathComponent:@"IndexedDB.sqlite3-shm"] error:nil];
     [[NSFileManager defaultManager] copyItemAtURL:url3.get() toURL:[frameIDBPath.get() URLByAppendingPathComponent:@"IndexedDB.sqlite3-wal"] error:nil];
-    
+
     RetainPtr<NSURL> frameIDBPath2 = [[fileIDBPath URLByAppendingPathComponent:@"https_webkit.org_0"] URLByAppendingPathComponent:@"WebsiteDataStoreCustomPaths"];
     [[NSFileManager defaultManager] createDirectoryAtURL:frameIDBPath2.get() withIntermediateDirectories:YES attributes:nil error:nil];
-    
+
     [[NSFileManager defaultManager] copyItemAtURL:url1.get() toURL:[frameIDBPath2.get() URLByAppendingPathComponent:@"IndexedDB.sqlite3"] error:nil];
     [[NSFileManager defaultManager] copyItemAtURL:url2.get() toURL:[frameIDBPath2.get() URLByAppendingPathComponent:@"IndexedDB.sqlite3-shm"] error:nil];
     [[NSFileManager defaultManager] copyItemAtURL:url3.get() toURL:[frameIDBPath2.get() URLByAppendingPathComponent:@"IndexedDB.sqlite3-wal"] error:nil];
@@ -262,6 +267,16 @@ TEST(WebKit, WebsiteDataStoreCustomPaths)
     EXPECT_FALSE([WKWebsiteDataStore _defaultDataStoreExists]);
 }
 
+TEST(WebKit, WebsiteDataStoreCustomPathsWithoutPrewarming)
+{
+    runWebsiteDataStoreCustomPaths(ShouldEnableProcessPrewarming::No);
+}
+
+TEST(WebKit, WebsiteDataStoreCustomPathsWithPrewarming)
+{
+    runWebsiteDataStoreCustomPaths(ShouldEnableProcessPrewarming::Yes);
+}
+
 TEST(WebKit, CustomDataStorePathsVersusCompletionHandlers)
 {
     // Copy the baked database files to the database directory