Diff
Modified: trunk/LayoutTests/ChangeLog (259858 => 259859)
--- trunk/LayoutTests/ChangeLog 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/LayoutTests/ChangeLog 2020-04-10 10:50:35 UTC (rev 259859)
@@ -1,3 +1,14 @@
+2020-04-10 Youenn Fablet <[email protected]>
+
+ SWServer should not run a service worker that is terminating
+ https://bugs.webkit.org/show_bug.cgi?id=210044
+
+ Reviewed by Chris Dumez.
+
+ * http/wpt/service-workers/resources/routines.js:
+ (async waitForServiceWorkerNoLongerRunning):
+ Use new internals API.
+
2020-04-09 Peng Liu <[email protected]>
REGRESSION: (r258434) [ Mac WK1 ] media/track/track-css-user-override.html is a flaky failure
Modified: trunk/LayoutTests/http/wpt/service-workers/resources/routines.js (259858 => 259859)
--- trunk/LayoutTests/http/wpt/service-workers/resources/routines.js 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/LayoutTests/http/wpt/service-workers/resources/routines.js 2020-04-10 10:50:35 UTC (rev 259859)
@@ -33,11 +33,8 @@
if (!window.internals)
return Promise.reject("requires internals");
- let count = 100;
- while (--count > 0 && await internals.isServiceWorkerRunning(worker)) {
- worker.postMessage("test");
- await new Promise(resolve => setTimeout(resolve, 50));
- }
- if (count === 0)
- return Promise.reject("service worker is still running");
+ const promise = internals.whenServiceWorkerIsTerminated(worker);
+ let timer = setInterval(() => worker.postMessage("test"), 50);
+ await promise;
+ clearInterval(timer);
}
Modified: trunk/Source/WebCore/ChangeLog (259858 => 259859)
--- trunk/Source/WebCore/ChangeLog 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/ChangeLog 2020-04-10 10:50:35 UTC (rev 259859)
@@ -1,3 +1,38 @@
+2020-04-10 Youenn Fablet <[email protected]>
+
+ SWServer should not run a service worker that is terminating
+ https://bugs.webkit.org/show_bug.cgi?id=210044
+
+ Reviewed by Chris Dumez.
+
+ If a test is being terminated and we want to restart it, we were previously running it right away.
+ This does not work well as the service worker process might still have the terminating service worker in its map.
+ Also, if the service worker is not able to terminate properly, we will kill the service worker process so there is no reason
+ to try running this service worker in this process.
+ Instead, wait for the service worker to terminate (which might include terminating the service worker process).
+
+ In addition, we remove the isServiceWorkerRunning internals API since this is potentially flaky as the service worker
+ might be terminated and rerunning in between two isServiceWorkerRunning checks.
+ Instead, we introduce whenServiceWorkerIsTerminated which will resolve as soon as the service worker goes to terminated.
+
+ Covered by existing spinning tests no longer crashing.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::whenServiceWorkerIsTerminated):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+ * workers/service/SWClientConnection.h:
+ (WebCore::SWClientConnection::whenServiceWorkerIsTerminatedForTesting):
+ * workers/service/server/SWServer.cpp:
+ (WebCore::SWServer::runServiceWorkerIfNecessary):
+ (WebCore::SWServer::runServiceWorker):
+ (WebCore::SWServer::workerContextTerminated):
+ * workers/service/server/SWServerWorker.cpp:
+ (WebCore::SWServerWorker::whenTerminated):
+ (WebCore::SWServerWorker::setState):
+ * workers/service/server/SWServerWorker.h:
+ (WebCore::SWServerWorker::isNotRunning const):
+
2020-04-10 Charlie Turner <[email protected]>
[EME][GStreamer] Relax proxy initialization assert
Modified: trunk/Source/WebCore/testing/Internals.cpp (259858 => 259859)
--- trunk/Source/WebCore/testing/Internals.cpp 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/testing/Internals.cpp 2020-04-10 10:50:35 UTC (rev 259859)
@@ -5144,10 +5144,10 @@
});
}
-void Internals::isServiceWorkerRunning(ServiceWorker& worker, DOMPromiseDeferred<IDLBoolean>&& promise)
+void Internals::whenServiceWorkerIsTerminated(ServiceWorker& worker, DOMPromiseDeferred<void>&& promise)
{
- return ServiceWorkerProvider::singleton().serviceWorkerConnection().isServiceWorkerRunning(worker.identifier(), [promise = WTFMove(promise)](bool result) mutable {
- promise.resolve(result);
+ return ServiceWorkerProvider::singleton().serviceWorkerConnection().whenServiceWorkerIsTerminatedForTesting(worker.identifier(), [promise = WTFMove(promise)]() mutable {
+ promise.resolve();
});
}
#endif
Modified: trunk/Source/WebCore/testing/Internals.h (259858 => 259859)
--- trunk/Source/WebCore/testing/Internals.h 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/testing/Internals.h 2020-04-10 10:50:35 UTC (rev 259859)
@@ -784,7 +784,7 @@
using HasRegistrationPromise = DOMPromiseDeferred<IDLBoolean>;
void hasServiceWorkerRegistration(const String& clientURL, HasRegistrationPromise&&);
void terminateServiceWorker(ServiceWorker&, DOMPromiseDeferred<void>&&);
- void isServiceWorkerRunning(ServiceWorker&, DOMPromiseDeferred<IDLBoolean>&&);
+ void whenServiceWorkerIsTerminated(ServiceWorker&, DOMPromiseDeferred<void>&&);
#endif
#if ENABLE(APPLE_PAY)
Modified: trunk/Source/WebCore/testing/Internals.idl (259858 => 259859)
--- trunk/Source/WebCore/testing/Internals.idl 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/testing/Internals.idl 2020-04-10 10:50:35 UTC (rev 259859)
@@ -770,7 +770,7 @@
[Conditional=SERVICE_WORKER] Promise<boolean> hasServiceWorkerRegistration(DOMString scopeURL);
[Conditional=SERVICE_WORKER] Promise<void> terminateServiceWorker(ServiceWorker worker);
- [Conditional=SERVICE_WORKER] Promise<boolean> isServiceWorkerRunning(ServiceWorker worker);
+ [Conditional=SERVICE_WORKER] Promise<void> whenServiceWorkerIsTerminated(ServiceWorker worker);
[CallWith=Document, Conditional=APPLE_PAY] readonly attribute MockPaymentCoordinator mockPaymentCoordinator;
Modified: trunk/Source/WebCore/workers/service/SWClientConnection.h (259858 => 259859)
--- trunk/Source/WebCore/workers/service/SWClientConnection.h 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/workers/service/SWClientConnection.h 2020-04-10 10:50:35 UTC (rev 259859)
@@ -86,7 +86,7 @@
virtual void finishFetchingScriptInServer(const ServiceWorkerFetchResult&) = 0;
virtual void storeRegistrationsOnDiskForTesting(CompletionHandler<void()>&& callback) { callback(); }
- virtual void isServiceWorkerRunning(ServiceWorkerIdentifier, CompletionHandler<void(bool)>&& callback) { callback(false); }
+ virtual void whenServiceWorkerIsTerminatedForTesting(ServiceWorkerIdentifier, CompletionHandler<void()>&& callback) { callback(); }
WEBCORE_EXPORT void registerServiceWorkerClients();
Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (259858 => 259859)
--- trunk/Source/WebCore/workers/service/server/SWServer.cpp 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp 2020-04-10 10:50:35 UTC (rev 259859)
@@ -685,6 +685,15 @@
return;
}
+ if (worker->isTerminating()) {
+ worker->whenTerminated([this, weakThis = makeWeakPtr(this), identifier, callback = WTFMove(callback)]() mutable {
+ if (!weakThis)
+ return callback(nullptr);
+ runServiceWorkerIfNecessary(identifier, WTFMove(callback));
+ });
+ return;
+ }
+
if (!contextConnection) {
auto& serviceWorkerRunRequestsForOrigin = m_serviceWorkerRunRequests.ensure(worker->registrableDomain(), [] {
return HashMap<ServiceWorkerIdentifier, Vector<RunServiceWorkerCallback>> { };
@@ -712,8 +721,9 @@
if (!worker->registration())
return false;
- auto addResult = m_runningOrTerminatingWorkers.add(identifier, *worker);
- ASSERT_UNUSED(addResult, addResult.isNewEntry || worker->isTerminating());
+ ASSERT(!worker->isTerminating());
+ ASSERT(!m_runningOrTerminatingWorkers.contains(identifier));
+ m_runningOrTerminatingWorkers.add(identifier, *worker);
worker->setState(SWServerWorker::State::Running);
@@ -738,15 +748,15 @@
void SWServer::workerContextTerminated(SWServerWorker& worker)
{
+ // At this point if no registrations are referencing the worker then it will be destroyed,
+ // removing itself from the m_workersByID map.
+ auto result = m_runningOrTerminatingWorkers.take(worker.identifier());
+ ASSERT_UNUSED(result, result && result->ptr() == &worker);
+
worker.setState(SWServerWorker::State::NotRunning);
if (auto* jobQueue = m_jobQueues.get(worker.registrationKey()))
jobQueue->cancelJobsFromServiceWorker(worker.identifier());
-
- // At this point if no registrations are referencing the worker then it will be destroyed,
- // removing itself from the m_workersByID map.
- auto result = m_runningOrTerminatingWorkers.take(worker.identifier());
- ASSERT_UNUSED(result, result && result->ptr() == &worker);
}
void SWServer::fireInstallEvent(SWServerWorker& worker)
Modified: trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp (259858 => 259859)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2020-04-10 10:50:35 UTC (rev 259859)
@@ -104,6 +104,12 @@
}
}
+void SWServerWorker::whenTerminated(CompletionHandler<void()>&& callback)
+{
+ ASSERT(isRunning() || isTerminating());
+ m_terminationCallbacks.append(WTFMove(callback));
+}
+
void SWServerWorker::startTermination(CompletionHandler<void()>&& callback)
{
auto* contextConnection = this->contextConnection();
@@ -296,6 +302,7 @@
void SWServerWorker::setState(State state)
{
ASSERT(state != State::Running || m_registration);
+ ASSERT(state != State::Running || m_state != State::Terminating);
m_state = state;
switch (state) {
Modified: trunk/Source/WebCore/workers/service/server/SWServerWorker.h (259858 => 259859)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2020-04-10 10:50:35 UTC (rev 259859)
@@ -63,6 +63,7 @@
WEBCORE_EXPORT ~SWServerWorker();
WEBCORE_EXPORT void terminate(CompletionHandler<void()>&& = [] { });
+ WEBCORE_EXPORT void whenTerminated(CompletionHandler<void()>&&);
WEBCORE_EXPORT void whenActivated(CompletionHandler<void(bool)>&&);
@@ -73,6 +74,7 @@
};
bool isRunning() const { return m_state == State::Running; }
bool isTerminating() const { return m_state == State::Terminating; }
+ bool isNotRunning() const { return m_state == State::NotRunning; }
void setState(State);
SWServer* server() { return m_server.get(); }
Modified: trunk/Source/WebKit/ChangeLog (259858 => 259859)
--- trunk/Source/WebKit/ChangeLog 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebKit/ChangeLog 2020-04-10 10:50:35 UTC (rev 259859)
@@ -1,3 +1,20 @@
+2020-04-10 Youenn Fablet <[email protected]>
+
+ SWServer should not run a service worker that is terminating
+ https://bugs.webkit.org/show_bug.cgi?id=210044
+
+ Reviewed by Chris Dumez.
+
+ Implement whenServiceWorkerIsTerminated check.
+
+ * NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
+ (WebKit::WebSWServerConnection::whenServiceWorkerIsTerminatedForTesting):
+ * NetworkProcess/ServiceWorker/WebSWServerConnection.h:
+ * NetworkProcess/ServiceWorker/WebSWServerConnection.messages.in:
+ * WebProcess/Storage/WebSWClientConnection.cpp:
+ (WebKit::WebSWClientConnection::whenServiceWorkerIsTerminatedForTesting):
+ * WebProcess/Storage/WebSWClientConnection.h:
+
2020-04-10 Commit Queue <[email protected]>
Unreviewed, reverting r259818.
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp (259858 => 259859)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp 2020-04-10 10:50:35 UTC (rev 259859)
@@ -464,10 +464,12 @@
worker->terminate(WTFMove(callback));
}
-void WebSWServerConnection::isServiceWorkerRunning(ServiceWorkerIdentifier identifier, CompletionHandler<void(bool)>&& completionHandler)
+void WebSWServerConnection::whenServiceWorkerIsTerminatedForTesting(WebCore::ServiceWorkerIdentifier identifier, CompletionHandler<void()>&& completionHandler)
{
auto* worker = SWServerWorker::existingWorkerForIdentifier(identifier);
- completionHandler(worker ? worker->isRunning() : false);
+ if (!worker || worker->isNotRunning())
+ return completionHandler();
+ worker->whenTerminated(WTFMove(completionHandler));
}
PAL::SessionID WebSWServerConnection::sessionID() const
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h (259858 => 259859)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h 2020-04-10 10:50:35 UTC (rev 259859)
@@ -103,7 +103,7 @@
void registerServiceWorkerClient(WebCore::SecurityOriginData&& topOrigin, WebCore::ServiceWorkerClientData&&, const Optional<WebCore::ServiceWorkerRegistrationIdentifier>&, String&& userAgent);
void unregisterServiceWorkerClient(const WebCore::ServiceWorkerClientIdentifier&);
void terminateWorkerFromClient(WebCore::ServiceWorkerIdentifier, CompletionHandler<void()>&&);
- void isServiceWorkerRunning(WebCore::ServiceWorkerIdentifier, CompletionHandler<void(bool)>&&);
+ void whenServiceWorkerIsTerminatedForTesting(WebCore::ServiceWorkerIdentifier, CompletionHandler<void()>&&);
void postMessageToServiceWorkerClient(WebCore::DocumentIdentifier destinationContextIdentifier, const WebCore::MessageWithMessagePorts&, WebCore::ServiceWorkerIdentifier sourceServiceWorkerIdentifier, const String& sourceOrigin) final;
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.messages.in (259858 => 259859)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.messages.in 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.messages.in 2020-04-10 10:50:35 UTC (rev 259859)
@@ -42,10 +42,10 @@
UnregisterServiceWorkerClient(struct WebCore::ServiceWorkerClientIdentifier identifier)
TerminateWorkerFromClient(WebCore::ServiceWorkerIdentifier workerIdentifier) -> () Async
+ WhenServiceWorkerIsTerminatedForTesting(WebCore::ServiceWorkerIdentifier workerIdentifier) -> () Async
SetThrottleState(bool isThrottleable)
StoreRegistrationsOnDisk() -> () Async
- IsServiceWorkerRunning(WebCore::ServiceWorkerIdentifier workerIdentifier) -> (bool isRunning) Async
}
#endif // ENABLE(SERVICE_WORKER)
Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp (259858 => 259859)
--- trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp 2020-04-10 10:50:35 UTC (rev 259859)
@@ -248,9 +248,9 @@
sendWithAsyncReply(Messages::WebSWServerConnection::TerminateWorkerFromClient { identifier }, WTFMove(callback));
}
-void WebSWClientConnection::isServiceWorkerRunning(ServiceWorkerIdentifier identifier, CompletionHandler<void(bool)>&& callback)
+void WebSWClientConnection::whenServiceWorkerIsTerminatedForTesting(ServiceWorkerIdentifier identifier, CompletionHandler<void()>&& callback)
{
- sendWithAsyncReply(Messages::WebSWServerConnection::IsServiceWorkerRunning { identifier }, WTFMove(callback));
+ sendWithAsyncReply(Messages::WebSWServerConnection::WhenServiceWorkerIsTerminatedForTesting { identifier }, WTFMove(callback));
}
void WebSWClientConnection::updateThrottleState()
Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.h (259858 => 259859)
--- trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.h 2020-04-10 10:32:59 UTC (rev 259858)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.h 2020-04-10 10:50:35 UTC (rev 259859)
@@ -86,7 +86,7 @@
void setDocumentIsControlled(WebCore::DocumentIdentifier, WebCore::ServiceWorkerRegistrationData&&, CompletionHandler<void(bool)>&&);
void getRegistrations(WebCore::SecurityOriginData&& topOrigin, const URL& clientURL, GetRegistrationsCallback&&) final;
- void isServiceWorkerRunning(WebCore::ServiceWorkerIdentifier, CompletionHandler<void(bool)>&&) final;
+ void whenServiceWorkerIsTerminatedForTesting(WebCore::ServiceWorkerIdentifier, CompletionHandler<void()>&&) final;
void didResolveRegistrationPromise(const WebCore::ServiceWorkerRegistrationKey&) final;
void storeRegistrationsOnDiskForTesting(CompletionHandler<void()>&&) final;
