Bug 1107592 part 2. Allow chrome JS to directly throw content DOMExceptions that will propagate out to the web script. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 02 Jan 2015 17:08:33 -0500
changeset 247739 0738d2b29e8bf59f60bfdb8f273bd8f730083e74
parent 247738 fea8c83aef600f036ee901c2a7dc9d3e5f00557b
child 247740 6a2660a3ca47f746603168592059db09a122a33d
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1107592
milestone37.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 1107592 part 2. Allow chrome JS to directly throw content DOMExceptions that will propagate out to the web script. r=peterv
dom/bindings/BindingUtils.cpp
dom/bindings/CallbackObject.cpp
dom/bindings/CallbackObject.h
dom/bindings/test/TestInterfaceJS.js
dom/bindings/test/mochitest.ini
dom/bindings/test/test_exception_options_from_jsimplemented.html
dom/webidl/TestInterfaceJS.webidl
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -29,16 +29,18 @@
 #include "xpcprivate.h"
 #include "XrayWrapper.h"
 #include "nsPrintfCString.h"
 #include "prprf.h"
 
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/DOMError.h"
 #include "mozilla/dom/DOMErrorBinding.h"
+#include "mozilla/dom/DOMException.h"
+#include "mozilla/dom/DOMExceptionBinding.h"
 #include "mozilla/dom/ElementBinding.h"
 #include "mozilla/dom/HTMLObjectElement.h"
 #include "mozilla/dom/HTMLObjectElementBinding.h"
 #include "mozilla/dom/HTMLSharedObjectElement.h"
 #include "mozilla/dom/HTMLEmbedElementBinding.h"
 #include "mozilla/dom/HTMLAppletElementBinding.h"
 #include "mozilla/dom/Promise.h"
 #include "WorkerPrivate.h"
@@ -211,33 +213,74 @@ ErrorResult::ReportJSException(JSContext
   JS::Rooted<JS::Value> exception(cx, mJSException);
   if (JS_WrapValue(cx, &exception)) {
     JS_SetPendingException(cx, exception);
   }
   mJSException = exception;
   // If JS_WrapValue failed, not much we can do about it...  No matter
   // what, go ahead and unroot mJSException.
   js::RemoveRawValueRoot(cx, &mJSException);
+
+  // We no longer have a useful exception but we do want to signal that an error
+  // occured.
+  mResult = NS_ERROR_FAILURE;
 }
 
 void
 ErrorResult::ReportJSExceptionFromJSImplementation(JSContext* aCx)
 {
   MOZ_ASSERT(!mMightHaveUnreportedJSException,
              "Why didn't you tell us you planned to handle JS exceptions?");
 
+  dom::DOMException* domException;
+  nsresult rv =
+    UNWRAP_OBJECT(DOMException, &mJSException.toObject(), domException);
+  if (NS_SUCCEEDED(rv)) {
+    // We actually have to create a new DOMException object, because the one we
+    // have has a stack that includes the chrome code that threw it, and in
+    // particular has the wrong file/line/column information.
+    nsString message;
+    domException->GetMessageMoz(message);
+    nsString name;
+    domException->GetName(name);
+    nsRefPtr<dom::DOMException> newException =
+      new dom::DOMException(nsresult(domException->Result()),
+                            NS_ConvertUTF16toUTF8(message),
+                            NS_ConvertUTF16toUTF8(name),
+                            domException->Code());
+    JS::Rooted<JS::Value> reflector(aCx);
+    if (!GetOrCreateDOMReflector(aCx, newException, &reflector)) {
+      // Well, that threw _an_ exception.  Let's forget ours.  We can just
+      // unroot and not change the value, since mJSException is completely
+      // ignored if mResult is not NS_ERROR_DOM_JS_EXCEPTION and we plan to
+      // change mResult to a different value.
+      js::RemoveRawValueRoot(aCx, &mJSException);
+
+      // We no longer have a useful exception but we do want to signal that an
+      // error occured.
+      mResult = NS_ERROR_FAILURE;
+
+      // But do make sure to not ReportJSException here, since we don't have one.
+      return;
+    }
+
+    mJSException = reflector;
+    ReportJSException(aCx);
+    return;
+  }
+
   dom::DOMError* domError;
-  nsresult rv = UNWRAP_OBJECT(DOMError, &mJSException.toObject(), domError);
+  rv = UNWRAP_OBJECT(DOMError, &mJSException.toObject(), domError);
   if (NS_FAILED(rv)) {
-    // Unwrapping really shouldn't fail here, if mExceptionHandling is set to
+    // Unwrapping really shouldn't fail here: if mExceptionHandling is set to
     // eRethrowContentExceptions then the CallSetup destructor only stores an
-    // exception if it unwraps to DOMError. If we reach this then either
-    // mExceptionHandling wasn't set to eRethrowContentExceptions and we
-    // shouldn't be calling ReportJSExceptionFromJSImplementation or something
-    // went really wrong.
+    // exception if it unwraps to DOMError or DOMException. If we reach this
+    // then either mExceptionHandling wasn't set to eRethrowContentExceptions
+    // and we shouldn't be calling ReportJSExceptionFromJSImplementation or
+    // something went really wrong.
     NS_RUNTIMEABORT("We stored a non-DOMError exception!");
   }
 
   nsString message;
   domError->GetMessage(message);
 
   JS_ReportError(aCx, "%hs", message.get());
   js::RemoveRawValueRoot(aCx, &mJSException);
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -3,16 +3,18 @@
 /* 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/. */
 
 #include "mozilla/dom/CallbackObject.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/DOMError.h"
 #include "mozilla/dom/DOMErrorBinding.h"
+#include "mozilla/dom/DOMException.h"
+#include "mozilla/dom/DOMExceptionBinding.h"
 #include "jsfriendapi.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsIXPConnect.h"
 #include "nsIScriptContext.h"
 #include "nsPIDOMWindow.h"
 #include "nsJSUtils.h"
 #include "nsIScriptSecurityManager.h"
 #include "xpcprivate.h"
@@ -185,31 +187,33 @@ CallbackObject::CallSetup::ShouldRethrow
 {
   if (mExceptionHandling == eRethrowExceptions) {
     return true;
   }
 
   MOZ_ASSERT(mExceptionHandling == eRethrowContentExceptions);
 
   // For eRethrowContentExceptions we only want to throw an exception if the
-  // object that was thrown is a DOMError object in the caller compartment
-  // (which we stored in mCompartment).
+  // object that was thrown is a DOMError or DOMException object in the caller
+  // compartment (which we stored in mCompartment).
 
   if (!aException.isObject()) {
     return false;
   }
 
   JS::Rooted<JSObject*> obj(mCx, &aException.toObject());
   obj = js::UncheckedUnwrap(obj, /* stopAtOuter = */ false);
   if (js::GetObjectCompartment(obj) != mCompartment) {
     return false;
   }
 
   DOMError* domError;
-  return NS_SUCCEEDED(UNWRAP_OBJECT(DOMError, obj, domError));
+  DOMException* domException;
+  return NS_SUCCEEDED(UNWRAP_OBJECT(DOMError, obj, domError)) ||
+         NS_SUCCEEDED(UNWRAP_OBJECT(DOMException, obj, domException));
 }
 
 CallbackObject::CallSetup::~CallSetup()
 {
   // To get our nesting right we have to destroy our JSAutoCompartment first.
   // In particular, we want to do this before we try reporting any exceptions,
   // so we end up reporting them while in the compartment of our entry point,
   // not whatever cross-compartment wrappper mCallback might be.
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -80,18 +80,18 @@ public:
   {
     return mIncumbentGlobal;
   }
 
   enum ExceptionHandling {
     // Report any exception and don't throw it to the caller code.
     eReportExceptions,
     // Throw an exception to the caller code if the thrown exception is a
-    // binding object for a DOMError from the caller's scope, otherwise report
-    // it.
+    // binding object for a DOMError or DOMException from the caller's scope,
+    // otherwise report it.
     eRethrowContentExceptions,
     // Throw any exception to the caller code.
     eRethrowExceptions
   };
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
   {
     return aMallocSizeOf(this);
--- a/dom/bindings/test/TestInterfaceJS.js
+++ b/dom/bindings/test/TestInterfaceJS.js
@@ -9,17 +9,20 @@ const Ci = Components.interfaces;
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
 function TestInterfaceJS(anyArg, objectArg) {}
 
 TestInterfaceJS.prototype = {
   classID: Components.ID("{2ac4e026-cf25-47d5-b067-78d553c3cad8}"),
   contractID: "@mozilla.org/dom/test-interface-js;1",
-  QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports]),
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports,
+                                         Ci.nsIDOMGlobalPropertyInitializer]),
+
+  init: function(win) { this._win = win; },
 
   __init: function (anyArg, objectArg, dictionaryArg) {
     this._anyAttr = undefined;
     this._objectAttr = null;
     this._anyArg = anyArg;
     this._objectArg = objectArg;
     this._dictionaryArg = dictionaryArg;
     this._cachedAttr = 15;
@@ -54,11 +57,20 @@ TestInterfaceJS.prototype = {
   returnBadUnion: function(x) { return 3; },
 
   get cachedAttr() { return this._cachedAttr; },
   setCachedAttr: function(n) { this._cachedAttr = n; },
   clearCachedAttrCache: function () { this.__DOM_IMPL__._clearCachedCachedAttrValue(); },
 
   testSequenceOverload: function(arg) {},
   testSequenceUnion: function(arg) {},
+
+  testThrowDOMError: function() {
+    throw new this._win.DOMError("NotSupportedError", "We are a DOMError");
+  },
+
+  testThrowDOMException: function() {
+    throw new this._win.DOMException("We are a DOMException",
+                                     "NotSupportedError");
+  },
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([TestInterfaceJS])
--- a/dom/bindings/test/mochitest.ini
+++ b/dom/bindings/test/mochitest.ini
@@ -48,8 +48,10 @@ skip-if = debug == false
 skip-if = debug == false
 [test_sequence_wrapping.html]
 [test_setWithNamedGetterNoNamedSetter.html]
 [test_throwing_method_noDCE.html]
 [test_treat_non_object_as_null.html]
 [test_traceProtos.html]
 [test_sequence_detection.html]
 skip-if = debug == false
+[test_exception_options_from_jsimplemented.html]
+skip-if = debug == false
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_exception_options_from_jsimplemented.html
@@ -0,0 +1,73 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1107592
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1107592</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 1107592 **/
+
+  SimpleTest.waitForExplicitFinish();
+
+  function doTest() {
+    var t = new TestInterfaceJS();
+    try {
+      t.testThrowDOMError();
+    } catch (e) {
+      ok(e instanceof Error, "Should have an Error here");
+      ok(!(e instanceof DOMException), "Should not have DOMException here");
+      ok(!("code" in e), "Should not have a 'code' property");
+      ise(e.name, "Error", "Should not have an interesting name here");
+      ise(e.message, "We are a DOMError", "Should have the right message");
+      ise(e.stack,
+          "doTest@http://mochi.test:8888/tests/dom/bindings/test/test_exception_options_from_jsimplemented.html:20:7\n",
+          "Exception stack should still only show our code");
+      ise(e.fileName,
+          "http://mochi.test:8888/tests/dom/bindings/test/test_exception_options_from_jsimplemented.html",
+          "Should have the right file name");
+      ise(e.lineNumber, 20, "Should have the right line number");
+      ise(e.columnNumber, 6, "Should have the right column number");
+    }
+
+    try {
+      t.testThrowDOMException();
+    } catch (e) {
+      ok(e instanceof Error, "Should also have an Error here");
+      ok(e instanceof DOMException, "Should have DOMException here");
+      ise(e.name, "NotSupportedError", "Should have the right name here");
+      ise(e.message, "We are a DOMException",
+          "Should also have the right message");
+      ise(e.code, DOMException.NOT_SUPPORTED_ERR,
+          "Should have the right 'code'");
+      ise(e.stack,
+          "doTest@http://mochi.test:8888/tests/dom/bindings/test/test_exception_options_from_jsimplemented.html:38:6\n",
+          "Exception stack should still only show our code");
+      ise(e.filename,
+          "http://mochi.test:8888/tests/dom/bindings/test/test_exception_options_from_jsimplemented.html",
+          "Should still have the right file name");
+      ise(e.lineNumber, 38, "Should still have the right line number");
+      todo_is(e.columnNumber, 6,
+              "No column number support for DOMException yet");
+    }
+    SimpleTest.finish();
+  }
+
+  SpecialPowers.pushPrefEnv({set: [['dom.expose_test_interfaces', true]]},
+                            doTest);
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1107592">Mozilla Bug 1107592</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>
--- a/dom/webidl/TestInterfaceJS.webidl
+++ b/dom/webidl/TestInterfaceJS.webidl
@@ -43,9 +43,16 @@ interface TestInterfaceJS {
   void setCachedAttr(short n);
   void clearCachedAttrCache();
 
   // Test for sequence overloading and union behavior
   void testSequenceOverload(sequence<DOMString> arg);
   void testSequenceOverload(DOMString arg);
 
   void testSequenceUnion((sequence<DOMString> or DOMString) arg);
+
+  // Tests for exception-throwing behavior
+  [Throws]
+  void testThrowDOMError();
+
+  [Throws]
+  void testThrowDOMException();
 };