From 49058aa9d5f283e63f32925ba868b3f4ae730d87 Mon Sep 17 00:00:00 2001 From: Andrew Sutherland Date: Tue, 24 Oct 2023 20:34:25 +0000 Subject: [PATCH] Bug 1856090 - Improve ServiceWorker state propagation. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D190245 --- dom/workers/ScriptLoader.cpp | 21 ++++++++++++++++++--- dom/workers/loader/CacheLoadHandler.cpp | 7 +++---- dom/workers/loader/CacheLoadHandler.h | 6 ++++-- dom/workers/loader/WorkerLoadContext.cpp | 7 +++++-- dom/workers/loader/WorkerLoadContext.h | 8 +++++++- dom/workers/loader/WorkerModuleLoader.cpp | 22 ++++++++++++++++------ 6 files changed, 53 insertions(+), 18 deletions(-) diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 156ac72f44f7..f9c3273fb91d 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -616,8 +616,22 @@ already_AddRefed WorkerScriptLoader::CreateScriptLoadRequest( Maybe clientInfo = GetGlobal()->GetClientInfo(); - RefPtr loadContext = - new WorkerLoadContext(kind, clientInfo, this); + // (For non-serviceworkers, this variable does not matter, but false best + // captures their behavior.) + bool onlyExistingCachedResourcesAllowed = false; + if (mWorkerRef->Private()->IsServiceWorker()) { + // https://w3c.github.io/ServiceWorker/#importscripts step 4: + // > 4. If serviceWorker’s state is not "parsed" or "installing": + // > 1. Return map[url] if it exists and a network error otherwise. + // + // So if our state is beyond installing, it's too late to make a request + // that would perform a new fetch which would be cached. + onlyExistingCachedResourcesAllowed = + mWorkerRef->Private()->GetServiceWorkerDescriptor().State() > + ServiceWorkerState::Installing; + } + RefPtr loadContext = new WorkerLoadContext( + kind, clientInfo, this, onlyExistingCachedResourcesAllowed); // Create ScriptLoadRequests for this WorkerScriptLoader ReferrerPolicy referrerPolicy = mWorkerRef->Private()->GetReferrerPolicy(); @@ -1429,7 +1443,8 @@ nsresult ScriptLoaderRunnable::Run() { handle->mRunnable = this; WorkerLoadContext* loadContext = handle->GetContext(); mCacheCreator->AddLoader(MakeNotNull>( - mWorkerRef, handle, loadContext->IsTopLevel(), mScriptLoader)); + mWorkerRef, handle, loadContext->IsTopLevel(), + loadContext->mOnlyExistingCachedResourcesAllowed, mScriptLoader)); } // The worker may have a null principal on first load, but in that case its diff --git a/dom/workers/loader/CacheLoadHandler.cpp b/dom/workers/loader/CacheLoadHandler.cpp index 1ffa82b07715..3e43ce224c56 100644 --- a/dom/workers/loader/CacheLoadHandler.cpp +++ b/dom/workers/loader/CacheLoadHandler.cpp @@ -237,13 +237,14 @@ void CacheCreator::DeleteCache(nsresult aReason) { CacheLoadHandler::CacheLoadHandler(ThreadSafeWorkerRef* aWorkerRef, ThreadSafeRequestHandle* aRequestHandle, bool aIsWorkerScript, + bool aOnlyExistingCachedResourcesAllowed, WorkerScriptLoader* aLoader) : mRequestHandle(aRequestHandle), mLoader(aLoader), mWorkerRef(aWorkerRef), mIsWorkerScript(aIsWorkerScript), mFailed(false), - mState(aWorkerRef->Private()->GetServiceWorkerDescriptor().State()) { + mOnlyExistingCachedResourcesAllowed(aOnlyExistingCachedResourcesAllowed) { MOZ_ASSERT(aWorkerRef); MOZ_ASSERT(aWorkerRef->Private()->IsServiceWorker()); mMainThreadEventTarget = aWorkerRef->Private()->MainThreadEventTarget(); @@ -373,9 +374,7 @@ void CacheLoadHandler::ResolvedCallback(JSContext* aCx, // storage was probably wiped without removing the service worker // registration. It can also happen for exposed reasons like the // service worker script calling importScripts() after install. - if (NS_WARN_IF(mIsWorkerScript || - (mState != ServiceWorkerState::Parsed && - mState != ServiceWorkerState::Installing))) { + if (NS_WARN_IF(mIsWorkerScript || mOnlyExistingCachedResourcesAllowed)) { Fail(NS_ERROR_DOM_INVALID_STATE_ERR); return; } diff --git a/dom/workers/loader/CacheLoadHandler.h b/dom/workers/loader/CacheLoadHandler.h index c14d5c1d4312..b1e164b79e95 100644 --- a/dom/workers/loader/CacheLoadHandler.h +++ b/dom/workers/loader/CacheLoadHandler.h @@ -82,7 +82,9 @@ class CacheLoadHandler final : public PromiseNativeHandler, CacheLoadHandler(ThreadSafeWorkerRef* aWorkerRef, ThreadSafeRequestHandle* aRequestHandle, - bool aIsWorkerScript, WorkerScriptLoader* aLoader); + bool aIsWorkerScript, + bool aOnlyExistingCachedResourcesAllowed, + WorkerScriptLoader* aLoader); void Fail(nsresult aRv); @@ -110,7 +112,7 @@ class CacheLoadHandler final : public PromiseNativeHandler, RefPtr mWorkerRef; const bool mIsWorkerScript; bool mFailed; - const ServiceWorkerState mState; + bool mOnlyExistingCachedResourcesAllowed; nsCOMPtr mPump; nsCOMPtr mBaseURI; mozilla::dom::ChannelInfo mChannelInfo; diff --git a/dom/workers/loader/WorkerLoadContext.cpp b/dom/workers/loader/WorkerLoadContext.cpp index 2e565bad4604..a788d5173c7a 100644 --- a/dom/workers/loader/WorkerLoadContext.cpp +++ b/dom/workers/loader/WorkerLoadContext.cpp @@ -13,11 +13,14 @@ namespace dom { WorkerLoadContext::WorkerLoadContext( Kind aKind, const Maybe& aClientInfo, - workerinternals::loader::WorkerScriptLoader* aScriptLoader) + workerinternals::loader::WorkerScriptLoader* aScriptLoader, + bool aOnlyExistingCachedResourcesAllowed) : JS::loader::LoadContextBase(JS::loader::ContextKind::Worker), mKind(aKind), mClientInfo(aClientInfo), - mScriptLoader(aScriptLoader){}; + mScriptLoader(aScriptLoader), + mOnlyExistingCachedResourcesAllowed( + aOnlyExistingCachedResourcesAllowed){}; ThreadSafeRequestHandle::ThreadSafeRequestHandle( JS::loader::ScriptLoadRequest* aRequest, nsISerialEventTarget* aSyncTarget) diff --git a/dom/workers/loader/WorkerLoadContext.h b/dom/workers/loader/WorkerLoadContext.h index 9829eb120937..97362f2871ef 100644 --- a/dom/workers/loader/WorkerLoadContext.h +++ b/dom/workers/loader/WorkerLoadContext.h @@ -98,7 +98,8 @@ class WorkerLoadContext : public JS::loader::LoadContextBase { }; WorkerLoadContext(Kind aKind, const Maybe& aClientInfo, - workerinternals::loader::WorkerScriptLoader* aScriptLoader); + workerinternals::loader::WorkerScriptLoader* aScriptLoader, + bool aOnlyExistingCachedResourcesAllowed); // Used to detect if the `is top-level` bit is set on a given module. bool IsTopLevel() { @@ -163,6 +164,11 @@ class WorkerLoadContext : public JS::loader::LoadContextBase { CacheStatus mCacheStatus = Uncached; + // If the requested script is not currently in the cache, should we initiate + // a request to fetch and cache it? Only ServiceWorkers that are being + // installed are allowed to go to the network (and then cache the result). + bool mOnlyExistingCachedResourcesAllowed = false; + bool IsAwaitingPromise() const { return bool(mCachePromise); } }; diff --git a/dom/workers/loader/WorkerModuleLoader.cpp b/dom/workers/loader/WorkerModuleLoader.cpp index c8e35d0768d1..c02d651a7c11 100644 --- a/dom/workers/loader/WorkerModuleLoader.cpp +++ b/dom/workers/loader/WorkerModuleLoader.cpp @@ -50,9 +50,10 @@ already_AddRefed WorkerModuleLoader::CreateStaticImport( // See Discussion in https://github.com/w3c/webappsec-csp/issues/336 Maybe clientInfo = GetGlobalObject()->GetClientInfo(); - RefPtr loadContext = - new WorkerLoadContext(WorkerLoadContext::Kind::StaticImport, clientInfo, - aParent->GetWorkerLoadContext()->mScriptLoader); + RefPtr loadContext = new WorkerLoadContext( + WorkerLoadContext::Kind::StaticImport, clientInfo, + aParent->GetWorkerLoadContext()->mScriptLoader, + aParent->GetWorkerLoadContext()->mOnlyExistingCachedResourcesAllowed); RefPtr request = new ModuleLoadRequest( aURI, aParent->ReferrerPolicy(), aParent->mFetchOptions, SRIMetadata(), aParent->mURI, loadContext, false, /* is top level */ @@ -91,6 +92,8 @@ already_AddRefed WorkerModuleLoader::CreateDynamicImport( } // Not supported for Service Workers. + // https://github.com/w3c/ServiceWorker/issues/1585 covers existing discussion + // about potentially supporting use of import(). if (workerPrivate->IsServiceWorker()) { return nullptr; } @@ -123,9 +126,16 @@ already_AddRefed WorkerModuleLoader::CreateDynamicImport( Maybe clientInfo = GetGlobalObject()->GetClientInfo(); - RefPtr context = - new WorkerLoadContext(WorkerLoadContext::Kind::DynamicImport, clientInfo, - GetCurrentScriptLoader()); + RefPtr context = new WorkerLoadContext( + WorkerLoadContext::Kind::DynamicImport, clientInfo, + GetCurrentScriptLoader(), + // When dynamic import is supported in ServiceWorkers, + // the current plan in onlyExistingCachedResourcesAllowed + // is that only existing cached resources will be + // allowed. (`import()` will not be used for caching + // side effects, but instead a specific method will be + // used during installation.) + true); ReferrerPolicy referrerPolicy = workerPrivate->GetReferrerPolicy(); RefPtr request = new ModuleLoadRequest(