Back out bug 1172246 because of mochitest-JP bustage. Who needs tests that we care about running on try anyway?
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 20 Nov 2015 18:00:27 -0500
changeset 309651 afb8ed11f93608aa12b42a71871d656428c1b36f
parent 309650 631ec6c50d2456f30fcc941aaad09390efe3ec42
child 309652 70e1d04f94870a4e3b59e71ddaee355c0f4af88d
push id1040
push userraliiev@mozilla.com
push dateMon, 29 Feb 2016 17:11:22 +0000
treeherdermozilla-release@8c3167321162 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1172246
milestone45.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
Back out bug 1172246 because of mochitest-JP bustage. Who needs tests that we care about running on try anyway?
dom/base/ScriptSettings.cpp
dom/base/ScriptSettings.h
dom/bindings/CallbackObject.cpp
dom/bindings/CallbackObject.h
dom/bindings/test/mochitest.ini
dom/bindings/test/test_callback_exceptions.html
--- a/dom/base/ScriptSettings.cpp
+++ b/dom/base/ScriptSettings.cpp
@@ -539,35 +539,26 @@ AutoJSAPI::ReportException()
       ClearException();
     }
   } else {
     NS_WARNING("OOMed while acquiring uncaught exception from JSAPI");
   }
 }
 
 bool
-AutoJSAPI::PeekException(JS::MutableHandle<JS::Value> aVal)
-{
-  MOZ_ASSERT_IF(mIsMainThread, CxPusherIsStackTop());
-  MOZ_ASSERT(HasException());
-  MOZ_ASSERT(js::GetContextCompartment(cx()));
-  if (!JS_GetPendingException(cx(), aVal)) {
-    return false;
-  }
-  return true;
-}
-
-bool
 AutoJSAPI::StealException(JS::MutableHandle<JS::Value> aVal)
 {
-  if (!PeekException(aVal)) {
-    return false;
-  }
-  JS_ClearPendingException(cx());
-  return true;
+    MOZ_ASSERT_IF(mIsMainThread, CxPusherIsStackTop());
+    MOZ_ASSERT(HasException());
+    MOZ_ASSERT(js::GetContextCompartment(cx()));
+    if (!JS_GetPendingException(cx(), aVal)) {
+      return false;
+    }
+    JS_ClearPendingException(cx());
+    return true;
 }
 
 AutoEntryScript::AutoEntryScript(nsIGlobalObject* aGlobalObject,
                                  const char *aReason,
                                  bool aIsMainThread,
                                  JSContext* aCx)
   : AutoJSAPI(aGlobalObject, aIsMainThread,
               aCx ? aCx : FindJSContext(aGlobalObject))
--- a/dom/base/ScriptSettings.h
+++ b/dom/base/ScriptSettings.h
@@ -286,24 +286,16 @@ public:
   // 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.
   bool StealException(JS::MutableHandle<JS::Value> aVal);
 
-  // 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.
-  bool PeekException(JS::MutableHandle<JS::Value> aVal);
-
   void ClearException() {
     MOZ_ASSERT_IF(NS_IsMainThread(), CxPusherIsStackTop());
     JS_ClearPendingException(cx());
   }
 
 protected:
   // Protected constructor, allowing subclasses to specify a particular cx to
   // be used. This constructor initialises the AutoJSAPI, so Init must NOT be
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -185,18 +185,22 @@ CallbackObject::CallSetup::CallSetup(Cal
   // Note that if the callback is a wrapper, this will not be the same
   // compartment that we ended up in with mAutoEntryScript above, because the
   // entry point is based off of the unwrapped callback (realCallback).
   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.
-  mAutoEntryScript->TakeOwnershipOfErrorReporting();
+  // 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);
+  }
 }
 
 bool
 CallbackObject::CallSetup::ShouldRethrowException(JS::Handle<JS::Value> aException)
 {
   if (mExceptionHandling == eRethrowExceptions) {
     if (!mCompartment) {
       // Caller didn't ask us to filter for only exceptions we subsume.
@@ -244,63 +248,58 @@ CallbackObject::CallSetup::~CallSetup()
   // so we end up reporting them while in the compartment of our entry point,
   // not whatever cross-compartment wrappper mCallback might be.
   // Be careful: the JSAutoCompartment might not have been constructed at all!
   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 = mAutoEntryScript->HasException();
+    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 (mAutoEntryScript->PeekException(&exn) &&
+        if (JS_GetPendingException(mCx, &exn) &&
             ShouldRethrowException(exn)) {
-          mAutoEntryScript->ClearException();
-          MOZ_ASSERT(!mAutoEntryScript->HasException());
           mErrorResult.ThrowJSException(mCx, exn);
+          JS_ClearPendingException(mCx);
           needToDealWithException = false;
         }
       }
     }
 
     if (needToDealWithException) {
       // Either we're supposed to report our exceptions, or we're supposed to
-      // re-throw them but we failed to get the exception value.  Either way,
+      // re-throw them but we failed to JS_GetPendingException.  Either way,
       // just report the pending exception, if any.
       //
       // We don't use nsJSUtils::ReportPendingException here because all it
       // does at this point is JS_SaveFrameChain and enter a compartment around
       // a JS_ReportPendingException call.  But our mAutoEntryScript should
       // already do a JS_SaveFrameChain and we are already in the compartment
       // we want to be in, so all nsJSUtils::ReportPendingException would do is
       // screw up our compartment, which is exactly what we do not want.
       //
       // XXXbz FIXME: bug 979525 means we don't always JS_SaveFrameChain here,
-      // so we need to go ahead and do that.  This is also the reason we don't
-      // just rely on ~AutoJSAPI reporting the exception for us.  I think if we
-      // didn't need to JS_SaveFrameChain here, we could just rely on that.
+      // so we need to go ahead and do that.
       JS::Rooted<JSObject*> oldGlobal(mCx, JS::CurrentGlobalOrNull(mCx));
       MOZ_ASSERT(oldGlobal, "How can we not have a global here??");
       bool saved = JS_SaveFrameChain(mCx);
       // Make sure the JSAutoCompartment goes out of scope before the
       // JS_RestoreFrameChain call!
       {
         JSAutoCompartment ac(mCx, oldGlobal);
         MOZ_ASSERT(!JS::DescribeScriptedCaller(mCx),
                    "Our comment above about JS_SaveFrameChain having been "
                    "called is a lie?");
-        // Note that we don't JS_ReportPendingException here because we want to
-        // go through our AutoEntryScript's reporting mechanism instead, since
-        // it currently owns error reporting.
-        mAutoEntryScript->ReportException();
+        JS_ReportPendingException(mCx);
       }
       if (saved) {
         JS_RestoreFrameChain(mCx);
       }
     }
   }
 
   mAutoIncumbentScript.reset();
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -256,16 +256,17 @@ 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
@@ -59,15 +59,14 @@ 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]
 [test_bug1123516_maplikesetlike.html]
 skip-if = debug == false
 [test_jsimplemented_eventhandler.html]
 skip-if = debug == false
 [test_iterable.html]
 skip-if = debug == false
\ No newline at end of file
deleted file mode 100644
--- a/dom/bindings/test/test_callback_exceptions.html
+++ /dev/null
@@ -1,17 +0,0 @@
-<!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>