Diff
Modified: trunk/LayoutTests/ChangeLog (278273 => 278274)
--- trunk/LayoutTests/ChangeLog 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/LayoutTests/ChangeLog 2021-05-31 08:29:02 UTC (rev 278274)
@@ -1,3 +1,16 @@
+2021-05-31 Youenn Fablet <you...@apple.com>
+
+ Go to network in case fetch event is not yet responded when being destroyed instead of failing the load
+ https://bugs.webkit.org/show_bug.cgi?id=226374
+ <rdar://78298472>
+
+ Reviewed by Alex Christensen.
+
+ * http/wpt/service-workers/fetch-worker-terminate-worker.js: Added.
+ (doTest):
+ * http/wpt/service-workers/fetch-worker-terminate.https-expected.txt: Added.
+ * http/wpt/service-workers/fetch-worker-terminate.https.html: Added.
+
2021-05-30 Wenson Hsieh <wenson_hs...@apple.com>
REGRESSION (r258118): SVG paths that contain a single move command incorrect client bounding rects
Added: trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate-worker.js (0 => 278274)
--- trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate-worker.js (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate-worker.js 2021-05-31 08:29:02 UTC (rev 278274)
@@ -0,0 +1,14 @@
+function doTest(event)
+{
+ if (!self.internals)
+ return;
+
+ if (event.request.url.includes("terminate-while-fetching")) {
+ event.respondWith(new Promise(resolve => {
+ // we do not resolve the promise.
+ setTimeout(() => internals.terminate(), 500);
+ }));
+ }
+}
+
+self.addEventListener("fetch", doTest);
Added: trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate.https-expected.txt (0 => 278274)
--- trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate.https-expected.txt (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate.https-expected.txt 2021-05-31 08:29:02 UTC (rev 278274)
@@ -0,0 +1,5 @@
+
+
+PASS Setup worker
+PASS Make sure a load goes to the network when worker terminates while load is not yet responded
+
Added: trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate.https.html (0 => 278274)
--- trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate.https.html (rev 0)
+++ trunk/LayoutTests/http/wpt/service-workers/fetch-worker-terminate.https.html 2021-05-31 08:29:02 UTC (rev 278274)
@@ -0,0 +1,34 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+var scope = "resources";
+var activeWorker;
+
+promise_test(async (test) => {
+ var registration = await navigator.serviceWorker.register("fetch-worker-terminate-worker.js", { scope : scope });
+ activeWorker = registration.active;
+ if (activeWorker)
+ return;
+ activeWorker = registration.installing;
+ return new Promise(resolve => {
+ activeWorker.addEventListener('statechange', () => {
+ if (activeWorker.state === "activated")
+ resolve();
+ });
+ });
+}, "Setup worker");
+
+promise_test(async (test) => {
+ const iframe = await with_iframe(scope);
+
+ const response = await iframe.contentWindow.fetch("/terminate-while-fetching");
+ assert_equals(response.status, 404);
+}, "Make sure a load goes to the network when worker terminates while load is not yet responded");
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (278273 => 278274)
--- trunk/Source/WebCore/ChangeLog 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/Source/WebCore/ChangeLog 2021-05-31 08:29:02 UTC (rev 278274)
@@ -1,3 +1,35 @@
+2021-05-31 Youenn Fablet <you...@apple.com>
+
+ Go to network in case fetch event is not yet responded when being destroyed instead of failing the load
+ https://bugs.webkit.org/show_bug.cgi?id=226374
+ <rdar://78298472>
+
+ Reviewed by Alex Christensen.
+
+ In case worker is terminated, instead of failing fetch events that are pending a response, we should go to the network.
+ This mirrors what is already done in ServiceWorkerFetchTask.
+
+ This can for instance happen in case a lot of fetches are done in parallel on the same service worker.
+ The service worker will do the fetch itself but given there are lots of fetches, some fetch might not start until other loads are complete.
+ This may trigger the fetch timeout which might then trigger terminating the worker.
+ We should probably revisit our fetch timeout policy now that we have added worker spin detection.
+
+ Test: http/wpt/service-workers/fetch-worker-terminate.https.html
+
+ * testing/ServiceWorkerInternals.cpp:
+ (WebCore::ServiceWorkerInternals::terminate):
+ (WebCore::ServiceWorkerInternals::waitForFetchEventToFinish):
+ * testing/ServiceWorkerInternals.h:
+ * testing/ServiceWorkerInternals.idl:
+ * workers/service/FetchEvent.cpp:
+ (WebCore::FetchEvent::~FetchEvent):
+ Update logging to only log the case where respondWith is called but fetch event is destroyed before processing the response.
+ Otherwise, we would log the case of respondWith being never called, which is happening often and leads to go to the network.
+ (WebCore::FetchEvent::processResponse):
+ * workers/service/FetchEvent.h:
+ * workers/service/context/ServiceWorkerFetch.cpp:
+ (WebCore::ServiceWorkerFetch::processResponse):
+
2021-05-30 Wenson Hsieh <wenson_hs...@apple.com>
REGRESSION (r258118): SVG paths that contain a single move command incorrect client bounding rects
Modified: trunk/Source/WebCore/testing/ServiceWorkerInternals.cpp (278273 => 278274)
--- trunk/Source/WebCore/testing/ServiceWorkerInternals.cpp 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/Source/WebCore/testing/ServiceWorkerInternals.cpp 2021-05-31 08:29:02 UTC (rev 278274)
@@ -50,13 +50,24 @@
});
}
+void ServiceWorkerInternals::terminate()
+{
+ callOnMainThread([identifier = m_identifier] () {
+ SWContextManager::singleton().terminateWorker(identifier, Seconds::infinity(), [] { });
+ });
+}
+
void ServiceWorkerInternals::waitForFetchEventToFinish(FetchEvent& event, DOMPromiseDeferred<IDLInterface<FetchResponse>>&& promise)
{
event.onResponse([promise = WTFMove(promise), event = makeRef(event)] (auto&& result) mutable {
- if (result.has_value())
- promise.resolve(WTFMove(result.value()));
- else
- promise.reject(TypeError, result.error().localizedDescription());
+ if (!result.has_value()) {
+ String description;
+ if (auto& error = result.error())
+ description = error->localizedDescription();
+ promise.reject(TypeError, description);
+ return;
+ }
+ promise.resolve(WTFMove(result.value()));
});
}
Modified: trunk/Source/WebCore/testing/ServiceWorkerInternals.h (278273 => 278274)
--- trunk/Source/WebCore/testing/ServiceWorkerInternals.h 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/Source/WebCore/testing/ServiceWorkerInternals.h 2021-05-31 08:29:02 UTC (rev 278274)
@@ -46,6 +46,8 @@
~ServiceWorkerInternals();
void setOnline(bool isOnline);
+ void terminate();
+
void waitForFetchEventToFinish(FetchEvent&, DOMPromiseDeferred<IDLInterface<FetchResponse>>&&);
Ref<FetchEvent> createBeingDispatchedFetchEvent(ScriptExecutionContext&);
Ref<FetchResponse> createOpaqueWithBlobBodyResponse(ScriptExecutionContext&);
Modified: trunk/Source/WebCore/testing/ServiceWorkerInternals.idl (278273 => 278274)
--- trunk/Source/WebCore/testing/ServiceWorkerInternals.idl 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/Source/WebCore/testing/ServiceWorkerInternals.idl 2021-05-31 08:29:02 UTC (rev 278274)
@@ -30,6 +30,8 @@
LegacyNoInterfaceObject,
] interface ServiceWorkerInternals {
undefined setOnline(boolean isOnline);
+ undefined terminate();
+
Promise<FetchResponse> waitForFetchEventToFinish(FetchEvent event);
[CallWith=ScriptExecutionContext] FetchEvent createBeingDispatchedFetchEvent();
[CallWith=ScriptExecutionContext] FetchResponse createOpaqueWithBlobBodyResponse();
Modified: trunk/Source/WebCore/workers/service/FetchEvent.cpp (278273 => 278274)
--- trunk/Source/WebCore/workers/service/FetchEvent.cpp 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/Source/WebCore/workers/service/FetchEvent.cpp 2021-05-31 08:29:02 UTC (rev 278274)
@@ -56,8 +56,8 @@
FetchEvent::~FetchEvent()
{
if (auto callback = WTFMove(m_onResponse)) {
- RELEASE_LOG_ERROR(ServiceWorker, "Fetch event is destroyed without a response, respondWithEntered=%d, waitToRespond=%d, respondWithError=%d, respondPromise=%d", m_respondWithEntered, m_waitToRespond, m_respondWithError, !!m_respondPromise);
- callback(makeUnexpected(ResourceError { errorDomainWebKitServiceWorker, 0, m_request->url(), "Fetch event is destroyed."_s, ResourceError::Type::Cancellation }));
+ RELEASE_LOG_ERROR_IF(m_respondWithEntered, ServiceWorker, "Fetch event is destroyed without a response, respondWithEntered=%d, waitToRespond=%d, respondWithError=%d, respondPromise=%d", m_respondWithEntered, m_waitToRespond, m_respondWithError, !!m_respondPromise);
+ callback(makeUnexpected(std::optional<ResourceError> { }));
}
}
@@ -106,7 +106,7 @@
processResponse(makeUnexpected(WTFMove(error)));
}
-void FetchEvent::processResponse(Expected<Ref<FetchResponse>, ResourceError>&& result)
+void FetchEvent::processResponse(Expected<Ref<FetchResponse>, std::optional<ResourceError>>&& result)
{
m_respondPromise = nullptr;
m_waitToRespond = false;
Modified: trunk/Source/WebCore/workers/service/FetchEvent.h (278273 => 278274)
--- trunk/Source/WebCore/workers/service/FetchEvent.h 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/Source/WebCore/workers/service/FetchEvent.h 2021-05-31 08:29:02 UTC (rev 278274)
@@ -59,7 +59,7 @@
ExceptionOr<void> respondWith(Ref<DOMPromise>&&);
- using ResponseCallback = CompletionHandler<void(Expected<Ref<FetchResponse>, ResourceError>&&)>;
+ using ResponseCallback = CompletionHandler<void(Expected<Ref<FetchResponse>, std::optional<ResourceError>>&&)>;
WEBCORE_EXPORT void onResponse(ResponseCallback&&);
FetchRequest& request() { return m_request.get(); }
@@ -75,7 +75,7 @@
WEBCORE_EXPORT FetchEvent(const AtomString&, Init&&, IsTrusted);
void promiseIsSettled();
- void processResponse(Expected<Ref<FetchResponse>, ResourceError>&&);
+ void processResponse(Expected<Ref<FetchResponse>, std::optional<ResourceError>>&&);
void respondWithError(ResourceError&&);
Ref<FetchRequest> m_request;
Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp (278273 => 278274)
--- trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp 2021-05-31 07:51:36 UTC (rev 278273)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp 2021-05-31 08:29:02 UTC (rev 278274)
@@ -65,10 +65,15 @@
return { };
}
-static void processResponse(Ref<Client>&& client, Expected<Ref<FetchResponse>, ResourceError>&& result, FetchOptions::Mode mode, FetchOptions::Redirect redirect, const URL& requestURL, CertificateInfo&& certificateInfo)
+static void processResponse(Ref<Client>&& client, Expected<Ref<FetchResponse>, std::optional<ResourceError>>&& result, FetchOptions::Mode mode, FetchOptions::Redirect redirect, const URL& requestURL, CertificateInfo&& certificateInfo)
{
if (!result.has_value()) {
- client->didFail(result.error());
+ auto& error = result.error();
+ if (!error) {
+ client->didNotHandle();
+ return;
+ }
+ client->didFail(*error);
return;
}
auto response = WTFMove(result.value());
