Bug 1367679. P2 - overload InvokeCallbackMethod() according to whether promise-chaining is supported. draft
authorJW Wang <jwwang@mozilla.com>
Wed, 31 May 2017 17:08:08 +0800
changeset 588113 cde4ba9d0dd75c0e0cf924b29b643cbbb6ffe104
parent 588053 cbe886b7c1bcdc258f6610c13e6532f17b09d419
child 588114 d4fc95136513ef4d0d427d0d72a20f5eaa90cf4b
push id61915
push userjwwang@mozilla.com
push dateFri, 02 Jun 2017 06:48:13 +0000
bugs1367679
milestone55.0a1
Bug 1367679. P2 - overload InvokeCallbackMethod() according to whether promise-chaining is supported. This patch fixes InvokeCallbackMethod() which should return null if promise-chaining is not supported. Before this patch, it could return non-null if one of the resolve/reject callbacks returns a MozPromise while the other not. MozReview-Commit-ID: 7YKNvRKEHQx
xpcom/threads/MozPromise.h
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -516,29 +516,35 @@ protected:
   static typename EnableIf<
     !TakesArgument<MethodType>::value,
     typename detail::MethodTrait<MethodType>::ReturnType>::Type
   InvokeMethod(ThisType* aThisVal, MethodType aMethod, ValueType&& aValue)
   {
     return (aThisVal->*aMethod)();
   }
 
-  template<typename ThisType, typename MethodType, typename ValueType>
-  static typename EnableIf<ReturnTypeIs<MethodType, RefPtr<MozPromise>>::value,
-                           already_AddRefed<MozPromise>>::Type
+  // Called when promise chaining is supported.
+  template<bool SupportChaining,
+           typename ThisType,
+           typename MethodType,
+           typename ValueType>
+  static typename EnableIf<SupportChaining, already_AddRefed<MozPromise>>::Type
   InvokeCallbackMethod(ThisType* aThisVal,
                        MethodType aMethod,
                        ValueType&& aValue)
   {
     return InvokeMethod(aThisVal, aMethod, Forward<ValueType>(aValue)).forget();
   }
 
-  template<typename ThisType, typename MethodType, typename ValueType>
-  static typename EnableIf<ReturnTypeIs<MethodType, void>::value,
-                           already_AddRefed<MozPromise>>::Type
+  // Called when promise chaining is not supported.
+  template<bool SupportChaining,
+           typename ThisType,
+           typename MethodType,
+           typename ValueType>
+  static typename EnableIf<!SupportChaining, already_AddRefed<MozPromise>>::Type
   InvokeCallbackMethod(ThisType* aThisVal,
                        MethodType aMethod,
                        ValueType&& aValue)
   {
     InvokeMethod(aThisVal, aMethod, Forward<ValueType>(aValue));
     return nullptr;
   }
 
@@ -588,20 +594,20 @@ protected:
     {
       return mCompletionPromise;
     }
 
     void DoResolveOrRejectInternal(ResolveOrRejectValue& aValue) override
     {
       RefPtr<MozPromise> result;
       if (aValue.IsResolve()) {
-        result = InvokeCallbackMethod(
+        result = InvokeCallbackMethod<SupportChaining::value>(
           mThisVal.get(), mResolveMethod, MaybeMove(aValue.ResolveValue()));
       } else {
-        result = InvokeCallbackMethod(
+        result = InvokeCallbackMethod<SupportChaining::value>(
           mThisVal.get(), mRejectMethod, MaybeMove(aValue.RejectValue()));
       }
 
       // Null out mThisVal after invoking the callback so that any references are
       // released predictably on the dispatch thread. Otherwise, it would be
       // released on whatever thread last drops its reference to the ThenValue,
       // which may or may not be ok.
       mThisVal = nullptr;
@@ -653,17 +659,17 @@ protected:
   protected:
     MozPromiseBase* CompletionPromise() const override
     {
       return mCompletionPromise;
     }
 
     void DoResolveOrRejectInternal(ResolveOrRejectValue& aValue) override
     {
-      RefPtr<MozPromise> result = InvokeCallbackMethod(
+      RefPtr<MozPromise> result = InvokeCallbackMethod<SupportChaining::value>(
         mThisVal.get(), mResolveRejectMethod, MaybeMove(aValue));
 
       // Null out mThisVal after invoking the callback so that any references are
       // released predictably on the dispatch thread. Otherwise, it would be
       // released on whatever thread last drops its reference to the ThenValue,
       // which may or may not be ok.
       mThisVal = nullptr;
 
@@ -726,23 +732,25 @@ protected:
     {
       // Note: The usage of InvokeCallbackMethod here requires that
       // ResolveFunction/RejectFunction are capture-lambdas (i.e. anonymous
       // classes with ::operator()), since it allows us to share code more easily.
       // We could fix this if need be, though it's quite easy to work around by
       // just capturing something.
       RefPtr<MozPromise> result;
       if (aValue.IsResolve()) {
-        result = InvokeCallbackMethod(mResolveFunction.ptr(),
-                                      &ResolveFunction::operator(),
-                                      MaybeMove(aValue.ResolveValue()));
+        result = InvokeCallbackMethod<SupportChaining::value>(
+          mResolveFunction.ptr(),
+          &ResolveFunction::operator(),
+          MaybeMove(aValue.ResolveValue()));
       } else {
-        result = InvokeCallbackMethod(mRejectFunction.ptr(),
-                                      &RejectFunction::operator(),
-                                      MaybeMove(aValue.RejectValue()));
+        result = InvokeCallbackMethod<SupportChaining::value>(
+          mRejectFunction.ptr(),
+          &RejectFunction::operator(),
+          MaybeMove(aValue.RejectValue()));
       }
 
       // Destroy callbacks after invocation so that any references in closures are
       // released predictably on the dispatch thread. Otherwise, they would be
       // released on whatever thread last drops its reference to the ThenValue,
       // which may or may not be ok.
       mResolveFunction.reset();
       mRejectFunction.reset();
@@ -798,20 +806,20 @@ protected:
 
     void DoResolveOrRejectInternal(ResolveOrRejectValue& aValue) override
     {
       // Note: The usage of InvokeCallbackMethod here requires that
       // ResolveRejectFunction is capture-lambdas (i.e. anonymous
       // classes with ::operator()), since it allows us to share code more easily.
       // We could fix this if need be, though it's quite easy to work around by
       // just capturing something.
-      RefPtr<MozPromise> result =
-        InvokeCallbackMethod(mResolveRejectFunction.ptr(),
-                             &ResolveRejectFunction::operator(),
-                             MaybeMove(aValue));
+      RefPtr<MozPromise> result = InvokeCallbackMethod<SupportChaining::value>(
+        mResolveRejectFunction.ptr(),
+        &ResolveRejectFunction::operator(),
+        MaybeMove(aValue));
 
       // Destroy callbacks after invocation so that any references in closures are
       // released predictably on the dispatch thread. Otherwise, they would be
       // released on whatever thread last drops its reference to the ThenValue,
       // which may or may not be ok.
       mResolveRejectFunction.reset();
 
       MOZ_DIAGNOSTIC_ASSERT(