Bug 1940287 - [devtools] Make callFunctionAndLogException re-throw the original exception. r=mccr8

Moving this existing helper from Console (idl) to ChromeUtils (WebIdl)
in order to be able to use aRv.MightThrowJSException and aRv.ThrowJSException
which are key ingredients to be able to re-throw the intercepted exception.

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

dom/base/ChromeUtils.cpp

@@ -2526,6 +2526,67 @@ bool ChromeUtils::ShouldResistFingerprinting(
                                        overriddenFingerprintingSettings);
 }
 
+/* static */
+void ChromeUtils::CallFunctionAndLogException(
+    GlobalObject& aGlobal, JS::Handle<JS::Value> aTargetGlobal,
+    JS::Handle<JS::Value> aFunction, JS::MutableHandle<JS::Value> aRetVal,
+    ErrorResult& aRv) {
+  JSContext* cx = aGlobal.Context();
+  if (!aTargetGlobal.isObject() || !aFunction.isObject()) {
+    aRv.Throw(NS_ERROR_INVALID_ARG);
+    return;
+  }
+
+  JS::Rooted<JS::Realm*> contextRealm(cx, JS::GetCurrentRealmOrNull(cx));
+  if (!contextRealm) {
+    aRv.Throw(NS_ERROR_INVALID_ARG);
+    return;
+  }
+
+  JS::Rooted<JSObject*> global(
+      cx, js::CheckedUnwrapDynamic(&aTargetGlobal.toObject(), cx));
+  if (!global) {
+    aRv.Throw(NS_ERROR_INVALID_ARG);
+    return;
+  }
+
+  // Use AutoJSAPI in order to trigger AutoJSAPI::ReportException
+  // which will do most of the work required for this function.
+  //
+  // We only have to pick the right global for which we want to flag
+  // the exception against.
+  dom::AutoJSAPI jsapi;
+  if (!jsapi.Init(global)) {
+    aRv.Throw(NS_ERROR_UNEXPECTED);
+    return;
+  }
+  JSContext* ccx = jsapi.cx();
+
+  // AutoJSAPI picks `aTargetGlobal` as execution compartment
+  // whereas we expect to run `aFunction` from the callsites compartment.
+  JSAutoRealm ar(ccx, JS::GetRealmGlobalOrNull(contextRealm));
+
+  JS::Rooted<JS::Value> funVal(ccx, aFunction);
+  if (!JS_WrapValue(ccx, &funVal)) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+  if (!JS_CallFunctionValue(ccx, nullptr, funVal, JS::HandleValueArray::empty(),
+                            aRetVal)) {
+    // Ensure re-throwing the exception which may have been thrown by
+    // `aFunction`
+    if (JS_IsExceptionPending(ccx)) {
+      JS::Rooted<JS::Value> exception(cx);
+      if (JS_GetPendingException(ccx, &exception)) {
+        if (JS_WrapValue(cx, &exception)) {
+          aRv.MightThrowJSException();
+          aRv.ThrowJSException(cx, exception);
+        }
+      }
+    }
+  }
+}
+
 std::atomic<uint32_t> ChromeUtils::sDevToolsOpenedCount = 0;
 
 /* static */

dom/base/ChromeUtils.h

@@ -321,6 +321,12 @@ class ChromeUtils {
       nsIRFPTargetSetIDL* aOverriddenFingerprintingSettings,
       const Optional<bool>& aIsPBM);
 
+  static void CallFunctionAndLogException(GlobalObject& aGlobal,
+                                          JS::Handle<JS::Value> aTargetGlobal,
+                                          JS::Handle<JS::Value> aFunction,
+                                          JS::MutableHandle<JS::Value> aRetval,
+                                          ErrorResult& aRv);
+
 #ifdef MOZ_WMF_CDM
   static already_AddRefed<Promise> GetWMFContentDecryptionModuleInformation(
       GlobalObject& aGlobal, ErrorResult& aRv);

dom/base/test/unit/test_chromeutils_callFunctionAndLogException.js

@@ -51,15 +51,20 @@ add_task(async function customScriptError() {
 
 add_task(async function callFunctionAndLogExceptionWithChromeGlobal() {
   try {
-    Services.console.callFunctionAndLogException(globalThis, function () {
+    ChromeUtils.callFunctionAndLogException(globalThis, function () {
       throw new Error("custom exception");
     });
-    Assert.fail("callFunctionAndLogException should throw");
+    Assert.ok(false, "callFunctionAndLogException should throw");
   } catch (e) {
     Assert.equal(
       e.name,
-      "NS_ERROR_XPC_JAVASCRIPT_ERROR",
+      "Error",
-      "callFunctionAndLogException thrown"
+      "callFunctionAndLogException thrown with the expected exception name"
+    );
+    Assert.equal(
+      e.message,
+      "custom exception",
+      "callFunctionAndLogException thrown with the expected message"
     );
   }
 
@@ -92,15 +97,20 @@ add_task(async function callFunctionAndLogExceptionWithChromeGlobal() {
 add_task(async function callFunctionAndLogExceptionWithContentGlobal() {
   const window = createContentWindow();
   try {
-    Services.console.callFunctionAndLogException(window, function () {
+    ChromeUtils.callFunctionAndLogException(window, function () {
       throw new Error("another custom exception");
     });
-    Assert.fail("callFunctionAndLogException should throw");
+    Assert.ok(false, "callFunctionAndLogException should throw");
   } catch (e) {
     Assert.equal(
       e.name,
-      "NS_ERROR_XPC_JAVASCRIPT_ERROR",
+      "Error",
-      "callFunctionAndLogException thrown"
+      "callFunctionAndLogException thrown with the expected exception name"
+    );
+    Assert.equal(
+      e.message,
+      "another custom exception",
+      "callFunctionAndLogException thrown with the expected message"
     );
   }
 
@@ -114,13 +124,13 @@ add_task(async function callFunctionAndLogExceptionWithContentGlobal() {
 
   Assert.equal(lastMessage.errorMessage, "Error: another custom exception");
   Assert.equal(lastMessage.sourceName, _TEST_FILE);
-  Assert.equal(lastMessage.lineNumber, 96);
+  Assert.equal(lastMessage.lineNumber, 101);
   Assert.equal(lastMessage.columnNumber, 13);
   Assert.equal(lastMessage.flags, Ci.nsIScriptError.errorFlag);
   Assert.equal(lastMessage.category, "content javascript");
   Assert.ok(lastMessage.stack, "It has a stack");
   Assert.equal(lastMessage.stack.source, _TEST_FILE);
-  Assert.equal(lastMessage.stack.line, 96);
+  Assert.equal(lastMessage.stack.line, 101);
   Assert.equal(lastMessage.stack.column, 13);
   Assert.ok(!!lastMessage.stack.parent, "stack has a parent frame");
   Assert.ok(
@@ -145,13 +155,18 @@ add_task(async function callFunctionAndLogExceptionForContentScriptSandboxes() {
     0
   );
   try {
-    Services.console.callFunctionAndLogException(window, sandbox.foo);
+    ChromeUtils.callFunctionAndLogException(window, sandbox.foo);
     Assert.fail("callFunctionAndLogException should throw");
   } catch (e) {
     Assert.equal(
       e.name,
-      "NS_ERROR_XPC_JAVASCRIPT_ERROR",
+      "Error",
-      "callFunctionAndLogException thrown"
+      "callFunctionAndLogException thrown with the expected exception name"
+    );
+    Assert.equal(
+      e.message,
+      "sandbox exception",
+      "callFunctionAndLogException thrown with the expected message"
     );
   }
 
@@ -198,15 +213,20 @@ add_task(
       0
     );
     try {
-      Services.console.callFunctionAndLogException(window, function () {
+      ChromeUtils.callFunctionAndLogException(window, function () {
         sandbox.foo();
       });
       Assert.fail("callFunctionAndLogException should throw");
     } catch (e) {
       Assert.equal(
         e.name,
-        "NS_ERROR_XPC_JAVASCRIPT_ERROR",
+        "Error",
-        "callFunctionAndLogException thrown"
+        "callFunctionAndLogException thrown with the expected exception name"
+      );
+      Assert.equal(
+        e.message,
+        "sandbox exception",
+        "callFunctionAndLogException thrown with the expected message"
       );
     }
 

dom/base/test/unit/xpcshell.toml

@@ -38,6 +38,8 @@ prefs = ["network.xhr.block_sync_system_requests=false"] # Bug 721336
 
 ["test_chromeutils_base64.js"]
 
+["test_chromeutils_callFunctionAndLogException.js"]
+
 ["test_chromeutils_defineLazyGetter.js"]
 
 ["test_chromeutils_isJSIdentifier.js"]

dom/chrome-webidl/ChromeUtils.webidl

@@ -796,6 +796,18 @@ partial namespace ChromeUtils {
 
   [Throws]
   ContentSecurityPolicy createCSPFromHeader(DOMString header, URI selfURI, Principal loadingPrincipal);
+
+  // This helper function executes `func` and redirects any exception
+  // that may be thrown while running it to the DevTools Console currently
+  // debugging `targetGlobal`.
+  //
+  // This helps flag the nsIScriptError with a particular innerWindowID
+  // which is especially useful for WebExtension content scripts
+  // where script are running in a Sandbox whose prototype is the content window.
+  // We expect content script exception to be flagged with the content window
+  // innerWindowID in order to appear in the tab's DevTools.
+  [ChromeOnly, Throws]
+  any callFunctionAndLogException(any targetGlobal, any func);
 };
 
 /*

xpcom/base/nsConsoleService.cpp

@@ -425,53 +425,6 @@ nsresult nsConsoleService::LogMessageWithMode(
   return NS_OK;
 }
 
-// See nsIConsoleService.idl for more info about this method
-NS_IMETHODIMP
-nsConsoleService::CallFunctionAndLogException(
-    JS::Handle<JS::Value> targetGlobal, JS::HandleValue function, JSContext* cx,
-    JS::MutableHandleValue retval) {
-  if (!targetGlobal.isObject() || !function.isObject()) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
-  JS::Rooted<JS::Realm*> contextRealm(cx, JS::GetCurrentRealmOrNull(cx));
-  if (!contextRealm) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
-  JS::Rooted<JSObject*> global(
-      cx, js::CheckedUnwrapDynamic(&targetGlobal.toObject(), cx));
-  if (!global) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
-  // Use AutoJSAPI in order to trigger AutoJSAPI::ReportException
-  // which will do most of the work required for this function.
-  //
-  // We only have to pick the right global for which we want to flag
-  // the exception against.
-  dom::AutoJSAPI jsapi;
-  if (!jsapi.Init(global)) {
-    return NS_ERROR_UNEXPECTED;
-  }
-  JSContext* ccx = jsapi.cx();
-
-  // AutoJSAPI picks `targetGlobal` as execution compartment
-  // whereas we expect to run `function` from the callsites compartment.
-  JSAutoRealm ar(ccx, JS::GetRealmGlobalOrNull(contextRealm));
-
-  JS::RootedValue funVal(ccx, function);
-  if (!JS_WrapValue(ccx, &funVal)) {
-    return NS_ERROR_FAILURE;
-  }
-  if (!JS_CallFunctionValue(ccx, nullptr, funVal, JS::HandleValueArray::empty(),
-                            retval)) {
-    return NS_ERROR_XPC_JAVASCRIPT_ERROR;
-  }
-
-  return NS_OK;
-}
-
 void nsConsoleService::CollectCurrentListeners(
     nsCOMArray<nsIConsoleListener>& aListeners) {
   MutexAutoLock lock(mLock);

xpcom/base/nsIConsoleService.idl

@@ -13,18 +13,6 @@ interface nsIConsoleService : nsISupports
 {
     void logMessage(in nsIConsoleMessage message);
 
-    // This helper function executes `func` and redirects any exception
-    // that may be thrown while running it to the DevTools Console currently
-    // debugging `targetGlobal`.
-    //
-    // This helps flag the nsIScriptError with a particular innerWindowID
-    // which is especially useful for WebExtension content scripts
-    // where script are running in a Sandbox whose prototype is the content window.
-    // We expect content script exception to be flaged with the content window
-    // innerWindowID in order to appear in the tab's DevTools.
-    [implicit_jscontext]
-    jsval callFunctionAndLogException(in jsval targetGlobal, in jsval func);
-
     // This is a variant of LogMessage which allows the caller to determine
     // if the message should be output to an OS-specific log. This is used on
     // B2G to control whether the message is logged to the android log or not.

xpcom/tests/unit/xpcshell.toml

@@ -33,8 +33,6 @@ fail-if = ["os == 'android'"]
 
 ["test_bug1434856.js"]
 
-["test_console_service_callFunctionAndLogException.js"]
-
 ["test_debugger_malloc_size_of.js"]
 
 ["test_error_iserror.js"]