Bug 1956163 - ScriptLoader now reports child module load failures as coming from the parent module. r=arai

When a child module fails to load, the parent module that requested it is now used as the source location for the error message.

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

dom/script/ScriptLoader.cpp

@@ -4027,11 +4027,25 @@ void ScriptLoader::ReportErrorToConsole(ScriptLoadRequest* aRequest,
   JS::ColumnNumberOneOrigin columnNo =
       aRequest->GetScriptLoadContext()->GetScriptColumnNumber();
 
+  SourceLocation loc{mDocument->GetDocumentURI(), lineNo,
+                     columnNo.oneOriginValue()};
+
+  // If this is a failed module load, and we know the parent module, then
+  // attribute the failure to the parent module, not the overall document.
+  if (aRequest->IsModuleRequest()) {
+    ModuleLoadRequest* modRequest = aRequest->AsModuleRequest();
+    if (!modRequest->IsTopLevel()) {
+      ModuleLoadRequest* parent = modRequest->mWaitingParentRequest;
+      if (parent) {
+        nsCString parentURL = parent->mURL;
+        loc = SourceLocation(std::move(parentURL));
+      }
+    }
+  }
+
   nsContentUtils::ReportToConsole(
       nsIScriptError::warningFlag, "Script Loader"_ns, mDocument,
-      nsContentUtils::eDOM_PROPERTIES, message, params,
-      SourceLocation{mDocument->GetDocumentURI(), lineNo,
-                     columnNo.oneOriginValue()});
+      nsContentUtils::eDOM_PROPERTIES, message, params, loc);
 }
 
 void ScriptLoader::ReportWarningToConsole(

dom/tests/browser/browser.toml

@@ -149,6 +149,13 @@ support-files = [
   "counter_server.sjs",
 ]
 
+["browser_scriptCache_container.js"]
+skip-if = ["!nightly_build"]
+support-files = [
+  "page_scriptCache_container.html",
+  "counter_server.sjs",
+]
+
 ["browser_scriptCache_evict_with_non_cacheable.js"]
 support-files = [
   "cacheable_non_cacheable_server.sjs",
@@ -162,13 +169,6 @@ support-files = [
   "page_scriptCache_load_events.html",
 ]
 
-["browser_scriptCache_container.js"]
-skip-if = ["!nightly_build"]
-support-files = [
-  "page_scriptCache_container.html",
-  "counter_server.sjs",
-]
-
 ["browser_scriptCache_partition.js"]
 skip-if = ["!nightly_build"]
 support-files = [
@@ -209,6 +209,14 @@ skip-if = [
 ]
 support-files = ["test_new_window_from_content_child.html"]
 
+["browser_test_report_missing_child_module.js"]
+support-files = [
+  "file_test_report_missing_child_module_grandparent.mjs",
+  "file_test_report_missing_child_module_parent.mjs",
+  "file_test_report_missing_child_module_parent_02.mjs",
+  "page_test_report_missing_child_module.html",
+]
+
 ["browser_test_toolbars_visibility.js"]
 https_first_disabled = true
 support-files = ["test_new_window_from_content_child.html"]

dom/tests/browser/browser_test_report_missing_child_module.js

@@ -0,0 +1,49 @@
+const TEST_URL =
+  "https://example.com/browser/dom/tests/browser/page_test_report_missing_child_module.html";
+
+const EXPECTED_MESSAGE = "Loading failed for the module with source ";
+
+function waitForError(expectedBadModule) {
+  return new Promise(resolve => {
+    const listener = {
+      QueryInterface: ChromeUtils.generateQI(["nsIConsoleListener"]),
+      observe(message) {
+        if (
+          message.message.includes(expectedBadModule) &&
+          message.message.includes(EXPECTED_MESSAGE)
+        ) {
+          message.QueryInterface(Ci.nsIScriptError);
+          Services.console.unregisterListener(listener);
+          resolve(message);
+        }
+      },
+    };
+    Services.console.registerListener(listener);
+  });
+}
+
+add_task(async function () {
+  // We need the console listeners in place BEFORE we load the test page.
+  const childLoadPromise = waitForError(
+    "/intentionally-non-existent-child-module.mjs"
+  );
+  const grandchildLoadPromise = waitForError(
+    "/intentionally-non-existent-grandchild-module.mjs"
+  );
+
+  await BrowserTestUtils.withNewTab(TEST_URL, async function () {
+    const childLoadError = await childLoadPromise;
+    Assert.stringContains(
+      childLoadError.sourceName,
+      "file_test_report_missing_child_module_parent.mjs",
+      "A failure to load a top-level module's child should be attributed to the top-level module"
+    );
+
+    const grandchildLoadError = await grandchildLoadPromise;
+    Assert.stringContains(
+      grandchildLoadError.sourceName,
+      "file_test_report_missing_child_module_parent_02.mjs",
+      "A failure to load a non-top-level module A's child B should be attributed to A"
+    );
+  });
+});

dom/tests/browser/file_test_report_missing_child_module_grandparent.mjs

@@ -0,0 +1,2 @@
+// eslint-disable-next-line import/no-unassigned-import
+import "./file_test_report_missing_child_module_parent_02.mjs";

dom/tests/browser/file_test_report_missing_child_module_parent.mjs

@@ -0,0 +1,4 @@
+// Do not create a file with the below name.
+
+// eslint-disable-next-line import/no-unassigned-import, import/no-unresolved
+import "./intentionally-non-existent-child-module.mjs";

dom/tests/browser/file_test_report_missing_child_module_parent_02.mjs

@@ -0,0 +1,4 @@
+// Do not create a file with the below name.
+
+// eslint-disable-next-line import/no-unassigned-import, import/no-unresolved
+import "./intentionally-non-existent-grandchild-module.mjs";

dom/tests/browser/page_test_report_missing_child_module.html

@@ -0,0 +1,18 @@
+<!doctype html>
+<html>
+  <head>
+    <meta charset="utf-8">
+  </head>
+  <body>
+    <!--
+      top-level module which imports a missing module
+    -->
+    <script type="module" src="file_test_report_missing_child_module_parent.mjs"></script>
+
+    <!--
+      top-level module which imports a child module, which in turn imports
+      a missing module
+    -->
+    <script type="module" src="file_test_report_missing_child_module_grandparent.mjs"></script>
+  </body>
+</html>