Bug 1357463 Add CopyableErrorResult to opt in to auto-cloning ErrorResult values. r=bz
authorBen Kelly <ben@wanderview.com>
Thu, 26 Apr 2018 13:50:56 -0700
changeset 415842 f557f72f5d462ecdc486e5ed9ba1fc930dfa3eee
parent 415841 1472f261db239683061a82d3ceeeb14c31bbc158
child 415843 c321bfd64e4d1bed809460d7f1e077368b6b3811
push id102673
push userbkelly@mozilla.com
push dateThu, 26 Apr 2018 20:51:04 +0000
treeherdermozilla-inbound@f557f72f5d46 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1357463
milestone61.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 1357463 Add CopyableErrorResult to opt in to auto-cloning ErrorResult values. r=bz
dom/bindings/BindingUtils.cpp
dom/bindings/ErrorIPCUtils.h
dom/bindings/ErrorResult.h
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -177,16 +177,22 @@ struct TErrorResult<CleanupPolicy>::Mess
 
   nsTArray<nsString> mArgs;
   dom::ErrNum mErrorNumber;
 
   bool HasCorrectNumberOfArguments()
   {
     return GetErrorArgCount(mErrorNumber) == mArgs.Length();
   }
+
+  bool operator==(const TErrorResult<CleanupPolicy>::Message& aRight)
+  {
+    return mErrorNumber == aRight.mErrorNumber &&
+           mArgs == aRight.mArgs;
+  }
 };
 
 template<typename CleanupPolicy>
 nsTArray<nsString>&
 TErrorResult<CleanupPolicy>::CreateErrorMessageHelper(const dom::ErrNum errorNumber,
                                                       nsresult errorType)
 {
   AssertInOwningThread();
@@ -328,16 +334,22 @@ template<typename CleanupPolicy>
 struct TErrorResult<CleanupPolicy>::DOMExceptionInfo {
   DOMExceptionInfo(nsresult rv, const nsACString& message)
     : mMessage(message)
     , mRv(rv)
   {}
 
   nsCString mMessage;
   nsresult mRv;
+
+  bool operator==(const TErrorResult<CleanupPolicy>::DOMExceptionInfo& aRight)
+  {
+    return mRv == aRight.mRv &&
+           mMessage == aRight.mMessage;
+  }
 };
 
 template<typename CleanupPolicy>
 void
 TErrorResult<CleanupPolicy>::SerializeDOMExceptionInfo(IPC::Message* aMsg) const
 {
   using namespace IPC;
   AssertInOwningThread();
@@ -497,17 +509,16 @@ TErrorResult<CleanupPolicy>::operator=(T
 }
 
 template<typename CleanupPolicy>
 void
 TErrorResult<CleanupPolicy>::CloneTo(TErrorResult& aRv) const
 {
   AssertInOwningThread();
   aRv.AssertInOwningThread();
-
   aRv.ClearUnionData();
   aRv.mResult = mResult;
 #ifdef DEBUG
   aRv.mMightHaveUnreportedJSException = mMightHaveUnreportedJSException;
 #endif
 
   if (IsErrorWithMessage()) {
 #ifdef DEBUG
@@ -607,16 +618,17 @@ TErrorResult<CleanupPolicy>::NoteJSConte
   } else {
     mResult = NS_ERROR_UNCATCHABLE_EXCEPTION;
   }
 }
 
 template class TErrorResult<JustAssertCleanupPolicy>;
 template class TErrorResult<AssertAndSuppressCleanupPolicy>;
 template class TErrorResult<JustSuppressCleanupPolicy>;
+template class TErrorResult<ThreadSafeJustSuppressCleanupPolicy>;
 
 } // namespace binding_danger
 
 namespace dom {
 
 bool
 DefineConstants(JSContext* cx, JS::Handle<JSObject*> obj,
                 const ConstantSpec* cs)
--- a/dom/bindings/ErrorIPCUtils.h
+++ b/dom/bindings/ErrorIPCUtils.h
@@ -74,11 +74,28 @@ struct ParamTraits<mozilla::ErrorResult>
                !readValue.DeserializeDOMExceptionInfo(aMsg, aIter)) {
       return false;
     }
     *aResult = Move(readValue);
     return true;
   }
 };
 
+template<>
+struct ParamTraits<mozilla::CopyableErrorResult>
+{
+  typedef mozilla::CopyableErrorResult paramType;
+
+  static void Write(Message* aMsg, const paramType& aParam)
+  {
+    ParamTraits<mozilla::ErrorResult>::Write(aMsg, aParam);
+  }
+
+  static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult)
+  {
+    mozilla::ErrorResult& ref = static_cast<mozilla::ErrorResult&>(*aResult);
+    return ParamTraits<mozilla::ErrorResult>::Read(aMsg, aIter, &ref);
+  }
+};
+
 } // namespace IPC
 
 #endif
--- a/dom/bindings/ErrorResult.h
+++ b/dom/bindings/ErrorResult.h
@@ -160,16 +160,17 @@ public:
 
   explicit TErrorResult(nsresult aRv)
     : TErrorResult()
   {
     AssignErrorCode(aRv);
   }
 
   operator ErrorResult&();
+  operator const ErrorResult&() const;
   operator OOMReporter&();
 
   void MOZ_MUST_RETURN_FROM_CALLER Throw(nsresult rv) {
     MOZ_ASSERT(NS_FAILED(rv), "Please don't try throwing success");
     AssignErrorCode(rv);
   }
 
   // This method acts identically to the `Throw` method, however, it does not
@@ -386,16 +387,18 @@ public:
     return mResult == rv;
   }
 
   // For use in logging ONLY.
   uint32_t ErrorCodeAsInt() const {
     return static_cast<uint32_t>(ErrorCode());
   }
 
+  bool operator==(const ErrorResult& aRight) const;
+
 protected:
   nsresult ErrorCode() const {
     return mResult;
   }
 
 private:
 #ifdef DEBUG
   enum UnionState {
@@ -434,17 +437,19 @@ private:
                                      Forward<Ts>(messageArgs)...);
 #ifdef DEBUG
     mUnionState = HasMessage;
 #endif // DEBUG
   }
 
   MOZ_ALWAYS_INLINE void AssertInOwningThread() const {
 #ifdef DEBUG
-    NS_ASSERT_OWNINGTHREAD(TErrorResult);
+    if (CleanupPolicy::assertSameThread) {
+      NS_ASSERT_OWNINGTHREAD(TErrorResult);
+    }
 #endif
   }
 
   void AssignErrorCode(nsresult aRv) {
     MOZ_ASSERT(aRv != NS_ERROR_INTERNAL_ERRORRESULT_TYPEERROR,
                "Use ThrowTypeError()");
     MOZ_ASSERT(aRv != NS_ERROR_INTERNAL_ERRORRESULT_RANGEERROR,
                "Use ThrowRangeError()");
@@ -561,26 +566,35 @@ private:
   // 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;
+  static const bool assertSameThread = true;
 };
 
 struct AssertAndSuppressCleanupPolicy {
   static const bool assertHandled = true;
   static const bool suppress = true;
+  static const bool assertSameThread = true;
 };
 
 struct JustSuppressCleanupPolicy {
   static const bool assertHandled = false;
   static const bool suppress = true;
+  static const bool assertSameThread = true;
+};
+
+struct ThreadSafeJustSuppressCleanupPolicy {
+  static const bool assertHandled = false;
+  static const bool suppress = true;
+  static const bool assertSameThread = 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::AssertAndSuppressCleanupPolicy>
@@ -620,24 +634,125 @@ private:
 
 template<typename CleanupPolicy>
 binding_danger::TErrorResult<CleanupPolicy>::operator ErrorResult&()
 {
   return *static_cast<ErrorResult*>(
      reinterpret_cast<TErrorResult<AssertAndSuppressCleanupPolicy>*>(this));
 }
 
+template<typename CleanupPolicy>
+binding_danger::TErrorResult<CleanupPolicy>::operator const ErrorResult&() const
+{
+  return *static_cast<const ErrorResult*>(
+     reinterpret_cast<const TErrorResult<AssertAndSuppressCleanupPolicy>*>(this));
+}
+
+template<typename CleanupPolicy>
+bool
+binding_danger::TErrorResult<CleanupPolicy>::operator==(const ErrorResult& aRight) const
+{
+  auto right = reinterpret_cast<const TErrorResult<CleanupPolicy>*>(&aRight);
+
+  if (mResult != right->mResult) {
+    return false;
+  }
+
+  if (IsJSException()) {
+    // js exceptions are always non-equal
+    return false;
+  }
+
+  if (IsErrorWithMessage()) {
+    return *mExtra.mMessage == *right->mExtra.mMessage;
+  }
+
+  if (IsDOMException()) {
+    return *mExtra.mDOMExceptionInfo == *right->mExtra.mDOMExceptionInfo;
+  }
+
+  return true;
+}
+
 // A class for use when an ErrorResult should just automatically be ignored.
 // This doesn't inherit from ErrorResult so we don't make two separate calls to
 // SuppressException.
 class IgnoredErrorResult :
     public binding_danger::TErrorResult<binding_danger::JustSuppressCleanupPolicy>
 {
 };
 
+// A class for use when an ErrorResult needs to be copied to a lambda, into
+// an IPDL structure, etc.  Since this will often involve crossing thread
+// boundaries this class will assert if you try to copy a JS exception.  Only
+// use this if you are propagating internal errors.  In general its best
+// to use ErrorResult by default and only convert to a CopyableErrorResult when
+// you need it.
+class CopyableErrorResult :
+    public binding_danger::TErrorResult<binding_danger::ThreadSafeJustSuppressCleanupPolicy>
+{
+  typedef binding_danger::TErrorResult<binding_danger::ThreadSafeJustSuppressCleanupPolicy> BaseErrorResult;
+
+public:
+  CopyableErrorResult()
+    : BaseErrorResult()
+  {}
+
+  explicit CopyableErrorResult(const ErrorResult& aRight)
+    : BaseErrorResult()
+  {
+    auto val = reinterpret_cast<const CopyableErrorResult&>(aRight);
+    operator=(val);
+  }
+
+  CopyableErrorResult(CopyableErrorResult&& aRHS)
+    : BaseErrorResult(Move(aRHS))
+  {}
+
+  explicit CopyableErrorResult(nsresult aRv)
+    : BaseErrorResult(aRv)
+  {}
+
+  void operator=(nsresult rv)
+  {
+    BaseErrorResult::operator=(rv);
+  }
+
+  CopyableErrorResult& operator=(CopyableErrorResult&& aRHS)
+  {
+    BaseErrorResult::operator=(Move(aRHS));
+    return *this;
+  }
+
+  CopyableErrorResult(const CopyableErrorResult& aRight)
+    : BaseErrorResult()
+  {
+    operator=(aRight);
+  }
+
+  CopyableErrorResult&
+  operator=(const CopyableErrorResult& aRight)
+  {
+    // We must not copy JS exceptions since it can too easily lead to
+    // off-thread use.  Assert this and fall back to a generic error
+    // in release builds.
+    MOZ_DIAGNOSTIC_ASSERT(!IsJSException(),
+                          "Attempt to copy to ErrorResult with a JS exception value.");
+    MOZ_DIAGNOSTIC_ASSERT(!aRight.IsJSException(),
+                          "Attempt to copy from ErrorResult with a JS exception value.");
+    if (aRight.IsJSException()) {
+      SuppressException();
+      Throw(NS_ERROR_FAILURE);
+    } else {
+      aRight.CloneTo(*this);
+    }
+    return *this;
+  }
+};
+
 namespace dom {
 namespace binding_detail {
 class FastErrorResult :
     public mozilla::binding_danger::TErrorResult<
       mozilla::binding_danger::JustAssertCleanupPolicy>
 {
 };
 } // namespace binding_detail