- Revision
- 274186
- Author
- achristen...@apple.com
- Date
- 2021-03-09 16:40:08 -0800 (Tue, 09 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
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 (274185 => 274186)
--- trunk/Source/WebKit/ChangeLog 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/ChangeLog 2021-03-10 00:40:08 UTC (rev 274186)
@@ -1,3 +1,33 @@
+2021-03-09 Alex Christensen <achristen...@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 <cdu...@apple.com>
Crash under WebViewImpl::flagsChanged()
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (274185 => 274186)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2021-03-10 00:40:08 UTC (rev 274186)
@@ -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 (274185 => 274186)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp 2021-03-10 00:40:08 UTC (rev 274186)
@@ -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
{
@@ -68,6 +72,7 @@
encoder << enablePrivateClickMeasurement;
encoder << enablePrivateClickMeasurementDebugMode;
+ encoder << websiteDataStoreParameters;
}
bool NetworkProcessCreationParameters::decode(IPC::Decoder& decoder, NetworkProcessCreationParameters& result)
@@ -138,6 +143,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 (274185 => 274186)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h 2021-03-10 00:40:08 UTC (rev 274186)
@@ -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&);
@@ -82,6 +87,8 @@
bool enablePrivateClickMeasurement { true };
bool enablePrivateClickMeasurementDebugMode { false };
+
+ Vector<WebsiteDataStoreParameters> websiteDataStoreParameters;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (274185 => 274186)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2021-03-10 00:40:08 UTC (rev 274186)
@@ -158,6 +158,10 @@
SandboxExtension::createHandle(parentBundleDirectory, SandboxExtension::Type::ReadOnly, parameters.parentBundleDirectoryExtensionHandle);
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);
}
@@ -1327,11 +1331,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 (274185 => 274186)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2021-03-10 00:40:08 UTC (rev 274186)
@@ -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 (274185 => 274186)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2021-03-10 00:40:08 UTC (rev 274186)
@@ -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;
@@ -1910,6 +1910,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 (274185 => 274186)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h 2021-03-09 23:59:29 UTC (rev 274185)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h 2021-03-10 00:40:08 UTC (rev 274186)
@@ -258,6 +258,7 @@
DeviceIdHashSaltStorage& deviceIdHashSaltStorage() { return m_deviceIdHashSaltStorage.get(); }
WebsiteDataStoreParameters parameters();
+ static Vector<WebsiteDataStoreParameters> parametersFromEachWebsiteDataStore();
void flushCookies(CompletionHandler<void()>&&);
void clearCachedCredentials();
