From 2bc689b541618281e849e5fea88eb4de17dd4b7d Mon Sep 17 00:00:00 2001
From: JW Wang <jwwang@mozilla.com>
Date: Wed, 31 May 2017 17:08:08 +0800
Subject: [PATCH] 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

--HG--
extra : rebase_source : 6429d9eef35efa0128e8b5967097850e6f4a4325
extra : intermediate-source : 6f73de7d2d5fb01be19fdf7d7037b506425eab18
extra : source : a1849e24b09b0b4e986ffaef14d2602541c1c6e8
---
 xpcom/threads/MozPromise.h | 46 ++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/xpcom/threads/MozPromise.h b/xpcom/threads/MozPromise.h
index 5f6a56cffa53..74b474999c54 100644
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -521,9 +521,12 @@ protected:
     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)
@@ -531,9 +534,12 @@ protected:
     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)
@@ -593,10 +599,10 @@ protected:
     {
       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()));
       }
 
@@ -658,7 +664,7 @@ protected:
 
     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
@@ -731,13 +737,15 @@ protected:
       // 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
@@ -803,10 +811,10 @@ protected:
       // 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