Bug 1362912. P1 - disallow promise chaining when any of the Then callbacks doesn't return a promise. draft
authorJW Wang <jwwang@mozilla.com>
Tue, 09 May 2017 23:11:42 +0800
changeset 575314 0d0b1aeb9e2ff5211dc8411526ee214a01549299
parent 575133 120d8562d4a53e4f78bd86c6f5076f6db265e5a3
child 575315 fc6a2748dcd08000a22693799d097ba6ca0eb0ce
push id58024
push userjwwang@mozilla.com
push dateWed, 10 May 2017 08:12:21 +0000
bugs1362912
milestone55.0a1
Bug 1362912. P1 - disallow promise chaining when any of the Then callbacks doesn't return a promise. A template won't be instatiated until used. We make it a compile error to call Then() on ThenCommand<false> to disallow promise chaining. MozReview-Commit-ID: BUCFgfX4FTJ
xpcom/threads/MozPromise.h
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -432,24 +432,21 @@ protected:
       if (Request::mDisconnected) {
         PROMISE_LOG("ThenValue::DoResolveOrReject disconnected - bailing out [this=%p]", this);
         return;
       }
 
       // Invoke the resolve or reject method.
       RefPtr<MozPromise> result = DoResolveOrRejectInternal(aValue);
 
-      // If there's a completion promise, resolve it appropriately with the
-      // result of the method.
-      if (RefPtr<Private> p = mCompletionPromise.forget()) {
-        if (result) {
-          result->ChainTo(p.forget(), "<chained completion promise>");
-        } else {
-          p->ResolveOrReject(aValue, "<completion of non-promise-returning method>");
-        }
+      MOZ_DIAGNOSTIC_ASSERT(!mCompletionPromise || result,
+        "Can't do promise chaining for a non-promise-returning method.");
+
+      if (mCompletionPromise && result) {
+        result->ChainTo(mCompletionPromise.forget(), "<chained completion promise>");
       }
     }
 
     RefPtr<AbstractThread> mResponseTarget; // May be released on any thread.
 #ifdef PROMISE_DEBUG
     uint32_t mMagic1 = sMagic;
 #endif
     RefPtr<Private> mCompletionPromise;
@@ -724,16 +721,17 @@ private:
    * A command object to store all information needed to make a request to
    * the promise. This allows us to delay the request until further use is
    * known (whether it is ->Then() again for more promise chaining or ->Track()
    * to terminate chaining and issue the request).
    *
    * This allows a unified syntax for promise chaining and disconnection
    * and feels more like its JS counterpart.
    */
+  template <bool SupportChaining>
   class ThenCommand
   {
     friend class MozPromise;
 
     ThenCommand(AbstractThread* aResponseThread,
                 const char* aCallSite,
                 already_AddRefed<ThenValueBase> aThenValue,
                 MozPromise* aReceiver)
@@ -754,18 +752,23 @@ private:
       if (mThenValue) {
         mReceiver->ThenInternal(mResponseThread, mThenValue, mCallSite);
       }
     }
 
     // Allow RefPtr<MozPromise> p = somePromise->Then();
     //       p->Then(thread1, ...);
     //       p->Then(thread2, ...);
+    template <typename...>
     operator RefPtr<MozPromise>()
     {
+      static_assert(SupportChaining,
+        "The resolve/reject callback needs to return a RefPtr<MozPromise> "
+        "in order to do promise chaining.");
+
       RefPtr<ThenValueBase> thenValue = mThenValue.forget();
       // mCompletionPromise must be created before ThenInternal() to avoid race.
       RefPtr<MozPromise::Private> p = new MozPromise::Private(
         "<completion promise>", true /* aIsCompletionPromise */);
       thenValue->mCompletionPromise = p;
       // Note ThenInternal() might nullify mCompletionPromise before return.
       // So we need to return p instead of mCompletionPromise.
       mReceiver->ThenInternal(mResponseThread, thenValue, mCallSite);
@@ -795,55 +798,101 @@ private:
 
   private:
     AbstractThread* mResponseThread;
     const char* mCallSite;
     RefPtr<ThenValueBase> mThenValue;
     MozPromise* mReceiver;
   };
 
+  template<typename Method>
+  using MethodReturnPromise =
+    ReturnTypeIs<Method, RefPtr<MozPromise>>;
+
+  template<typename Function>
+  using FunctionReturnPromise =
+    MethodReturnPromise<decltype(&Function::operator())>;
+
+  template <typename M1, typename... Ms>
+  struct MethodThenCommand
+  {
+    static const bool value =
+      MethodThenCommand<M1>::value && MethodThenCommand<Ms...>::value;
+    using type = ThenCommand<value>;
+  };
+
+  template <typename M1>
+  struct MethodThenCommand<M1>
+  {
+    static const bool value = MethodReturnPromise<M1>::value;
+    using type = ThenCommand<value>;
+  };
+
+  template <typename F1, typename... Fs>
+  struct FunctionThenCommand
+  {
+    static const bool value =
+      FunctionThenCommand<F1>::value && FunctionThenCommand<Fs...>::value;
+    using type = ThenCommand<value>;
+  };
+
+  template <typename F1>
+  struct FunctionThenCommand<F1>
+  {
+    static const bool value = FunctionReturnPromise<F1>::value;
+    using type = ThenCommand<value>;
+  };
+
 public:
   template<typename ThisType, typename ResolveMethodType, typename RejectMethodType>
-  ThenCommand Then(AbstractThread* aResponseThread, const char* aCallSite,
+  typename MethodThenCommand<ResolveMethodType, RejectMethodType>::type
+  Then(AbstractThread* aResponseThread, const char* aCallSite,
     ThisType* aThisVal, ResolveMethodType aResolveMethod, RejectMethodType aRejectMethod)
   {
     using ThenType = MethodThenValue<ThisType, ResolveMethodType, RejectMethodType>;
     RefPtr<ThenValueBase> thenValue = new ThenType(aResponseThread,
        aThisVal, aResolveMethod, aRejectMethod, aCallSite);
-    return ThenCommand(aResponseThread, aCallSite, thenValue.forget(), this);
+    return typename MethodThenCommand<ResolveMethodType, RejectMethodType>::type(
+      aResponseThread, aCallSite, thenValue.forget(), this);
   }
 
   template<typename ThisType, typename ResolveRejectMethodType>
-  ThenCommand Then(AbstractThread* aResponseThread, const char* aCallSite,
+  typename MethodThenCommand<ResolveRejectMethodType>::type
+  Then(AbstractThread* aResponseThread, const char* aCallSite,
     ThisType* aThisVal, ResolveRejectMethodType aResolveRejectMethod)
   {
     using ThenType = MethodThenValue<ThisType, ResolveRejectMethodType, void>;
     RefPtr<ThenValueBase> thenValue = new ThenType(aResponseThread,
        aThisVal, aResolveRejectMethod, aCallSite);
-    return ThenCommand(aResponseThread, aCallSite, thenValue.forget(), this);
+    return typename MethodThenCommand<ResolveRejectMethodType>::type(
+      aResponseThread, aCallSite, thenValue.forget(), this);
   }
 
   template<typename ResolveFunction, typename RejectFunction>
-  ThenCommand Then(AbstractThread* aResponseThread, const char* aCallSite,
+  typename FunctionThenCommand<ResolveFunction, RejectFunction>::type
+  Then(AbstractThread* aResponseThread, const char* aCallSite,
     ResolveFunction&& aResolveFunction, RejectFunction&& aRejectFunction)
   {
     using ThenType = FunctionThenValue<ResolveFunction, RejectFunction>;
     RefPtr<ThenValueBase> thenValue = new ThenType(aResponseThread,
       Move(aResolveFunction), Move(aRejectFunction), aCallSite);
-    return ThenCommand(aResponseThread, aCallSite, thenValue.forget(), this);
+    return typename FunctionThenCommand<ResolveFunction, RejectFunction>::type(
+      aResponseThread, aCallSite, thenValue.forget(), this);
   }
 
   template<typename ResolveRejectFunction>
-  ThenCommand Then(AbstractThread* aResponseThread, const char* aCallSite,
+  typename FunctionThenCommand<ResolveRejectFunction>::type
+  Then(AbstractThread* aResponseThread, const char* aCallSite,
                    ResolveRejectFunction&& aResolveRejectFunction)
   {
     using ThenType = FunctionThenValue<ResolveRejectFunction, void>;
     RefPtr<ThenValueBase> thenValue = new ThenType(aResponseThread,
       Move(aResolveRejectFunction), aCallSite);
-    return ThenCommand(aResponseThread, aCallSite, thenValue.forget(), this);
+    return typename FunctionThenCommand<ResolveRejectFunction>::type(
+      aResponseThread, aCallSite, thenValue.forget(), this);
   }
 
   void ChainTo(already_AddRefed<Private> aChainedPromise, const char* aCallSite)
   {
     MutexAutoLock lock(mMutex);
     MOZ_DIAGNOSTIC_ASSERT(!IsExclusive || !mHaveRequest);
     mHaveRequest = true;
     RefPtr<Private> chainedPromise = aChainedPromise;