Bug 1681664 - Allow Top-level await modules which fail to evaluate due to user action to fail without crashing. r=jonco,mconley

Differential Revision: https://phabricator.services.mozilla.com/D99809
2020-12-18 12:37:36 +00:00 · 2020-12-18 12:37:36 +00:00 · a1a1c495ea
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@ -3021,7 +3021,13 @@ nsresult ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) {
      JS::Rooted<JS::Value> rval(cx);

      rv = nsJSUtils::ModuleEvaluate(cx, module, &rval);
-      MOZ_ASSERT(NS_FAILED(rv) == aes.HasException());
+
+      if (NS_SUCCEEDED(rv)) {
+        // If we have an infinite loop in a module, which is stopped by the
+        // user, the module evaluation will fail, but we will not have an
+        // AutoEntryScript exception.
+        MOZ_ASSERT(!aes.HasException());
+      }

      if (NS_FAILED(rv)) {
        LOG(("ScriptLoadRequest (%p):   evaluation failed", aRequest));
@ -3036,14 +3042,22 @@ nsresult ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) {
        }
      } else {
        // Path for when Top Level Await is enabled
-        JS::Rooted<JSObject*> aEvaluationPromise(cx, &rval.toObject());
+        JS::Rooted<JSObject*> aEvaluationPromise(cx);
+        if (NS_SUCCEEDED(rv)) {
+          // If the user cancels the evaluation on an infinite loop, we need
+          // to skip this step. In that case, ModuleEvaluate will not return a
+          // promise, rval will be undefined. We should treat it as a failed
+          // evaluation, and reject appropriately.
+          aEvaluationPromise.set(&rval.toObject());
+        }
        if (request->IsDynamicImport()) {
          FinishDynamicImport(cx, request, rv, aEvaluationPromise);
        } else {
          // If this is not a dynamic import, and if the promise is rejected,
          // the value is unwrapped from the promise value.
          if (!JS::ThrowOnModuleEvaluationFailure(cx, aEvaluationPromise)) {
-            LOG(("ScriptLoadRequest (%p):   evaluation failed", aRequest));
+            LOG(("ScriptLoadRequest (%p):   evaluation failed on throw",
+                 aRequest));
            // For a dynamic import, the promise is rejected.  Otherwise an
            // error is either reported by AutoEntryScript.
            rv = NS_OK;
--- a/js/xpconnect/tests/mochitest/bug1681664_helper.js
+++ b/js/xpconnect/tests/mochitest/bug1681664_helper.js
@ -0,0 +1 @@
+while (true) {};
--- a/js/xpconnect/tests/mochitest/mochitest.ini
+++ b/js/xpconnect/tests/mochitest/mochitest.ini
@ -38,9 +38,11 @@ support-files =
  test2_bug629331.html
  finalizationRegistry_worker.js
  private_field_worker.js
+  bug1681664_helper.js
 prefs =
  javascript.options.weakrefs=true
  javascript.options.experimental.private_fields=true
+  javascript.options.experimental.top_level_await=true

 [test_bug384632.html]
 [test_bug390488.html]
@ -99,6 +101,7 @@ skip-if = toolkit == "android" && debug && !is_fennec
 [test_bug1094930.html]
 [test_bug1158558.html]
 [test_bug1448048.html]
+[test_bug1681664.html]
 [test_crosscompartment_weakmap.html]
 [test_enable_privilege.html]
 [test_frameWrapping.html]
@ -123,4 +126,4 @@ skip-if = (debug == false)
 [test_weakRefs_cross_compartment.html]
 [test_weakRefs_collected_wrapper.html]
 [test_private_field_dom.html]
-[test_private_field_worker.html]
+[test_private_field_worker.html]
--- a/js/xpconnect/tests/mochitest/test_bug1681664.html
+++ b/js/xpconnect/tests/mochitest/test_bug1681664.html
@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <title>Test page for bug 1681664</title>
+    <script src="/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+    <script>
+      SimpleTest.waitForExplicitFinish()
+      async function init() {
+        var observer = {
+          observe(subject, topic, data) {
+            if (topic === "process-hang-report") {
+              var report = subject.QueryInterface(Ci.nsIHangReport);
+              report.terminateScript();
+              Services.obs.removeObserver(observer, "process-hang-report");
+            }
+          }
+        }
+
+        Services.obs.addObserver(observer, "process-hang-report");
+        try {
+          await import("test_bug1681664.js");
+          result.textContent = "FAIL";
+        } catch (ex) {
+          result.textContent = "PASS";
+        }
+      }
+    </script>
+  </head>
+  <body>
+    <p id="result"></p>
+    <script>
+      (async function() {
+        await init();
+        is(result.textContent, "PASS", "Infinite loop script should not cause browser crash");
+        SimpleTest.finish()
+      })();
+    </script>
+  </body>
+</html>
+