IndexedDB.IndexedDBTempFileSize API test times out with process prewarming enabled
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Nov 2018 22:01:31 +0000 (22:01 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Nov 2018 22:01:31 +0000 (22:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191671
<rdar://problem/46086062>

Reviewed by Alex Christensen.

Source/WebKit:

Enabling process prewarming caused IndexedDB.IndexedDBTempFileSize API to time out and print
the following line:
"Attempted to create a NetworkLoad with a session (id=2) that does not exist."

This actually identified a pre-existing bug with our handling of non-default data store.
Whenever a page starts using a data store, we call WebProcessPool::pageBeginUsingWebsiteDataStore()
which will call NetworkProcessProxy::addSession() if the network process was already started
to let the network process know about this non-default session. There are several issues with the
current model:
1. If the network process was not created yet when pageBeginUsingWebsiteDataStore() is called,
   then the network process will not know about the non-default session when it actually gets
   started later on. This is unlikely to happen in practice, except in case of network process
   crash because we create the network process as soon as we initialize the first WebProcessProxy.
2. Even if we successfuly managed to register the session with the network process proxy, we get
   in trouble if the network process crashes or is terminated later on as we do not re-register
   those sessions with the new network process.

To address these 2 issues, WebProcessPool::ensureNetworkProcess() now takes care of registering
all the non-default sessions (that are associated with this process pool) with the new network
process. The WebProcessPool knows about these sessions because it adds them to m_sessionToPagesMap
whenever WebProcessPool::pageBeginUsingWebsiteDataStore() is called, even if the network process
proxy was not created yet.

The reason the IndexedDB.IndexedDBTempFileSize API test was failing was because it did:
1. A load in a WebView V1 with a non-default session
2. Process prewarming kicked in after this load and would create a new WebProcessProxy.
3. Terminate the network process
4. Another load in a WebView V2 with the same non-defaut session, which would reuse the
  prewarmed process. Because the network process was terminated, constructing the new
  page would not register the session ID with the new network process when
  pageBeginUsingWebsiteDataStore() is called.
-> The load would hang because the new network process would not know about the
   non-default session when started later on.

The bug was previously hidden without process prewarming because step 4 would create a *new*
WebProcessProxy and WebProcessPool::initializeNewWebProcess() would call ensureNetworkProcess()
so that pageBeginUsingWebsiteDataStore() would successfuly register the session with the
network process later on.

I wrote a second API test (WebKit.DoLoadWithNonDefaultDataStoreAfterTerminatingNetworkProcess)
to demonstrate the pre-existing bug without process prewarming enabled.

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

Tools:

Add API test coverage.

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

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebProcessPool.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm

index c559fd4..64c4a14 100644 (file)
@@ -1,3 +1,56 @@
+2018-11-15  Chris Dumez  <cdumez@apple.com>
+
+        IndexedDB.IndexedDBTempFileSize API test times out with process prewarming enabled
+        https://bugs.webkit.org/show_bug.cgi?id=191671
+        <rdar://problem/46086062>
+
+        Reviewed by Alex Christensen.
+
+        Enabling process prewarming caused IndexedDB.IndexedDBTempFileSize API to time out and print
+        the following line:
+        "Attempted to create a NetworkLoad with a session (id=2) that does not exist."
+
+        This actually identified a pre-existing bug with our handling of non-default data store.
+        Whenever a page starts using a data store, we call WebProcessPool::pageBeginUsingWebsiteDataStore()
+        which will call NetworkProcessProxy::addSession() if the network process was already started
+        to let the network process know about this non-default session. There are several issues with the
+        current model:
+        1. If the network process was not created yet when pageBeginUsingWebsiteDataStore() is called,
+           then the network process will not know about the non-default session when it actually gets
+           started later on. This is unlikely to happen in practice, except in case of network process
+           crash because we create the network process as soon as we initialize the first WebProcessProxy.
+        2. Even if we successfuly managed to register the session with the network process proxy, we get
+           in trouble if the network process crashes or is terminated later on as we do not re-register
+           those sessions with the new network process.
+
+        To address these 2 issues, WebProcessPool::ensureNetworkProcess() now takes care of registering
+        all the non-default sessions (that are associated with this process pool) with the new network
+        process. The WebProcessPool knows about these sessions because it adds them to m_sessionToPagesMap
+        whenever WebProcessPool::pageBeginUsingWebsiteDataStore() is called, even if the network process
+        proxy was not created yet.
+
+        The reason the IndexedDB.IndexedDBTempFileSize API test was failing was because it did:
+        1. A load in a WebView V1 with a non-default session
+        2. Process prewarming kicked in after this load and would create a new WebProcessProxy.
+        3. Terminate the network process
+        4. Another load in a WebView V2 with the same non-defaut session, which would reuse the
+          prewarmed process. Because the network process was terminated, constructing the new
+          page would not register the session ID with the new network process when
+          pageBeginUsingWebsiteDataStore() is called.
+        -> The load would hang because the new network process would not know about the
+           non-default session when started later on.
+
+        The bug was previously hidden without process prewarming because step 4 would create a *new*
+        WebProcessProxy and WebProcessPool::initializeNewWebProcess() would call ensureNetworkProcess()
+        so that pageBeginUsingWebsiteDataStore() would successfuly register the session with the
+        network process later on.
+
+        I wrote a second API test (WebKit.DoLoadWithNonDefaultDataStoreAfterTerminatingNetworkProcess)
+        to demonstrate the pre-existing bug without process prewarming enabled.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::ensureNetworkProcess):
+
 2018-11-15  Vivek Seth  <v_seth@apple.com>
 
         Create feature flag (HTTPS_UPGRADE)
index 11711a7..9dcd061 100644 (file)
@@ -568,6 +568,12 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
         withWebsiteDataStore->clearPendingCookies();
     }
 
+    // Make sure the network process knowns about all the sessions that have been registered before it started.
+    for (auto& sessionID : m_sessionToPagesMap.keys()) {
+        if (auto* websiteDataStore = WebsiteDataStore::existingNonDefaultDataStoreForSessionID(sessionID))
+            m_networkProcess->addSession(*websiteDataStore);
+    }
+
     if (m_websiteDataStore)
         m_websiteDataStore->websiteDataStore().didCreateNetworkProcess();
 
index cbe1378..e1a269e 100644 (file)
@@ -1,3 +1,17 @@
+2018-11-15  Chris Dumez  <cdumez@apple.com>
+
+        IndexedDB.IndexedDBTempFileSize API test times out with process prewarming enabled
+        https://bugs.webkit.org/show_bug.cgi?id=191671
+        <rdar://problem/46086062>
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+        * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
+        (TEST):
+
 2018-11-15  Oriol Brufau  <obrufau@igalia.com>
 
         [css-logical] Implement flow-relative margin, padding and border shorthands
index a83fb03..7c34528 100644 (file)
@@ -2690,6 +2690,42 @@ TEST(ProcessSwap, SwapOnLoadHTMLString)
     done = false;
 }
 
+TEST(ProcessSwap, UsePrewarmedProcessAfterTerminatingNetworkProcess)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto websiteDataStoreConfiguration = adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    webViewConfiguration.get().websiteDataStore = [[[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()] autorelease];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    TestWebKitAPI::Util::spinRunLoop(1);
+
+    [processPool _terminateNetworkProcess];
+
+    auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    [webView2 setNavigationDelegate:delegate.get()];
+    request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView2 loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+}
+
 #if PLATFORM(MAC)
 
 TEST(ProcessSwap, GoBackToSuspendedPageWithMainFrameIDThatIsNotOne)
index f1d8aa5..a4f2468 100644 (file)
 
 #import "PlatformUtilities.h"
 #import "Test.h"
-#import <WebKit/WebKit.h>
-
+#import "TestNavigationDelegate.h"
 #import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WKUserContentControllerPrivate.h>
 #import <WebKit/WKWebViewConfigurationPrivate.h>
 #import <WebKit/WKWebViewPrivate.h>
 #import <WebKit/WKWebsiteDataStorePrivate.h>
+#import <WebKit/WebKit.h>
 #import <WebKit/_WKProcessPoolConfiguration.h>
 #import <WebKit/_WKUserStyleSheet.h>
 #import <WebKit/_WKWebsiteDataStoreConfiguration.h>
@@ -426,4 +426,26 @@ TEST(WebKit, WebsiteDataStoreEphemeral)
     EXPECT_FALSE([[NSFileManager defaultManager] fileExistsAtPath:defaultResourceLoadStatisticsFilePath.path]);
 }
 
+TEST(WebKit, DoLoadWithNonDefaultDataStoreAfterTerminatingNetworkProcess)
+{
+    auto websiteDataStoreConfiguration = adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    webViewConfiguration.get().websiteDataStore = [[[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()] autorelease];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+    [webView _test_waitForDidFinishNavigation];
+
+    TestWebKitAPI::Util::spinRunLoop(1);
+
+    [webViewConfiguration.get().processPool _terminateNetworkProcess];
+
+    request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+    [webView _test_waitForDidFinishNavigation];
+}
+
 #endif