Bug 1129877 - Separate the creator and consumer APIs for MediaPromise. v2 r=mattwoodrow a=lmandel
authorBobby Holley <bobbyholley@gmail.com>
Mon, 09 Feb 2015 18:47:49 -0800
changeset 249848 9ca7d843248de1235a21830e9c854e7c1a05777f
parent 249847 f1717862392963e008e094b8933c8b239f1928e6
child 249849 5f012ce3f3eafd3cc9c8ce734fab518e5b81542f
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow, lmandel
bugs1129877
milestone37.0a2
Bug 1129877 - Separate the creator and consumer APIs for MediaPromise. v2 r=mattwoodrow a=lmandel This causes the buggy code that caused the crash to fail to compile.
dom/media/MediaPromise.h
--- a/dom/media/MediaPromise.h
+++ b/dom/media/MediaPromise.h
@@ -57,38 +57,52 @@ template<typename T> class MediaPromiseH
 template<typename ResolveValueT, typename RejectValueT, bool IsExclusive>
 class MediaPromise
 {
 public:
   typedef ResolveValueT ResolveValueType;
   typedef RejectValueT RejectValueType;
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaPromise)
+
+protected:
+  // MediaPromise is the public type, and never constructed directly. Construct
+  // a MediaPromise::Private, defined below.
   explicit MediaPromise(const char* aCreationSite)
     : mCreationSite(aCreationSite)
     , mMutex("MediaPromise Mutex")
     , mHaveConsumer(false)
   {
     PROMISE_LOG("%s creating MediaPromise (%p)", mCreationSite, this);
   }
 
+public:
+  // MediaPromise::Private allows us to separate the public interface (upon which
+  // consumers of the promise may invoke methods like Then()) from the private
+  // interface (upon which the creator of the promise may invoke Resolve() or
+  // Reject()). APIs should create and store a MediaPromise::Private (usually
+  // via a MediaPromiseHolder), and return a MediaPromise to consumers.
+  //
+  // NB: We can include the definition of this class inline once B2G ICS is gone.
+  class Private;
+
   static nsRefPtr<MediaPromise>
   CreateAndResolve(ResolveValueType aResolveValue, const char* aResolveSite)
   {
-    nsRefPtr<MediaPromise> p = new MediaPromise(aResolveSite);
+    nsRefPtr<typename MediaPromise::Private> p = new MediaPromise::Private(aResolveSite);
     p->Resolve(aResolveValue, aResolveSite);
-    return p;
+    return Move(p);
   }
 
   static nsRefPtr<MediaPromise>
   CreateAndReject(RejectValueType aRejectValue, const char* aRejectSite)
   {
-    nsRefPtr<MediaPromise> p = new MediaPromise(aRejectSite);
+    nsRefPtr<typename MediaPromise::Private> p = new MediaPromise::Private(aRejectSite);
     p->Reject(aRejectValue, aRejectSite);
-    return p;
+    return Move(p);
   }
 
   class Consumer
   {
   public:
     NS_INLINE_DECL_THREADSAFE_REFCOUNTING(Consumer)
 
     void Disconnect()
@@ -324,66 +338,48 @@ public:
   void Then(TargetType* aResponseTarget, const char* aCallSite, ThisType* aThisVal,
             ResolveMethodType aResolveMethod, RejectMethodType aRejectMethod)
   {
     nsRefPtr<Consumer> c =
       RefableThen(aResponseTarget, aCallSite, aThisVal, aResolveMethod, aRejectMethod);
     return;
   }
 
-  void ChainTo(already_AddRefed<MediaPromise> aChainedPromise, const char* aCallSite)
+  void ChainTo(already_AddRefed<Private> aChainedPromise, const char* aCallSite)
   {
     MutexAutoLock lock(mMutex);
     MOZ_DIAGNOSTIC_ASSERT(!IsExclusive || !mHaveConsumer);
     mHaveConsumer = true;
-    nsRefPtr<MediaPromise> chainedPromise = aChainedPromise;
+    nsRefPtr<Private> chainedPromise = aChainedPromise;
     PROMISE_LOG("%s invoking Chain() [this=%p, chainedPromise=%p, isPending=%d]",
                 aCallSite, this, chainedPromise.get(), (int) IsPending());
     if (!IsPending()) {
       ForwardTo(chainedPromise);
     } else {
       mChainedPromises.AppendElement(chainedPromise);
     }
   }
 
-  void Resolve(ResolveValueType aResolveValue, const char* aResolveSite)
-  {
-    MutexAutoLock lock(mMutex);
-    MOZ_ASSERT(IsPending());
-    PROMISE_LOG("%s resolving MediaPromise (%p created at %s)", aResolveSite, this, mCreationSite);
-    mResolveValue.emplace(aResolveValue);
-    DispatchAll();
-  }
-
-  void Reject(RejectValueType aRejectValue, const char* aRejectSite)
-  {
-    MutexAutoLock lock(mMutex);
-    MOZ_ASSERT(IsPending());
-    PROMISE_LOG("%s rejecting MediaPromise (%p created at %s)", aRejectSite, this, mCreationSite);
-    mRejectValue.emplace(aRejectValue);
-    DispatchAll();
-  }
-
 protected:
   bool IsPending() { return mResolveValue.isNothing() && mRejectValue.isNothing(); }
   void DispatchAll()
   {
     mMutex.AssertCurrentThreadOwns();
     for (size_t i = 0; i < mThenValues.Length(); ++i) {
       mThenValues[i]->Dispatch(this);
     }
     mThenValues.Clear();
 
     for (size_t i = 0; i < mChainedPromises.Length(); ++i) {
       ForwardTo(mChainedPromises[i]);
     }
     mChainedPromises.Clear();
   }
 
-  void ForwardTo(MediaPromise* aOther)
+  void ForwardTo(Private* aOther)
   {
     MOZ_ASSERT(!IsPending());
     if (mResolveValue.isSome()) {
       aOther->Resolve(mResolveValue.ref(), "<chained promise>");
     } else {
       aOther->Reject(mRejectValue.ref(), "<chained promise>");
     }
   }
@@ -396,20 +392,46 @@ protected:
     MOZ_ASSERT(mChainedPromises.IsEmpty());
   };
 
   const char* mCreationSite; // For logging
   Mutex mMutex;
   Maybe<ResolveValueType> mResolveValue;
   Maybe<RejectValueType> mRejectValue;
   nsTArray<nsRefPtr<ThenValueBase>> mThenValues;
-  nsTArray<nsRefPtr<MediaPromise>> mChainedPromises;
+  nsTArray<nsRefPtr<Private>> mChainedPromises;
   bool mHaveConsumer;
 };
 
+template<typename ResolveValueT, typename RejectValueT, bool IsExclusive>
+class MediaPromise<ResolveValueT, RejectValueT, IsExclusive>::Private
+  : public MediaPromise<ResolveValueT, RejectValueT, IsExclusive>
+{
+public:
+  explicit Private(const char* aCreationSite) : MediaPromise(aCreationSite) {}
+
+  void Resolve(ResolveValueT aResolveValue, const char* aResolveSite)
+  {
+    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(IsPending());
+    PROMISE_LOG("%s resolving MediaPromise (%p created at %s)", aResolveSite, this, mCreationSite);
+    mResolveValue.emplace(aResolveValue);
+    DispatchAll();
+  }
+
+  void Reject(RejectValueT aRejectValue, const char* aRejectSite)
+  {
+    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(IsPending());
+    PROMISE_LOG("%s rejecting MediaPromise (%p created at %s)", aRejectSite, this, mCreationSite);
+    mRejectValue.emplace(aRejectValue);
+    DispatchAll();
+  }
+};
+
 /*
  * Class to encapsulate a promise for a particular role. Use this as the member
  * variable for a class whose method returns a promise.
  */
 template<typename PromiseType>
 class MediaPromiseHolder
 {
 public:
@@ -418,40 +440,40 @@ public:
 
   ~MediaPromiseHolder() { MOZ_ASSERT(!mPromise); }
 
   already_AddRefed<PromiseType> Ensure(const char* aMethodName) {
     if (mMonitor) {
       mMonitor->AssertCurrentThreadOwns();
     }
     if (!mPromise) {
-      mPromise = new PromiseType(aMethodName);
+      mPromise = new (typename PromiseType::Private)(aMethodName);
     }
-    nsRefPtr<PromiseType> p = mPromise;
+    nsRefPtr<PromiseType> p = mPromise.get();
     return p.forget();
   }
 
   // Provide a Monitor that should always be held when accessing this instance.
   void SetMonitor(Monitor* aMonitor) { mMonitor = aMonitor; }
 
   bool IsEmpty()
   {
     if (mMonitor) {
       mMonitor->AssertCurrentThreadOwns();
     }
     return !mPromise;
   }
 
-  already_AddRefed<PromiseType> Steal()
+  already_AddRefed<typename PromiseType::Private> Steal()
   {
     if (mMonitor) {
       mMonitor->AssertCurrentThreadOwns();
     }
 
-    nsRefPtr<PromiseType> p = mPromise;
+    nsRefPtr<typename PromiseType::Private> p = mPromise;
     mPromise = nullptr;
     return p.forget();
   }
 
   void Resolve(typename PromiseType::ResolveValueType aResolveValue,
                const char* aMethodName)
   {
     if (mMonitor) {
@@ -486,17 +508,17 @@ public:
   {
     if (!IsEmpty()) {
       Reject(aRejectValue, aMethodName);
     }
   }
 
 private:
   Monitor* mMonitor;
-  nsRefPtr<PromiseType> mPromise;
+  nsRefPtr<typename PromiseType::Private> mPromise;
 };
 
 /*
  * Class to encapsulate a MediaPromise::Consumer reference. Use this as the member
  * variable for a class waiting on a media promise.
  */
 template<typename PromiseType>
 class MediaPromiseConsumerHolder
@@ -586,41 +608,41 @@ protected:
   Arg1Type mArg1;
   Arg2Type mArg2;
 };
 
 template<typename PromiseType>
 class ProxyRunnable : public nsRunnable
 {
 public:
-  ProxyRunnable(PromiseType* aProxyPromise, MethodCallBase<PromiseType>* aMethodCall)
+  ProxyRunnable(typename PromiseType::Private* aProxyPromise, MethodCallBase<PromiseType>* aMethodCall)
     : mProxyPromise(aProxyPromise), mMethodCall(aMethodCall) {}
 
   NS_IMETHODIMP Run()
   {
     nsRefPtr<PromiseType> p = mMethodCall->Invoke();
     mMethodCall = nullptr;
     p->ChainTo(mProxyPromise.forget(), "<Proxy Promise>");
     return NS_OK;
   }
 
 private:
-  nsRefPtr<PromiseType> mProxyPromise;
+  nsRefPtr<typename PromiseType::Private> mProxyPromise;
   nsAutoPtr<MethodCallBase<PromiseType>> mMethodCall;
 };
 
 template<typename PromiseType, typename TargetType>
 static nsRefPtr<PromiseType>
 ProxyInternal(TargetType* aTarget, MethodCallBase<PromiseType>* aMethodCall, const char* aCallerName)
 {
-  nsRefPtr<PromiseType> p = new PromiseType(aCallerName);
+  nsRefPtr<typename PromiseType::Private> p = new (typename PromiseType::Private)(aCallerName);
   nsRefPtr<ProxyRunnable<PromiseType>> r = new ProxyRunnable<PromiseType>(p, aMethodCall);
   nsresult rv = detail::DispatchMediaPromiseRunnable(aTarget, r);
   MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
-  return p;
+  return Move(p);
 }
 
 } // namespace detail
 
 template<typename PromiseType, typename TargetType, typename ThisType>
 static nsRefPtr<PromiseType>
 ProxyMediaCall(TargetType* aTarget, ThisType* aThisVal, const char* aCallerName,
                nsRefPtr<PromiseType>(ThisType::*aMethod)())