From 66f73c547b6c9b2eb195f145802befaf404331f4 Mon Sep 17 00:00:00 2001 From: Norisz Fay Date: Fri, 15 Mar 2024 13:28:07 +0200 Subject: [PATCH] Backed out 4 changesets (bug 1877703) as requested by dev for causing Bug 1885443 Backed out changeset 5eb60e36ef79 (bug 1877703) Backed out changeset 5921d1fb831e (bug 1877703) Backed out changeset 64281b11237e (bug 1877703) Backed out changeset 7ee0827809fb (bug 1877703) --- .../jsmodules/importmaps/classic_script.js | 1 - .../test/jsmodules/importmaps/mochitest.toml | 5 - .../jsmodules/importmaps/module_chain_1.mjs | 2 - .../jsmodules/importmaps/module_chain_2.mjs | 1 - ...test_dynamic_importMap_load_completes.html | 33 ------ ...ynamic_importMap_with_external_script.html | 81 -------------- dom/script/ScriptLoadContext.h | 2 +- dom/script/ScriptLoader.cpp | 23 ++-- js/loader/LoadContextBase.h | 3 - js/loader/LoadedScript.cpp | 4 - js/loader/LoadedScript.h | 6 -- js/loader/ModuleLoadRequest.cpp | 4 - js/loader/ModuleLoaderBase.cpp | 100 +++++------------- js/loader/ModuleLoaderBase.h | 22 ++-- js/public/Modules.h | 2 - js/src/vm/Modules.cpp | 5 - 16 files changed, 45 insertions(+), 249 deletions(-) delete mode 100644 dom/base/test/jsmodules/importmaps/classic_script.js delete mode 100644 dom/base/test/jsmodules/importmaps/module_chain_1.mjs delete mode 100644 dom/base/test/jsmodules/importmaps/module_chain_2.mjs delete mode 100644 dom/base/test/jsmodules/importmaps/test_dynamic_importMap_load_completes.html delete mode 100644 dom/base/test/jsmodules/importmaps/test_dynamic_importMap_with_external_script.html diff --git a/dom/base/test/jsmodules/importmaps/classic_script.js b/dom/base/test/jsmodules/importmaps/classic_script.js deleted file mode 100644 index d7ae0be05441..000000000000 --- a/dom/base/test/jsmodules/importmaps/classic_script.js +++ /dev/null @@ -1 +0,0 @@ -// Empty script. diff --git a/dom/base/test/jsmodules/importmaps/mochitest.toml b/dom/base/test/jsmodules/importmaps/mochitest.toml index 72ca6dc10c7d..422945572292 100644 --- a/dom/base/test/jsmodules/importmaps/mochitest.toml +++ b/dom/base/test/jsmodules/importmaps/mochitest.toml @@ -3,9 +3,6 @@ support-files = [ "bug_1865410_module_a.mjs", "bug_1865410_module_b.mjs", "bug_1873417.mjs", - "classic_script.js", - "module_chain_1.mjs", - "module_chain_2.mjs", "module_importMap_with_external_script_0.mjs", "module_importMap_with_external_script_1.mjs", "module_importMap_with_external_script_2.mjs", @@ -34,5 +31,3 @@ support-files = [ ["test_bug_1873417.html"] ["test_importMap_with_external_script.html"] -["test_dynamic_importMap_with_external_script.html"] -["test_dynamic_importMap_load_completes.html"] diff --git a/dom/base/test/jsmodules/importmaps/module_chain_1.mjs b/dom/base/test/jsmodules/importmaps/module_chain_1.mjs deleted file mode 100644 index d9515fab7f3f..000000000000 --- a/dom/base/test/jsmodules/importmaps/module_chain_1.mjs +++ /dev/null @@ -1,2 +0,0 @@ -// eslint-disable-next-line import/no-unassigned-import -import {} from "./module_chain_2.mjs"; diff --git a/dom/base/test/jsmodules/importmaps/module_chain_2.mjs b/dom/base/test/jsmodules/importmaps/module_chain_2.mjs deleted file mode 100644 index ce12406a767c..000000000000 --- a/dom/base/test/jsmodules/importmaps/module_chain_2.mjs +++ /dev/null @@ -1 +0,0 @@ -loaded = true; diff --git a/dom/base/test/jsmodules/importmaps/test_dynamic_importMap_load_completes.html b/dom/base/test/jsmodules/importmaps/test_dynamic_importMap_load_completes.html deleted file mode 100644 index da354c1ca057..000000000000 --- a/dom/base/test/jsmodules/importmaps/test_dynamic_importMap_load_completes.html +++ /dev/null @@ -1,33 +0,0 @@ - - - -Test script loading complets when there's a dynamicly inserted import map - - - - - - - - - - - - - - - - - diff --git a/dom/base/test/jsmodules/importmaps/test_dynamic_importMap_with_external_script.html b/dom/base/test/jsmodules/importmaps/test_dynamic_importMap_with_external_script.html deleted file mode 100644 index b78992fb874f..000000000000 --- a/dom/base/test/jsmodules/importmaps/test_dynamic_importMap_with_external_script.html +++ /dev/null @@ -1,81 +0,0 @@ - - - -Test speculative preload of external script doesn't conflict with import map - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/dom/script/ScriptLoadContext.h b/dom/script/ScriptLoadContext.h index d32553c4d44b..74f7c6fe4ffd 100644 --- a/dom/script/ScriptLoadContext.h +++ b/dom/script/ScriptLoadContext.h @@ -147,7 +147,7 @@ class ScriptLoadContext : public JS::loader::LoadContextBase, static void PrioritizeAsPreload(nsIChannel* aChannel); - bool IsPreload() const override; + bool IsPreload() const; bool CompileStarted() const; diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 092937cc9eaf..4fa08e074d37 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -1097,6 +1097,19 @@ bool ScriptLoader::ProcessExternalScript(nsIScriptElement* aElement, return false; } + if (request && request->IsModuleRequest() && + mModuleLoader->HasImportMapRegistered() && + request->mState > ScriptLoadRequest::State::Compiling) { + // We don't preload module scripts after seeing an import map but a script + // can dynamically insert an import map after preloading has happened. + // + // In the case of an import map is inserted after preloading has happened, + // We also check if the request has started loading imports, if not then we + // can reuse the preloaded request. + request->Cancel(); + request = nullptr; + } + if (request) { // Use the preload request. @@ -1386,16 +1399,6 @@ bool ScriptLoader::ProcessInlineScript(nsIScriptElement* aElement, return false; } - // Remove any module preloads. Module specifier resolution is invalidated by - // adding an import map, and incorrect dependencies may have been loaded. - mPreloads.RemoveElementsBy([](const PreloadInfo& info) { - if (info.mRequest->IsModuleRequest()) { - info.mRequest->Cancel(); - return true; - } - return false; - }); - // TODO: Bug 1781758: Move RegisterImportMap into EvaluateScriptElement. // // https://html.spec.whatwg.org/multipage/scripting.html#execute-the-script-element diff --git a/js/loader/LoadContextBase.h b/js/loader/LoadContextBase.h index 5dc78070cb41..a8ce04554685 100644 --- a/js/loader/LoadContextBase.h +++ b/js/loader/LoadContextBase.h @@ -52,9 +52,6 @@ class LoadContextBase : public nsISupports { // Used to output a string for the Gecko Profiler. virtual void GetProfilerLabel(nsACString& aOutString); - // Whether this is a preload, for contexts that support them. - virtual bool IsPreload() const { return false; } - // Casting to the different contexts bool IsWindowContext() const { return mKind == ContextKind::Window; } mozilla::dom::ScriptLoadContext* AsWindowContext(); diff --git a/js/loader/LoadedScript.cpp b/js/loader/LoadedScript.cpp index 6712b35f49f9..7ddd04168ebf 100644 --- a/js/loader/LoadedScript.cpp +++ b/js/loader/LoadedScript.cpp @@ -207,7 +207,6 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_END ModuleScript::ModuleScript(mozilla::dom::ReferrerPolicy aReferrerPolicy, ScriptFetchOptions* aFetchOptions, nsIURI* aURI) : LoadedScript(ScriptKind::eModule, aReferrerPolicy, aFetchOptions, aURI), - mHadImportMap(false), mDebuggerDataInitialized(false) { MOZ_ASSERT(!ModuleRecord()); MOZ_ASSERT(!HasParseError()); @@ -280,9 +279,6 @@ void ModuleScript::SetErrorToRethrow(const JS::Value& aError) { mErrorToRethrow = aError; } -void ModuleScript::SetForPreload(bool aValue) { mForPreload = aValue; } -void ModuleScript::SetHadImportMap(bool aValue) { mHadImportMap = aValue; } - void ModuleScript::SetDebuggerDataInitialized() { MOZ_ASSERT(ModuleRecord()); MOZ_ASSERT(!mDebuggerDataInitialized); diff --git a/js/loader/LoadedScript.h b/js/loader/LoadedScript.h index e5bb28d60f7a..ca6d1fc17967 100644 --- a/js/loader/LoadedScript.h +++ b/js/loader/LoadedScript.h @@ -332,8 +332,6 @@ class ModuleScript final : public LoadedScript { JS::Heap mModuleRecord; JS::Heap mParseError; JS::Heap mErrorToRethrow; - bool mForPreload; - bool mHadImportMap; bool mDebuggerDataInitialized; ~ModuleScript(); @@ -354,8 +352,6 @@ class ModuleScript final : public LoadedScript { void SetModuleRecord(JS::Handle aModuleRecord); void SetParseError(const JS::Value& aError); void SetErrorToRethrow(const JS::Value& aError); - void SetForPreload(bool aValue); - void SetHadImportMap(bool aValue); void SetDebuggerDataInitialized(); JSObject* ModuleRecord() const { return mModuleRecord; } @@ -364,8 +360,6 @@ class ModuleScript final : public LoadedScript { JS::Value ErrorToRethrow() const { return mErrorToRethrow; } bool HasParseError() const { return !mParseError.isUndefined(); } bool HasErrorToRethrow() const { return !mErrorToRethrow.isUndefined(); } - bool ForPreload() const { return mForPreload; } - bool HadImportMap() const { return mHadImportMap; } bool DebuggerDataInitialized() const { return mDebuggerDataInitialized; } void Shutdown(); diff --git a/js/loader/ModuleLoadRequest.cpp b/js/loader/ModuleLoadRequest.cpp index 7313563df9d5..7e188160fc05 100644 --- a/js/loader/ModuleLoadRequest.cpp +++ b/js/loader/ModuleLoadRequest.cpp @@ -205,7 +205,6 @@ void ModuleLoadRequest::CheckModuleDependenciesLoaded() { if (!mModuleScript || mModuleScript->HasParseError()) { return; } - for (const auto& childRequest : mImports) { ModuleScript* childScript = childRequest->mModuleScript; if (!childScript) { @@ -214,9 +213,6 @@ void ModuleLoadRequest::CheckModuleDependenciesLoaded() { childRequest.get())); return; } - - MOZ_DIAGNOSTIC_ASSERT(mModuleScript->HadImportMap() == - childScript->HadImportMap()); } LOG(("ScriptLoadRequest (%p): all ok", this)); diff --git a/js/loader/ModuleLoaderBase.cpp b/js/loader/ModuleLoaderBase.cpp index e125017f6815..228c96ad6932 100644 --- a/js/loader/ModuleLoaderBase.cpp +++ b/js/loader/ModuleLoaderBase.cpp @@ -55,17 +55,17 @@ mozilla::LazyLogModule ModuleLoaderBase::gModuleLoaderBaseLog( MOZ_LOG_TEST(ModuleLoaderBase::gModuleLoaderBaseLog, mozilla::LogLevel::Debug) ////////////////////////////////////////////////////////////// -// ModuleLoaderBase::LoadingRequest +// ModuleLoaderBase::WaitingRequests ////////////////////////////////////////////////////////////// -NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ModuleLoaderBase::LoadingRequest) +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ModuleLoaderBase::WaitingRequests) NS_INTERFACE_MAP_ENTRY(nsISupports) NS_INTERFACE_MAP_END -NS_IMPL_CYCLE_COLLECTION(ModuleLoaderBase::LoadingRequest, mRequest, mWaiting) +NS_IMPL_CYCLE_COLLECTION(ModuleLoaderBase::WaitingRequests, mWaiting) -NS_IMPL_CYCLE_COLLECTING_ADDREF(ModuleLoaderBase::LoadingRequest) -NS_IMPL_CYCLE_COLLECTING_RELEASE(ModuleLoaderBase::LoadingRequest) +NS_IMPL_CYCLE_COLLECTING_ADDREF(ModuleLoaderBase::WaitingRequests) +NS_IMPL_CYCLE_COLLECTING_RELEASE(ModuleLoaderBase::WaitingRequests) ////////////////////////////////////////////////////////////// // ModuleLoaderBase @@ -509,9 +509,7 @@ void ModuleLoaderBase::SetModuleFetchStarted(ModuleLoadRequest* aRequest) { MOZ_ASSERT(aRequest->IsFetching() || aRequest->IsPendingFetchingError()); MOZ_ASSERT(!ModuleMapContainsURL(aRequest->mURI)); - RefPtr loadingRequest = new LoadingRequest(); - loadingRequest->mRequest = aRequest; - mFetchingModules.InsertOrUpdate(aRequest->mURI, loadingRequest); + mFetchingModules.InsertOrUpdate(aRequest->mURI, nullptr); } void ModuleLoaderBase::SetModuleFetchFinishedAndResumeWaitingRequests( @@ -528,8 +526,9 @@ void ModuleLoaderBase::SetModuleFetchFinishedAndResumeWaitingRequests( "%u)", aRequest, aRequest->mModuleScript.get(), unsigned(aResult))); - auto entry = mFetchingModules.Lookup(aRequest->mURI); - if (!entry) { + RefPtr waitingRequests; + if (!mFetchingModules.Remove(aRequest->mURI, + getter_AddRefs(waitingRequests))) { LOG( ("ScriptLoadRequest (%p): Key not found in mFetchingModules, " "assuming we have an inline module or have finished fetching already", @@ -537,35 +536,20 @@ void ModuleLoaderBase::SetModuleFetchFinishedAndResumeWaitingRequests( return; } - // It's possible for a request to be cancelled and removed from the fetching - // modules map and a new request started for the same URI and added to the - // map. In this case we don't want the first cancelled request to complete the - // later request (which will cause it to fail) so we ignore it. - RefPtr loadingRequest = entry.Data(); - if (loadingRequest->mRequest != aRequest) { - MOZ_ASSERT(aRequest->IsCanceled()); - LOG( - ("ScriptLoadRequest (%p): Ignoring completion of cancelled request " - "that was removed from the map", - aRequest)); - return; - } - - MOZ_ALWAYS_TRUE(mFetchingModules.Remove(aRequest->mURI)); - RefPtr moduleScript(aRequest->mModuleScript); MOZ_ASSERT(NS_FAILED(aResult) == !moduleScript); mFetchedModules.InsertOrUpdate(aRequest->mURI, RefPtr{moduleScript}); - LOG(("ScriptLoadRequest (%p): Resuming waiting requests", aRequest)); - MOZ_ASSERT(loadingRequest->mRequest == aRequest); - ResumeWaitingRequests(loadingRequest, bool(moduleScript)); + if (waitingRequests) { + LOG(("ScriptLoadRequest (%p): Resuming waiting requests", aRequest)); + ResumeWaitingRequests(waitingRequests, bool(moduleScript)); + } } -void ModuleLoaderBase::ResumeWaitingRequests(LoadingRequest* aLoadingRequest, +void ModuleLoaderBase::ResumeWaitingRequests(WaitingRequests* aWaitingRequests, bool aSuccess) { - for (ModuleLoadRequest* request : aLoadingRequest->mWaiting) { + for (ModuleLoadRequest* request : aWaitingRequests->mWaiting) { ResumeWaitingRequest(request, aSuccess); } } @@ -584,8 +568,13 @@ void ModuleLoaderBase::WaitForModuleFetch(ModuleLoadRequest* aRequest) { MOZ_ASSERT(ModuleMapContainsURL(uri)); if (auto entry = mFetchingModules.Lookup(uri)) { - RefPtr loadingRequest = entry.Data(); - loadingRequest->mWaiting.AppendElement(aRequest); + RefPtr waitingRequests = entry.Data(); + if (!waitingRequests) { + waitingRequests = new WaitingRequests(); + mFetchingModules.InsertOrUpdate(uri, waitingRequests); + } + + waitingRequests->mWaiting.AppendElement(aRequest); return; } @@ -689,8 +678,6 @@ nsresult ModuleLoaderBase::CreateModuleScript(ModuleLoadRequest* aRequest) { aRequest->mLoadedScript->AsModuleScript(); aRequest->mModuleScript = moduleScript; - moduleScript->SetForPreload(aRequest->mLoadContext->IsPreload()); - if (!module) { LOG(("ScriptLoadRequest (%p): compilation failed (%d)", aRequest, unsigned(rv))); @@ -782,7 +769,6 @@ ResolveResult ModuleLoaderBase::ResolveModuleSpecifier( // Import Maps are not supported on workers/worklets. // See https://github.com/WICG/import-maps/issues/2 MOZ_ASSERT_IF(!NS_IsMainThread(), mImportMap == nullptr); - // Forward to the updated 'Resolve a module specifier' algorithm defined in // the Import Maps spec. return ImportMap::ResolveModuleSpecifier(mImportMap.get(), mLoader, aScript, @@ -857,7 +843,6 @@ void ModuleLoaderBase::StartFetchingModuleDependencies( MOZ_ASSERT(visitedSet->Contains(aRequest->mURI)); aRequest->mState = ModuleLoadRequest::State::LoadingImports; - aRequest->mModuleScript->SetHadImportMap(HasImportMapRegistered()); nsCOMArray urls; nsresult rv = ResolveRequestedModules(aRequest, &urls); @@ -1049,9 +1034,9 @@ void ModuleLoaderBase::Shutdown() { CancelAndClearDynamicImports(); for (const auto& entry : mFetchingModules) { - RefPtr loadingRequest(entry.GetData()); - if (loadingRequest) { - ResumeWaitingRequests(loadingRequest, false); + RefPtr waitingRequests(entry.GetData()); + if (waitingRequests) { + ResumeWaitingRequests(waitingRequests, false); } } @@ -1115,9 +1100,6 @@ JS::Value ModuleLoaderBase::FindFirstParseError(ModuleLoadRequest* aRequest) { } for (ModuleLoadRequest* childRequest : aRequest->mImports) { - MOZ_DIAGNOSTIC_ASSERT(moduleScript->HadImportMap() == - childRequest->mModuleScript->HadImportMap()); - JS::Value error = FindFirstParseError(childRequest); if (!error.isUndefined()) { return error; @@ -1411,38 +1393,6 @@ void ModuleLoaderBase::RegisterImportMap(UniquePtr aImportMap) { // Step 3. Set global's import map to result's import map. mImportMap = std::move(aImportMap); - - // Any import resolution has been invalidated by the addition of the import - // map. If speculative preloading is currently fetching any modules then - // cancel their requests and remove them from the map. - // - // The cancelled requests will still complete later so we have to check this - // in SetModuleFetchFinishedAndResumeWaitingRequests. - for (const auto& entry : mFetchingModules) { - LoadingRequest* loadingRequest = entry.GetData(); - MOZ_DIAGNOSTIC_ASSERT(loadingRequest->mRequest->mLoadContext->IsPreload()); - loadingRequest->mRequest->Cancel(); - for (const auto& request : loadingRequest->mWaiting) { - MOZ_DIAGNOSTIC_ASSERT(request->mLoadContext->IsPreload()); - request->Cancel(); - } - } - mFetchingModules.Clear(); - - // If speculative preloading has added modules to the module map, remove - // them. - for (const auto& entry : mFetchedModules) { - ModuleScript* script = entry.GetData(); - if (script) { - MOZ_DIAGNOSTIC_ASSERT(script->ForPreload()); - MOZ_DIAGNOSTIC_ASSERT(!script->HadImportMap()); - if (JSObject* module = script->ModuleRecord()) { - MOZ_DIAGNOSTIC_ASSERT(!JS::ModuleIsLinked(module)); - } - script->Shutdown(); - } - } - mFetchedModules.Clear(); } void ModuleLoaderBase::CopyModulesTo(ModuleLoaderBase* aDest) { diff --git a/js/loader/ModuleLoaderBase.h b/js/loader/ModuleLoaderBase.h index d06da0afca7b..2c2c385a3081 100644 --- a/js/loader/ModuleLoaderBase.h +++ b/js/loader/ModuleLoaderBase.h @@ -165,30 +165,20 @@ class ScriptLoaderInterface : public nsISupports { */ class ModuleLoaderBase : public nsISupports { /* - * Represents an ongoing load operation for a URI initiated for one request - * and which may have other requests waiting for it to complete. - * - * These are tracked in the mFetchingModules map. + * The set of requests that are waiting for an ongoing fetch to complete. */ - class LoadingRequest final : public nsISupports { - virtual ~LoadingRequest() = default; + class WaitingRequests final : public nsISupports { + virtual ~WaitingRequests() = default; public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS - NS_DECL_CYCLE_COLLECTION_CLASS(LoadingRequest) + NS_DECL_CYCLE_COLLECTION_CLASS(WaitingRequests) - // The request that initiated the load and which is currently fetching or - // being compiled. - RefPtr mRequest; - - // A list of any other requests for the same URI that are waiting for the - // initial load to complete. These will be resumed by ResumeWaitingRequests - // when that happens. nsTArray> mWaiting; }; // Module map - nsRefPtrHashtable mFetchingModules; + nsRefPtrHashtable mFetchingModules; nsRefPtrHashtable mFetchedModules; // List of dynamic imports that are currently being loaded. @@ -431,7 +421,7 @@ class ModuleLoaderBase : public nsISupports { void SetModuleFetchFinishedAndResumeWaitingRequests( ModuleLoadRequest* aRequest, nsresult aResult); - void ResumeWaitingRequests(LoadingRequest* aLoadingRequest, bool aSuccess); + void ResumeWaitingRequests(WaitingRequests* aWaitingRequests, bool aSuccess); void ResumeWaitingRequest(ModuleLoadRequest* aRequest, bool aSuccess); void StartFetchingModuleDependencies(ModuleLoadRequest* aRequest); diff --git a/js/public/Modules.h b/js/public/Modules.h index 4a8d45acb07d..2e7192e120fa 100644 --- a/js/public/Modules.h +++ b/js/public/Modules.h @@ -299,8 +299,6 @@ extern JS_PUBLIC_API JSObject* GetModuleEnvironment( */ extern JS_PUBLIC_API void ClearModuleEnvironment(JSObject* moduleObj); -extern JS_PUBLIC_API bool ModuleIsLinked(JSObject* moduleObj); - } // namespace JS #endif // js_Modules_h diff --git a/js/src/vm/Modules.cpp b/js/src/vm/Modules.cpp index 6f6565261f92..f461e7bec198 100644 --- a/js/src/vm/Modules.cpp +++ b/js/src/vm/Modules.cpp @@ -327,11 +327,6 @@ JS_PUBLIC_API void JS::ClearModuleEnvironment(JSObject* moduleObj) { } } -JS_PUBLIC_API bool JS::ModuleIsLinked(JSObject* moduleObj) { - AssertHeapIsIdle(); - return moduleObj->as().status() != ModuleStatus::Unlinked; -} - //////////////////////////////////////////////////////////////////////////////// // Internal implementation