Bug 1409852 - Expose an API in ChromeUtils to detect chrome JS dev errors;r=bz
authorDavid Teller <dteller@mozilla.com>
Thu, 16 Nov 2017 10:48:45 +0100
changeset 449097 d49a1de5d569881d555e1b75250747aa58bd7ad2
parent 449096 0ea178ea953acb063d2581b3d2e889235a80ed71
child 449098 ba9e7b6956191e8212e5eb3dcbdc3301db10a27f
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1409852
milestone59.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 1409852 - Expose an API in ChromeUtils to detect chrome JS dev errors;r=bz MozReview-Commit-ID: GluMLeQOHTZ
dom/base/ChromeUtils.cpp
dom/base/ChromeUtils.h
dom/base/test/unit/test_js_dev_error_interceptor.js
dom/base/test/unit/xpcshell.ini
dom/webidl/ChromeUtils.webidl
dom/webidl/moz.build
xpcom/base/CycleCollectedJSRuntime.cpp
xpcom/base/CycleCollectedJSRuntime.h
--- a/dom/base/ChromeUtils.cpp
+++ b/dom/base/ChromeUtils.cpp
@@ -6,16 +6,17 @@
 
 #include "ChromeUtils.h"
 
 #include "jsfriendapi.h"
 #include "WrapperFactory.h"
 
 #include "mozilla/Base64.h"
 #include "mozilla/BasePrincipal.h"
+#include "mozilla/CycleCollectedJSRuntime.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/dom/IdleDeadline.h"
 #include "mozilla/dom/UnionTypes.h"
 #include "mozilla/dom/WindowBinding.h" // For IdleRequestCallback/Options
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 namespace dom {
@@ -428,10 +429,37 @@ ChromeUtils::IsOriginAttributesEqual(con
                                      const dom::OriginAttributesDictionary& aB)
 {
   return aA.mAppId == aB.mAppId &&
          aA.mInIsolatedMozBrowser == aB.mInIsolatedMozBrowser &&
          aA.mUserContextId == aB.mUserContextId &&
          aA.mPrivateBrowsingId == aB.mPrivateBrowsingId;
 }
 
+#ifdef NIGHTLY_BUILD
+/* static */ void
+ChromeUtils::GetRecentJSDevError(GlobalObject& aGlobal,
+                                JS::MutableHandleValue aRetval,
+                                ErrorResult& aRv)
+{
+  aRetval.setUndefined();
+  auto runtime = CycleCollectedJSRuntime::Get();
+  MOZ_ASSERT(runtime);
+
+  auto cx = aGlobal.Context();
+  if (!runtime->GetRecentDevError(cx, aRetval)) {
+    aRv.NoteJSContextException(cx);
+    return;
+  }
+}
+
+/* static */ void
+ChromeUtils::ClearRecentJSDevError(GlobalObject&)
+{
+  auto runtime = CycleCollectedJSRuntime::Get();
+  MOZ_ASSERT(runtime);
+
+  runtime->ClearRecentDevError();
+}
+#endif // NIGHTLY_BUILD
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/base/ChromeUtils.h
+++ b/dom/base/ChromeUtils.h
@@ -143,14 +143,20 @@ public:
                            JS::HandleObject aTarget,
                            JS::MutableHandleObject aRetval,
                            ErrorResult& aRv);
 
   static void IdleDispatch(const GlobalObject& global,
                            IdleRequestCallback& callback,
                            const IdleRequestOptions& options,
                            ErrorResult& aRv);
+
+  static void GetRecentJSDevError(GlobalObject& aGlobal,
+                                  JS::MutableHandleValue aRetval,
+                                  ErrorResult& aRv);
+
+  static void ClearRecentJSDevError(GlobalObject& aGlobal);
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_ChromeUtils__
new file mode 100644
--- /dev/null
+++ b/dom/base/test/unit/test_js_dev_error_interceptor.js
@@ -0,0 +1,56 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* 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";
+
+function errors() {
+    return [
+        // The following two errors MUST NOT be captured.
+        new Error("This is an error: " + Math.random()),
+        new RangeError("This is a RangeError: " + Math.random()),
+        "This is a string: " + Math.random(),
+        null,
+        undefined,
+        Math.random(),
+        {},
+
+        // The following errors MUST be captured.
+        new TypeError("This is a TypeError: " + Math.random()),
+        new SyntaxError("This is a SyntaxError: " + Math.random()),
+        new ReferenceError("This is a ReferenceError: " + Math.random())
+    ]
+}
+
+function isDeveloperError(e) {
+    if (e == null || typeof e != "object") {
+        return false;
+    }
+
+    return e.constructor == TypeError
+        || e.constructor == SyntaxError
+        || e.constructor == ReferenceError;
+}
+
+function run_test() {
+    ChromeUtils.clearRecentJSDevError();
+    Assert.equal(ChromeUtils.recentJSDevError, undefined);
+
+    for (let exn of errors()) {
+        ChromeUtils.clearRecentJSDevError();
+        try {
+            throw exn;
+        } catch (e) {
+            // Discard error.
+        }
+        if (isDeveloperError(exn)) {
+            Assert.equal(ChromeUtils.recentJSDevError.message, "" + exn);
+        } else {
+            Assert.equal(ChromeUtils.recentJSDevError, undefined);
+        }
+        ChromeUtils.clearRecentJSDevError();
+        Assert.equal(ChromeUtils.recentJSDevError, undefined);
+    }
+};
+
--- a/dom/base/test/unit/xpcshell.ini
+++ b/dom/base/test/unit/xpcshell.ini
@@ -49,9 +49,12 @@ head = head_xml.js
 head = head_xml.js
 [test_xml_serializer.js]
 head = head_xml.js
 [test_xmlserializer.js]
 [test_cancelPrefetch.js]
 [test_chromeutils_base64.js]
 [test_generate_xpath.js]
 head = head_xml.js
+[test_js_dev_error_interceptor.js]
+# This feature is implemented only in NIGHTLY.
+run-if = nightly_build
 
--- a/dom/webidl/ChromeUtils.webidl
+++ b/dom/webidl/ChromeUtils.webidl
@@ -84,16 +84,50 @@ namespace ChromeUtils {
    * @param string The string to decode.
    * @param options Additional decoding options.
    * @returns The decoded buffer.
    */
   [Throws, NewObject]
   ArrayBuffer base64URLDecode(ByteString string,
                               Base64URLDecodeOptions options);
 
+#ifdef NIGHTLY_BUILD
+
+  /**
+   * If the chrome code has thrown a JavaScript Dev Error
+   * in the current JSRuntime. the first such error, or `undefined`
+   * otherwise.
+   *
+   * A JavaScript Dev Error is an exception thrown by JavaScript
+   * code that matches both conditions:
+   * - it was thrown by chrome code;
+   * - it is either a `ReferenceError`, a `TypeError` or a `SyntaxError`.
+   *
+   * Such errors are stored regardless of whether they have been
+   * caught.
+   *
+   * This mechanism is designed to help ensure that the code of
+   * Firefox is free from Dev Errors, even if they are accidentally
+   * caught by clients.
+   *
+   * The object returned is not an exception. It has fields:
+   * - DOMString stack
+   * - DOMString filename
+   * - DOMString lineNumber
+   * - DOMString message
+   */
+  [Throws]
+  readonly attribute any recentJSDevError;
+
+  /**
+   * Reset `recentJSDevError` to `undefined` for the current JSRuntime.
+   */
+  void clearRecentJSDevError();
+#endif // NIGHTLY_BUILD
+
   /**
    * IF YOU ADD NEW METHODS HERE, MAKE SURE THEY ARE THREAD-SAFE.
    */
 };
 
 /**
  * Additional ChromeUtils methods that are _not_ thread-safe, and hence not
  * exposed in workers.
--- a/dom/webidl/moz.build
+++ b/dom/webidl/moz.build
@@ -358,16 +358,17 @@ with Files("WindowOrWorkerGlobalScope.we
 with Files("Worker*"):
     BUG_COMPONENT = ("Core", "DOM: Workers")
 
 GENERATED_WEBIDL_FILES = [
     'CSS2Properties.webidl',
 ]
 
 PREPROCESSED_WEBIDL_FILES = [
+    'ChromeUtils.webidl',
     'Navigator.webidl',
     'Node.webidl',
     'Window.webidl',
 ]
 
 WEBIDL_FILES = [
     'AbortController.webidl',
     'AbortSignal.webidl',
@@ -420,17 +421,16 @@ WEBIDL_FILES = [
     'CDATASection.webidl',
     'ChannelMergerNode.webidl',
     'ChannelSplitterNode.webidl',
     'ChannelWrapper.webidl',
     'CharacterData.webidl',
     'CheckerboardReportService.webidl',
     'ChildNode.webidl',
     'ChromeNodeList.webidl',
-    'ChromeUtils.webidl',
     'Client.webidl',
     'Clients.webidl',
     'ClipboardEvent.webidl',
     'CommandEvent.webidl',
     'Comment.webidl',
     'CompositionEvent.webidl',
     'Console.webidl',
     'ConstantSourceNode.webidl',
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -92,16 +92,21 @@
 #endif
 
 #include "nsIException.h"
 #include "nsIPlatformInfo.h"
 #include "nsThread.h"
 #include "nsThreadUtils.h"
 #include "xpcpublic.h"
 
+#ifdef NIGHTLY_BUILD
+// For performance reasons, we make the JS Dev Error Interceptor a Nightly-only feature.
+#define MOZ_JS_DEV_ERROR_INTERCEPTOR = 1
+#endif // NIGHTLY_BUILD
+
 using namespace mozilla;
 using namespace mozilla::dom;
 
 namespace mozilla {
 
 struct DeferredFinalizeFunctionHolder
 {
   DeferredFinalizeFunction run;
@@ -551,21 +556,28 @@ CycleCollectedJSRuntime::CycleCollectedJ
 
   static js::DOMCallbacks DOMcallbacks = {
     InstanceClassHasProtoAtDepth
   };
   SetDOMCallbacks(aCx, &DOMcallbacks);
   js::SetScriptEnvironmentPreparer(aCx, &mEnvironmentPreparer);
 
   JS::dbg::SetDebuggerMallocSizeOf(aCx, moz_malloc_size_of);
+
+#ifdef MOZ_JS_DEV_ERROR_INTERCEPTOR
+  JS_SetErrorInterceptorCallback(mJSRuntime, &mErrorInterceptor);
+#endif // MOZ_JS_DEV_ERROR_INTERCEPTOR
 }
 
 void
 CycleCollectedJSRuntime::Shutdown(JSContext* cx)
 {
+#ifdef MOZ_JS_DEV_ERROR_INTERCEPTOR
+  mErrorInterceptor.Shutdown(mJSRuntime);
+#endif // MOZ_JS_DEV_ERROR_INTERCEPTOR
   JS_RemoveExtraGCRootsTracer(cx, TraceBlackJS, this);
   JS_RemoveExtraGCRootsTracer(cx, TraceGrayJS, this);
 #ifdef DEBUG
   mShutdownCalled = true;
 #endif
 }
 
 CycleCollectedJSRuntime::~CycleCollectedJSRuntime()
@@ -1552,8 +1564,118 @@ CycleCollectedJSRuntime::EnvironmentPrep
 CycleCollectedJSRuntime::Get()
 {
   auto context = CycleCollectedJSContext::Get();
   if (context) {
     return context->Runtime();
   }
   return nullptr;
 }
+
+#ifdef MOZ_JS_DEV_ERROR_INTERCEPTOR
+
+namespace js {
+extern void DumpValue(const JS::Value& val);
+}
+
+void
+CycleCollectedJSRuntime::ErrorInterceptor::Shutdown(JSRuntime* rt)
+{
+  JS_SetErrorInterceptorCallback(rt, nullptr);
+  mThrownError.reset();
+}
+
+/* virtual */ void
+CycleCollectedJSRuntime::ErrorInterceptor::interceptError(JSContext* cx, const JS::Value& exn)
+{
+  if (mThrownError) {
+    // We already have an error, we don't need anything more.
+    return;
+  }
+
+  if (!nsContentUtils::ThreadsafeIsSystemCaller(cx)) {
+    // We are only interested in chrome code.
+    return;
+  }
+
+  const auto type = JS_GetErrorType(exn);
+  if (!type) {
+    // This is not one of the primitive error types.
+    return;
+  }
+
+  switch (*type) {
+    case JSExnType::JSEXN_REFERENCEERR:
+    case JSExnType::JSEXN_SYNTAXERR:
+    case JSExnType::JSEXN_TYPEERR:
+      break;
+    default:
+      // Not one of the errors we are interested in.
+      return;
+  }
+
+  // Now copy the details of the exception locally.
+  // While copying the details of an exception could be expensive, in most runs,
+  // this will be done at most once during the execution of the process, so the
+  // total cost should be reasonable.
+  JS::RootedValue value(cx, exn);
+
+  ErrorDetails details;
+  details.mType = *type;
+  // If `exn` isn't an exception object, `ExtractErrorValues` could end up calling
+  // `toString()`, which could in turn end up throwing an error. While this should
+  // work, we want to avoid that complex use case.
+  // Fortunately, we have already checked above that `exn` is an exception object,
+  // so nothing such should happen.
+  nsContentUtils::ExtractErrorValues(cx, value, details.mFilename, &details.mLine, &details.mColumn, details.mMessage);
+
+  nsAutoCString stack;
+  JS::UniqueChars buf = JS::FormatStackDump(cx, nullptr, /* showArgs = */ false, /* showLocals = */ false, /* showThisProps = */ false);
+  stack.Append(buf.get());
+  CopyUTF8toUTF16(buf.get(), details.mStack);
+
+  mThrownError.emplace(Move(details));
+}
+
+void
+CycleCollectedJSRuntime::ClearRecentDevError()
+{
+  mErrorInterceptor.mThrownError.reset();
+}
+
+bool
+CycleCollectedJSRuntime::GetRecentDevError(JSContext*cx, JS::MutableHandle<JS::Value> error)
+{
+  if (!mErrorInterceptor.mThrownError) {
+    return true;
+  }
+
+  // Create a copy of the exception.
+  JS::RootedObject obj(cx, JS_NewPlainObject(cx));
+  if (!obj) {
+    return false;
+  }
+
+  JS::RootedValue message(cx);
+  JS::RootedValue filename(cx);
+  JS::RootedValue stack(cx);
+  if (!ToJSValue(cx, mErrorInterceptor.mThrownError->mMessage, &message) ||
+      !ToJSValue(cx, mErrorInterceptor.mThrownError->mFilename, &filename) ||
+      !ToJSValue(cx, mErrorInterceptor.mThrownError->mStack, &stack)) {
+    return false;
+  }
+
+  // Build the object.
+  const auto FLAGS = JSPROP_READONLY | JSPROP_ENUMERATE | JSPROP_PERMANENT;
+  if (!JS_DefineProperty(cx, obj, "message", message, FLAGS) ||
+      !JS_DefineProperty(cx, obj, "fileName", filename, FLAGS) ||
+      !JS_DefineProperty(cx, obj, "lineNumber", mErrorInterceptor.mThrownError->mLine, FLAGS) ||
+      !JS_DefineProperty(cx, obj, "stack", stack, FLAGS)) {
+    return false;
+  }
+
+  // Pass the result.
+  error.setObject(*obj);
+  return true;
+}
+#endif // MOZ_JS_DEV_ERROR_INTERCEPTOR
+
+#undef MOZ_JS_DEV_ERROR_INTERCEPTOR
--- a/xpcom/base/CycleCollectedJSRuntime.h
+++ b/xpcom/base/CycleCollectedJSRuntime.h
@@ -323,16 +323,21 @@ public:
 
   // Get the current thread's CycleCollectedJSRuntime.  Returns null if there
   // isn't one.
   static CycleCollectedJSRuntime* Get();
 
   void AddContext(CycleCollectedJSContext* aContext);
   void RemoveContext(CycleCollectedJSContext* aContext);
 
+#ifdef NIGHTLY_BUILD
+  bool GetRecentDevError(JSContext* aContext, JS::MutableHandle<JS::Value> aError);
+  void ClearRecentDevError();
+#endif // defined(NIGHTLY_BUILD)
+
 private:
   LinkedList<CycleCollectedJSContext> mContexts;
 
   JSGCThingParticipant mGCThingCycleCollectorGlobal;
 
   JSZoneParticipant mJSZoneCycleCollectorGlobal;
 
   JSRuntime* mJSRuntime;
@@ -367,16 +372,47 @@ private:
   struct EnvironmentPreparer : public js::ScriptEnvironmentPreparer {
     void invoke(JS::HandleObject scope, Closure& closure) override;
   };
   EnvironmentPreparer mEnvironmentPreparer;
 
 #ifdef DEBUG
   bool mShutdownCalled;
 #endif
+
+#ifdef NIGHTLY_BUILD
+  // Implementation of the error interceptor.
+  // Built on nightly only to avoid any possible performance impact on release
+
+  struct ErrorInterceptor final : public JSErrorInterceptor {
+    virtual void interceptError(JSContext* cx, const JS::Value& val) override;
+    void Shutdown(JSRuntime* rt);
+
+    // Copy of the details of the exception.
+    // We store this rather than the exception itself to avoid dealing with complicated
+    // garbage-collection scenarios, e.g. a JSContext being killed while we still hold
+    // onto an exception thrown from it.
+    struct ErrorDetails {
+      nsString mFilename;
+      nsString mMessage;
+      nsString mStack;
+      JSExnType mType;
+      uint32_t mLine;
+      uint32_t mColumn;
+    };
+
+    // If we have encountered at least one developer error,
+    // the first error we have encountered. Otherwise, or
+    // if we have reset since the latest error, `None`.
+    Maybe<ErrorDetails> mThrownError;
+  };
+  ErrorInterceptor mErrorInterceptor;
+
+#endif // defined(NIGHTLY_BUILD)
+
 };
 
 void TraceScriptHolder(nsISupports* aHolder, JSTracer* aTracer);
 
 // Returns true if the JS::TraceKind is one the cycle collector cares about.
 inline bool AddToCCKind(JS::TraceKind aKind)
 {
   return aKind == JS::TraceKind::Object ||