From 4b06c284a3c296824a612349fb043bcce0d4d560 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Mon, 9 Feb 2015 18:47:49 -0800 Subject: [PATCH] Bug 1129877 - Separate the creator and consumer APIs for MediaPromise. v2 r=mattwoodrow This causes the buggy code that caused the crash to fail to compile. --- dom/media/MediaPromise.h | 92 +++++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 35 deletions(-) diff --git a/dom/media/MediaPromise.h b/dom/media/MediaPromise.h index 0c4cb329f1d8..cb26ac38711c 100644 --- a/dom/media/MediaPromise.h +++ b/dom/media/MediaPromise.h @@ -62,6 +62,10 @@ public: 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") @@ -70,20 +74,30 @@ public: 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 CreateAndResolve(ResolveValueType aResolveValue, const char* aResolveSite) { - nsRefPtr p = new MediaPromise(aResolveSite); + nsRefPtr p = new MediaPromise::Private(aResolveSite); p->Resolve(aResolveValue, aResolveSite); - return p; + return Move(p); } static nsRefPtr CreateAndReject(RejectValueType aRejectValue, const char* aRejectSite) { - nsRefPtr p = new MediaPromise(aRejectSite); + nsRefPtr p = new MediaPromise::Private(aRejectSite); p->Reject(aRejectValue, aRejectSite); - return p; + return Move(p); } class Consumer @@ -328,12 +342,12 @@ public: return; } - void ChainTo(already_AddRefed aChainedPromise, const char* aCallSite) + void ChainTo(already_AddRefed aChainedPromise, const char* aCallSite) { MutexAutoLock lock(mMutex); MOZ_DIAGNOSTIC_ASSERT(!IsExclusive || !mHaveConsumer); mHaveConsumer = true; - nsRefPtr chainedPromise = aChainedPromise; + nsRefPtr chainedPromise = aChainedPromise; PROMISE_LOG("%s invoking Chain() [this=%p, chainedPromise=%p, isPending=%d]", aCallSite, this, chainedPromise.get(), (int) IsPending()); if (!IsPending()) { @@ -343,24 +357,6 @@ public: } } - 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() @@ -377,7 +373,7 @@ protected: mChainedPromises.Clear(); } - void ForwardTo(MediaPromise* aOther) + void ForwardTo(Private* aOther) { MOZ_ASSERT(!IsPending()); if (mResolveValue.isSome()) { @@ -400,10 +396,36 @@ protected: Maybe mResolveValue; Maybe mRejectValue; nsTArray> mThenValues; - nsTArray> mChainedPromises; + nsTArray> mChainedPromises; bool mHaveConsumer; }; +template +class MediaPromise::Private + : public MediaPromise +{ +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. @@ -422,9 +444,9 @@ public: mMonitor->AssertCurrentThreadOwns(); } if (!mPromise) { - mPromise = new PromiseType(aMethodName); + mPromise = new (typename PromiseType::Private)(aMethodName); } - nsRefPtr p = mPromise; + nsRefPtr p = mPromise.get(); return p.forget(); } @@ -439,13 +461,13 @@ public: return !mPromise; } - already_AddRefed Steal() + already_AddRefed Steal() { if (mMonitor) { mMonitor->AssertCurrentThreadOwns(); } - nsRefPtr p = mPromise; + nsRefPtr p = mPromise; mPromise = nullptr; return p.forget(); } @@ -490,7 +512,7 @@ public: private: Monitor* mMonitor; - nsRefPtr mPromise; + nsRefPtr mPromise; }; /* @@ -590,7 +612,7 @@ template class ProxyRunnable : public nsRunnable { public: - ProxyRunnable(PromiseType* aProxyPromise, MethodCallBase* aMethodCall) + ProxyRunnable(typename PromiseType::Private* aProxyPromise, MethodCallBase* aMethodCall) : mProxyPromise(aProxyPromise), mMethodCall(aMethodCall) {} NS_IMETHODIMP Run() @@ -602,7 +624,7 @@ public: } private: - nsRefPtr mProxyPromise; + nsRefPtr mProxyPromise; nsAutoPtr> mMethodCall; }; @@ -610,11 +632,11 @@ template static nsRefPtr ProxyInternal(TargetType* aTarget, MethodCallBase* aMethodCall, const char* aCallerName) { - nsRefPtr p = new PromiseType(aCallerName); + nsRefPtr p = new (typename PromiseType::Private)(aCallerName); nsRefPtr> r = new ProxyRunnable(p, aMethodCall); nsresult rv = detail::DispatchMediaPromiseRunnable(aTarget, r); MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv)); - return p; + return Move(p); } } // namespace detail