Changeset 274186 in webkit

Timestamp:

Mar 9, 2021, 4:40:08 PM (6 years ago)

Author:

achristensen@apple.com

Message:

REGRESSION (r272376): [iOS] ASSERTION FAILED: sessionID.isEphemeral()

!path.isEmpty() in WebKit::NetworkProcess::swServerForSession

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

Reviewed by Geoff Garen.

Because NetworkProcess::CreateNetworkConnectionToWebProcess is sent with SendOption::DispatchMessageEvenWhenWaitingForSyncReply, it is possible
for two messages of type NetworkProcess::AddWebsiteDataStore and two messages of type NetworkProcess::CreateNetworkConnectionToWebProcess to be in the queue,
but the second NetworkProcess::CreateNetworkConnectionToWebProcess jumps to the front of the line while the UI process is waiting for the reply to the first.
Then, when calling NetworkProcess::swServerForSession we expect the session's parameters to have been initialized in the network process already, but we cut
ahead of the parameter initialization message. This is a realistically rare condition that can only be hit when using SPI, but it is hit in the
ResourceLoadStatistics.StoreSuspension API test. To fix this, we send the WebsiteDataStoreParameters from each WebsiteDataStore in the NetworkProcessCreationParameters.
To avoid doing extra work, we introduce an early return in NetworkProcessProxy::addSession if we have already added parameters from this session to the network process.

• NetworkProcess/NetworkProcess.cpp:

(WebKit::NetworkProcess::initializeNetworkProcess):
(WebKit::NetworkProcess::addSessionStorageQuotaManager):

• NetworkProcess/NetworkProcessCreationParameters.cpp:

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

• NetworkProcess/NetworkProcessCreationParameters.h:
• UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::NetworkProcessProxy::sendCreationParametersToNewProcess):
(WebKit::NetworkProcessProxy::addSession):

• UIProcess/Network/NetworkProcessProxy.h:
• UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::parametersFromEachWebsiteDataStore):

• UIProcess/WebsiteData/WebsiteDataStore.h:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/NetworkProcess.cpp (modified) (2 diffs)
• NetworkProcess/NetworkProcessCreationParameters.cpp (modified) (4 diffs)
• NetworkProcess/NetworkProcessCreationParameters.h (modified) (2 diffs)
• UIProcess/Network/NetworkProcessProxy.cpp (modified) (2 diffs)
• UIProcess/Network/NetworkProcessProxy.h (modified) (1 diff)
• UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (2 diffs)
• UIProcess/WebsiteData/WebsiteDataStore.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-03-09  Alex Christensen  <achristensen@webkit.org>
+
+        REGRESSION (r272376): [iOS] ASSERTION FAILED: sessionID.isEphemeral() || !path.isEmpty() in WebKit::NetworkProcess::swServerForSession
+        https://bugs.webkit.org/show_bug.cgi?id=222713
+
+        Reviewed by Geoff Garen.
+
+        Because NetworkProcess::CreateNetworkConnectionToWebProcess is sent with SendOption::DispatchMessageEvenWhenWaitingForSyncReply, it is possible
+        for two messages of type NetworkProcess::AddWebsiteDataStore and two messages of type NetworkProcess::CreateNetworkConnectionToWebProcess to be in the queue,
+        but the second NetworkProcess::CreateNetworkConnectionToWebProcess jumps to the front of the line while the UI process is waiting for the reply to the first.
+        Then, when calling NetworkProcess::swServerForSession we expect the session's parameters to have been initialized in the network process already, but we cut
+        ahead of the parameter initialization message.  This is a realistically rare condition that can only be hit when using SPI, but it is hit in the
+        ResourceLoadStatistics.StoreSuspension API test.  To fix this, we send the WebsiteDataStoreParameters from each WebsiteDataStore in the NetworkProcessCreationParameters.
+        To avoid doing extra work, we introduce an early return in NetworkProcessProxy::addSession if we have already added parameters from this session to the network process.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::initializeNetworkProcess):
+        (WebKit::NetworkProcess::addSessionStorageQuotaManager):
+        * NetworkProcess/NetworkProcessCreationParameters.cpp:
+        (WebKit::NetworkProcessCreationParameters::encode const):
+        (WebKit::NetworkProcessCreationParameters::decode):
+        * NetworkProcess/NetworkProcessCreationParameters.h:
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::sendCreationParametersToNewProcess):
+        (WebKit::NetworkProcessProxy::addSession):
+        * UIProcess/Network/NetworkProcessProxy.h:
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::parametersFromEachWebsiteDataStore):
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+
 2021-03-09  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp

@@ -354 +354 @@
         registerURLSchemeAsNoAccess(scheme);
     
+    for (auto&& websiteDataStoreParameters : WTFMove(parameters.websiteDataStoreParameters))
+        addWebsiteDataStore(WTFMove(websiteDataStoreParameters));
+
     RELEASE_LOG(Process, "%p - NetworkProcess::initializeNetworkProcess: Presenting processPID=%d", this, WebCore::presentingApplicationPID());
 }
@@ -431 +434 @@
     if (isNewEntry)
         SandboxExtension::consumePermanently(cacheRootPathHandle);
+    else
+        ASSERT_NOT_REACHED();
 }
 

trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp

@@ -29 +29 @@
 #include "ArgumentCoders.h"
 #include "WebCoreArgumentCoders.h"
+#include "WebsiteDataStoreParameters.h"
 
 #if PLATFORM(COCOA)
@@ -37 +38 @@
 
 NetworkProcessCreationParameters::NetworkProcessCreationParameters() = default;
+NetworkProcessCreationParameters::~NetworkProcessCreationParameters() = default;
+NetworkProcessCreationParameters::NetworkProcessCreationParameters(NetworkProcessCreationParameters&&) = default;
+NetworkProcessCreationParameters& NetworkProcessCreationParameters::operator=(NetworkProcessCreationParameters&&) = default;
 
 void NetworkProcessCreationParameters::encode(IPC::Encoder& encoder) const
@@ -69 +73 @@
     encoder << enablePrivateClickMeasurement;
     encoder << enablePrivateClickMeasurementDebugMode;
+    encoder << websiteDataStoreParameters;
 }
 
@@ -139 +144 @@
         return false;
 
+    Optional<Vector<WebsiteDataStoreParameters>> websiteDataStoreParameters;
+    decoder >> websiteDataStoreParameters;
+    if (!websiteDataStoreParameters)
+        return false;
+    result.websiteDataStoreParameters = WTFMove(*websiteDataStoreParameters);
+
     return true;
 }

trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h

@@ -44 +44 @@
 namespace WebKit {
 
+struct WebsiteDataStoreParameters;
+
 struct NetworkProcessCreationParameters {
     NetworkProcessCreationParameters();
+    NetworkProcessCreationParameters(NetworkProcessCreationParameters&&);
+    ~NetworkProcessCreationParameters();
+    NetworkProcessCreationParameters& operator=(NetworkProcessCreationParameters&&);
 
     void encode(IPC::Encoder&) const;
@@ -83 +88 @@
     bool enablePrivateClickMeasurement { true };
     bool enablePrivateClickMeasurementDebugMode { false };
+
+    Vector<WebsiteDataStoreParameters> websiteDataStoreParameters;
 };
 

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

@@ -159 +159 @@
     SandboxExtension::createHandleForTemporaryFile(emptyString(), SandboxExtension::Type::ReadWrite, parameters.tempDirectoryExtensionHandle);
 #endif
+    parameters.websiteDataStoreParameters = WebsiteDataStore::parametersFromEachWebsiteDataStore();
+    WebsiteDataStore::forEachWebsiteDataStore([this](auto& websiteDataStore) {
+        addSession(websiteDataStore, SendParametersToNetworkProcess::No);
+    });
     WebProcessPool::platformInitializeNetworkProcess(parameters);
     send(Messages::NetworkProcess::InitializeNetworkProcess(parameters), 0);
@@ -1328 +1332 @@
 }
 
-void NetworkProcessProxy::addSession(WebsiteDataStore& store)
-{
-    m_websiteDataStores.add(store);
-
-    if (canSendMessage())
+void NetworkProcessProxy::addSession(WebsiteDataStore& store, SendParametersToNetworkProcess sendParametersToNetworkProcess)
+{
+    auto addResult = m_websiteDataStores.add(store);
+    if (!addResult.isNewEntry)
+        return;
+
+    if (canSendMessage() && sendParametersToNetworkProcess == SendParametersToNetworkProcess::Yes)
         send(Messages::NetworkProcess::AddWebsiteDataStore { store.parameters() }, 0);
     auto sessionID = store.sessionID();

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h

@@ -222 +222 @@
 #endif
 
-    void addSession(WebsiteDataStore&);
+    enum class SendParametersToNetworkProcess : bool { No, Yes };
+    void addSession(WebsiteDataStore&, SendParametersToNetworkProcess);
     void removeSession(WebsiteDataStore&);
     

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

@@ -209 +209 @@
     if (!m_networkProcess) {
         m_networkProcess = networkProcessForSession(m_sessionID);
-        m_networkProcess->addSession(*this);
+        m_networkProcess->addSession(*this, NetworkProcessProxy::SendParametersToNetworkProcess::Yes);
     }
 
@@ -1911 +1911 @@
 }
 
+Vector<WebsiteDataStoreParameters> WebsiteDataStore::parametersFromEachWebsiteDataStore()
+{
+    Vector<WebsiteDataStoreParameters> parameters;
+    parameters.reserveInitialCapacity(allDataStores().size());
+    for (auto* dataStore : allDataStores().values())
+        parameters.uncheckedAppend(dataStore->parameters());
+    return parameters;
+}
+
 WebsiteDataStoreParameters WebsiteDataStore::parameters()
 {

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h

@@ -259 +259 @@
 
     WebsiteDataStoreParameters parameters();
+    static Vector<WebsiteDataStoreParameters> parametersFromEachWebsiteDataStore();
 
     void flushCookies(CompletionHandler<void()>&&);