- Revision
- 290352
- Author
- [email protected]
- Date
- 2022-02-22 23:35:12 -0800 (Tue, 22 Feb 2022)
Log Message
http/wpt/push-api/onpush-disabled.html fails
https://bugs.webkit.org/show_bug.cgi?id=236874
<rdar://problem/89176154>
Reviewed by Youenn Fablet.
Source/WebCore:
Before running the test, we would construct a new WKWebView with the Push API setting disabled.
We would then run the test, which would launch a service worker. Normally, we would expect the
service worker to inherit the settings from the WKWebView. However, it was not reliably
happening because the settings for the service worker are stored on the WebProcessPool and
WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess() was selecting a
random WebProcessPool object (the first one returned by processPools()).
To address the issue, we now pass the ProcessIdentifier of the client that requested the
service worker connection so that establishServiceWorkerContextConnectionToNetworkProcess() can
now prioritize the WebProcessPool of that process. Also, as an optimization, we now prioritize
this client process to run the service worker (assuming it is origin-clean).
No new tests, unskipped existing test.
* workers/service/server/SWServer.cpp:
(WebCore::SWServer::createContextConnection):
* workers/service/server/SWServer.h:
Source/WebKit:
Before running the test, we would construct a new WKWebView with the Push API setting disabled.
We would then run the test, which would launch a service worker. Normally, we would expect the
service worker to inherit the settings from the WKWebView. However, it was not reliably
happening because the settings for the service worker are stored on the WebProcessPool and
WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess() was selecting a
random WebProcessPool object (the first one returned by processPools()).
To address the issue, we now pass the ProcessIdentifier of the client that requested the
service worker connection so that establishServiceWorkerContextConnectionToNetworkProcess() can
now prioritize the WebProcessPool of that process. Also, as an optimization, we now prioritize
this client process to run the service worker (assuming it is origin-clean).
* NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::ensureSWServer):
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::establishServiceWorkerContextConnectionToNetworkProcess):
* UIProcess/Network/NetworkProcessProxy.h:
* UIProcess/Network/NetworkProcessProxy.messages.in:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess):
* UIProcess/WebProcessPool.h:
LayoutTests:
Unskip test that should no longer be flaky.
* TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (290351 => 290352)
--- trunk/LayoutTests/ChangeLog 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/LayoutTests/ChangeLog 2022-02-23 07:35:12 UTC (rev 290352)
@@ -1,3 +1,15 @@
+2022-02-22 Chris Dumez <[email protected]>
+
+ http/wpt/push-api/onpush-disabled.html fails
+ https://bugs.webkit.org/show_bug.cgi?id=236874
+ <rdar://problem/89176154>
+
+ Reviewed by Youenn Fablet.
+
+ Unskip test that should no longer be flaky.
+
+ * TestExpectations:
+
2022-02-22 Tim Nguyen <[email protected]>
Use of showModalDialog should appear as a warning in WI console
Modified: trunk/LayoutTests/TestExpectations (290351 => 290352)
--- trunk/LayoutTests/TestExpectations 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/LayoutTests/TestExpectations 2022-02-23 07:35:12 UTC (rev 290352)
@@ -5165,7 +5165,5 @@
js/dom/reflect-set-onto-dom.html [ Skip ]
userscripts/user-script-plugin-document.html [ Skip ]
-webkit.org/b/236874 http/wpt/push-api/onpush-disabled.html [ Failure ]
-
# This is marked as passing on WK2.
fast/canvas/large-getImageData.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (290351 => 290352)
--- trunk/Source/WebCore/ChangeLog 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebCore/ChangeLog 2022-02-23 07:35:12 UTC (rev 290352)
@@ -1,3 +1,29 @@
+2022-02-22 Chris Dumez <[email protected]>
+
+ http/wpt/push-api/onpush-disabled.html fails
+ https://bugs.webkit.org/show_bug.cgi?id=236874
+ <rdar://problem/89176154>
+
+ Reviewed by Youenn Fablet.
+
+ Before running the test, we would construct a new WKWebView with the Push API setting disabled.
+ We would then run the test, which would launch a service worker. Normally, we would expect the
+ service worker to inherit the settings from the WKWebView. However, it was not reliably
+ happening because the settings for the service worker are stored on the WebProcessPool and
+ WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess() was selecting a
+ random WebProcessPool object (the first one returned by processPools()).
+
+ To address the issue, we now pass the ProcessIdentifier of the client that requested the
+ service worker connection so that establishServiceWorkerContextConnectionToNetworkProcess() can
+ now prioritize the WebProcessPool of that process. Also, as an optimization, we now prioritize
+ this client process to run the service worker (assuming it is origin-clean).
+
+ No new tests, unskipped existing test.
+
+ * workers/service/server/SWServer.cpp:
+ (WebCore::SWServer::createContextConnection):
+ * workers/service/server/SWServer.h:
+
2022-02-22 Tim Nguyen <[email protected]>
Use of showModalDialog should appear as a warning in WI console
Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (290351 => 290352)
--- trunk/Source/WebCore/workers/service/server/SWServer.cpp 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp 2022-02-23 07:35:12 UTC (rev 290352)
@@ -1177,8 +1177,14 @@
RELEASE_LOG(ServiceWorker, "SWServer::createContextConnection will create a connection");
+ std::optional<ProcessIdentifier> requestingProcessIdentifier;
+ if (auto it = m_clientsByRegistrableDomain.find(registrableDomain); it != m_clientsByRegistrableDomain.end()) {
+ if (!it->value.isEmpty())
+ requestingProcessIdentifier = it->value.begin()->processIdentifier();
+ }
+
m_pendingConnectionDomains.add(registrableDomain);
- m_createContextConnectionCallback(registrableDomain, serviceWorkerPageIdentifier, [this, weakThis = WeakPtr { *this }, registrableDomain, serviceWorkerPageIdentifier] {
+ m_createContextConnectionCallback(registrableDomain, requestingProcessIdentifier, serviceWorkerPageIdentifier, [this, weakThis = WeakPtr { *this }, registrableDomain, serviceWorkerPageIdentifier] {
if (!weakThis)
return;
Modified: trunk/Source/WebCore/workers/service/server/SWServer.h (290351 => 290352)
--- trunk/Source/WebCore/workers/service/server/SWServer.h 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebCore/workers/service/server/SWServer.h 2022-02-23 07:35:12 UTC (rev 290352)
@@ -126,7 +126,7 @@
};
using SoftUpdateCallback = Function<void(ServiceWorkerJobData&& jobData, bool shouldRefreshCache, ResourceRequest&&, CompletionHandler<void(const WorkerFetchResult&)>&&)>;
- using CreateContextConnectionCallback = Function<void(const WebCore::RegistrableDomain&, std::optional<ScriptExecutionContextIdentifier>, CompletionHandler<void()>&&)>;
+ using CreateContextConnectionCallback = Function<void(const WebCore::RegistrableDomain&, std::optional<ProcessIdentifier> requestingProcessIdentifier, std::optional<ScriptExecutionContextIdentifier>, CompletionHandler<void()>&&)>;
using AppBoundDomainsCallback = Function<void(CompletionHandler<void(HashSet<WebCore::RegistrableDomain>&&)>&&)>;
WEBCORE_EXPORT SWServer(UniqueRef<SWOriginStore>&&, bool processTerminationDelayEnabled, String&& registrationDatabaseDirectory, PAL::SessionID, bool shouldRunServiceWorkersOnMainThreadForTesting, bool hasServiceWorkerEntitlement, SoftUpdateCallback&&, CreateContextConnectionCallback&&, AppBoundDomainsCallback&&);
Modified: trunk/Source/WebKit/ChangeLog (290351 => 290352)
--- trunk/Source/WebKit/ChangeLog 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebKit/ChangeLog 2022-02-23 07:35:12 UTC (rev 290352)
@@ -1,3 +1,33 @@
+2022-02-22 Chris Dumez <[email protected]>
+
+ http/wpt/push-api/onpush-disabled.html fails
+ https://bugs.webkit.org/show_bug.cgi?id=236874
+ <rdar://problem/89176154>
+
+ Reviewed by Youenn Fablet.
+
+ Before running the test, we would construct a new WKWebView with the Push API setting disabled.
+ We would then run the test, which would launch a service worker. Normally, we would expect the
+ service worker to inherit the settings from the WKWebView. However, it was not reliably
+ happening because the settings for the service worker are stored on the WebProcessPool and
+ WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess() was selecting a
+ random WebProcessPool object (the first one returned by processPools()).
+
+ To address the issue, we now pass the ProcessIdentifier of the client that requested the
+ service worker connection so that establishServiceWorkerContextConnectionToNetworkProcess() can
+ now prioritize the WebProcessPool of that process. Also, as an optimization, we now prioritize
+ this client process to run the service worker (assuming it is origin-clean).
+
+ * NetworkProcess/NetworkSession.cpp:
+ (WebKit::NetworkSession::ensureSWServer):
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::establishServiceWorkerContextConnectionToNetworkProcess):
+ * UIProcess/Network/NetworkProcessProxy.h:
+ * UIProcess/Network/NetworkProcessProxy.messages.in:
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess):
+ * UIProcess/WebProcessPool.h:
+
2022-02-22 Simon Fraser <[email protected]>
With DOM Rendering in GPU process, every display results in new IOSurface allocation
Modified: trunk/Source/WebKit/NetworkProcess/NetworkSession.cpp (290351 => 290352)
--- trunk/Source/WebKit/NetworkProcess/NetworkSession.cpp 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebKit/NetworkProcess/NetworkSession.cpp 2022-02-23 07:35:12 UTC (rev 290352)
@@ -626,9 +626,9 @@
#endif
m_swServer = makeUnique<SWServer>(makeUniqueRef<WebSWOriginStore>(), info.processTerminationDelayEnabled, WTFMove(path), m_sessionID, shouldRunServiceWorkersOnMainThreadForTesting(), m_networkProcess->parentProcessHasServiceWorkerEntitlement(), [this](auto&& jobData, bool shouldRefreshCache, auto&& request, auto&& completionHandler) mutable {
ServiceWorkerSoftUpdateLoader::start(this, WTFMove(jobData), shouldRefreshCache, WTFMove(request), WTFMove(completionHandler));
- }, [this](auto& registrableDomain, std::optional<ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, auto&& completionHandler) {
+ }, [this](auto& registrableDomain, std::optional<ProcessIdentifier> requestingProcessIdentifier, std::optional<ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, auto&& completionHandler) {
ASSERT(!registrableDomain.isEmpty());
- m_networkProcess->parentProcessConnection()->sendWithAsyncReply(Messages::NetworkProcessProxy::EstablishServiceWorkerContextConnectionToNetworkProcess { registrableDomain, serviceWorkerPageIdentifier, m_sessionID }, WTFMove(completionHandler), 0);
+ m_networkProcess->parentProcessConnection()->sendWithAsyncReply(Messages::NetworkProcessProxy::EstablishServiceWorkerContextConnectionToNetworkProcess { registrableDomain, requestingProcessIdentifier, serviceWorkerPageIdentifier, m_sessionID }, WTFMove(completionHandler), 0);
}, WTFMove(appBoundDomainsCallback));
}
return *m_swServer;
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (290351 => 290352)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2022-02-23 07:35:12 UTC (rev 290352)
@@ -1448,9 +1448,9 @@
}
#if ENABLE(SERVICE_WORKER)
-void NetworkProcessProxy::establishServiceWorkerContextConnectionToNetworkProcess(RegistrableDomain&& registrableDomain, std::optional<ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID sessionID, CompletionHandler<void()>&& completionHandler)
+void NetworkProcessProxy::establishServiceWorkerContextConnectionToNetworkProcess(RegistrableDomain&& registrableDomain, std::optional<WebCore::ProcessIdentifier> requestingProcessIdentifier, std::optional<ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID sessionID, CompletionHandler<void()>&& completionHandler)
{
- WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess(WTFMove(registrableDomain), serviceWorkerPageIdentifier, sessionID, WTFMove(completionHandler));
+ WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess(WTFMove(registrableDomain), requestingProcessIdentifier, serviceWorkerPageIdentifier, sessionID, WTFMove(completionHandler));
}
void NetworkProcessProxy::startServiceWorkerBackgroundProcessing(WebCore::ProcessIdentifier serviceWorkerProcessIdentifier)
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (290351 => 290352)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h 2022-02-23 07:35:12 UTC (rev 290352)
@@ -333,7 +333,7 @@
#endif
#if ENABLE(SERVICE_WORKER)
- void establishServiceWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain&&, std::optional<WebCore::ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID, CompletionHandler<void()>&&);
+ void establishServiceWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain&&, std::optional<WebCore::ProcessIdentifier> requestingProcessIdentifier, std::optional<WebCore::ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID, CompletionHandler<void()>&&);
void startServiceWorkerBackgroundProcessing(WebCore::ProcessIdentifier serviceWorkerProcessIdentifier);
void endServiceWorkerBackgroundProcessing(WebCore::ProcessIdentifier serviceWorkerProcessIdentifier);
#endif
Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in (290351 => 290352)
--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in 2022-02-23 07:35:12 UTC (rev 290352)
@@ -53,7 +53,7 @@
TerminateWebProcess(WebCore::ProcessIdentifier webProcessIdentifier)
#if ENABLE(SERVICE_WORKER)
- EstablishServiceWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain registrableDomain, std::optional<WebCore::ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID sessionID) -> () Async
+ EstablishServiceWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain registrableDomain, std::optional<WebCore::ProcessIdentifier> requestingProcessIdentifier, std::optional<WebCore::ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID sessionID) -> () Async
StartServiceWorkerBackgroundProcessing(WebCore::ProcessIdentifier serviceWorkerProcessIdentifier)
EndServiceWorkerBackgroundProcessing(WebCore::ProcessIdentifier serviceWorkerProcessIdentifier)
#endif
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (290351 => 290352)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2022-02-23 07:35:12 UTC (rev 290352)
@@ -526,7 +526,7 @@
bool WebProcessPool::s_useSeparateServiceWorkerProcess = false;
#if ENABLE(SERVICE_WORKER)
-void WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess(RegistrableDomain&& registrableDomain, std::optional<ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID sessionID, CompletionHandler<void()>&& completionHandler)
+void WebProcessPool::establishServiceWorkerContextConnectionToNetworkProcess(RegistrableDomain&& registrableDomain, std::optional<WebCore::ProcessIdentifier> requestingProcessIdentifier, std::optional<ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID sessionID, CompletionHandler<void()>&& completionHandler)
{
auto* websiteDataStore = WebsiteDataStore::existingDataStoreForSessionID(sessionID);
if (!websiteDataStore)
@@ -534,23 +534,33 @@
if (!processPools().size())
static NeverDestroyed<Ref<WebProcessPool>> serviceWorkerProcessPool(WebProcessPool::create(API::ProcessPoolConfiguration::create().get()));
- // Arbitrarily choose the first process pool to host the service worker process.
- auto* processPool = processPools()[0];
+ RefPtr<WebProcessProxy> requestingProcess = requestingProcessIdentifier ? WebProcessProxy::processForIdentifier(*requestingProcessIdentifier) : nullptr;
+ WebProcessPool* processPool = requestingProcess ? &requestingProcess->processPool() : processPools()[0];
ASSERT(processPool);
WebProcessProxy* serviceWorkerProcessProxy { nullptr };
+ auto useProcessForServiceWorkers = [&](WebProcessProxy& process) {
+ serviceWorkerProcessProxy = &process;
+ process.enableRemoteWorkers(RemoteWorkerType::ServiceWorker, processPool->userContentControllerIdentifierForRemoteWorkers());
+ if (process.isInProcessCache()) {
+ processPool->webProcessCache().removeProcess(process, WebProcessCache::ShouldShutDownProcess::No);
+ ASSERT(!process.isInProcessCache());
+ }
+ };
+
if (serviceWorkerPageIdentifier) {
// This is a service worker for a service worker page so we need to make sure we use use the page's WebProcess for the service worker.
- if ((serviceWorkerProcessProxy = WebProcessProxy::processForIdentifier(serviceWorkerPageIdentifier->processIdentifier()))) {
- serviceWorkerProcessProxy->enableRemoteWorkers(RemoteWorkerType::ServiceWorker, processPool->userContentControllerIdentifierForRemoteWorkers());
- if (serviceWorkerProcessProxy->isInProcessCache()) {
- processPool->webProcessCache().removeProcess(*serviceWorkerProcessProxy, WebProcessCache::ShouldShutDownProcess::No);
- ASSERT(!serviceWorkerProcessProxy->isInProcessCache());
- }
- }
+ if (RefPtr process = WebProcessProxy::processForIdentifier(serviceWorkerPageIdentifier->processIdentifier()))
+ useProcessForServiceWorkers(*process);
}
+ // Prioritize the requesting WebProcess for running the service worker.
+ if (!serviceWorkerProcessProxy && !s_useSeparateServiceWorkerProcess && requestingProcess) {
+ if (&requestingProcess->websiteDataStore() == websiteDataStore && requestingProcess->isMatchingRegistrableDomain(registrableDomain))
+ useProcessForServiceWorkers(*requestingProcess);
+ }
+
if (!serviceWorkerProcessProxy && !s_useSeparateServiceWorkerProcess) {
for (auto& process : processPool->m_processes) {
if (process.ptr() == processPool->m_prewarmedProcess.get() || process->isDummyProcessProxy())
@@ -560,14 +570,8 @@
if (!process->isMatchingRegistrableDomain(registrableDomain))
continue;
- serviceWorkerProcessProxy = process.ptr();
- serviceWorkerProcessProxy->enableRemoteWorkers(RemoteWorkerType::ServiceWorker, processPool->userContentControllerIdentifierForRemoteWorkers());
+ useProcessForServiceWorkers(process);
- if (serviceWorkerProcessProxy->isInProcessCache()) {
- processPool->webProcessCache().removeProcess(*serviceWorkerProcessProxy, WebProcessCache::ShouldShutDownProcess::No);
- ASSERT(!serviceWorkerProcessProxy->isInProcessCache());
- }
-
WEBPROCESSPOOL_RELEASE_LOG_STATIC(ServiceWorker, "establishServiceWorkerContextConnectionToNetworkProcess reusing an existing web process (process=%p, PID=%d)", serviceWorkerProcessProxy, serviceWorkerProcessProxy->processIdentifier());
break;
}
@@ -585,7 +589,7 @@
remoteWorkerProcesses().add(*serviceWorkerProcessProxy);
- auto preferencesStore = processPool->m_remoteWorkerPreferences ? processPool->m_remoteWorkerPreferences.value() : processPool->m_defaultPageGroup->preferences().store();
+ auto& preferencesStore = processPool->m_remoteWorkerPreferences ? processPool->m_remoteWorkerPreferences.value() : processPool->m_defaultPageGroup->preferences().store();
serviceWorkerProcessProxy->establishServiceWorkerContext(preferencesStore, registrableDomain, serviceWorkerPageIdentifier, WTFMove(completionHandler));
if (!processPool->m_remoteWorkerUserAgent.isNull())
serviceWorkerProcessProxy->setRemoteWorkerUserAgent(processPool->m_remoteWorkerUserAgent);
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (290351 => 290352)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.h 2022-02-23 07:23:15 UTC (rev 290351)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h 2022-02-23 07:35:12 UTC (rev 290352)
@@ -379,7 +379,7 @@
void removeFromRemoteWorkerProcesses(WebProcessProxy&);
#if ENABLE(SERVICE_WORKER)
- static void establishServiceWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain&&, std::optional<WebCore::ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID, CompletionHandler<void()>&&);
+ static void establishServiceWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain&&, std::optional<WebCore::ProcessIdentifier> requestingProcessIdentifier, std::optional<WebCore::ScriptExecutionContextIdentifier> serviceWorkerPageIdentifier, PAL::SessionID, CompletionHandler<void()>&&);
size_t serviceWorkerProxiesCount() const;
bool hasServiceWorkerForegroundActivityForTesting() const;
bool hasServiceWorkerBackgroundActivityForTesting() const;
