Bug 1321471. Part 2 - remove CompletionPromise() to enforce use of ThenPromise(). r=jwwang
authorJW Wang <jwwang@mozilla.com>
Fri, 02 Dec 2016 10:40:01 +0800
changeset 325065 24424daa3526192a331fa0f75f67fb33790f6546
parent 325064 31c6008a6773d09b0f84d6db482d25b88d2c1639
child 325066 46b896e481eb216c5410d0bc31ff7e1c5c30393a
push id31028
push userkwierso@gmail.com
push dateFri, 02 Dec 2016 20:27:00 +0000
treeherdermozilla-central@919596f62a27 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang
bugs1321471
milestone53.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 1321471. Part 2 - remove CompletionPromise() to enforce use of ThenPromise(). r=jwwang MozReview-Commit-ID: 9zC3JnzumES
xpcom/threads/MozPromise.h
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -273,18 +273,16 @@ public:
   {
   public:
     virtual void Disconnect() = 0;
 
     // MSVC complains when an inner class (ThenValueBase::{Resolve,Reject}Runnable)
     // tries to access an inherited protected member.
     bool IsDisconnected() const { return mDisconnected; }
 
-    virtual MozPromise* CompletionPromise() = 0;
-
     virtual void AssertIsDead() = 0;
 
   protected:
     Request() : mComplete(false), mDisconnected(false) {}
     virtual ~Request() {}
 
     bool mComplete;
     bool mDisconnected;
@@ -295,16 +293,18 @@ protected:
   /*
    * A ThenValue tracks a single consumer waiting on the promise. When a consumer
    * invokes promise->Then(...), a ThenValue is created. Once the Promise is
    * resolved or rejected, a {Resolve,Reject}Runnable is dispatched, which
    * invokes the resolve/reject method and then deletes the ThenValue.
    */
   class ThenValueBase : public Request
   {
+    friend class MozPromise;
+
   public:
     class ResolveOrRejectRunnable : public Runnable
     {
     public:
       ResolveOrRejectRunnable(ThenValueBase* aThenValue, MozPromise* aPromise)
         : mThenValue(aThenValue)
         , mPromise(aPromise)
       {
@@ -328,39 +328,20 @@ protected:
       }
 
     private:
       RefPtr<ThenValueBase> mThenValue;
       RefPtr<MozPromise> mPromise;
     };
 
     ThenValueBase(AbstractThread* aResponseTarget,
-                  const char* aCallSite,
-                  bool aInitCompletionPromise = false)
+                  const char* aCallSite)
       : mResponseTarget(aResponseTarget)
       , mCallSite(aCallSite)
-      , mInitCompletionPromise(aInitCompletionPromise)
-    {
-      if (mInitCompletionPromise) {
-        mCompletionPromise = new MozPromise::Private(
-          "<completion promise>", true /* aIsCompletionPromise */);
-      }
-    }
-
-    MozPromise* CompletionPromise() override
-    {
-      MOZ_DIAGNOSTIC_ASSERT(mInitCompletionPromise ||
-                            mResponseTarget->IsCurrentThreadIn());
-      MOZ_DIAGNOSTIC_ASSERT(!Request::mComplete);
-      if (!mInitCompletionPromise && !mCompletionPromise) {
-        mCompletionPromise = new MozPromise::Private(
-          "<completion promise>", true /* aIsCompletionPromise */);
-      }
-      return mCompletionPromise;
-    }
+    { }
 
     void AssertIsDead() override
     {
       // We want to assert that this ThenValues is dead - that is to say, that
       // there are no consumers waiting for the result. In the case of a normal
       // ThenValue, we check that it has been disconnected, which is the way
       // that the consumer signals that it no longer wishes to hear about the
       // result. If this ThenValue has a completion promise (which is mutually
@@ -439,20 +420,16 @@ protected:
 
     // Declaring RefPtr<MozPromise::Private> here causes build failures
     // on MSVC because MozPromise::Private is only forward-declared at this
     // point. This hack can go away when we inline-declare MozPromise::Private,
     // which is blocked on the B2G ICS compiler being too old.
     RefPtr<MozPromise> mCompletionPromise;
 
     const char* mCallSite;
-
-    // True if mCompletionPromise should be initialized in the constructor
-    // to make CompletionPromise() thread-safe.
-    const bool mInitCompletionPromise;
   };
 
   /*
    * We create two overloads for invoking Resolve/Reject Methods so as to
    * make the resolve/reject value argument "optional".
    */
 
   template<typename ThisType, typename MethodType, typename ValueType>
@@ -494,18 +471,18 @@ protected:
   }
 
   template<typename ThisType, typename ResolveMethodType, typename RejectMethodType>
   class MethodThenValue : public ThenValueBase
   {
   public:
     MethodThenValue(AbstractThread* aResponseTarget, ThisType* aThisVal,
                     ResolveMethodType aResolveMethod, RejectMethodType aRejectMethod,
-                    const char* aCallSite, bool aInitCompletionPromise = false)
-      : ThenValueBase(aResponseTarget, aCallSite, aInitCompletionPromise)
+                    const char* aCallSite)
+      : ThenValueBase(aResponseTarget, aCallSite)
       , mThisVal(aThisVal)
       , mResolveMethod(aResolveMethod)
       , mRejectMethod(aRejectMethod) {}
 
   virtual void Disconnect() override
   {
     ThenValueBase::Disconnect();
 
@@ -543,19 +520,18 @@ protected:
   // NB: We could use std::function here instead of a template if it were supported. :-(
   template<typename ResolveFunction, typename RejectFunction>
   class FunctionThenValue : public ThenValueBase
   {
   public:
     FunctionThenValue(AbstractThread* aResponseTarget,
                       ResolveFunction&& aResolveFunction,
                       RejectFunction&& aRejectFunction,
-                      const char* aCallSite,
-                      bool aInitCompletionPromise = false)
-      : ThenValueBase(aResponseTarget, aCallSite, aInitCompletionPromise)
+                      const char* aCallSite)
+      : ThenValueBase(aResponseTarget, aCallSite)
     {
       mResolveFunction.emplace(Move(aResolveFunction));
       mRejectFunction.emplace(Move(aRejectFunction));
     }
 
   virtual void Disconnect() override
   {
     ThenValueBase::Disconnect();
@@ -632,49 +608,50 @@ public:
                        ResolveFunction&& aResolveFunction, RejectFunction&& aRejectFunction)
   {
     RefPtr<ThenValueBase> thenValue = new FunctionThenValue<ResolveFunction, RejectFunction>(aResponseThread,
                                               Move(aResolveFunction), Move(aRejectFunction), aCallSite);
     ThenInternal(aResponseThread, thenValue, aCallSite);
     return thenValue.forget(); // Implicit conversion from already_AddRefed<ThenValueBase> to RefPtr<Request>.
   }
 
-  // Equivalent to Then(target, ...)->CompletionPromise()
-  // without the restriction that CompletionPromise() must be called on the
-  // |target| thread. So ThenPromise() can be called on any thread as Then().
+  // ThenPromise() can be called on any thread as Then().
   // The syntax is close to JS promise and makes promise chaining easier
   // where you can do: p->ThenPromise()->ThenPromise()->ThenPromise();
   //
   // Note you would have to call Then() instead when the result needs to be held
   // by a MozPromiseRequestHolder for future disconnection.
-  //
-  // TODO: replace Then()->CompletionPromise() with ThenPromise() and
-  // stop exposing CompletionPromise() to the client code.
   template<typename ThisType, typename ResolveMethodType, typename RejectMethodType>
   MOZ_MUST_USE RefPtr<MozPromise>
   ThenPromise(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, true /* aInitCompletionPromise */);
+    RefPtr<ThenValueBase> thenValue = new ThenType(
+      aResponseThread, aThisVal, aResolveMethod, aRejectMethod, aCallSite);
+    // mCompletionPromise must be created before ThenInternal() to avoid race.
+    thenValue->mCompletionPromise = new MozPromise::Private(
+      "<completion promise>", true /* aIsCompletionPromise */);
     ThenInternal(aResponseThread, thenValue, aCallSite);
-    return thenValue->CompletionPromise();
+    return thenValue->mCompletionPromise;
   }
 
   template<typename ResolveFunction, typename RejectFunction>
   MOZ_MUST_USE RefPtr<MozPromise>
   ThenPromise(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, true /* aInitCompletionPromise */);
+    RefPtr<ThenValueBase> thenValue = new ThenType(
+      aResponseThread, Move(aResolveFunction), Move(aRejectFunction), aCallSite);
+    // mCompletionPromise must be created before ThenInternal() to avoid race.
+    thenValue->mCompletionPromise = new MozPromise::Private(
+      "<completion promise>", true /* aIsCompletionPromise */);
     ThenInternal(aResponseThread, thenValue, aCallSite);
-    return thenValue->CompletionPromise();
+    return thenValue->mCompletionPromise;
   }
 
   void ChainTo(already_AddRefed<Private> aChainedPromise, const char* aCallSite)
   {
     MutexAutoLock lock(mMutex);
     MOZ_DIAGNOSTIC_ASSERT(!IsExclusive || !mHaveRequest);
     mHaveRequest = true;
     RefPtr<Private> chainedPromise = aChainedPromise;