Bug 1877703 - Part 3: Also remove currently fetching preload modules from the module map when import map is registered r=smaug

A further problem with dynamically inserted import maps was discovered where
sometimes module scripts would never execute. This happens when the script is
still being fetched when the import map is added.

To fix this, cancel all fetching module preloads as well when an import map is
registered and remove them from the module map. In theory this shouldn't be
necessary but I wasn't able to make the tests pass without this step.

For simplicity also remove all module preloads from the scriptloader's list of
preload requests rather than detecting and ignoring them later on.

Differential Revision: https://phabricator.services.mozilla.com/D204202

dom/script/ScriptLoader.cpp

@@ -1097,19 +1097,6 @@ 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.
 
@@ -1399,6 +1386,16 @@ 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

js/loader/ModuleLoaderBase.cpp

@@ -528,9 +528,8 @@ void ModuleLoaderBase::SetModuleFetchFinishedAndResumeWaitingRequests(
        "%u)",
        aRequest, aRequest->mModuleScript.get(), unsigned(aResult)));
 
-  RefPtr<LoadingRequest> loadingRequest;
-  if (!mFetchingModules.Remove(aRequest->mURI,
-                               getter_AddRefs(loadingRequest))) {
+  auto entry = mFetchingModules.Lookup(aRequest->mURI);
+  if (!entry) {
     LOG(
         ("ScriptLoadRequest (%p): Key not found in mFetchingModules, "
          "assuming we have an inline module or have finished fetching already",
@@ -538,6 +537,22 @@ 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> 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> moduleScript(aRequest->mModuleScript);
   MOZ_ASSERT(NS_FAILED(aResult) == !moduleScript);
 
@@ -1397,9 +1412,25 @@ void ModuleLoaderBase::RegisterImportMap(UniquePtr<ImportMap> 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. Any import resolution has been invalidated by the addition of the
-  // import map.
+  // them.
   for (const auto& entry : mFetchedModules) {
     ModuleScript* script = entry.GetData();
     if (script) {