Bug 1842798 - Part 2: Remove use of MozPromise to wait for module imp…

…orts to load r=smaug

This replaces the use of a promise to wait for all imports of a module to load
with a counter of reminaing imports in the parent and a pointer to the parent
that is waiting in the child. The parent request is updated immediately rather
than by dispatching a runnable.

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

dom/workers/ScriptLoader.cpp

@@ -1221,6 +1221,13 @@ bool WorkerScriptLoader::EvaluateScript(JSContext* aCx,
 }
 
 void WorkerScriptLoader::TryShutdown() {
+  {
+    MutexAutoLock lock(CleanUpLock());
+    if (CleanedUp()) {
+      return;
+    }
+  }
+
   if (AllScriptsExecuted() && AllModuleRequestsLoaded()) {
     ShutdownScriptLoader(!mExecutionAborted, mMutedErrorFlag);
   }

js/loader/ModuleLoadRequest.cpp

@@ -27,14 +27,15 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(ModuleLoadRequest)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(ModuleLoadRequest,
                                                 ScriptLoadRequest)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoader, mModuleScript, mImports, mRootModule)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoader, mRootModule, mModuleScript, mImports,
+                                  mWaitingParentRequest)
   tmp->ClearDynamicImport();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ModuleLoadRequest,
                                                   ScriptLoadRequest)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLoader, mModuleScript, mImports,
-                                    mRootModule)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLoader, mRootModule, mModuleScript,
+                                    mImports, mWaitingParentRequest)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(ModuleLoadRequest,
@@ -81,25 +82,34 @@ void ModuleLoadRequest::Cancel() {
     return;
   }
 
+  if (IsReadyToRun()) {
+    return;
+  }
+
   ScriptLoadRequest::Cancel();
+
   mModuleScript = nullptr;
   CancelImports();
-  mReady.RejectIfExists(NS_ERROR_DOM_ABORT_ERR, __func__);
+
+  if (mWaitingParentRequest) {
+    ChildLoadComplete(false);
+  }
 }
 
 void ModuleLoadRequest::SetReady() {
+  MOZ_ASSERT(!IsReadyToRun());
+
   // Mark a module as ready to execute. This means that this module and all it
   // dependencies have had their source loaded, parsed as a module and the
   // modules instantiated.
-  //
-  // The mReady promise is used to ensure that when all dependencies of a module
-  // have become ready, DependenciesLoaded is called on that module
-  // request. This is set up in StartFetchingModuleDependencies.
 
   AssertAllImportsReady();
 
   ScriptLoadRequest::SetReady();
-  mReady.ResolveIfExists(true, __func__);
+
+  if (mWaitingParentRequest) {
+    ChildLoadComplete(true);
+  }
 }
 
 void ModuleLoadRequest::ModuleLoaded() {
@@ -157,6 +167,11 @@ void ModuleLoadRequest::ModuleErrored() {
   MOZ_ASSERT(IsErrored());
 
   CancelImports();
+  if (IsReadyToRun()) {
+    // Cancelling an outstanding import will error this request.
+    return;
+  }
+
   SetReady();
   LoadFinished();
 }

js/loader/ModuleLoadRequest.h

@@ -11,7 +11,6 @@
 #include "ScriptLoadRequest.h"
 #include "ModuleLoaderBase.h"
 #include "mozilla/Assertions.h"
-#include "mozilla/MozPromise.h"
 #include "js/RootingAPI.h"
 #include "js/Value.h"
 #include "nsURIHashKey.h"
@@ -36,7 +35,10 @@ class VisitedURLSet : public nsTHashtable<nsURIHashKey> {
 // multiple imports of the same module.
 
 class ModuleLoadRequest final : public ScriptLoadRequest {
-  ~ModuleLoadRequest() = default;
+  ~ModuleLoadRequest() {
+    MOZ_ASSERT(!mWaitingParentRequest);
+    MOZ_ASSERT(mAwaitingImports == 0);
+  }
 
   ModuleLoadRequest(const ModuleLoadRequest& aOther) = delete;
   ModuleLoadRequest(ModuleLoadRequest&& aOther) = delete;
@@ -47,10 +49,6 @@ class ModuleLoadRequest final : public ScriptLoadRequest {
                                                          ScriptLoadRequest)
   using SRIMetadata = mozilla::dom::SRIMetadata;
 
-  template <typename T>
-  using MozPromiseHolder = mozilla::MozPromiseHolder<T>;
-  using GenericPromise = mozilla::GenericPromise;
-
   ModuleLoadRequest(nsIURI* aURI, ScriptFetchOptions* aFetchOptions,
                     const SRIMetadata& aIntegrity, nsIURI* aReferrer,
                     LoadContextBase* aContext, bool aIsTopLevel,
@@ -114,6 +112,8 @@ class ModuleLoadRequest final : public ScriptLoadRequest {
   void StartDynamicImport() { mLoader->StartDynamicImport(this); }
   void ProcessDynamicImport() { mLoader->ProcessDynamicImport(this); }
 
+  void ChildLoadComplete(bool aSuccess);
+
  private:
   void LoadFinished();
   void CancelImports();
@@ -146,14 +146,17 @@ class ModuleLoadRequest final : public ScriptLoadRequest {
   // failure.
   RefPtr<ModuleScript> mModuleScript;
 
-  // A promise that is completed on successful load of this module and all of
-  // its dependencies, indicating that the module is ready for instantiation and
-  // evaluation.
-  MozPromiseHolder<GenericPromise> mReady;
-
   // Array of imported modules.
   nsTArray<RefPtr<ModuleLoadRequest>> mImports;
 
+  // Parent module (i.e. importer of this module) that is waiting for this
+  // module and its dependencies to load, or null.
+  RefPtr<ModuleLoadRequest> mWaitingParentRequest;
+
+  // Number of child modules (i.e. imported modules) that this module is waiting
+  // for.
+  size_t mAwaitingImports = 0;
+
   // Set of module URLs visited while fetching the module graph this request is
   // part of.
   RefPtr<VisitedURLSet> mVisitedSet;

js/loader/ModuleLoaderBase.cpp

@@ -851,7 +851,7 @@ void ModuleLoaderBase::StartFetchingModuleDependencies(
 
   MOZ_ASSERT(aRequest->mModuleScript);
   MOZ_ASSERT(!aRequest->mModuleScript->HasParseError());
-  MOZ_ASSERT(!aRequest->IsReadyToRun());
+  MOZ_ASSERT(aRequest->IsFetching() || aRequest->IsCompiling());
 
   auto visitedSet = aRequest->mVisitedSet;
   MOZ_ASSERT(visitedSet->Contains(aRequest->mURI));
@@ -885,26 +885,18 @@ void ModuleLoaderBase::StartFetchingModuleDependencies(
     return;
   }
 
+  MOZ_ASSERT(aRequest->mAwaitingImports == 0);
+  aRequest->mAwaitingImports = urls.Count();
+
   // For each url in urls, fetch a module script graph given url, module
   // script's CORS setting, and module script's settings object.
-  nsTArray<RefPtr<mozilla::GenericPromise>> importsReady;
   for (auto* url : urls) {
-    RefPtr<mozilla::GenericPromise> childReady =
-        StartFetchingModuleAndDependencies(aRequest, url);
-    importsReady.AppendElement(childReady);
+    StartFetchingModuleAndDependencies(aRequest, url);
   }
-
-  // Wait for all imports to become ready.
-  RefPtr<mozilla::GenericPromise::AllPromiseType> allReady =
-      mozilla::GenericPromise::All(mEventTarget, importsReady);
-  allReady->Then(mEventTarget, __func__, aRequest,
-                 &ModuleLoadRequest::DependenciesLoaded,
-                 &ModuleLoadRequest::ModuleErrored);
 }
 
-RefPtr<mozilla::GenericPromise>
-ModuleLoaderBase::StartFetchingModuleAndDependencies(ModuleLoadRequest* aParent,
-                                                     nsIURI* aURI) {
+void ModuleLoaderBase::StartFetchingModuleAndDependencies(
+    ModuleLoadRequest* aParent, nsIURI* aURI) {
   MOZ_ASSERT(aURI);
 
   RefPtr<ModuleLoadRequest> childRequest = CreateStaticImport(aURI, aParent);
@@ -924,20 +916,38 @@ ModuleLoaderBase::StartFetchingModuleAndDependencies(ModuleLoadRequest* aParent,
          url2.get()));
   }
 
-  RefPtr<mozilla::GenericPromise> ready = childRequest->mReady.Ensure(__func__);
+  MOZ_ASSERT(!childRequest->mWaitingParentRequest);
+  childRequest->mWaitingParentRequest = aParent;
 
   nsresult rv = StartModuleLoad(childRequest);
   if (NS_FAILED(rv)) {
     MOZ_ASSERT(!childRequest->mModuleScript);
     LOG(("ScriptLoadRequest (%p):   rejecting %p", aParent,
-         &childRequest->mReady));
+         childRequest.get()));
 
     mLoader->ReportErrorToConsole(childRequest, rv);
-    childRequest->mReady.Reject(rv, __func__);
-    return ready;
+    childRequest->LoadFailed();
+  }
+}
+
+void ModuleLoadRequest::ChildLoadComplete(bool aSuccess) {
+  RefPtr<ModuleLoadRequest> parent = mWaitingParentRequest;
+  MOZ_ASSERT(parent);
+  MOZ_ASSERT(parent->mAwaitingImports != 0);
+
+  mWaitingParentRequest = nullptr;
+  parent->mAwaitingImports--;
+
+  if (parent->IsReadyToRun()) {
+    MOZ_ASSERT_IF(!aSuccess, parent->IsErrored());
+    return;
   }
 
-  return ready;
+  if (!aSuccess) {
+    parent->ModuleErrored();
+  } else if (parent->mAwaitingImports == 0) {
+    parent->DependenciesLoaded();
+  }
 }
 
 void ModuleLoaderBase::StartDynamicImport(ModuleLoadRequest* aRequest) {

js/loader/ModuleLoaderBase.h

@@ -23,7 +23,6 @@
 #include "mozilla/CORSMode.h"
 #include "mozilla/dom/JSExecutionContext.h"
 #include "mozilla/MaybeOneOf.h"
-#include "mozilla/MozPromise.h"
 #include "mozilla/UniquePtr.h"
 #include "ResolveResult.h"
 
@@ -163,8 +162,6 @@ class ScriptLoaderInterface : public nsISupports {
  * 10. The client calls EvaluateModule() to execute the top-level module.
  */
 class ModuleLoaderBase : public nsISupports {
-  using GenericPromise = mozilla::GenericPromise;
-
   /*
    * The set of requests that are waiting for an ongoing fetch to complete.
    */
@@ -193,8 +190,8 @@ class ModuleLoaderBase : public nsISupports {
   bool mImportMapsAllowed = true;
 
  protected:
-  // Event handler used to process MozPromise actions, used internally to wait
-  // for fetches to finish and for imports to become avilable.
+  // Event handler used to dispatch runnables, used internally to wait for
+  // fetches to finish and for imports to become avilable.
   nsCOMPtr<nsISerialEventTarget> mEventTarget;
   RefPtr<ScriptLoaderInterface> mLoader;
 
@@ -388,8 +385,8 @@ class ModuleLoaderBase : public nsISupports {
 
   void StartFetchingModuleDependencies(ModuleLoadRequest* aRequest);
 
-  RefPtr<GenericPromise> StartFetchingModuleAndDependencies(
-      ModuleLoadRequest* aParent, nsIURI* aURI);
+  void StartFetchingModuleAndDependencies(ModuleLoadRequest* aParent,
+                                          nsIURI* aURI);
 
   /**
    * Shorthand Wrapper for JSAPI FinishDynamicImport function for the reject