Bug 1358882 - Check for failed instantiation when starting to fetch module dependencies r=smaug
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 02 May 2017 18:01:51 +0100
changeset 424339 4e6acfa334ce3fdd7dba207afe81e03bde8ae6f3
parent 424338 f6d10bc8c36c28ac5a1964b1d2ba7436af44c6aa
child 424340 9758617c18334b61d3e6d6f5432c0485483af518
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1358882
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1358882 - Check for failed instantiation when starting to fetch module dependencies r=smaug
dom/script/ModuleLoadRequest.cpp
dom/script/ScriptLoader.cpp
testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-2.html.ini
testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html.ini
--- a/dom/script/ModuleLoadRequest.cpp
+++ b/dom/script/ModuleLoadRequest.cpp
@@ -6,16 +6,20 @@
 
 #include "ModuleLoadRequest.h"
 #include "ModuleScript.h"
 #include "ScriptLoader.h"
 
 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)
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(ModuleLoadRequest, ScriptLoadRequest,
                                    mBaseURL,
                                    mLoader,
                                    mParent,
                                    mModuleScript,
@@ -48,56 +52,70 @@ ModuleLoadRequest::Cancel()
     mImports[i]->Cancel();
   }
   mReady.RejectIfExists(NS_ERROR_FAILURE, __func__);
 }
 
 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());
   }
 #endif
 
   ScriptLoadRequest::SetReady();
   mReady.ResolveIfExists(true, __func__);
 }
 
 void
 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);
 }
 
 void
 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;
   }
 
   SetReady();
   mLoader->ProcessLoadedModuleTree(this);
   mLoader = nullptr;
   mParent = nullptr;
 }
 
 void
 ModuleLoadRequest::LoadFailed()
 {
+  LOG(("ScriptLoadRequest (%p): Module load failed", this));
+
   Cancel();
   mLoader->ProcessLoadedModuleTree(this);
   mLoader = nullptr;
   mParent = nullptr;
 }
 
 } // dom namespace
 } // mozilla namespace
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -67,19 +67,22 @@
 using JS::SourceBufferHolder;
 
 namespace mozilla {
 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.
 static NS_NAMED_LITERAL_CSTRING(kNullMimeType, "javascript/null");
 
 //////////////////////////////////////////////////////////////
 // ScriptLoader::PreloadInfo
 //////////////////////////////////////////////////////////////
@@ -352,16 +355,23 @@ ScriptLoader::SetModuleFetchStarted(Modu
 }
 
 void
 ScriptLoader::SetModuleFetchFinishedAndResumeWaitingRequests(ModuleLoadRequest* aRequest,
                                                              nsresult aResult)
 {
   // 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());
 
   RefPtr<GenericPromise::Private> promise;
   MOZ_ALWAYS_TRUE(mFetchingModules.Remove(aRequest->mURI, getter_AddRefs(promise)));
 
   RefPtr<ModuleScript> ms(aRequest->mModuleScript);
   MOZ_ASSERT(NS_SUCCEEDED(aResult) == (ms != nullptr));
@@ -395,16 +405,22 @@ ScriptLoader::WaitForModuleFetch(ModuleL
   }
 
   return GenericPromise::CreateAndResolve(true, __func__);
 }
 
 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);
   return ms;
 }
 
 nsresult
 ScriptLoader::ProcessFetchedModuleSource(ModuleLoadRequest* aRequest)
@@ -641,18 +657,23 @@ ResolveRequestedModules(ModuleLoadReques
 
   return NS_OK;
 }
 
 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;
   nsresult rv = ResolveRequestedModules(aRequest, urls);
   if (NS_FAILED(rv)) {
     aRequest->LoadFailed();
     return;
   }
@@ -785,16 +806,18 @@ ScriptLoader::ProcessLoadedModuleTree(Mo
 
 bool
 ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest)
 {
   // Perform eager instantiation of the loaded module tree.
 
   MOZ_ASSERT(aRequest);
 
+  LOG(("ScriptLoadRequest (%p): Instantiate module tree", aRequest));
+
   ModuleScript* ms = aRequest->mModuleScript;
   MOZ_ASSERT(ms);
   if (!ms->ModuleRecord()) {
     return false;
   }
 
   AutoJSAPI jsapi;
   if (NS_WARN_IF(!jsapi.Init(ms->ModuleRecord()))) {
@@ -804,16 +827,17 @@ ScriptLoader::InstantiateModuleTree(Modu
   nsresult rv = EnsureModuleResolveHook(jsapi.cx());
   NS_ENSURE_SUCCESS(rv, false);
 
   JS::Rooted<JSObject*> module(jsapi.cx(), ms->ModuleRecord());
   bool ok = NS_SUCCEEDED(nsJSUtils::ModuleDeclarationInstantiation(jsapi.cx(), module));
 
   JS::RootedValue exception(jsapi.cx());
   if (!ok) {
+    LOG(("ScriptLoadRequest (%p): Instantiate failed", aRequest));
     MOZ_ASSERT(jsapi.HasException());
     if (!jsapi.StealException(&exception)) {
       return false;
     }
     MOZ_ASSERT(!exception.isUndefined());
   }
 
   // Mark this module and any uninstantiated dependencies found via depth-first
@@ -872,30 +896,38 @@ ScriptLoader::StartLoad(ScriptLoadReques
   NS_ENSURE_TRUE(mDocument, NS_ERROR_NULL_POINTER);
   aRequest->mDataType = ScriptLoadRequest::DataType::Unknown;
 
   // If this document is sandboxed without 'allow-scripts', abort.
   if (mDocument->HasScriptsBlockedBySandbox()) {
     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,
                &ModuleLoadRequest::LoadFailed);
       return NS_OK;
     }
 
     // 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()
                                           ? nsIContentPolicy::TYPE_INTERNAL_SCRIPT_PRELOAD
                                           : nsIContentPolicy::TYPE_INTERNAL_SCRIPT;
   nsCOMPtr<nsINode> context;
   if (aRequest->mElement) {
     context = do_QueryInterface(aRequest->mElement);
--- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-2.html.ini
+++ b/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
--- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html.ini
+++ b/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