Bug 1172246. Make sure CallSetup's handling of exceptions it wants to deal with itself works even when the callable is a JSNative that use the JS_Report*Error APIs instead of throwing exceptions in the usual way. r=bholley
☠☠ backed out by cb6c32d220d2 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 08 Jun 2015 16:16:27 -0400
changeset 280077 a2a7009adafca29064d8f8a0ffc3fe41fbd96105
parent 280076 1db2b848699a2677386cf52f3fc9085402736ecf
child 280078 e1a0969cbc53f15269aeabfe218a38dd9ece322e
push id897
push userjlund@mozilla.com
push dateMon, 14 Sep 2015 18:56:12 +0000
treeherdermozilla-release@9411e2d2b214 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1172246
milestone41.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 1172246. Make sure CallSetup's handling of exceptions it wants to deal with itself works even when the callable is a JSNative that use the JS_Report*Error APIs instead of throwing exceptions in the usual way. r=bholley
dom/bindings/CallbackObject.cpp
dom/bindings/CallbackObject.h
dom/bindings/test/mochitest.ini
dom/bindings/test/test_callback_exceptions.html
dom/imptests/testharness.js
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -182,18 +182,17 @@ CallbackObject::CallSetup::CallSetup(Cal
   mAc.emplace(cx, *mRootedCallable);
 
   // And now we're ready to go.
   mCx = cx;
 
   // Make sure the JS engine doesn't report exceptions we want to re-throw
   if ((mCompartment && mExceptionHandling == eRethrowContentExceptions) ||
       mExceptionHandling == eRethrowExceptions) {
-    mSavedJSContextOptions = JS::ContextOptionsRef(cx);
-    JS::ContextOptionsRef(cx).setDontReportUncaught(true);
+    mAutoEntryScript->TakeOwnershipOfErrorReporting();
   }
 }
 
 bool
 CallbackObject::CallSetup::ShouldRethrowException(JS::Handle<JS::Value> aException)
 {
   if (mExceptionHandling == eRethrowExceptions) {
     if (!mCompartment) {
@@ -253,19 +252,18 @@ CallbackObject::CallSetup::~CallSetup()
   mAc.reset();
 
   // Now, if we have a JSContext, report any pending errors on it, unless we
   // were told to re-throw them.
   if (mCx) {
     bool needToDealWithException = JS_IsExceptionPending(mCx);
     if ((mCompartment && mExceptionHandling == eRethrowContentExceptions) ||
         mExceptionHandling == eRethrowExceptions) {
-      // Restore the old context options
-      JS::ContextOptionsRef(mCx) = mSavedJSContextOptions;
       mErrorResult.MightThrowJSException();
+      MOZ_ASSERT(mAutoEntryScript->OwnsErrorReporting());
       if (needToDealWithException) {
         JS::Rooted<JS::Value> exn(mCx);
         if (JS_GetPendingException(mCx, &exn) &&
             ShouldRethrowException(exn)) {
           mErrorResult.ThrowJSException(mCx, exn);
           JS_ClearPendingException(mCx);
           needToDealWithException = false;
         }
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -214,17 +214,16 @@ protected:
     // we pop the JSContext. Though in practice we'll often manually order
     // those two things.
     Maybe<JSAutoCompartment> mAc;
 
     // An ErrorResult to possibly re-throw exceptions on and whether
     // we should re-throw them.
     ErrorResult& mErrorResult;
     const ExceptionHandling mExceptionHandling;
-    JS::ContextOptions mSavedJSContextOptions;
     const bool mIsMainThread;
   };
 };
 
 template<class WebIDLCallbackT, class XPCOMCallbackT>
 class CallbackObjectHolder;
 
 template<class T, class U>
--- a/dom/bindings/test/mochitest.ini
+++ b/dom/bindings/test/mochitest.ini
@@ -56,8 +56,9 @@ skip-if = debug == false
 skip-if = debug == false
 [test_exception_options_from_jsimplemented.html]
 skip-if = debug == false
 [test_promise_rejections_from_jsimplemented.html]
 skip-if = debug == false
 [test_worker_UnwrapArg.html]
 [test_unforgeablesonexpando.html]
 [test_crossOriginWindowSymbolAccess.html]
+[test_callback_exceptions.html]
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_callback_exceptions.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Test for ...</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<script>
+promise_test(function(t) {
+  var iterator = document.createNodeIterator(document, NodeFilter.SHOW_ALL, JSON.parse);
+  return promise_rejects(t, new SyntaxError,
+                         Promise.resolve().then(iterator.nextNode.bind(iterator)));
+}, "Trying to use JSON.parse as filter should throw a catchable SyntaxError exception even when the filter is invoked async");
+
+promise_test(function(t) {
+  return promise_rejects(t, new SyntaxError, Promise.resolve('{').then(JSON.parse));
+}, "Trying to use JSON.parse as a promise callback should allow the next promise to handle the resulting exception.");
+</script>
--- a/dom/imptests/testharness.js
+++ b/dom/imptests/testharness.js
@@ -459,16 +459,22 @@ policies and contribution forms [3].
                     if (value instanceof AssertionError) {
                         throw value;
                     }
                     assert(false, "promise_test", null,
                            "Unhandled rejection with value: ${value}", {value:value});
                 }));
     }
 
+    function promise_rejects(test, expected, promise) {
+        return promise.then(test.unreached_func("Should have rejected.")).catch(function(e) {
+            assert_throws(expected, function() { throw e });
+        });
+    }
+
     /**
      * This constructor helper allows DOM events to be handled using Promises,
      * which can make it a lot easier to test a very specific series of events,
      * including ensuring that unexpected events are not fired at any point.
      */
     function EventWatcher(test, watchedNode, eventTypes)
     {
         if (typeof eventTypes == 'string') {
@@ -574,16 +580,17 @@ policies and contribution forms [3].
     function on_event(object, event, callback)
     {
         object.addEventListener(event, callback, false);
     }
 
     expose(test, 'test');
     expose(async_test, 'async_test');
     expose(promise_test, 'promise_test');
+    expose(promise_rejects, 'promise_rejects');
     expose(generate_tests, 'generate_tests');
     expose(setup, 'setup');
     expose(done, 'done');
     expose(on_event, 'on_event');
 
     /*
      * Return a string truncated to the given length, with ... added at the end
      * if it was longer.