Log Message
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.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (250757 => 250758)
--- trunk/LayoutTests/ChangeLog 2019-10-04 23:43:24 UTC (rev 250757)
+++ trunk/LayoutTests/ChangeLog 2019-10-05 00:40:04 UTC (rev 250758)
@@ -1,5 +1,17 @@
2019-10-04 Chris Dumez <cdu...@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 <cdu...@apple.com>
+
Allow pages using IDBIndex to enter the PageCache
https://bugs.webkit.org/show_bug.cgi?id=202430
<rdar://problem/55887918>
Added: trunk/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt (0 => 250758)
--- trunk/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https-expected.txt 2019-10-05 00:40:04 UTC (rev 250758)
@@ -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
+
Added: trunk/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https.html (0 => 250758)
--- trunk/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https.html (rev 0)
+++ trunk/LayoutTests/http/tests/workers/service/page-cache-service-worker-pending-promise.https.html 2019-10-05 00:40:04 UTC (rev 250758)
@@ -0,0 +1,45 @@
+<!-- webkit-test-runner [ enablePageCache=true ] -->
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<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>
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt (250757 => 250758)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt 2019-10-04 23:43:24 UTC (rev 250757)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https-expected.txt 2019-10-05 00:40:04 UTC (rev 250758)
@@ -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
Modified: trunk/Source/WebCore/ChangeLog (250757 => 250758)
--- trunk/Source/WebCore/ChangeLog 2019-10-04 23:43:24 UTC (rev 250757)
+++ trunk/Source/WebCore/ChangeLog 2019-10-05 00:40:04 UTC (rev 250758)
@@ -1,5 +1,30 @@
2019-10-04 Chris Dumez <cdu...@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 <cdu...@apple.com>
+
Allow pages using IDBIndex to enter the PageCache
https://bugs.webkit.org/show_bug.cgi?id=202430
<rdar://problem/55887918>
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp (250757 => 250758)
--- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp 2019-10-04 23:43:24 UTC (rev 250757)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp 2019-10-05 00:40:04 UTC (rev 250758)
@@ -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 = "" shouldNotifyWhenResolved, notifyIfExitEarly = WTFMove(notifyIfExitEarly)]() mutable {
+ notifyIfExitEarly.release();
- scriptExecutionContext()->postTask([this, protectedThis = RefPtr<ServiceWorkerContainer>(this), promise = WTFMove(promise), jobIdentifier = job.identifier(), data = "" shouldNotifyWhenResolved](ScriptExecutionContext& context) mutable {
- if (isStopped()) {
- if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
- notifyRegistrationIsSettled(data.key);
- return;
- }
+ auto registration = ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(data));
- auto registration = ServiceWorkerRegistration::getOrCreate(context, *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()
Modified: trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h (250757 => 250758)
--- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h 2019-10-04 23:43:24 UTC (rev 250757)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h 2019-10-05 00:40:04 UTC (rev 250758)
@@ -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
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes