ServiceWorkerContainer should never prevent a page from entering the back/forward cache
https://bugs.webkit.org/show_bug.cgi?id=202603
Reviewed by Geoff Garen.
Source/WebCore:
Make it so that ServiceWorkerContainer can suspend, even if they have pending promises.
We now queue all promise resolutions to a SuspendableTaskQueue to make sure that those
promises do not get resolved while in the page cache.
Test: http/tests/workers/service/page-cache-service-worker-pending-promise.https.html
* workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::ServiceWorkerContainer):
(WebCore::ServiceWorkerContainer::ready):
(WebCore::ServiceWorkerContainer::getRegistration):
(WebCore::ServiceWorkerContainer::getRegistrations):
(WebCore::ServiceWorkerContainer::jobFailedWithException):
(WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):
(WebCore::ServiceWorkerContainer::jobResolvedWithUnregistrationResult):
(WebCore::ServiceWorkerContainer::jobFailedLoadingScript):
(WebCore::ServiceWorkerContainer::canSuspendForDocumentSuspension const):
* workers/service/ServiceWorkerContainer.h:
LayoutTests:
Add layout test coverage.
* http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt: Added.
* http/tests/workers/service/page-cache-service-worker-pending-promise.https.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@250758 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8a56cee..2fd44bb 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,5 +1,17 @@
2019-10-04 Chris Dumez <cdumez@apple.com>
+ ServiceWorkerContainer should never prevent a page from entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=202603
+
+ Reviewed by Geoff Garen.
+
+ Add layout test coverage.
+
+ * http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt: Added.
+ * http/tests/workers/service/page-cache-service-worker-pending-promise.https.html: Added.
+
+2019-10-04 Chris Dumez <cdumez@apple.com>
+
Allow pages using IDBIndex to enter the PageCache
https://bugs.webkit.org/show_bug.cgi?id=202430
<rdar://problem/55887918>
diff --git a/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt b/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt
new file mode 100644
index 0000000..a450b20
--- /dev/null
+++ b/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt
@@ -0,0 +1,15 @@
+Tests that a page with a pending service worker promise is able to enter the page cache.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page was restored from Page Cache
+PASS Unregistered successfuly
+PASS !!restoredFromPageCache is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https.html b/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https.html
new file mode 100644
index 0000000..09f0568
--- /dev/null
+++ b/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https.html
@@ -0,0 +1,45 @@
+<!-- webkit-test-runner [ enablePageCache=true ] -->
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+description("Tests that a page with a pending service worker promise is able to enter the page cache.");
+jsTestIsAsync = true;
+
+let restoredFromPageCache = false;
+
+window.addEventListener("pageshow", function(event) {
+ debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+ if (event.persisted) {
+ testPassed("Page was restored from Page Cache");
+ restoredFromPageCache = true;
+ }
+});
+
+window.addEventListener("pagehide", function(event) {
+ debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+ if (!event.persisted) {
+ testFailed("Page failed to enter the Page Cache");
+ finishJSTest();
+ }
+
+ registration.unregister().then(() => {
+ testPassed("Unregistered successfuly");
+ shouldBeTrue("!!restoredFromPageCache");
+ finishJSTest();
+ }, () => {
+ testFailed("Failed to unregister");
+ finishJSTest();
+ });
+});
+
+onload = () => {
+ navigator.serviceWorker.register("resources/basic-fetch-worker.js", { scope: "/workers/service/resources/" }).then((_registration) => {
+ registration = _registration;
+ window.location = "/navigation/resources/page-cache-helper.html";
+ });
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt
index 812adc2..0ea9d58 100644
--- a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt
+++ b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt
@@ -1,3 +1,4 @@
-FAIL Test skipWaiting while a client is not being controlled promise_test: Unhandled rejection with value: object "TypeError: null is not an object (evaluating 'document.body.appendChild')"
+PASS Test skipWaiting while a client is not being controlled
+PASS skipWaiting
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0f457911..3baba08 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,30 @@
2019-10-04 Chris Dumez <cdumez@apple.com>
+ ServiceWorkerContainer should never prevent a page from entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=202603
+
+ Reviewed by Geoff Garen.
+
+ Make it so that ServiceWorkerContainer can suspend, even if they have pending promises.
+ We now queue all promise resolutions to a SuspendableTaskQueue to make sure that those
+ promises do not get resolved while in the page cache.
+
+ Test: http/tests/workers/service/page-cache-service-worker-pending-promise.https.html
+
+ * workers/service/ServiceWorkerContainer.cpp:
+ (WebCore::ServiceWorkerContainer::ServiceWorkerContainer):
+ (WebCore::ServiceWorkerContainer::ready):
+ (WebCore::ServiceWorkerContainer::getRegistration):
+ (WebCore::ServiceWorkerContainer::getRegistrations):
+ (WebCore::ServiceWorkerContainer::jobFailedWithException):
+ (WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):
+ (WebCore::ServiceWorkerContainer::jobResolvedWithUnregistrationResult):
+ (WebCore::ServiceWorkerContainer::jobFailedLoadingScript):
+ (WebCore::ServiceWorkerContainer::canSuspendForDocumentSuspension const):
+ * workers/service/ServiceWorkerContainer.h:
+
+2019-10-04 Chris Dumez <cdumez@apple.com>
+
Allow pages using IDBIndex to enter the PageCache
https://bugs.webkit.org/show_bug.cgi?id=202430
<rdar://problem/55887918>
diff --git a/Source/WebCore/workers/service/ServiceWorkerContainer.cpp b/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
index 310e40f..28d5dbe 100644
--- a/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
+++ b/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
@@ -72,6 +72,7 @@
: ActiveDOMObject(context)
, m_navigator(navigator)
, m_messageQueue(GenericEventQueue::create(*this))
+ , m_taskQueue(SuspendableTaskQueue::create(context))
{
suspendIfNeeded();
@@ -107,11 +108,10 @@
auto& context = *scriptExecutionContext();
ensureSWClientConnection().whenRegistrationReady(context.topOrigin().data(), context.url(), [this, protectedThis = makeRef(*this)](auto&& registrationData) mutable {
- if (m_isStopped)
- return;
-
- auto registration = ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(registrationData));
- m_readyPromise->resolve(WTFMove(registration));
+ m_taskQueue->enqueueTask([this, registrationData = WTFMove(registrationData)]() mutable {
+ auto registration = ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(registrationData));
+ m_readyPromise->resolve(WTFMove(registration));
+ });
});
}
return *m_readyPromise;
@@ -270,14 +270,13 @@
}
ensureSWClientConnection().matchRegistration(SecurityOriginData { context.topOrigin().data() }, parsedURL, [this, protectedThis = makeRef(*this), promise = WTFMove(promise)](auto&& result) mutable {
- if (m_isStopped)
- return;
-
- if (!result) {
- promise->resolve();
- return;
- }
- promise->resolve<IDLInterface<ServiceWorkerRegistration>>(ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(result.value())));
+ m_taskQueue->enqueueTask([this, promise = WTFMove(promise), result = WTFMove(result)]() mutable {
+ if (!result) {
+ promise->resolve();
+ return;
+ }
+ promise->resolve<IDLInterface<ServiceWorkerRegistration>>(ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(result.value())));
+ });
});
}
@@ -303,13 +302,12 @@
auto& context = *scriptExecutionContext();
ensureSWClientConnection().getRegistrations(SecurityOriginData { context.topOrigin().data() }, context.url(), [this, protectedThis = makeRef(*this), promise = WTFMove(promise)] (auto&& registrationDatas) mutable {
- if (m_isStopped)
- return;
-
- auto registrations = WTF::map(WTFMove(registrationDatas), [&](auto&& registrationData) {
- return ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(registrationData));
+ m_taskQueue->enqueueTask([this, promise = WTFMove(promise), registrationDatas = WTFMove(registrationDatas)]() mutable {
+ auto registrations = WTF::map(WTFMove(registrationDatas), [&](auto&& registrationData) {
+ return ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(registrationData));
+ });
+ promise->resolve<IDLSequence<IDLInterface<ServiceWorkerRegistration>>>(WTFMove(registrations));
});
- promise->resolve<IDLSequence<IDLInterface<ServiceWorkerRegistration>>>(WTFMove(registrations));
});
}
@@ -336,11 +334,9 @@
if (!promise)
return;
- if (auto* context = scriptExecutionContext()) {
- context->postTask([promise = WTFMove(promise), exception](auto&) mutable {
- promise->reject(exception);
- });
- }
+ m_taskQueue->enqueueTask([promise = WTFMove(promise), exception]() mutable {
+ promise->reject(exception);
+ });
}
void ServiceWorkerContainer::fireUpdateFoundEvent(ServiceWorkerRegistrationIdentifier identifier)
@@ -371,7 +367,7 @@
destroyJob(job);
});
- auto notifyIfExitEarly = WTF::makeScopeExit([this, &data, &shouldNotifyWhenResolved] {
+ auto notifyIfExitEarly = WTF::makeScopeExit([this, protectedThis = makeRef(*this), &data, &shouldNotifyWhenResolved] {
if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
notifyRegistrationIsSettled(data.key);
});
@@ -383,22 +379,16 @@
if (!promise)
return;
- notifyIfExitEarly.release();
+ m_taskQueue->enqueueTask([this, promise = WTFMove(promise), jobIdentifier = job.identifier(), data = WTFMove(data), shouldNotifyWhenResolved, notifyIfExitEarly = WTFMove(notifyIfExitEarly)]() mutable {
+ notifyIfExitEarly.release();
- scriptExecutionContext()->postTask([this, protectedThis = RefPtr<ServiceWorkerContainer>(this), promise = WTFMove(promise), jobIdentifier = job.identifier(), data = WTFMove(data), shouldNotifyWhenResolved](ScriptExecutionContext& context) mutable {
- if (isStopped()) {
- if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
- notifyRegistrationIsSettled(data.key);
- return;
- }
-
- auto registration = ServiceWorkerRegistration::getOrCreate(context, *this, WTFMove(data));
+ auto registration = ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(data));
CONTAINER_RELEASE_LOG_IF_ALLOWED("jobResolvedWithRegistration: Resolving promise for job %" PRIu64 ". Registration ID: %" PRIu64, jobIdentifier.toUInt64(), registration->identifier().toUInt64());
if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes) {
m_ongoingSettledRegistrations.add(++m_lastOngoingSettledRegistrationIdentifier, registration->data().key);
- promise->whenSettled([this, protectedThis = WTFMove(protectedThis), identifier = m_lastOngoingSettledRegistrationIdentifier] {
+ promise->whenSettled([this, protectedThis = makeRef(*this), identifier = m_lastOngoingSettledRegistrationIdentifier] {
notifyRegistrationIsSettled(m_ongoingSettledRegistrations.take(identifier));
});
}
@@ -444,7 +434,7 @@
return;
}
- context->postTask([promise = job.takePromise(), unregistrationResult](auto&) mutable {
+ m_taskQueue->enqueueTask([promise = job.takePromise(), unregistrationResult]() mutable {
promise->resolve<IDLBoolean>(unregistrationResult);
});
}
@@ -490,8 +480,11 @@
CONTAINER_RELEASE_LOG_ERROR_IF_ALLOWED("jobFinishedLoadingScript: Failed to fetch script for job %" PRIu64 ", error: %s", job.identifier().toUInt64(), error.localizedDescription().utf8().data());
- if (auto promise = job.takePromise())
- promise->reject(WTFMove(exception));
+ if (auto promise = job.takePromise()) {
+ m_taskQueue->enqueueTask([promise = WTFMove(promise), exception = WTFMove(exception)]() mutable {
+ promise->reject(WTFMove(exception));
+ });
+ }
notifyFailedFetchingScript(job, error);
destroyJob(job);
@@ -521,7 +514,7 @@
bool ServiceWorkerContainer::canSuspendForDocumentSuspension() const
{
- return !hasPendingActivity();
+ return true;
}
SWClientConnection& ServiceWorkerContainer::ensureSWClientConnection()
diff --git a/Source/WebCore/workers/service/ServiceWorkerContainer.h b/Source/WebCore/workers/service/ServiceWorkerContainer.h
index 2e0d9d0..461e70e 100644
--- a/Source/WebCore/workers/service/ServiceWorkerContainer.h
+++ b/Source/WebCore/workers/service/ServiceWorkerContainer.h
@@ -36,6 +36,7 @@
#include "ServiceWorkerJobClient.h"
#include "ServiceWorkerRegistration.h"
#include "ServiceWorkerRegistrationOptions.h"
+#include "SuspendableTaskQueue.h"
#include "WorkerType.h"
#include <wtf/Threading.h>
@@ -144,6 +145,7 @@
uint64_t m_lastOngoingSettledRegistrationIdentifier { 0 };
HashMap<uint64_t, ServiceWorkerRegistrationKey> m_ongoingSettledRegistrations;
UniqueRef<GenericEventQueue> m_messageQueue;
+ UniqueRef<SuspendableTaskQueue> m_taskQueue;
};
} // namespace WebCore
