[webkit-changes] [275047] trunk/Source/WebKit

Thu, 25 Mar 2021 10:36:52 -0700

Title: [275047] trunk/Source/WebKit
Revision
275047
Author
[email protected]
Date
2021-03-25 10:36:00 -0700 (Thu, 25 Mar 2021)

Log Message

REGRESSION (r272376): [iOS] ASSERTION FAILED: sessionID.isEphemeral() || !path.isEmpty() in WebKit::NetworkProcess::swServerForSession
https://bugs.webkit.org/show_bug.cgi?id=222713

Patch by Alex Christensen <[email protected]> on 2021-03-25
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:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (275046 => 275047)


--- trunk/Source/WebKit/ChangeLog	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/ChangeLog	2021-03-25 17:36:00 UTC (rev 275047)
@@ -1,3 +1,33 @@
+2021-03-25  Alex Christensen  <[email protected]>
+
+        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-25  John Wilander  <[email protected]>
 
         PCM: Rename attributeOn to attributionDestination and change the IDL data type of attributionSourceId to unsigned long

Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (275046 => 275047)


--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp	2021-03-25 17:36:00 UTC (rev 275047)
@@ -353,6 +353,9 @@
     for (auto& scheme : parameters.urlSchemesRegisteredAsNoAccess)
         registerURLSchemeAsNoAccess(scheme);
     
+    for (auto&& websiteDataStoreParameters : WTFMove(parameters.websiteDataStoreParameters))
+        addWebsiteDataStore(WTFMove(websiteDataStoreParameters));
+
     RELEASE_LOG(Process, "%p - NetworkProcess::initializeNetworkProcess: Presenting processPID=%d", this, WebCore::presentingApplicationPID());
 }
 
@@ -430,6 +433,8 @@
     }).isNewEntry;
     if (isNewEntry)
         SandboxExtension::consumePermanently(cacheRootPathHandle);
+    else
+        ASSERT_NOT_REACHED();
 }
 
 void NetworkProcess::removeSessionStorageQuotaManager(PAL::SessionID sessionID)

Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp (275046 => 275047)


--- trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp	2021-03-25 17:36:00 UTC (rev 275047)
@@ -28,6 +28,7 @@
 
 #include "ArgumentCoders.h"
 #include "WebCoreArgumentCoders.h"
+#include "WebsiteDataStoreParameters.h"
 
 #if PLATFORM(COCOA)
 #include "ArgumentCodersCF.h"
@@ -36,6 +37,9 @@
 namespace WebKit {
 
 NetworkProcessCreationParameters::NetworkProcessCreationParameters() = default;
+NetworkProcessCreationParameters::~NetworkProcessCreationParameters() = default;
+NetworkProcessCreationParameters::NetworkProcessCreationParameters(NetworkProcessCreationParameters&&) = default;
+NetworkProcessCreationParameters& NetworkProcessCreationParameters::operator=(NetworkProcessCreationParameters&&) = default;
 
 void NetworkProcessCreationParameters::encode(IPC::Encoder& encoder) const
 {
@@ -67,6 +71,7 @@
 
     encoder << enablePrivateClickMeasurement;
     encoder << enablePrivateClickMeasurementDebugMode;
+    encoder << websiteDataStoreParameters;
 }
 
 bool NetworkProcessCreationParameters::decode(IPC::Decoder& decoder, NetworkProcessCreationParameters& result)
@@ -135,6 +140,12 @@
     if (!decoder.decode(result.enablePrivateClickMeasurementDebugMode))
         return false;
 
+    Optional<Vector<WebsiteDataStoreParameters>> websiteDataStoreParameters;
+    decoder >> websiteDataStoreParameters;
+    if (!websiteDataStoreParameters)
+        return false;
+    result.websiteDataStoreParameters = WTFMove(*websiteDataStoreParameters);
+
     return true;
 }

Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h (275046 => 275047)


--- trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h	2021-03-25 17:36:00 UTC (rev 275047)
@@ -43,8 +43,13 @@
 
 namespace WebKit {
 
+struct WebsiteDataStoreParameters;
+
 struct NetworkProcessCreationParameters {
     NetworkProcessCreationParameters();
+    NetworkProcessCreationParameters(NetworkProcessCreationParameters&&);
+    ~NetworkProcessCreationParameters();
+    NetworkProcessCreationParameters& operator=(NetworkProcessCreationParameters&&);
 
     void encode(IPC::Encoder&) const;
     static WARN_UNUSED_RETURN bool decode(IPC::Decoder&, NetworkProcessCreationParameters&);
@@ -81,6 +86,8 @@
 
     bool enablePrivateClickMeasurement { true };
     bool enablePrivateClickMeasurementDebugMode { false };
+
+    Vector<WebsiteDataStoreParameters> websiteDataStoreParameters;
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (275046 => 275047)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2021-03-25 17:36:00 UTC (rev 275047)
@@ -158,6 +158,14 @@
         SandboxExtension::createHandle(parentBundleDirectory, SandboxExtension::Type::ReadOnly, parameters.parentBundleDirectoryExtensionHandle);
     SandboxExtension::createHandleForTemporaryFile(emptyString(), SandboxExtension::Type::ReadWrite, parameters.tempDirectoryExtensionHandle);
 #endif
+
+#if !PLATFORM(GTK) && !PLATFORM(WPE) // GTK and WPE don't use defaultNetworkProcess
+    parameters.websiteDataStoreParameters = WebsiteDataStore::parametersFromEachWebsiteDataStore();
+    WebsiteDataStore::forEachWebsiteDataStore([this](auto& websiteDataStore) {
+        addSession(websiteDataStore, SendParametersToNetworkProcess::No);
+    });
+#endif
+
     WebProcessPool::platformInitializeNetworkProcess(parameters);
     send(Messages::NetworkProcess::InitializeNetworkProcess(parameters), 0);
 }
@@ -1327,11 +1335,13 @@
     sendWithAsyncReply(Messages::NetworkProcess::FlushCookies(sessionID), WTFMove(completionHandler));
 }
 
-void NetworkProcessProxy::addSession(WebsiteDataStore& store)
+void NetworkProcessProxy::addSession(WebsiteDataStore& store, SendParametersToNetworkProcess sendParametersToNetworkProcess)
 {
-    m_websiteDataStores.add(store);
+    auto addResult = m_websiteDataStores.add(store);
+    if (!addResult.isNewEntry)
+        return;
 
-    if (canSendMessage())
+    if (canSendMessage() && sendParametersToNetworkProcess == SendParametersToNetworkProcess::Yes)
         send(Messages::NetworkProcess::AddWebsiteDataStore { store.parameters() }, 0);
     auto sessionID = store.sessionID();
     if (!sessionID.isEphemeral()) {

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (275046 => 275047)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h	2021-03-25 17:36:00 UTC (rev 275047)
@@ -221,7 +221,8 @@
     void didDestroyWebUserContentControllerProxy(WebUserContentControllerProxy&);
 #endif
 
-    void addSession(WebsiteDataStore&);
+    enum class SendParametersToNetworkProcess : bool { No, Yes };
+    void addSession(WebsiteDataStore&, SendParametersToNetworkProcess);
     void removeSession(WebsiteDataStore&);
     
 #if ENABLE(INDEXED_DATABASE)

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (275046 => 275047)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2021-03-25 17:36:00 UTC (rev 275047)
@@ -208,7 +208,7 @@
 {
     if (!m_networkProcess) {
         m_networkProcess = networkProcessForSession(m_sessionID);
-        m_networkProcess->addSession(*this);
+        m_networkProcess->addSession(*this, NetworkProcessProxy::SendParametersToNetworkProcess::Yes);
     }
 
     return *m_networkProcess;
@@ -1906,6 +1906,15 @@
         processPool->setCacheModelSynchronouslyForTesting(cacheModel);
 }
 
+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()
 {
     WebsiteDataStoreParameters parameters;

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h (275046 => 275047)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	2021-03-25 17:34:41 UTC (rev 275046)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h	2021-03-25 17:36:00 UTC (rev 275047)
@@ -258,6 +258,7 @@
     DeviceIdHashSaltStorage& deviceIdHashSaltStorage() { return m_deviceIdHashSaltStorage.get(); }
 
     WebsiteDataStoreParameters parameters();
+    static Vector<WebsiteDataStoreParameters> parametersFromEachWebsiteDataStore();
 
     void flushCookies(CompletionHandler<void()>&&);
     void clearCachedCredentials();

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to