Bug 1367679. P2 - overload InvokeCallbackMethod() according to whether promise-chaining is supported. r=gerald
authorJW Wang <jwwang@mozilla.com>
Wed, 31 May 2017 17:08:08 +0800
changeset 361965 fcc064923628236b2732ba50d5227e47357c0279
parent 361964 324ef6c1803078225fba97edf9ca56ae18e146d4
child 361966 3297397ead64b04a1c92cb38cd367fa20b8e68ff
push id31952
push usercbook@mozilla.com
push dateFri, 02 Jun 2017 12:17:25 +0000
treeherdermozilla-central@194c009d6295 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1367679
milestone55.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 1367679. P2 - overload InvokeCallbackMethod() according to whether promise-chaining is supported. r=gerald 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(