Bug 1441333: Part 2 - Allow passing an error stack to reportError. draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 01 Mar 2018 13:57:27 -0800
changeset 762299 dbe4439d214c69a3036dd7108ab301cbcb522d4d
parent 762298 9abf4101d5dda2713e307fc45953c4a61ca72f5a
child 762300 ec9e15b5aae028c2c55e20c62dafe3729413ae26
push id101126
push usermaglione.k@gmail.com
push dateFri, 02 Mar 2018 00:45:19 +0000
bugs1441333
milestone60.0a1
Bug 1441333: Part 2 - Allow passing an error stack to reportError. Building on top of part 1, we need a way to link a saved caller location to a reported error message. This change allows us to pass a stack to `reportError` when called with a string. This part was written before part 3, and could probably be removed in favor of using createError in every call. But this method also has the advantage of being more consise and somewhate more efficient. MozReview-Commit-ID: 39jfCg9AerY
js/xpconnect/idl/xpccomponents.idl
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/tests/unit/test_getCallerLocation.js
--- a/js/xpconnect/idl/xpccomponents.idl
+++ b/js/xpconnect/idl/xpccomponents.idl
@@ -144,18 +144,22 @@ interface nsIXPCComponents_Utils : nsISu
      *
      * It will report a JS Error object to the JS console, and return. It
      * is meant for use in exception handler blocks which want to "eat"
      * an exception, but still want to report it to the console.
      *
      * It must be called with one param, usually an object which was caught by
      * an exception handler.  If it is not a JS error object, the parameter
      * is converted to a string and reported as a new error.
+     *
+     * If called with two parameters, and the first parameter is not an
+     * object, the second parameter is used as the stack for the error report.
      */
-    [implicit_jscontext] void reportError(in jsval error);
+    [implicit_jscontext]
+    void reportError(in jsval error, [optional] in jsval stack);
 
     readonly attribute nsIXPCComponents_utils_Sandbox Sandbox;
 
     /*
      * evalInSandbox is designed to be called from JavaScript only.
      *
      * evalInSandbox evaluates the provided source string in the given sandbox.
      * It returns the result of the evaluation to the caller.
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -2051,17 +2051,17 @@ nsXPCComponents_Utils::GetSandbox(nsIXPC
         mSandbox = NewSandboxConstructor();
 
     nsCOMPtr<nsIXPCComponents_utils_Sandbox> rval = mSandbox;
     rval.forget(aSandbox);
     return NS_OK;
 }
 
 NS_IMETHODIMP
-nsXPCComponents_Utils::ReportError(HandleValue error, JSContext* cx)
+nsXPCComponents_Utils::ReportError(HandleValue error, HandleValue stack, JSContext* cx)
 {
     // This function shall never fail! Silently eat any failure conditions.
 
     nsCOMPtr<nsIConsoleService> console(do_GetService(NS_CONSOLESERVICE_CONTRACTID));
     if (!console)
         return NS_OK;
 
     nsGlobalWindowInner* globalWin = CurrentWindowOrNull(cx);
@@ -2077,29 +2077,54 @@ nsXPCComponents_Utils::ReportError(Handl
         JS::RootedObject stackVal(cx,
           FindExceptionStackForConsoleReport(win, error));
         if (stackVal) {
             scripterr = new nsScriptErrorWithStack(stackVal);
         }
     }
 
     nsString fileName;
-    int32_t lineNo = 0;
+    uint32_t lineNo = 0;
 
     if (!scripterr) {
-        nsCOMPtr<nsIStackFrame> frame = dom::GetCurrentJSStack();
-        if (frame) {
-            frame->GetFilename(cx, fileName);
-            lineNo = frame->GetLineNumber(cx);
-            JS::Rooted<JS::Value> stack(cx);
-            nsresult rv = frame->GetNativeSavedFrame(&stack);
-            if (NS_SUCCEEDED(rv) && stack.isObject()) {
-              JS::Rooted<JSObject*> stackObj(cx, &stack.toObject());
-              scripterr = new nsScriptErrorWithStack(stackObj);
+        RootedObject stackObj(cx);
+        if (stack.isObject()) {
+            if (!JS::IsSavedFrame(&stack.toObject())) {
+                return NS_ERROR_INVALID_ARG;
+            }
+
+            stackObj = &stack.toObject();
+
+            if (GetSavedFrameLine(cx, stackObj, &lineNo) != SavedFrameResult::Ok) {
+                JS_ClearPendingException(cx);
             }
+
+            RootedString source(cx);
+            nsAutoJSString str;
+            if (GetSavedFrameSource(cx, stackObj, &source) == SavedFrameResult::Ok &&
+                str.init(cx, source)) {
+                fileName = str;
+            } else {
+                JS_ClearPendingException(cx);
+            }
+        } else {
+            nsCOMPtr<nsIStackFrame> frame = dom::GetCurrentJSStack();
+            if (frame) {
+                frame->GetFilename(cx, fileName);
+                lineNo = frame->GetLineNumber(cx);
+                JS::Rooted<JS::Value> stack(cx);
+                nsresult rv = frame->GetNativeSavedFrame(&stack);
+                if (NS_SUCCEEDED(rv) && stack.isObject()) {
+                  stackObj = &stack.toObject();
+                }
+            }
+        }
+
+        if (stackObj) {
+            scripterr = new nsScriptErrorWithStack(stackObj);
         }
     }
 
     if (!scripterr) {
         scripterr = new nsScriptError();
     }
 
     if (err) {
--- a/js/xpconnect/tests/unit/test_getCallerLocation.js
+++ b/js/xpconnect/tests/unit/test_getCallerLocation.js
@@ -1,15 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 Cu.importGlobalProperties(["ChromeUtils"]);
 
+ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm");
+
 add_task(async function() {
   const sandbox = Cu.Sandbox("http://example.com/");
 
   function foo() {
     return bar();
   }
 
   function bar() {
@@ -37,9 +39,23 @@ add_task(async function() {
 
   equal(frame.source, "thing.js", "Frame source");
   equal(frame.line, 5, "Frame line");
   equal(frame.column, 14, "Frame column");
   equal(frame.functionDisplayName, "it", "Frame function name");
   equal(frame.parent, null, "Frame parent");
 
   equal(String(frame), "it@thing.js:5:14\n", "Stringified frame");
+
+
+  // reportError
+
+  let {messages} = await AddonTestUtils.promiseConsoleOutput(() => {
+    Cu.reportError("Meh", frame);
+  });
+
+  equal(messages[0].stack, frame, "reportError stack frame");
+  equal(messages[0].message, '[JavaScript Error: "Meh" {file: "thing.js" line: 5}]\nit@thing.js:5:14\n');
+
+  Assert.throws(() => { Cu.reportError("Meh", {}); },
+                err => err.result == Cr.NS_ERROR_INVALID_ARG,
+                "reportError should throw when passed a non-SavedFrame object");
 });