Changeset 250351 in webkit

Timestamp:

Sep 25, 2019 12:16:36 PM (5 years ago)

Author:

achristensen@apple.com

Message:

Replace WebsiteDataStoreParameters::privateSessionParameters with re-initializing all sessions immediately upon NetworkProcess resumption
​https://bugs.webkit.org/show_bug.cgi?id=202211

Reviewed by Tim Horton.

Source/WebKit:

Re-adding an ephemeral session after a NetworkProcess crash based on guessing that all its parameters are empty was added in r227590 with a test.
That test passes even when that re-adding code is removed because we re-add all sessions with parameters from the UIProcess when we restart a NetworkProcess.
I move the addition of non-default sessions to the initialization message instead of messages after the initialization message to remove race conditions that
might cause loads to happen before the NetworkProcess has received its second message from the UIProcess. I also add a unit test to verify that session
resumption also works with non-default persistent WebsiteDataStores.

• NetworkProcess/NetworkProcess.cpp:

(WebKit::NetworkProcess::initializeNetworkProcess):

• NetworkProcess/NetworkProcessCreationParameters.cpp:

(WebKit::NetworkProcessCreationParameters::encode const):
(WebKit::NetworkProcessCreationParameters::decode):

• NetworkProcess/NetworkProcessCreationParameters.h:
• NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::startNetworkLoad):

• NetworkProcess/NetworkSessionCreationParameters.cpp:

(WebKit::NetworkSessionCreationParameters::privateSessionParameters): Deleted.

• NetworkProcess/NetworkSessionCreationParameters.h:
• Shared/WebsiteDataStoreParameters.cpp:

(WebKit::WebsiteDataStoreParameters::privateSessionParameters): Deleted.

• Shared/WebsiteDataStoreParameters.h:

(WebKit::WebsiteDataStoreParameters::legacyPrivateSessionParameters): Deleted.

• UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::ensureNetworkProcess):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm:

(checkRecoveryAfterCrash):
(TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcess.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp (modified) (2 diffs)
• Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.h (modified) (1 diff)
• Source/WebKit/Shared/WebsiteDataStoreParameters.cpp (modified) (1 diff)
• Source/WebKit/Shared/WebsiteDataStoreParameters.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm (modified) (4 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-09-25  Alex Christensen  <achristensen@webkit.org>
+
+        Replace WebsiteDataStoreParameters::privateSessionParameters with re-initializing all sessions immediately upon NetworkProcess resumption
+        https://bugs.webkit.org/show_bug.cgi?id=202211
+
+        Reviewed by Tim Horton.
+
+        Re-adding an ephemeral session after a NetworkProcess crash based on guessing that all its parameters are empty was added in r227590 with a test.
+        That test passes even when that re-adding code is removed because we re-add all sessions with parameters from the UIProcess when we restart a NetworkProcess.
+        I move the addition of non-default sessions to the initialization message instead of messages after the initialization message to remove race conditions that
+        might cause loads to happen before the NetworkProcess has received its second message from the UIProcess.  I also add a unit test to verify that session
+        resumption also works with non-default persistent WebsiteDataStores.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::initializeNetworkProcess):
+        * NetworkProcess/NetworkProcessCreationParameters.cpp:
+        (WebKit::NetworkProcessCreationParameters::encode const):
+        (WebKit::NetworkProcessCreationParameters::decode):
+        * NetworkProcess/NetworkProcessCreationParameters.h:
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::startNetworkLoad):
+        * NetworkProcess/NetworkSessionCreationParameters.cpp:
+        (WebKit::NetworkSessionCreationParameters::privateSessionParameters): Deleted.
+        * NetworkProcess/NetworkSessionCreationParameters.h:
+        * Shared/WebsiteDataStoreParameters.cpp:
+        (WebKit::WebsiteDataStoreParameters::privateSessionParameters): Deleted.
+        * Shared/WebsiteDataStoreParameters.h:
+        (WebKit::WebsiteDataStoreParameters::legacyPrivateSessionParameters): Deleted.
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::ensureNetworkProcess):
+
 2019-09-25  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp

@@ -323 +323 @@
     auto sessionID = parameters.defaultDataStoreParameters.networkSessionParameters.sessionID;
     setSession(sessionID, NetworkSession::create(*this, WTFMove(parameters.defaultDataStoreParameters.networkSessionParameters)));
+    for (auto&& parameters : WTFMove(parameters.nonDefaultDataStoreParameters))
+        addWebsiteDataStore(WTFMove(parameters));
 
 #if ENABLE(INDEXED_DATABASE)

trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp

@@ -67 +67 @@
 #endif
     encoder << defaultDataStoreParameters;
+    encoder << nonDefaultDataStoreParameters;
 #if USE(SOUP)
     encoder.encodeEnum(cookieAcceptPolicy);
@@ -156 +157 @@
     result.defaultDataStoreParameters = WTFMove(*defaultDataStoreParameters);
 
+    Optional<Vector<WebsiteDataStoreParameters>> nonDefaultDataStoreParameters;
+    decoder >> nonDefaultDataStoreParameters;
+    if (!nonDefaultDataStoreParameters)
+        return false;
+    result.nonDefaultDataStoreParameters = WTFMove(*nonDefaultDataStoreParameters);
+    
 #if USE(SOUP)
     if (!decoder.decodeEnum(result.cookieAcceptPolicy))

trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h

@@ -83 +83 @@
 
     WebsiteDataStoreParameters defaultDataStoreParameters;
+    Vector<WebsiteDataStoreParameters> nonDefaultDataStoreParameters;
     
 #if USE(SOUP)

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

@@ -303 +303 @@
 
     auto* networkSession = m_connection->networkSession();
-    if (!networkSession && sessionID().isEphemeral()) {
-        m_connection->networkProcess().addWebsiteDataStore(WebsiteDataStoreParameters::privateSessionParameters(sessionID()));
-        networkSession = m_connection->networkProcess().networkSession(sessionID());
-    }
     if (!networkSession) {
         WTFLogAlways("Attempted to create a NetworkLoad with a session (id=%" PRIu64 ") that does not exist.", sessionID().toUInt64());

trunk/Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.cpp

@@ -39 +39 @@
 namespace WebKit {
 
-NetworkSessionCreationParameters NetworkSessionCreationParameters::privateSessionParameters(const PAL::SessionID& sessionID)
-{
-    return {
-        sessionID
-        , { }
-        , AllowsCellularAccess::Yes
-#if PLATFORM(COCOA)
-        , { }
-        , { }
-        , { }
-        , false
-        , { }
-        , { }
-        , { }
-        , false
-#endif
-#if USE(SOUP)
-        , { }
-        , SoupCookiePersistentStorageType::Text
-#endif
-#if USE(CURL)
-        , { }
-        , { }
-#endif
-        , { }
-        , { }
-        , false
-        , false
-        , { }
-        , { }
-        , { }
-        , { }
-        , { }
-        , { }
-        , { }
-        , { }
-        , { }
-    };
-}
-
 void NetworkSessionCreationParameters::encode(IPC::Encoder& encoder) const
 {

trunk/Source/WebKit/NetworkProcess/NetworkSessionCreationParameters.h

@@ -58 +58 @@
     void encode(IPC::Encoder&) const;
     static Optional<NetworkSessionCreationParameters> decode(IPC::Decoder&);
-    static NetworkSessionCreationParameters privateSessionParameters(const PAL::SessionID&);
     
     PAL::SessionID sessionID { PAL::SessionID::defaultSessionID() };

trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.cpp

@@ -151 +151 @@
 }
 
-WebsiteDataStoreParameters WebsiteDataStoreParameters::privateSessionParameters(PAL::SessionID sessionID)
-{
-    ASSERT(sessionID.isEphemeral());
-    return { { }, { }, { }, NetworkSessionCreationParameters::privateSessionParameters(sessionID)
-#if ENABLE(INDEXED_DATABASE)
-        , { }, { }
-#if PLATFORM(IOS_FAMILY)
-        , { }
-#endif
-#endif
-#if ENABLE(SERVICE_WORKER)
-        , { }, { }
-#endif
-        , { }, { }
-    };
-}
-
 } // namespace WebKit

trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.h

@@ -47 +47 @@
     ~WebsiteDataStoreParameters();
 
-    static WebsiteDataStoreParameters legacyPrivateSessionParameters() { return privateSessionParameters(PAL::SessionID::legacyPrivateSessionID()); }
-    static WebsiteDataStoreParameters privateSessionParameters(PAL::SessionID);
-
     void encode(IPC::Encoder&) const;
     static Optional<WebsiteDataStoreParameters> decode(IPC::Decoder&);

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -634 +634 @@
     platformInitializeNetworkProcess(parameters);
 
+    // Make sure the network process knows about all the sessions that have been registered before it started.
+    for (auto& sessionID : m_sessionToPageIDsMap.keys()) {
+        if (auto* websiteDataStore = WebsiteDataStore::existingNonDefaultDataStoreForSessionID(sessionID))
+            parameters.nonDefaultDataStoreParameters.append(websiteDataStore->parameters());
+    }
+
     // Initialize the network process.
     networkProcess->send(Messages::NetworkProcess::InitializeNetworkProcess(parameters), 0);
@@ -649 +655 @@
         networkProcess->addSession(makeRef(*withWebsiteDataStore));
         withWebsiteDataStore->clearPendingCookies();
-    }
-
-    // Make sure the network process knows about all the sessions that have been registered before it started.
-    for (auto& sessionID : m_sessionToPageIDsMap.keys()) {
-        if (auto* websiteDataStore = WebsiteDataStore::existingNonDefaultDataStoreForSessionID(sessionID))
-            networkProcess->addSession(*websiteDataStore);
     }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2019-09-25  Alex Christensen  <achristensen@webkit.org>
+
+        Replace WebsiteDataStoreParameters::privateSessionParameters with re-initializing all sessions immediately upon NetworkProcess resumption
+        https://bugs.webkit.org/show_bug.cgi?id=202211
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm:
+        (checkRecoveryAfterCrash):
+        (TEST):
+
 2019-09-25  Aakash Jain  <aakash_jain@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm

@@ -30 +30 @@
 #import "TestWKWebView.h"
 #import <WebKit/WKProcessPoolPrivate.h>
+#import <WebKit/WKWebsiteDataStorePrivate.h>
+#import <WebKit/_WKWebsiteDataStoreConfiguration.h>
 #import <wtf/RetainPtr.h>
 
@@ -56 +58 @@
 @end
 
-TEST(WebKit, NetworkProcessCrashNonPersistentDataStore)
+static void checkRecoveryAfterCrash(WKWebsiteDataStore *dataStore)
 {
     NSURL *simple = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
@@ -62 +64 @@
     
     auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
-    [configuration setWebsiteDataStore:[WKWebsiteDataStore nonPersistentDataStore]];
+    [configuration setWebsiteDataStore:dataStore];
     auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
     auto delegate = adoptNS([[CrashDelegate alloc] init]);
@@ -75 +77 @@
     TestWebKitAPI::Util::run(&done);
 }
+
+TEST(WebKit, NetworkProcessCrashNonPersistentDataStore)
+{
+    checkRecoveryAfterCrash([WKWebsiteDataStore nonPersistentDataStore]);
+}
+
+TEST(WebKit, NetworkProcessCrashNonDefaultPersistentDataStore)
+{
+    checkRecoveryAfterCrash([[[WKWebsiteDataStore alloc] _initWithConfiguration:[[[_WKWebsiteDataStoreConfiguration alloc] init] autorelease]] autorelease]);
+}