Diff
Modified: trunk/LayoutTests/ChangeLog (255057 => 255058)
--- trunk/LayoutTests/ChangeLog 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/LayoutTests/ChangeLog 2020-01-24 08:23:07 UTC (rev 255058)
@@ -1,3 +1,13 @@
+2020-01-24 youenn fablet <[email protected]>
+
+ Make sure fetch tasks go to network if service worker never gets to activated
+ https://bugs.webkit.org/show_bug.cgi?id=206648
+
+ Reviewed by Chris Dumez.
+
+ * http/wpt/service-workers/service-worker-spinning-activate.https-expected.txt:
+ * http/wpt/service-workers/service-worker-spinning-activate.https.html:
+
2020-01-23 Diego Pino Garcia <[email protected]>
[GTK] Gardening, rebaselines and update TestExpectations
Modified: trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-activate.https-expected.txt (255057 => 255058)
--- trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-activate.https-expected.txt 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-activate.https-expected.txt 2020-01-24 08:23:07 UTC (rev 255058)
@@ -1,4 +1,5 @@
+
PASS Spin in activate
PASS Spin after activate
Modified: trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-activate.https.html (255057 => 255058)
--- trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-activate.https.html 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-activate.https.html 2020-01-24 08:23:07 UTC (rev 255058)
@@ -4,6 +4,7 @@
<script src=""
<script src=""
<script src=""
+<script src=""
</head>
<body>
<script>
@@ -13,6 +14,20 @@
await waitForState(registration.installing, "activating");
+ var promises = [];
+
+ promises.push(with_iframe("spin-activate/1"));
+ promises.push(with_iframe("spin-activate/2"));
+ promises.push(with_iframe("spin-activate/3"));
+ promises.push(with_iframe("spin-activate/4"));
+ promises.push(with_iframe("spin-activate/5"));
+ promises.push(with_iframe("spin-activate/6"));
+ promises.push(with_iframe("spin-activate/7"));
+ promises.push(with_iframe("spin-activate/8"));
+ promises.push(with_iframe("spin-activate/9"));
+
+ await Promise.all(promises);
+
return waitForServiceWorkerNoLongerRunning(worker);
}, "Spin in activate");
Modified: trunk/Source/WebCore/ChangeLog (255057 => 255058)
--- trunk/Source/WebCore/ChangeLog 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebCore/ChangeLog 2020-01-24 08:23:07 UTC (rev 255058)
@@ -1,5 +1,27 @@
2020-01-24 youenn fablet <[email protected]>
+ Make sure fetch tasks go to network if service worker never gets to activated
+ https://bugs.webkit.org/show_bug.cgi?id=206648
+
+ Reviewed by Chris Dumez.
+
+ In case worker context process crashes, the SWServerWorker gets set to NotRunning.
+ If the SWServerWorker has pending activating completion handlers, they will never be called until the worker is destroyed.
+ But the worker may never be destroyed until its registration is destroyed.
+ This may trigger service worker fetch task hangs.
+
+ To fix this, make sure to call activating completion handlers whenever the SWServerWorker state is changed to either Terminating or NotRunning.
+
+ Covered by updated test.
+
+ * workers/service/server/SWServerWorker.cpp:
+ (WebCore::SWServerWorker::~SWServerWorker):
+ (WebCore::SWServerWorker::whenActivated):
+ (WebCore::SWServerWorker::setState):
+ * workers/service/server/SWServerWorker.h:
+
+2020-01-24 youenn fablet <[email protected]>
+
Make sure DOMCacheStorage::retrieveCaches always calls its completionHandler
https://bugs.webkit.org/show_bug.cgi?id=206647
Modified: trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp (255057 => 255058)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.cpp 2020-01-24 08:23:07 UTC (rev 255058)
@@ -69,6 +69,7 @@
SWServerWorker::~SWServerWorker()
{
+ ASSERT(m_whenActivatedHandlers.isEmpty());
callWhenActivatedHandler(false);
auto taken = allWorkers().take(identifier());
@@ -210,12 +211,13 @@
m_registration->tryActivate();
}
-void SWServerWorker::whenActivated(WTF::Function<void(bool)>&& handler)
+void SWServerWorker::whenActivated(CompletionHandler<void(bool)>&& handler)
{
if (state() == ServiceWorkerState::Activated) {
handler(true);
return;
}
+ ASSERT(state() == ServiceWorkerState::Activating);
m_whenActivatedHandlers.append(WTFMove(handler));
}
@@ -249,8 +251,15 @@
ASSERT(state != State::Running || m_registration);
m_state = state;
- if (state == State::Running)
+ switch (state) {
+ case State::Running:
m_shouldSkipHandleFetch = false;
+ break;
+ case State::Terminating:
+ case State::NotRunning:
+ callWhenActivatedHandler(false);
+ break;
+ }
}
SWServerRegistration* SWServerWorker::registration() const
Modified: trunk/Source/WebCore/workers/service/server/SWServerWorker.h (255057 => 255058)
--- trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebCore/workers/service/server/SWServerWorker.h 2020-01-24 08:23:07 UTC (rev 255058)
@@ -62,7 +62,7 @@
void terminate();
- WEBCORE_EXPORT void whenActivated(WTF::Function<void(bool)>&&);
+ WEBCORE_EXPORT void whenActivated(CompletionHandler<void(bool)>&&);
enum class State {
Running,
@@ -137,7 +137,7 @@
mutable Optional<ClientOrigin> m_origin;
RegistrableDomain m_registrableDomain;
bool m_isSkipWaitingFlagSet { false };
- Vector<Function<void(bool)>> m_whenActivatedHandlers;
+ Vector<CompletionHandler<void(bool)>> m_whenActivatedHandlers;
HashMap<URL, ServiceWorkerContextData::ImportedScript> m_scriptResourceMap;
bool m_shouldSkipHandleFetch;
bool m_hasTimedOutAnyFetchTasks { false };
Modified: trunk/Source/WebKit/ChangeLog (255057 => 255058)
--- trunk/Source/WebKit/ChangeLog 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebKit/ChangeLog 2020-01-24 08:23:07 UTC (rev 255058)
@@ -1,3 +1,32 @@
+2020-01-24 youenn fablet <[email protected]>
+
+ Make sure fetch tasks go to network if service worker never gets to activated
+ https://bugs.webkit.org/show_bug.cgi?id=206648
+
+ Reviewed by Chris Dumez.
+
+ In case activating completion handlers are not called, the fetch task timeout should kick in and make the load go to network process.
+ The issue is that our code was using the context connection to do so.
+ If the fetch task is waiting for the worker activation, the context connection might not be set and the timeout will be a no-op.
+
+ To fix this, the fetch task will do as if its context is closed when the timeout fires.
+ The fetck task now has a weak pointer to the WebSWServerConnection and will use to terminate the service worker as done previously.
+
+ We no longer handle all ongoing fetch tasks of the ongoing service worker.
+ Each individual fetch task timeout provides the same level of protection.
+ The service worker will anyway get terminated which will race to finalize the service worker fetch tasks with each of their timeout.
+
+ * NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:
+ (WebKit::ServiceWorkerFetchTask::ServiceWorkerFetchTask):
+ (WebKit::ServiceWorkerFetchTask::timeoutTimerFired):
+ * NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h:
+ * NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
+ (WebKit::WebSWServerConnection::createFetchTask):
+ (WebKit::WebSWServerConnection::fetchTaskTimedOut):
+ * NetworkProcess/ServiceWorker/WebSWServerConnection.h:
+ * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
+ (WebKit::WebSWServerToContextConnection::fetchTaskTimedOut):
+
2020-01-23 Jiewen Tan <[email protected]>
Unreviewed, a follow up on r254894
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp (255057 => 255058)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp 2020-01-24 08:23:07 UTC (rev 255058)
@@ -50,8 +50,9 @@
namespace WebKit {
-ServiceWorkerFetchTask::ServiceWorkerFetchTask(NetworkResourceLoader& loader, ResourceRequest&& request, SWServerConnectionIdentifier serverConnectionIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, ServiceWorkerRegistrationIdentifier serviceWorkerRegistrationIdentifier, bool shouldSoftUpdate)
- : m_loader(loader)
+ServiceWorkerFetchTask::ServiceWorkerFetchTask(WebSWServerConnection& swServerConnection, NetworkResourceLoader& loader, ResourceRequest&& request, SWServerConnectionIdentifier serverConnectionIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, ServiceWorkerRegistrationIdentifier serviceWorkerRegistrationIdentifier, bool shouldSoftUpdate)
+ : m_swServerConnection(makeWeakPtr(swServerConnection))
+ , m_loader(loader)
, m_fetchIdentifier(WebCore::FetchIdentifier::generate())
, m_serverConnectionIdentifier(serverConnectionIdentifier)
, m_serviceWorkerIdentifier(serviceWorkerIdentifier)
@@ -239,8 +240,11 @@
softUpdateIfNeeded();
RELEASE_LOG_ERROR_IF_ALLOWED("timeoutTimerFired: (hasServiceWorkerConnection=%d)", !!m_serviceWorkerConnection);
- if (m_serviceWorkerConnection)
- m_serviceWorkerConnection->fetchTaskTimedOut(serviceWorkerIdentifier());
+
+ contextClosed();
+
+ if (m_swServerConnection)
+ m_swServerConnection->fetchTaskTimedOut(serviceWorkerIdentifier());
}
void ServiceWorkerFetchTask::softUpdateIfNeeded()
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h (255057 => 255058)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.h 2020-01-24 08:23:07 UTC (rev 255058)
@@ -51,12 +51,13 @@
namespace WebKit {
class NetworkResourceLoader;
+class WebSWServerConnection;
class WebSWServerToContextConnection;
class ServiceWorkerFetchTask : public CanMakeWeakPtr<ServiceWorkerFetchTask> {
WTF_MAKE_FAST_ALLOCATED;
public:
- ServiceWorkerFetchTask(NetworkResourceLoader&, WebCore::ResourceRequest&&, WebCore::SWServerConnectionIdentifier, WebCore::ServiceWorkerIdentifier, WebCore::ServiceWorkerRegistrationIdentifier, bool shouldSoftUpdate);
+ ServiceWorkerFetchTask(WebSWServerConnection&, NetworkResourceLoader&, WebCore::ResourceRequest&&, WebCore::SWServerConnectionIdentifier, WebCore::ServiceWorkerIdentifier, WebCore::ServiceWorkerRegistrationIdentifier, bool shouldSoftUpdate);
~ServiceWorkerFetchTask();
void start(WebSWServerToContextConnection&);
@@ -91,6 +92,7 @@
template<typename Message> bool sendToServiceWorker(Message&&);
template<typename Message> bool sendToClient(Message&&);
+ WeakPtr<WebSWServerConnection> m_swServerConnection;
NetworkResourceLoader& m_loader;
WeakPtr<WebSWServerToContextConnection> m_serviceWorkerConnection;
WebCore::FetchIdentifier m_fetchIdentifier;
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp (255057 => 255058)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp 2020-01-24 08:23:07 UTC (rev 255058)
@@ -185,7 +185,7 @@
return nullptr;
}
- auto task = makeUnique<ServiceWorkerFetchTask>(loader, ResourceRequest { request }, identifier(), worker->identifier(), *serviceWorkerRegistrationIdentifier, shouldSoftUpdate);
+ auto task = makeUnique<ServiceWorkerFetchTask>(*this, loader, ResourceRequest { request }, identifier(), worker->identifier(), *serviceWorkerRegistrationIdentifier, shouldSoftUpdate);
startFetch(*task, *worker);
return task;
}
@@ -427,6 +427,17 @@
static_cast<WebSWServerToContextConnection&>(connection).send(WTFMove(message));
}
+void WebSWServerConnection::fetchTaskTimedOut(ServiceWorkerIdentifier serviceWorkerIdentifier)
+{
+ auto* worker = server().workerByID(serviceWorkerIdentifier);
+ if (!worker)
+ return;
+
+ worker->setHasTimedOutAnyFetchTasks();
+ if (worker->isRunning())
+ server().syncTerminateWorker(*worker);
+}
+
} // namespace WebKit
#endif // ENABLE(SERVICE_WORKER)
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h (255057 => 255058)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h 2020-01-24 08:23:07 UTC (rev 255058)
@@ -69,6 +69,7 @@
PAL::SessionID sessionID() const;
std::unique_ptr<ServiceWorkerFetchTask> createFetchTask(NetworkResourceLoader&, const WebCore::ResourceRequest&);
+ void fetchTaskTimedOut(WebCore::ServiceWorkerIdentifier);
private:
// Implement SWServer::Connection (Messages to the client WebProcess)
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp (255057 => 255058)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp 2020-01-24 08:23:07 UTC (rev 255058)
@@ -164,28 +164,6 @@
m_ongoingFetches.remove(task.fetchIdentifier());
}
-void WebSWServerToContextConnection::fetchTaskTimedOut(ServiceWorkerIdentifier serviceWorkerIdentifier)
-{
- // Gather all fetches in this service worker.
- Vector<ServiceWorkerFetchTask*> fetches;
- for (auto& fetchTask : m_ongoingFetches.values()) {
- if (fetchTask->serviceWorkerIdentifier() == serviceWorkerIdentifier)
- fetches.append(fetchTask.get());
- }
-
- // Signal load failure for them.
- for (auto* fetchTask : fetches)
- fetchTask->contextClosed();
-
- if (m_server) {
- if (auto* worker = m_server->workerByID(serviceWorkerIdentifier)) {
- worker->setHasTimedOutAnyFetchTasks();
- if (worker->isRunning())
- m_server->syncTerminateWorker(*worker);
- }
- }
-}
-
WebCore::ProcessIdentifier WebSWServerToContextConnection::webProcessIdentifier() const
{
return m_connection.webProcessIdentifier();
Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h (255057 => 255058)
--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h 2020-01-24 08:19:26 UTC (rev 255057)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h 2020-01-24 08:23:07 UTC (rev 255058)
@@ -70,8 +70,6 @@
void setThrottleState(bool isThrottleable);
bool isThrottleable() const { return m_isThrottleable; }
- void fetchTaskTimedOut(WebCore::ServiceWorkerIdentifier);
-
void registerFetch(ServiceWorkerFetchTask&);
void unregisterFetch(ServiceWorkerFetchTask&);
