Bug 1358882 - Check for failed instantiation when starting to fetch module dependencies r=smaug

dom/script/ModuleLoadRequest.cpp

@@ -11,6 +11,10 @@
 namespace mozilla {
 namespace dom {
 
+#undef LOG
+#define LOG(args) \
+  MOZ_LOG(ScriptLoader::gScriptLoaderLog, mozilla::LogLevel::Debug, args)
+
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(ModuleLoadRequest)
 NS_INTERFACE_MAP_END_INHERITING(ScriptLoadRequest)
 
@@ -53,6 +57,14 @@ ModuleLoadRequest::Cancel()
 void
 ModuleLoadRequest::SetReady()
 {
+  // 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.
+
 #ifdef DEBUG
   for (size_t i = 0; i < mImports.Length(); i++) {
     MOZ_ASSERT(mImports[i]->IsReadyToRun());
@@ -69,6 +81,8 @@ ModuleLoadRequest::ModuleLoaded()
   // A module that was found to be marked as fetching in the module map has now
   // been loaded.
 
+  LOG(("ScriptLoadRequest (%p): Module loaded", this));
+
   mModuleScript = mLoader->GetFetchedModule(mURI);
   mLoader->StartFetchingModuleDependencies(this);
 }
@@ -79,6 +93,8 @@ ModuleLoadRequest::DependenciesLoaded()
   // The module and all of its dependencies have been successfully fetched and
   // compiled.
 
+  LOG(("ScriptLoadRequest (%p): Module dependencies loaded", this));
+
   if (!mLoader->InstantiateModuleTree(this)) {
     LoadFailed();
     return;
@@ -93,6 +109,8 @@ ModuleLoadRequest::DependenciesLoaded()
 void
 ModuleLoadRequest::LoadFailed()
 {
+  LOG(("ScriptLoadRequest (%p): Module load failed", this));
+
   Cancel();
   mLoader->ProcessLoadedModuleTree(this);
   mLoader = nullptr;

dom/script/ScriptLoader.cpp

@@ -72,9 +72,12 @@ namespace dom {
 LazyLogModule ScriptLoader::gCspPRLog("CSP");
 LazyLogModule ScriptLoader::gScriptLoaderLog("ScriptLoader");
 
+#undef LOG
 #define LOG(args) \
   MOZ_LOG(gScriptLoaderLog, mozilla::LogLevel::Debug, args)
 
+#define LOG_ENABLED() \
+  MOZ_LOG_TEST(ScriptLoader::gScriptLoaderLog, mozilla::LogLevel::Debug)
 
 // Alternate Data MIME type used by the ScriptLoader to register that we want to
 // store bytecode without reading it.
@@ -357,6 +360,13 @@ ScriptLoader::SetModuleFetchFinishedAndResumeWaitingRequests(ModuleLoadRequest*
 {
   // Update module map with the result of fetching a single module script.  The
   // module script pointer is nullptr on error.
+  //
+  // If any requests for the same URL are waiting on this one to complete, they
+  // will have ModuleLoaded or LoadFailed on them when the promise is
+  // resolved/rejected. This is set up in StartLoad.
+
+  LOG(("ScriptLoadRequest (%p): Module fetch finished (result == %u)",
+       aRequest, aResult));
 
   MOZ_ASSERT(!aRequest->IsReadyToRun());
 
@@ -400,6 +410,12 @@ ScriptLoader::WaitForModuleFetch(ModuleLoadRequest* aRequest)
 ModuleScript*
 ScriptLoader::GetFetchedModule(nsIURI* aURL) const
 {
+  if (LOG_ENABLED()) {
+    nsAutoCString url;
+    aURL->GetAsciiSpec(url);
+    LOG(("GetFetchedModule %s", url.get()));
+  }
+
   bool found;
   ModuleScript* ms = mFetchedModules.GetWeak(aURL, &found);
   MOZ_ASSERT(found);
@@ -646,8 +662,13 @@ void
 ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest)
 {
   MOZ_ASSERT(aRequest->mModuleScript);
-  MOZ_ASSERT(!aRequest->mModuleScript->InstantiationFailed());
   MOZ_ASSERT(!aRequest->IsReadyToRun());
+
+  if (aRequest->mModuleScript->InstantiationFailed()) {
+    aRequest->LoadFailed();
+    return;
+  }
+
   aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports;
 
   nsCOMArray<nsIURI> urls;
@@ -790,6 +811,8 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest)
 
   MOZ_ASSERT(aRequest);
 
+  LOG(("ScriptLoadRequest (%p): Instantiate module tree", aRequest));
+
   ModuleScript* ms = aRequest->mModuleScript;
   MOZ_ASSERT(ms);
   if (!ms->ModuleRecord()) {
@@ -809,6 +832,7 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest)
 
   JS::RootedValue exception(jsapi.cx());
   if (!ok) {
+    LOG(("ScriptLoadRequest (%p): Instantiate failed", aRequest));
     MOZ_ASSERT(jsapi.HasException());
     if (!jsapi.StealException(&exception)) {
       return false;
@@ -877,11 +901,18 @@ ScriptLoader::StartLoad(ScriptLoadRequest* aRequest)
     return NS_OK;
   }
 
+  if (LOG_ENABLED()) {
+    nsAutoCString url;
+    aRequest->mURI->GetAsciiSpec(url);
+    LOG(("ScriptLoadRequest (%p): Start Load (url = %s)", aRequest, url.get()));
+  }
+
   if (aRequest->IsModuleRequest()) {
     // Check whether the module has been fetched or is currently being fetched,
     // and if so wait for it.
     ModuleLoadRequest* request = aRequest->AsModuleRequest();
     if (ModuleMapContainsModule(request)) {
+      LOG(("ScriptLoadRequest (%p): Waiting for module fetch", aRequest));
       WaitForModuleFetch(request)
         ->Then(GetMainThreadSerialEventTarget(), __func__, request,
                &ModuleLoadRequest::ModuleLoaded,
@@ -891,6 +922,7 @@ ScriptLoader::StartLoad(ScriptLoadRequest* aRequest)
 
     // Otherwise put the URL in the module map and mark it as fetching.
     SetModuleFetchStarted(request);
+    LOG(("ScriptLoadRequest (%p): Start fetching module", aRequest));
   }
 
   nsContentPolicyType contentPolicyType = aRequest->IsPreload()

testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-2.html.ini

@@ -1,5 +1,4 @@
 [instantiation-error-2.html]
   type: testharness
-  expected:
-    if not e10s: CRASH
-    if e10s: CRASH
+  [Test that missing exports lead to SyntaxError events on window and load events on script, and that exceptions are remembered]
+    expected: FAIL

testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html.ini

@@ -1,5 +1,4 @@
 [instantiation-error-3.html]
   type: testharness
-  expected:
-    if not e10s: CRASH
-    if e10s: CRASH
+  [Test that unresolvable cycles lead to SyntaxError events on window and load events on script, and that exceptions are remembered]
+    expected: FAIL