Bug 1224007 part 5. Get rid of ErrorResult::StealJSException. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 20 Nov 2015 16:29:41 -0500
changeset 307793 e597563f32ff62d4fb5ae612cebf93a630b20621
parent 307792 169d9adca23f59e0a0e4ce5ce57b572e0e4cde11
child 307794 ecb3051bba081b246f8504d23ab568617e9248c5
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1224007
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
Bug 1224007 part 5. Get rid of ErrorResult::StealJSException. r=peterv
dom/bindings/BindingUtils.cpp
dom/bindings/ErrorResult.h
dom/fetch/Fetch.cpp
dom/promise/Promise.cpp
dom/promise/PromiseCallback.cpp
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -266,33 +266,16 @@ ErrorResult::SetPendingJSException(JSCon
   // We no longer have a useful exception but we do want to signal that an error
   // occured.
   mResult = NS_ERROR_FAILURE;
 #ifdef DEBUG
   mUnionState = HasNothing;
 #endif // DEBUG
 }
 
-void
-ErrorResult::StealJSException(JSContext* cx,
-                              JS::MutableHandle<JS::Value> value)
-{
-  MOZ_ASSERT(!mMightHaveUnreportedJSException,
-             "Must call WouldReportJSException unconditionally in all codepaths that might call StealJSException");
-  MOZ_ASSERT(IsJSException(), "No exception to steal");
-  MOZ_ASSERT(mUnionState == HasJSException);
-
-  value.set(mJSException);
-  js::RemoveRawValueRoot(cx, &mJSException);
-  mResult = NS_OK;
-#ifdef DEBUG
-  mUnionState = HasNothing;
-#endif // DEBUG
-}
-
 struct ErrorResult::DOMExceptionInfo {
   DOMExceptionInfo(nsresult rv, const nsACString& message)
     : mMessage(message)
     , mRv(rv)
   {}
 
   nsCString mMessage;
   nsresult mRv;
--- a/dom/bindings/ErrorResult.h
+++ b/dom/bindings/ErrorResult.h
@@ -183,18 +183,19 @@ public:
                                        Forward<Ts>(messageArgs)...);
   }
 
   bool IsErrorWithMessage() const { return ErrorCode() == NS_ERROR_TYPE_ERR || ErrorCode() == NS_ERROR_RANGE_ERR; }
 
   // Facilities for throwing a preexisting JS exception value via this
   // ErrorResult.  The contract is that any code which might end up calling
   // ThrowJSException() must call MightThrowJSException() even if no exception
-  // is being thrown.  Code that would call StealJSException as needed must
-  // first call WouldReportJSException even if this ErrorResult has not failed.
+  // is being thrown.  Code that conditionally calls ToJSValue on this
+  // ErrorResult only if Failed() must first call WouldReportJSException even if
+  // this ErrorResult has not failed.
   //
   // The exn argument to ThrowJSException can be in any compartment.  It does
   // not have to be in the compartment of cx.  If someone later uses it, they
   // will wrap it into whatever compartment they're working in, as needed.
   void ThrowJSException(JSContext* cx, JS::Handle<JS::Value> exn);
   bool IsJSException() const { return ErrorCode() == NS_ERROR_DOM_JS_EXCEPTION; }
 
   // Facilities for throwing a DOMException.  If an empty message string is
@@ -219,21 +220,16 @@ public:
   // Support for uncatchable exceptions.
   void ThrowUncatchableException() {
     Throw(NS_ERROR_UNCATCHABLE_EXCEPTION);
   }
   bool IsUncatchableException() const {
     return ErrorCode() == NS_ERROR_UNCATCHABLE_EXCEPTION;
   }
 
-  // StealJSException steals the JS Exception from the object. This method must
-  // be called only if IsJSException() returns true. This method also resets the
-  // error code to NS_OK.
-  void StealJSException(JSContext* cx, JS::MutableHandle<JS::Value> value);
-
   void MOZ_ALWAYS_INLINE MightThrowJSException()
   {
 #ifdef DEBUG
     mMightHaveUnreportedJSException = true;
 #endif
   }
   void MOZ_ALWAYS_INLINE WouldReportJSException()
   {
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -1059,23 +1059,17 @@ FetchBody<Derived>::ContinueConsumeBody(
       break;
     }
     default:
       NS_NOTREACHED("Unexpected consume body type");
   }
 
   error.WouldReportJSException();
   if (error.Failed()) {
-    if (error.IsJSException()) {
-      JS::Rooted<JS::Value> exn(cx);
-      error.StealJSException(cx, &exn);
-      localPromise->MaybeReject(cx, exn);
-    } else {
-      localPromise->MaybeReject(error);
-    }
+    localPromise->MaybeReject(error);
   }
 }
 
 template <class Derived>
 already_AddRefed<Promise>
 FetchBody<Derived>::ConsumeBody(ConsumeType aType, ErrorResult& aRv)
 {
   mConsumeType = aType;
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -233,19 +233,18 @@ protected:
 
     mThen->Call(rootedThenable, resolveFunc, rejectFunc, rv,
                 "promise thenable", CallbackObject::eRethrowExceptions,
                 mPromise->Compartment());
 
     rv.WouldReportJSException();
     if (rv.Failed()) {
       JS::Rooted<JS::Value> exn(cx);
-      if (rv.IsJSException()) {
-        rv.StealJSException(cx, &exn);
-      } else {
+      { // Scope for JSAutoCompartment
+
         // Convert the ErrorResult to a JS exception object that we can reject
         // ourselves with.  This will be exactly the exception that would get
         // thrown from a binding method whose ErrorResult ended up with
         // whatever is on "rv" right now.
         JSAutoCompartment ac(cx, mPromise->GlobalJSObject());
         DebugOnly<bool> conversionResult = ToJSValue(cx, rv, &exn);
         MOZ_ASSERT(conversionResult);
       }
@@ -691,35 +690,29 @@ Promise::CallInitFunction(const GlobalOb
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
 
   aInit.Call(resolveFunc, rejectFunc, aRv, "promise initializer",
              CallbackObject::eRethrowExceptions, Compartment());
   aRv.WouldReportJSException();
 
-  if (aRv.IsJSException()) {
-    JS::Rooted<JS::Value> value(cx);
-    aRv.StealJSException(cx, &value);
-
-    // we want the same behavior as this JS implementation:
+  if (aRv.Failed()) {
+    // We want the same behavior as this JS implementation:
+    //
     // function Promise(arg) { try { arg(a, b); } catch (e) { this.reject(e); }}
-    if (!JS_WrapValue(cx, &value)) {
-      aRv.Throw(NS_ERROR_UNEXPECTED);
-      return;
-    }
-
+    //
+    // In particular, that means not using MaybeReject(aRv) here, since that
+    // would create the exception object in our reflector compartment, while we
+    // want to create it in whatever the current compartment on cx is.
+    JS::Rooted<JS::Value> value(cx);
+    DebugOnly<bool> conversionResult = ToJSValue(cx, aRv, &value);
+    MOZ_ASSERT(conversionResult);
     MaybeRejectInternal(cx, value);
   }
-
-  // Else aRv is an error.  We _could_ reject ourselves with that error, but
-  // we're just going to propagate aRv out to the binding code, which will then
-  // throw us away and create a new promise rejected with the error on aRv.  So
-  // there's no need to worry about rejecting ourselves here; the bindings
-  // will do the right thing.
 }
 
 /* static */ already_AddRefed<Promise>
 Promise::Resolve(const GlobalObject& aGlobal,
                  JS::Handle<JS::Value> aValue, ErrorResult& aRv)
 {
   // If a Promise was passed, just return it.
   if (aValue.isObject()) {
--- a/dom/promise/PromiseCallback.cpp
+++ b/dom/promise/PromiseCallback.cpp
@@ -220,24 +220,17 @@ WrapperPromiseCallback::Call(JSContext* 
                   CallbackObject::eRethrowExceptions,
                   mNextPromise->Compartment());
 
   rv.WouldReportJSException();
 
   // PromiseReactionTask step 7
   if (rv.Failed()) {
     JS::Rooted<JS::Value> value(aCx);
-    if (rv.IsJSException()) {
-      rv.StealJSException(aCx, &value);
-
-      if (!JS_WrapValue(aCx, &value)) {
-        NS_WARNING("Failed to wrap value into the right compartment.");
-        return NS_ERROR_FAILURE;
-      }
-    } else {
+    { // Scope for JSAutoCompartment
       // Convert the ErrorResult to a JS exception object that we can reject
       // ourselves with.  This will be exactly the exception that would get
       // thrown from a binding method whose ErrorResult ended up with whatever
       // is on "rv" right now.
       JSAutoCompartment ac(aCx, mNextPromise->GlobalJSObject());
       DebugOnly<bool> conversionResult = ToJSValue(aCx, rv, &value);
       MOZ_ASSERT(conversionResult);
     }