Bug 996060 Part 2 - Use JS engine stack if necessary when reporting errors, r=bz.
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 01 Apr 2019 12:17:43 -0600
changeset 468506 8e21496aa97ba70c531bede8b6adf6fbc0eb29f8
parent 468505 36c7e4619a2978b3d4849820b19db7c9568db6fa
child 468507 ad5283f158e7bd91d73f0117ad06bbc7d0d83aad
push id112729
push userbhackett@mozilla.com
push dateTue, 09 Apr 2019 15:23:45 +0000
treeherdermozilla-inbound@7d16d7014ed3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs996060
milestone68.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 996060 Part 2 - Use JS engine stack if necessary when reporting errors, r=bz. Differential Revision: https://phabricator.services.mozilla.com/D25645
dom/base/nsJSEnvironment.cpp
dom/script/ScriptSettings.cpp
dom/script/ScriptSettings.h
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/src/xpcpublic.h
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -208,29 +208,37 @@ static TimeDuration sGCUnnotifiedTotalTi
 
 static const char* ProcessNameForCollectorLog() {
   return XRE_GetProcessType() == GeckoProcessType_Default ? "default"
                                                           : "content";
 }
 
 namespace xpc {
 
-// This handles JS Exceptions (via ExceptionStackOrNull), as well as DOM and XPC
-// Exceptions.
+// This handles JS Exceptions (via ExceptionStackOrNull), DOM and XPC
+// Exceptions, and arbitrary values that were associated with a stack by the
+// JS engine when they were thrown, as specified by exceptionStack.
 //
 // Note that the returned stackObj and stackGlobal are _not_ wrapped into the
 // compartment of exceptionValue.
 void FindExceptionStackForConsoleReport(nsPIDOMWindowInner* win,
                                         JS::HandleValue exceptionValue,
+                                        JS::HandleObject exceptionStack,
                                         JS::MutableHandleObject stackObj,
                                         JS::MutableHandleObject stackGlobal) {
   stackObj.set(nullptr);
   stackGlobal.set(nullptr);
 
   if (!exceptionValue.isObject()) {
+    // Use the stack provided by the JS engine, if available. This will not be
+    // a wrapper.
+    if (exceptionStack) {
+      stackObj.set(exceptionStack);
+      stackGlobal.set(JS::GetNonCCWObjectGlobal(exceptionStack));
+    }
     return;
   }
 
   if (win && win->AsGlobal()->IsDying()) {
     // Pretend like we have no stack, so we don't end up keeping the global
     // alive via the stack.
     return;
   }
@@ -252,16 +260,21 @@ void FindExceptionStackForConsoleReport(
 
   // It is not a JS Exception, try DOM Exception.
   RefPtr<Exception> exception;
   UNWRAP_OBJECT(DOMException, exceptionObject, exception);
   if (!exception) {
     // Not a DOM Exception, try XPC Exception.
     UNWRAP_OBJECT(Exception, exceptionObject, exception);
     if (!exception) {
+      // As above, use the stack provided by the JS engine, if available.
+      if (exceptionStack) {
+        stackObj.set(exceptionStack);
+        stackGlobal.set(JS::GetNonCCWObjectGlobal(exceptionStack));
+      }
       return;
     }
   }
 
   nsCOMPtr<nsIStackFrame> stack = exception->GetLocation();
   if (!stack) {
     return;
   }
@@ -401,21 +414,23 @@ bool NS_HandleScriptError(nsIScriptGloba
     --errorDepth;
   }
   return called;
 }
 
 class ScriptErrorEvent : public Runnable {
  public:
   ScriptErrorEvent(nsPIDOMWindowInner* aWindow, JS::RootingContext* aRootingCx,
-                   xpc::ErrorReport* aReport, JS::Handle<JS::Value> aError)
+                   xpc::ErrorReport* aReport, JS::Handle<JS::Value> aError,
+                   JS::Handle<JSObject*> aErrorStack)
       : mozilla::Runnable("ScriptErrorEvent"),
         mWindow(aWindow),
         mReport(aReport),
-        mError(aRootingCx, aError) {}
+        mError(aRootingCx, aError),
+        mErrorStack(aRootingCx, aErrorStack) {}
 
   NS_IMETHOD Run() override {
     nsEventStatus status = nsEventStatus_eIgnore;
     nsPIDOMWindowInner* win = mWindow;
     MOZ_ASSERT(win);
     MOZ_ASSERT(NS_IsMainThread());
     // First, notify the DOM that we have a script error, but only if
     // our window is still the current inner.
@@ -450,45 +465,47 @@ class ScriptErrorEvent : public Runnable
 
       EventDispatcher::DispatchDOMEvent(win, nullptr, event, presContext,
                                         &status);
     }
 
     if (status != nsEventStatus_eConsumeNoDefault) {
       JS::Rooted<JSObject*> stack(rootingCx);
       JS::Rooted<JSObject*> stackGlobal(rootingCx);
-      xpc::FindExceptionStackForConsoleReport(win, mError, &stack,
+      xpc::FindExceptionStackForConsoleReport(win, mError, mErrorStack, &stack,
                                               &stackGlobal);
       mReport->LogToConsoleWithStack(stack, stackGlobal,
                                      JS::ExceptionTimeWarpTarget(mError));
     }
 
     return NS_OK;
   }
 
  private:
   nsCOMPtr<nsPIDOMWindowInner> mWindow;
   RefPtr<xpc::ErrorReport> mReport;
   JS::PersistentRootedValue mError;
+  JS::PersistentRootedObject mErrorStack;
 
   static bool sHandlingScriptError;
 };
 
 bool ScriptErrorEvent::sHandlingScriptError = false;
 
 // This temporarily lives here to avoid code churn. It will go away entirely
 // soon.
 namespace xpc {
 
 void DispatchScriptErrorEvent(nsPIDOMWindowInner* win,
                               JS::RootingContext* rootingCx,
                               xpc::ErrorReport* xpcReport,
-                              JS::Handle<JS::Value> exception) {
+                              JS::Handle<JS::Value> exception,
+                              JS::Handle<JSObject*> exceptionStack) {
   nsContentUtils::AddScriptRunner(
-      new ScriptErrorEvent(win, rootingCx, xpcReport, exception));
+      new ScriptErrorEvent(win, rootingCx, xpcReport, exception, exceptionStack));
 }
 
 } /* namespace xpc */
 
 #ifdef DEBUG
 // A couple of useful functions to call when you're debugging.
 nsGlobalWindowInner* JSObject2Win(JSObject* obj) {
   return xpc::WindowOrNull(obj);
--- a/dom/script/ScriptSettings.cpp
+++ b/dom/script/ScriptSettings.cpp
@@ -489,35 +489,36 @@ void AutoJSAPI::ReportException() {
         // worker's global was created. Use the debugger global instead.
         errorGlobal = GetCurrentThreadWorkerDebuggerGlobal();
       }
     }
   }
   MOZ_ASSERT(JS_IsGlobalObject(errorGlobal));
   JSAutoRealm ar(cx(), errorGlobal);
   JS::Rooted<JS::Value> exn(cx());
+  JS::Rooted<JSObject*> exnStack(cx());
   js::ErrorReport jsReport(cx());
-  if (StealException(&exn) &&
+  if (StealExceptionAndStack(&exn, &exnStack) &&
       jsReport.init(cx(), exn, js::ErrorReport::WithSideEffects)) {
     if (mIsMainThread) {
       RefPtr<xpc::ErrorReport> xpcReport = new xpc::ErrorReport();
 
       RefPtr<nsGlobalWindowInner> win = xpc::WindowOrNull(errorGlobal);
       nsPIDOMWindowInner* inner = win ? win->AsInner() : nullptr;
       bool isChrome = nsContentUtils::IsSystemPrincipal(
           nsContentUtils::ObjectPrincipal(errorGlobal));
       xpcReport->Init(jsReport.report(), jsReport.toStringResult().c_str(),
                       isChrome, inner ? inner->WindowID() : 0);
       if (inner && jsReport.report()->errorNumber != JSMSG_OUT_OF_MEMORY) {
         JS::RootingContext* rcx = JS::RootingContext::get(cx());
-        DispatchScriptErrorEvent(inner, rcx, xpcReport, exn);
+        DispatchScriptErrorEvent(inner, rcx, xpcReport, exn, exnStack);
       } else {
         JS::Rooted<JSObject*> stack(cx());
         JS::Rooted<JSObject*> stackGlobal(cx());
-        xpc::FindExceptionStackForConsoleReport(inner, exn, &stack,
+        xpc::FindExceptionStackForConsoleReport(inner, exn, exnStack, &stack,
                                                 &stackGlobal);
         xpcReport->LogToConsoleWithStack(stack, stackGlobal);
       }
     } else {
       // On a worker, we just use the worker error reporting mechanism and don't
       // bother with xpc::ErrorReport.  This will ensure that all the right
       // events (which are a lot more complicated than in the window case) get
       // fired.
@@ -544,19 +545,26 @@ bool AutoJSAPI::PeekException(JS::Mutabl
   MOZ_ASSERT(js::GetContextRealm(cx()));
   if (!JS_GetPendingException(cx(), aVal)) {
     return false;
   }
   return true;
 }
 
 bool AutoJSAPI::StealException(JS::MutableHandle<JS::Value> aVal) {
+  JS::Rooted<JSObject*> stack(cx());
+  return StealExceptionAndStack(aVal, &stack);
+}
+
+bool AutoJSAPI::StealExceptionAndStack(JS::MutableHandle<JS::Value> aVal,
+                                       JS::MutableHandle<JSObject*> aStack) {
   if (!PeekException(aVal)) {
     return false;
   }
+  aStack.set(JS::GetPendingExceptionStack(cx()));
   JS_ClearPendingException(cx());
   return true;
 }
 
 #ifdef DEBUG
 bool AutoJSAPI::IsStackTop() const {
   return ScriptSettingsStack::TopNonIncumbentScript() == this;
 }
--- a/dom/script/ScriptSettings.h
+++ b/dom/script/ScriptSettings.h
@@ -267,16 +267,22 @@ class MOZ_STACK_CLASS AutoJSAPI : protec
   // Transfers ownership of the current exception from the JS engine to the
   // caller. Callers must ensure that HasException() is true, and that cx()
   // is in a non-null compartment.
   //
   // Note that this fails if and only if we OOM while wrapping the exception
   // into the current compartment.
   MOZ_MUST_USE bool StealException(JS::MutableHandle<JS::Value> aVal);
 
+  // As for StealException(), but put the saved frames for any stack trace
+  // associated with the point the exception was thrown into aStack.
+  // aVal will be in the current compartment, but aStack might not be.
+  MOZ_MUST_USE bool StealExceptionAndStack(JS::MutableHandle<JS::Value> aVal,
+                                           JS::MutableHandle<JSObject*> aStack);
+
   // Peek the current exception from the JS engine, without stealing it.
   // Callers must ensure that HasException() is true, and that cx() is in a
   // non-null compartment.
   //
   // Note that this fails if and only if we OOM while wrapping the exception
   // into the current compartment.
   MOZ_MUST_USE bool PeekException(JS::MutableHandle<JS::Value> aVal);
 
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -1343,17 +1343,17 @@ nsXPCComponents_Utils::ReportError(Handl
   RootedObject errorObj(cx, error.isObject() ? &error.toObject() : nullptr);
   JSErrorReport* err = errorObj ? JS_ErrorFromException(cx, errorObj) : nullptr;
 
   nsCOMPtr<nsIScriptError> scripterr;
 
   if (errorObj) {
     JS::RootedObject stackVal(cx);
     JS::RootedObject stackGlobal(cx);
-    FindExceptionStackForConsoleReport(win, error, &stackVal, &stackGlobal);
+    FindExceptionStackForConsoleReport(win, error, nullptr, &stackVal, &stackGlobal);
     if (stackVal) {
       scripterr = new nsScriptErrorWithStack(stackVal, stackGlobal);
     }
   }
 
   nsString fileName;
   uint32_t lineNo = 0;
 
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -621,36 +621,39 @@ class ErrorReport : public ErrorBase {
 
  private:
   ~ErrorReport() {}
 };
 
 void DispatchScriptErrorEvent(nsPIDOMWindowInner* win,
                               JS::RootingContext* rootingCx,
                               xpc::ErrorReport* xpcReport,
-                              JS::Handle<JS::Value> exception);
+                              JS::Handle<JS::Value> exception,
+                              JS::Handle<JSObject*> exceptionStack);
 
 // Get a stack (as stackObj outparam) of the sort that can be passed to
 // xpc::ErrorReport::LogToConsoleWithStack from the given exception value.  Can
-// be nullptr if the exception value doesn't have an associated stack.  The
+// be nullptr if the exception value doesn't have an associated stack, and if
+// there is no stack supplied by the JS engine in exceptionStack.  The
 // returned stack, if any, may also not be in the same compartment as
 // exceptionValue.
 //
 // The "win" argument passed in here should be the same as the window whose
 // WindowID() is used to initialize the xpc::ErrorReport.  This may be null, of
 // course.  If it's not null, this function may return a null stack object if
 // the window is far enough gone, because in those cases we don't want to have
 // the stack in the console message keeping the window alive.
 //
 // If this function sets stackObj to a non-null value, stackGlobal is set to
 // either the JS exception object's global or the global of the SavedFrame we
 // got from a DOM or XPConnect exception. In all cases, stackGlobal is an
 // unwrapped global object and is same-compartment with stackObj.
 void FindExceptionStackForConsoleReport(nsPIDOMWindowInner* win,
                                         JS::HandleValue exceptionValue,
+                                        JS::HandleObject exceptionStack,
                                         JS::MutableHandleObject stackObj,
                                         JS::MutableHandleObject stackGlobal);
 
 // Return a name for the realm.
 // This function makes reasonable efforts to make this name both mostly
 // human-readable and unique. However, there are no guarantees of either
 // property.
 extern void GetCurrentRealmName(JSContext*, nsCString& name);