Bug 933378 part 1. Introduce a TErrorResult class that will serve as a base class for various ErrorResult-like subclasses. No actual behavior changes so far. r=bkelly

dom/bindings/BindingUtils.cpp

@@ -143,9 +143,12 @@ ThrowNoSetterArg(JSContext* aCx, prototypes::ID aProtoId)
 
 } // namespace dom
 
-struct ErrorResult::Message {
-  Message() { MOZ_COUNT_CTOR(ErrorResult::Message); }
-  ~Message() { MOZ_COUNT_DTOR(ErrorResult::Message); }
+namespace binding_danger {
+
+template<typename CleanupPolicy>
+struct TErrorResult<CleanupPolicy>::Message {
+  Message() { MOZ_COUNT_CTOR(TErrorResult::Message); }
+  ~Message() { MOZ_COUNT_DTOR(TErrorResult::Message); }
 
   nsTArray<nsString> mArgs;
   dom::ErrNum mErrorNumber;
@@ -156,8 +159,10 @@ struct ErrorResult::Message {
   }
 };
 
+template<typename CleanupPolicy>
 nsTArray<nsString>&
-ErrorResult::CreateErrorMessageHelper(const dom::ErrNum errorNumber, nsresult errorType)
+TErrorResult<CleanupPolicy>::CreateErrorMessageHelper(const dom::ErrNum errorNumber,
+                                                      nsresult errorType)
 {
   AssertInOwningThread();
   mResult = errorType;
@@ -167,8 +172,9 @@ ErrorResult::CreateErrorMessageHelper(const dom::ErrNum errorNumber, nsresult er
   return mMessage->mArgs;
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SerializeMessage(IPC::Message* aMsg) const
+TErrorResult<CleanupPolicy>::SerializeMessage(IPC::Message* aMsg) const
 {
   using namespace IPC;
   AssertInOwningThread();
@@ -178,8 +184,10 @@ ErrorResult::SerializeMessage(IPC::Message* aMsg) const
   WriteParam(aMsg, mMessage->mErrorNumber);
 }
 
+template<typename CleanupPolicy>
 bool
-ErrorResult::DeserializeMessage(const IPC::Message* aMsg, PickleIterator* aIter)
+TErrorResult<CleanupPolicy>::DeserializeMessage(const IPC::Message* aMsg,
+                                                PickleIterator* aIter)
 {
   using namespace IPC;
   AssertInOwningThread();
@@ -200,8 +208,9 @@ ErrorResult::DeserializeMessage(const IPC::Message* aMsg, PickleIterator* aIter)
   return true;
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SetPendingExceptionWithMessage(JSContext* aCx)
+TErrorResult<CleanupPolicy>::SetPendingExceptionWithMessage(JSContext* aCx)
 {
   AssertInOwningThread();
   MOZ_ASSERT(mMessage, "SetPendingExceptionWithMessage() can be called only once");
@@ -224,8 +233,9 @@ ErrorResult::SetPendingExceptionWithMessage(JSContext* aCx)
   mResult = NS_OK;
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::ClearMessage()
+TErrorResult<CleanupPolicy>::ClearMessage()
 {
   AssertInOwningThread();
   MOZ_ASSERT(IsErrorWithMessage());
@@ -236,8 +246,9 @@ ErrorResult::ClearMessage()
 #endif // DEBUG
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::ThrowJSException(JSContext* cx, JS::Handle<JS::Value> exn)
+TErrorResult<CleanupPolicy>::ThrowJSException(JSContext* cx, JS::Handle<JS::Value> exn)
 {
   AssertInOwningThread();
   MOZ_ASSERT(mMightHaveUnreportedJSException,
@@ -249,7 +260,7 @@ ErrorResult::ThrowJSException(JSContext* cx, JS::Handle<JS::Value> exn)
   // don't set it to exn yet, because we don't want to do that until after we
   // root.
   mJSException.setUndefined();
-  if (!js::AddRawValueRoot(cx, &mJSException, "ErrorResult::mJSException")) {
+  if (!js::AddRawValueRoot(cx, &mJSException, "TErrorResult::mJSException")) {
     // Don't use NS_ERROR_DOM_JS_EXCEPTION, because that indicates we have
     // in fact rooted mJSException.
     mResult = NS_ERROR_OUT_OF_MEMORY;
@@ -262,8 +273,9 @@ ErrorResult::ThrowJSException(JSContext* cx, JS::Handle<JS::Value> exn)
   }
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SetPendingJSException(JSContext* cx)
+TErrorResult<CleanupPolicy>::SetPendingJSException(JSContext* cx)
 {
   AssertInOwningThread();
   MOZ_ASSERT(!mMightHaveUnreportedJSException,
@@ -285,7 +297,8 @@ ErrorResult::SetPendingJSException(JSContext* cx)
 #endif // DEBUG
 }
 
-struct ErrorResult::DOMExceptionInfo {
+template<typename CleanupPolicy>
+struct TErrorResult<CleanupPolicy>::DOMExceptionInfo {
   DOMExceptionInfo(nsresult rv, const nsACString& message)
     : mMessage(message)
     , mRv(rv)
@@ -295,8 +308,9 @@ struct ErrorResult::DOMExceptionInfo {
   nsresult mRv;
 };
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SerializeDOMExceptionInfo(IPC::Message* aMsg) const
+TErrorResult<CleanupPolicy>::SerializeDOMExceptionInfo(IPC::Message* aMsg) const
 {
   using namespace IPC;
   AssertInOwningThread();
@@ -306,8 +320,10 @@ ErrorResult::SerializeDOMExceptionInfo(IPC::Message* aMsg) const
   WriteParam(aMsg, mDOMExceptionInfo->mRv);
 }
 
+template<typename CleanupPolicy>
 bool
-ErrorResult::DeserializeDOMExceptionInfo(const IPC::Message* aMsg, PickleIterator* aIter)
+TErrorResult<CleanupPolicy>::DeserializeDOMExceptionInfo(const IPC::Message* aMsg,
+                                                         PickleIterator* aIter)
 {
   using namespace IPC;
   AssertInOwningThread();
@@ -327,8 +343,10 @@ ErrorResult::DeserializeDOMExceptionInfo(const IPC::Message* aMsg, PickleIterato
   return true;
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::ThrowDOMException(nsresult rv, const nsACString& message)
+TErrorResult<CleanupPolicy>::ThrowDOMException(nsresult rv,
+                                               const nsACString& message)
 {
   AssertInOwningThread();
   ClearUnionData();
@@ -340,8 +358,9 @@ ErrorResult::ThrowDOMException(nsresult rv, const nsACString& message)
 #endif
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SetPendingDOMException(JSContext* cx)
+TErrorResult<CleanupPolicy>::SetPendingDOMException(JSContext* cx)
 {
   AssertInOwningThread();
   MOZ_ASSERT(mDOMExceptionInfo,
@@ -354,8 +373,9 @@ ErrorResult::SetPendingDOMException(JSContext* cx)
   mResult = NS_OK;
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::ClearDOMExceptionInfo()
+TErrorResult<CleanupPolicy>::ClearDOMExceptionInfo()
 {
   AssertInOwningThread();
   MOZ_ASSERT(IsDOMException());
@@ -367,8 +387,9 @@ ErrorResult::ClearDOMExceptionInfo()
 #endif // DEBUG
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::ClearUnionData()
+TErrorResult<CleanupPolicy>::ClearUnionData()
 {
   AssertInOwningThread();
   if (IsJSException()) {
@@ -386,8 +407,9 @@ ErrorResult::ClearUnionData()
   }
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SetPendingGenericErrorException(JSContext* cx)
+TErrorResult<CleanupPolicy>::SetPendingGenericErrorException(JSContext* cx)
 {
   AssertInOwningThread();
   MOZ_ASSERT(!IsErrorWithMessage());
@@ -397,8 +419,9 @@ ErrorResult::SetPendingGenericErrorException(JSContext* cx)
   mResult = NS_OK;
 }
 
-ErrorResult&
-ErrorResult::operator=(ErrorResult&& aRHS)
+template<typename CleanupPolicy>
+TErrorResult<CleanupPolicy>&
+TErrorResult<CleanupPolicy>::operator=(TErrorResult<CleanupPolicy>&& aRHS)
 {
   AssertInOwningThread();
   aRHS.AssertInOwningThread();
@@ -417,7 +440,7 @@ ErrorResult::operator=(ErrorResult&& aRHS)
     JSContext* cx = nsContentUtils::RootingCx();
     MOZ_ASSERT(cx);
     mJSException.setUndefined();
-    if (!js::AddRawValueRoot(cx, &mJSException, "ErrorResult::mJSException")) {
+    if (!js::AddRawValueRoot(cx, &mJSException, "TErrorResult::mJSException")) {
       MOZ_CRASH("Could not root mJSException, we're about to OOM");
     }
     mJSException = aRHS.mJSException;
@@ -443,8 +466,9 @@ ErrorResult::operator=(ErrorResult&& aRHS)
   return *this;
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::CloneTo(ErrorResult& aRv) const
+TErrorResult<CleanupPolicy>::CloneTo(TErrorResult& aRv) const
 {
   AssertInOwningThread();
   aRv.AssertInOwningThread();
@@ -478,8 +502,9 @@ ErrorResult::CloneTo(ErrorResult& aRv) const
   }
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SuppressException()
+TErrorResult<CleanupPolicy>::SuppressException()
 {
   AssertInOwningThread();
   WouldReportJSException();
@@ -489,8 +514,9 @@ ErrorResult::SuppressException()
   mResult = NS_OK;
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::SetPendingException(JSContext* cx)
+TErrorResult<CleanupPolicy>::SetPendingException(JSContext* cx)
 {
   AssertInOwningThread();
   if (IsUncatchableException()) {
@@ -522,8 +548,9 @@ ErrorResult::SetPendingException(JSContext* cx)
   SetPendingGenericErrorException(cx);
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::StealExceptionFromJSContext(JSContext* cx)
+TErrorResult<CleanupPolicy>::StealExceptionFromJSContext(JSContext* cx)
 {
   AssertInOwningThread();
   MOZ_ASSERT(mMightHaveUnreportedJSException,
@@ -539,8 +566,9 @@ ErrorResult::StealExceptionFromJSContext(JSContext* cx)
   JS_ClearPendingException(cx);
 }
 
+template<typename CleanupPolicy>
 void
-ErrorResult::NoteJSContextException(JSContext* aCx)
+TErrorResult<CleanupPolicy>::NoteJSContextException(JSContext* aCx)
 {
   AssertInOwningThread();
   if (JS_IsExceptionPending(aCx)) {
@@ -550,6 +578,10 @@ ErrorResult::NoteJSContextException(JSContext* aCx)
   }
 }
 
+template class TErrorResult<JustAssertCleanupPolicy>;
+
+} // namespace binding_danger
+
 namespace dom {
 
 bool

dom/bindings/ErrorResult.h

@@ -5,9 +5,10 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /**
- * A struct for tracking exceptions that need to be thrown to JS.
+ * A set of structs for tracking exceptions that need to be thrown to JS:
+ * ErrorResult and IgnoredErrorResult.
  *
- * Conceptually, an ErrorResult represents either success or an exception in the
+ * Conceptually, these structs represent either success or an exception in the
  * process of being thrown.  This means that a failing ErrorResult _must_ be
  * handled in one of the following ways before coming off the stack:
  *
@@ -17,6 +18,8 @@
  *    MaybeSetPendingException.
  * 4) Converted to an exception JS::Value (probably to then reject a Promise
  *    with) via dom::ToJSValue.
+ *
+ * An IgnoredErrorResult will automatically do the first of those four things.
  */
 
 #ifndef mozilla_ErrorResult_h
@@ -88,9 +91,25 @@ struct StringArrayAppender
 
 } // namespace dom
 
-class ErrorResult {
+class ErrorResult;
+
+namespace binding_danger {
+
+/**
+ * Templated implementation class for various ErrorResult-like things.  The
+ * instantiations differ only in terms of their cleanup policies (used in the
+ * destructor), which they can specify via the template argument.  Note that
+ * this means it's safe to reinterpret_cast between the instantiations unless
+ * you plan to invoke the destructor through such a cast pointer.
+ *
+ * A cleanup policy consists of two booleans: whether to assert that we've been
+ * reported or suppressed, and whether to then go ahead and suppress the
+ * exception.
+ */
+template<typename CleanupPolicy>
+class TErrorResult {
 public:
-  ErrorResult()
+  TErrorResult()
     : mResult(NS_OK)
 #ifdef DEBUG
     , mMightHaveUnreportedJSException(false)
@@ -99,29 +118,35 @@ public:
   {
   }
 
-#ifdef DEBUG
-  ~ErrorResult() {
-    // Consumers should have called one of MaybeSetPendingException
-    // (possibly via ToJSValue), StealNSResult, and SuppressException
-    MOZ_ASSERT(!Failed());
-    MOZ_ASSERT(!mMightHaveUnreportedJSException);
-    MOZ_ASSERT(mUnionState == HasNothing);
-    NS_ASSERT_OWNINGTHREAD(ErrorResult);
-  }
-#endif // DEBUG
+  ~TErrorResult() {
+    NS_ASSERT_OWNINGTHREAD(TErrorResult);
 
-  ErrorResult(ErrorResult&& aRHS)
+    if (CleanupPolicy::assertHandled) {
+      // Consumers should have called one of MaybeSetPendingException
+      // (possibly via ToJSValue), StealNSResult, and SuppressException
+      AssertReportedOrSuppressed();
+    }
+
+    if (CleanupPolicy::suppress) {
+      SuppressException();
+    }
+
+    // And now assert that we're in a good final state.
+    AssertReportedOrSuppressed();
+  }
+
+  TErrorResult(TErrorResult&& aRHS)
     // Initialize mResult and whatever else we need to default-initialize, so
     // the ClearUnionData call in our operator= will do the right thing
     // (nothing).
-    : ErrorResult()
+    : TErrorResult()
   {
     *this = Move(aRHS);
   }
-  ErrorResult& operator=(ErrorResult&& aRHS);
+  TErrorResult& operator=(TErrorResult&& aRHS);
 
-  explicit ErrorResult(nsresult aRv)
-    : ErrorResult()
+  explicit TErrorResult(nsresult aRv)
+    : TErrorResult()
   {
     AssignErrorCode(aRv);
   }
@@ -131,18 +156,18 @@ public:
     AssignErrorCode(rv);
   }
 
-  // Duplicate our current state on the given ErrorResult object.  Any existing
-  // errors or messages on the target will be suppressed before cloning.  Our
-  // own error state remains unchanged.
-  void CloneTo(ErrorResult& aRv) const;
+  // Duplicate our current state on the given TErrorResult object.  Any
+  // existing errors or messages on the target will be suppressed before
+  // cloning.  Our own error state remains unchanged.
+  void CloneTo(TErrorResult& aRv) const;
 
   // Use SuppressException when you want to suppress any exception that might be
-  // on the ErrorResult.  After this call, the ErrorResult will be back a "no
+  // on the TErrorResult.  After this call, the TErrorResult will be back a "no
   // exception thrown" state.
   void SuppressException();
 
-  // Use StealNSResult() when you want to safely convert the ErrorResult to an
-  // nsresult that you will then return to a caller.  This will
+  // Use StealNSResult() when you want to safely convert the TErrorResult to
+  // an nsresult that you will then return to a caller.  This will
   // SuppressException(), since there will no longer be a way to report it.
   nsresult StealNSResult() {
     nsresult rv = ErrorCode();
@@ -150,11 +175,11 @@ public:
     return rv;
   }
 
-  // Use MaybeSetPendingException to convert an ErrorResult to a pending
+  // Use MaybeSetPendingException to convert a TErrorResult to a pending
   // exception on the given JSContext.  This is the normal "throw an exception"
   // codepath.
   //
-  // The return value is false if the ErrorResult represents success, true
+  // The return value is false if the TErrorResult represents success, true
   // otherwise.  This does mean that in JSAPI method implementations you can't
   // just use this as |return rv.MaybeSetPendingException(cx)| (though you could
   // |return !rv.MaybeSetPendingException(cx)|), but in practice pretty much any
@@ -175,7 +200,7 @@ public:
   // considered equivalent to a JSAPI failure in terms of what callers should do
   // after true is returned.
   //
-  // After this call, the ErrorResult will no longer return true from Failed(),
+  // After this call, the TErrorResult will no longer return true from Failed(),
   // since the exception will have moved to the JSContext.
   bool MaybeSetPendingException(JSContext* cx)
   {
@@ -189,7 +214,7 @@ public:
   }
 
   // Use StealExceptionFromJSContext to convert a pending exception on a
-  // JSContext to an ErrorResult.  This function must be called only when a
+  // JSContext to a TErrorResult.  This function must be called only when a
   // JSAPI operation failed.  It assumes that lack of pending exception on the
   // JSContext means an uncatchable exception was thrown.
   //
@@ -217,11 +242,11 @@ public:
   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
+  // TErrorResult.  The contract is that any code which might end up calling
   // ThrowJSException() or StealExceptionFromJSContext() must call
   // MightThrowJSException() even if no exception 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.
+  // conditionally calls ToJSValue on this TErrorResult only if Failed() must
+  // first call WouldReportJSException even if this TErrorResult 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
@@ -237,12 +262,12 @@ public:
   void ThrowDOMException(nsresult rv, const nsACString& message = EmptyCString());
   bool IsDOMException() const { return ErrorCode() == NS_ERROR_DOM_DOMEXCEPTION; }
 
-  // Flag on the ErrorResult that whatever needs throwing has been
+  // Flag on the TErrorResult that whatever needs throwing has been
   // thrown on the JSContext already and we should not mess with it.
   // If nothing was thrown, this becomes an uncatchable exception.
   void NoteJSContextException(JSContext* aCx);
 
-  // Check whether the ErrorResult says to just throw whatever is on
+  // Check whether the TErrorResult says to just throw whatever is on
   // the JSContext already.
   bool IsJSContextException() {
     return ErrorCode() == NS_ERROR_DOM_EXCEPTION_ON_JSCONTEXT;
@@ -308,6 +333,7 @@ private:
   };
 #endif // DEBUG
 
+  friend struct IPC::ParamTraits<TErrorResult>;
   friend struct IPC::ParamTraits<ErrorResult>;
   void SerializeMessage(IPC::Message* aMsg) const;
   bool DeserializeMessage(const IPC::Message* aMsg, PickleIterator* aIter);
@@ -315,7 +341,7 @@ private:
   void SerializeDOMExceptionInfo(IPC::Message* aMsg) const;
   bool DeserializeDOMExceptionInfo(const IPC::Message* aMsg, PickleIterator* aIter);
 
-  // Helper method that creates a new Message for this ErrorResult,
+  // Helper method that creates a new Message for this TErrorResult,
   // and returns the arguments array from that Message.
   nsTArray<nsString>& CreateErrorMessageHelper(const dom::ErrNum errorNumber, nsresult errorType);
 
@@ -339,7 +365,7 @@ private:
   }
 
   void AssertInOwningThread() const {
-    NS_ASSERT_OWNINGTHREAD(ErrorResult);
+    NS_ASSERT_OWNINGTHREAD(TErrorResult);
   }
 
   void AssignErrorCode(nsresult aRv) {
@@ -377,6 +403,12 @@ private:
   void SetPendingDOMException(JSContext* cx);
   void SetPendingGenericErrorException(JSContext* cx);
 
+  MOZ_ALWAYS_INLINE void AssertReportedOrSuppressed()
+  {
+    MOZ_ASSERT(!Failed());
+    MOZ_ASSERT(!mMightHaveUnreportedJSException);
+    MOZ_ASSERT(mUnionState == HasNothing);
+  }
 
   // Special values of mResult:
   // NS_ERROR_TYPE_ERR -- ThrowTypeError() called on us.
@@ -411,10 +443,54 @@ private:
   // something.
   UnionState mUnionState;
 
-  // The thread that created this ErrorResult
+  // The thread that created this TErrorResult
   NS_DECL_OWNINGTHREAD;
 #endif
 
+  // Not to be implemented, to make sure people always pass this by
+  // reference, not by value.
+  TErrorResult(const TErrorResult&) = delete;
+  void operator=(const TErrorResult&) = delete;
+};
+
+struct JustAssertCleanupPolicy {
+  static const bool assertHandled = true;
+  static const bool suppress = false;
+};
+
+} // namespace binding_danger
+
+// A class people should normally use on the stack when they plan to actually
+// do something with the exception.
+class ErrorResult : public binding_danger::TErrorResult<binding_danger::JustAssertCleanupPolicy>
+{
+  typedef binding_danger::TErrorResult<binding_danger::JustAssertCleanupPolicy> BaseErrorResult;
+
+public:
+  ErrorResult()
+    : BaseErrorResult()
+  {}
+
+  ErrorResult(ErrorResult&& aRHS)
+    : BaseErrorResult(Move(aRHS))
+  {}
+
+  explicit ErrorResult(nsresult aRv)
+    : BaseErrorResult(aRv)
+  {}
+
+  void operator=(nsresult rv)
+  {
+    BaseErrorResult::operator=(rv);
+  }
+
+  ErrorResult& operator=(ErrorResult&& aRHS)
+  {
+    BaseErrorResult::operator=(Move(aRHS));
+    return *this;
+  }
+
+private:
   // Not to be implemented, to make sure people always pass this by
   // reference, not by value.
   ErrorResult(const ErrorResult&) = delete;