Bug 1759881 - Part 1: Make list of dynamic import requests private r=yulia

This restricts access to mDynamicImportRequests and adds the appropriate accessors.

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

dom/script/ModuleLoader.cpp

@@ -440,14 +440,9 @@ void ModuleLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) {
   MOZ_ASSERT(aRequest->IsReadyToRun());
 
   if (aRequest->IsTopLevel()) {
-    if (aRequest->IsDynamicImport()) {
+    if (aRequest->IsDynamicImport() ||
-      MOZ_ASSERT(aRequest->isInList());
+        (aRequest->GetLoadContext()->mIsInline &&
-      RefPtr<ScriptLoadRequest> req = mDynamicImportRequests.Steal(aRequest);
+         aRequest->GetLoadContext()->GetParserCreated() == NOT_FROM_PARSER)) {
-      GetScriptLoader()->RunScriptWhenSafe(aRequest);
-    } else if (aRequest->GetLoadContext()->mIsInline &&
-               aRequest->GetLoadContext()->GetParserCreated() ==
-                   NOT_FROM_PARSER) {
-      MOZ_ASSERT(!aRequest->isInList());
       GetScriptLoader()->RunScriptWhenSafe(aRequest);
     } else {
       GetScriptLoader()->MaybeMoveToLoadedList(aRequest);

dom/script/ScriptLoader.cpp

@@ -2516,7 +2516,7 @@ bool ScriptLoader::HasPendingRequests() {
          !mLoadedAsyncRequests.isEmpty() ||
          !mNonAsyncExternalScriptInsertedRequests.isEmpty() ||
          !mDeferRequests.isEmpty() ||
-         !mModuleLoader->mDynamicImportRequests.isEmpty() ||
+         mModuleLoader->HasPendingDynamicImports() ||
          !mPendingChildLoaders.IsEmpty();
   // mOffThreadCompilingRequests are already being processed.
 }
@@ -2982,13 +2982,7 @@ void ScriptLoader::HandleLoadError(ScriptLoadRequest* aRequest,
     if (modReq->IsDynamicImport()) {
       MOZ_ASSERT(modReq->IsTopLevel());
       if (aRequest->isInList()) {
-        RefPtr<ScriptLoadRequest> req =
+        mModuleLoader->CancelDynamicImport(modReq, aResult);
-            mModuleLoader->mDynamicImportRequests.Steal(aRequest);
-        modReq->Cancel();
-        // FinishDynamicImport must happen exactly once for each dynamic import
-        // request. If the load is aborted we do it when we remove the request
-        // from mDynamicImportRequests.
-        mModuleLoader->FinishDynamicImportAndReject(modReq, aResult);
       }
     } else {
       MOZ_ASSERT(!modReq->IsTopLevel());
@@ -3223,17 +3217,17 @@ nsresult ScriptLoader::PrepareLoadedRequest(ScriptLoadRequest* aRequest,
   // inserting the request in the array. However it's an unlikely case
   // so if you see this assertion it is likely something else that is
   // wrong, especially if you see it more than once.
-  NS_ASSERTION(mDeferRequests.Contains(aRequest) ||
+  NS_ASSERTION(
-                   mLoadingAsyncRequests.Contains(aRequest) ||
+      mDeferRequests.Contains(aRequest) ||
-                   mNonAsyncExternalScriptInsertedRequests.Contains(aRequest) ||
+          mLoadingAsyncRequests.Contains(aRequest) ||
-                   mXSLTRequests.Contains(aRequest) ||
+          mNonAsyncExternalScriptInsertedRequests.Contains(aRequest) ||
-                   mModuleLoader->mDynamicImportRequests.Contains(aRequest) ||
+          mXSLTRequests.Contains(aRequest) ||
-                   (aRequest->IsModuleRequest() &&
+          (aRequest->IsModuleRequest() &&
-                    !aRequest->AsModuleRequest()->IsTopLevel() &&
+           (mModuleLoader->HasDynamicImport(aRequest->AsModuleRequest()) ||
-                    !aRequest->isInList()) ||
+            !aRequest->AsModuleRequest()->IsTopLevel())) ||
-                   mPreloads.Contains(aRequest, PreloadRequestComparator()) ||
+          mPreloads.Contains(aRequest, PreloadRequestComparator()) ||
-                   mParserBlockingRequest == aRequest,
+          mParserBlockingRequest == aRequest,
-               "aRequest should be pending!");
+      "aRequest should be pending!");
 
   nsCOMPtr<nsIURI> uri;
   rv = channel->GetOriginalURI(getter_AddRefs(uri));

js/loader/ModuleLoadRequest.cpp

@@ -180,7 +180,12 @@ void ModuleLoadRequest::LoadFailed() {
 }
 
 void ModuleLoadRequest::LoadFinished() {
-  mLoader->ProcessLoadedModuleTree(this);
+  RefPtr<ModuleLoadRequest> request(this);
+  if (IsTopLevel() && IsDynamicImport()) {
+    mLoader->RemoveDynamicImport(request);
+  }
+
+  mLoader->ProcessLoadedModuleTree(request);
 
   mLoader = nullptr;
 }

js/loader/ModuleLoaderBase.cpp

@@ -622,6 +622,31 @@ ModuleLoaderBase::~ModuleLoaderBase() {
   LOG(("ModuleLoaderBase::~ModuleLoaderBase %p", this));
 }
 
+bool ModuleLoaderBase::HasPendingDynamicImports() const {
+  return !mDynamicImportRequests.isEmpty();
+}
+
+void ModuleLoaderBase::CancelDynamicImport(ModuleLoadRequest* aRequest,
+                                           nsresult aResult) {
+  RefPtr<ScriptLoadRequest> req = mDynamicImportRequests.Steal(aRequest);
+  aRequest->Cancel();
+  // FinishDynamicImport must happen exactly once for each dynamic import
+  // request. If the load is aborted we do it when we remove the request
+  // from mDynamicImportRequests.
+  FinishDynamicImportAndReject(aRequest, aResult);
+}
+
+void ModuleLoaderBase::RemoveDynamicImport(ModuleLoadRequest* aRequest) {
+  MOZ_ASSERT(aRequest->IsDynamicImport());
+  mDynamicImportRequests.Remove(aRequest);
+}
+
+#ifdef DEBUG
+bool ModuleLoaderBase::HasDynamicImport(ModuleLoadRequest* aRequest) const {
+  return mDynamicImportRequests.Contains(aRequest);
+}
+#endif
+
 JS::Value ModuleLoaderBase::FindFirstParseError(ModuleLoadRequest* aRequest) {
   MOZ_ASSERT(aRequest);
 

js/loader/ModuleLoaderBase.h

@@ -89,10 +89,14 @@ class ModuleLoaderBase : public nsISupports {
       mFetchingModules;
   nsRefPtrHashtable<ModuleMapKey, ModuleScript> mFetchedModules;
 
+  // List of dynamic imports that are currently being loaded.
+  ScriptLoadRequestList mDynamicImportRequests;
+
  protected:
-  virtual ~ModuleLoaderBase();
   RefPtr<ScriptLoaderInterface> mLoader;
 
+  virtual ~ModuleLoaderBase();
+
  public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(ModuleLoaderBase)
@@ -101,7 +105,6 @@ class ModuleLoaderBase : public nsISupports {
   using ScriptFetchOptions = JS::loader::ScriptFetchOptions;
   using ScriptLoadRequest = JS::loader::ScriptLoadRequest;
   using ModuleLoadRequest = JS::loader::ModuleLoadRequest;
-  ScriptLoadRequestList mDynamicImportRequests;
 
   using MaybeSourceText =
       mozilla::MaybeOneOf<JS::SourceText<char16_t>, JS::SourceText<Utf8Unit>>;
@@ -122,6 +125,13 @@ class ModuleLoaderBase : public nsISupports {
   virtual already_AddRefed<ModuleLoadRequest> CreateStaticImport(
       nsIURI* aURI, ModuleLoadRequest* aParent) = 0;
 
+  bool HasPendingDynamicImports() const;
+  void CancelDynamicImport(ModuleLoadRequest* aRequest, nsresult aResult);
+  void RemoveDynamicImport(ModuleLoadRequest* aRequest);
+#ifdef DEBUG
+  bool HasDynamicImport(ModuleLoadRequest* aRequest) const;
+#endif
+
   // Helper function to set up the global correctly for dynamic imports.
   nsresult EvaluateModule(ScriptLoadRequest* aRequest);