diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index b20ee757dbf3..b4ee06b7f31c 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -946,7 +946,7 @@ void HTMLMediaElement::NotifyMediaStreamTracksAvailable(DOMMediaStream* aStream) NotifyOwnerDocumentActivityChanged(); } - mReadyStateUpdater->Notify(); + mWatchManager.ManualNotify(&HTMLMediaElement::UpdateReadyStateInternal); } void HTMLMediaElement::LoadFromSourceChildren() @@ -2042,10 +2042,10 @@ HTMLMediaElement::LookupMediaElementURITable(nsIURI* aURI) HTMLMediaElement::HTMLMediaElement(already_AddRefed& aNodeInfo) : nsGenericHTMLElement(aNodeInfo), + mWatchManager(this), mCurrentLoadID(0), mNetworkState(nsIDOMHTMLMediaElement::NETWORK_EMPTY), mReadyState(nsIDOMHTMLMediaElement::HAVE_NOTHING, "HTMLMediaElement::mReadyState"), - mReadyStateUpdater("HTMLMediaElement::mReadyStateUpdater"), mLoadWaitStatus(NOT_WAITING), mVolume(1.0), mPreloadAction(PRELOAD_UNDEFINED), @@ -2110,11 +2110,10 @@ HTMLMediaElement::HTMLMediaElement(already_AddRefed& aNo NotifyOwnerDocumentActivityChanged(); MOZ_ASSERT(NS_IsMainThread()); - mReadyStateUpdater->AddWeakCallback(this, &HTMLMediaElement::UpdateReadyStateInternal); - mReadyStateUpdater->Watch(mDownloadSuspendedByCache); + mWatchManager.Watch(mDownloadSuspendedByCache, &HTMLMediaElement::UpdateReadyStateInternal); // Paradoxically, there is a self-edge whereby UpdateReadyStateInternal refuses // to run until mReadyState reaches at least HAVE_METADATA by some other means. - mReadyStateUpdater->Watch(mReadyState); + mWatchManager.Watch(mReadyState, &HTMLMediaElement::UpdateReadyStateInternal); } HTMLMediaElement::~HTMLMediaElement() @@ -3073,7 +3072,7 @@ void HTMLMediaElement::SetupSrcMediaStreamPlayback(DOMMediaStream* aStream) // playing a stream, we'll need to add a CombineWithPrincipal call here. mMediaStreamListener = new StreamListener(this, "HTMLMediaElement::mMediaStreamListener"); mMediaStreamSizeListener = new StreamSizeListener(this); - mReadyStateUpdater->Watch(*mMediaStreamListener); + mWatchManager.Watch(*mMediaStreamListener, &HTMLMediaElement::UpdateReadyStateInternal); GetSrcMediaStream()->AddListener(mMediaStreamListener); // Listen for an initial image size on mSrcStream so we can get results even @@ -3123,7 +3122,7 @@ void HTMLMediaElement::EndSrcMediaStreamPlayback() } // Kill its reference to this element - mReadyStateUpdater->Unwatch(*mMediaStreamListener); + mWatchManager.Unwatch(*mMediaStreamListener, &HTMLMediaElement::UpdateReadyStateInternal); mMediaStreamListener->Forget(); mMediaStreamListener = nullptr; mMediaStreamSizeListener->Forget(); @@ -3226,7 +3225,7 @@ void HTMLMediaElement::MetadataLoaded(const MediaInfo* aInfo, if (!aInfo->HasVideo()) { ResetState(); } else { - mReadyStateUpdater->Notify(); + mWatchManager.ManualNotify(&HTMLMediaElement::UpdateReadyStateInternal); } if (IsVideo() && aInfo->HasVideo()) { @@ -3887,7 +3886,7 @@ void HTMLMediaElement::UpdateMediaSize(const nsIntSize& aSize) } mMediaInfo.mVideo.mDisplay = aSize; - mReadyStateUpdater->Notify(); + mWatchManager.ManualNotify(&HTMLMediaElement::UpdateReadyStateInternal); } void HTMLMediaElement::UpdateInitialMediaSize(const nsIntSize& aSize) diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index f3b906d89da6..655c0f9efad0 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -217,6 +217,9 @@ public: // Dispatch events virtual nsresult DispatchAsyncEvent(const nsAString& aName) final override; + // Triggers a recomputation of readyState. + void UpdateReadyState() override { UpdateReadyStateInternal(); } + // Dispatch events that were raised while in the bfcache nsresult DispatchPendingMediaEvents(); @@ -642,17 +645,7 @@ protected: class StreamSizeListener; MediaDecoderOwner::NextFrameStatus NextFrameStatus(); - - void SetDecoder(MediaDecoder* aDecoder) - { - if (mDecoder) { - mReadyStateUpdater->Unwatch(mDecoder->ReadyStateWatchTarget()); - } - mDecoder = aDecoder; - if (mDecoder) { - mReadyStateUpdater->Watch(mDecoder->ReadyStateWatchTarget()); - } - } + void SetDecoder(MediaDecoder* aDecoder) { mDecoder = aDecoder; } virtual void GetItemValueText(DOMString& text) override; virtual void SetItemValueText(const nsAString& text) override; @@ -1032,6 +1025,9 @@ protected: // At most one of mDecoder and mSrcStream can be non-null. nsRefPtr mDecoder; + // State-watching manager. + WatchManager mWatchManager; + // A reference to the VideoFrameContainer which contains the current frame // of video to display. nsRefPtr mVideoFrameContainer; @@ -1102,8 +1098,6 @@ protected: nsMediaNetworkState mNetworkState; Watchable mReadyState; - WatcherHolder mReadyStateUpdater; - enum LoadAlgorithmState { // No load algorithm instance is waiting for a source to be added to the // media in order to continue loading. diff --git a/dom/media/MediaDecoder.cpp b/dom/media/MediaDecoder.cpp index a66c074d5d24..7d42ff98f559 100644 --- a/dom/media/MediaDecoder.cpp +++ b/dom/media/MediaDecoder.cpp @@ -585,7 +585,7 @@ bool MediaDecoder::IsInfinite() } MediaDecoder::MediaDecoder() : - mReadyStateWatchTarget("MediaDecoder::mReadyStateWatchTarget"), + mWatchManager(this), mDecoderPosition(0), mPlaybackPosition(0), mCurrentTime(0.0), @@ -636,8 +636,8 @@ MediaDecoder::MediaDecoder() : mAudioChannel = AudioChannelService::GetDefaultAudioChannel(); // Initialize watchers. - mReadyStateWatchTarget->Watch(mPlayState); - mReadyStateWatchTarget->Watch(mNextFrameStatus); + mWatchManager.Watch(mPlayState, &MediaDecoder::UpdateReadyState); + mWatchManager.Watch(mNextFrameStatus, &MediaDecoder::UpdateReadyState); } bool MediaDecoder::Init(MediaDecoderOwner* aOwner) @@ -1648,7 +1648,7 @@ void MediaDecoder::NotifyDataArrived(const char* aBuffer, uint32_t aLength, int6 // ReadyState computation depends on MediaDecoder::CanPlayThrough, which // depends on the download rate. - mReadyStateWatchTarget->Notify(); + UpdateReadyState(); } // Provide access to the state machine object diff --git a/dom/media/MediaDecoder.h b/dom/media/MediaDecoder.h index efa1cda05861..cb3a63ec47b8 100644 --- a/dom/media/MediaDecoder.h +++ b/dom/media/MediaDecoder.h @@ -1028,7 +1028,12 @@ public: GetFrameStatistics().NotifyDecodedFrames(aParsed, aDecoded, aDropped); } - WatchTarget& ReadyStateWatchTarget() { return *mReadyStateWatchTarget; } + void UpdateReadyState() + { + if (mOwner) { + mOwner->UpdateReadyState(); + } + } virtual MediaDecoderOwner::NextFrameStatus NextFrameStatus() { return mNextFrameStatus; } @@ -1047,7 +1052,8 @@ protected: // Return true if the decoder has reached the end of playback bool IsEnded() const; - WatcherHolder mReadyStateWatchTarget; + // State-watching manager. + WatchManager mWatchManager; // NextFrameStatus, mirrored from the state machine. Mirror::Holder mNextFrameStatus; diff --git a/dom/media/MediaDecoderOwner.h b/dom/media/MediaDecoderOwner.h index 98b7ca9bfb02..42fe1cea8b82 100644 --- a/dom/media/MediaDecoderOwner.h +++ b/dom/media/MediaDecoderOwner.h @@ -24,6 +24,9 @@ public: // Dispatch an asynchronous event to the decoder owner virtual nsresult DispatchAsyncEvent(const nsAString& aName) = 0; + // Triggers a recomputation of readyState. + virtual void UpdateReadyState() = 0; + /** * Fires a timeupdate event. If aPeriodic is true, the event will only * be fired if we've not fired a timeupdate event (for any reason) in the diff --git a/dom/media/MediaDecoderStateMachine.cpp b/dom/media/MediaDecoderStateMachine.cpp index 21f47c507035..7b0960eb02a8 100644 --- a/dom/media/MediaDecoderStateMachine.cpp +++ b/dom/media/MediaDecoderStateMachine.cpp @@ -202,6 +202,7 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder, MediaDecoderReader* aReader, bool aRealTime) : mDecoder(aDecoder), + mWatchManager(this), mRealTime(aRealTime), mDispatchedStateMachine(false), mDelayedScheduler(this), @@ -210,7 +211,6 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder, mStartTime(-1), mEndTime(-1), mDurationSet(false), - mNextFrameStatusUpdater("MediaDecoderStateMachine::mNextFrameStatusUpdater"), mFragmentEndTime(-1), mReader(aReader), mCurrentFrameTime(0), @@ -265,11 +265,8 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder, mNextPlayState.Init(mTaskQueue, MediaDecoder::PLAY_STATE_PAUSED, "MediaDecoderStateMachine::mNextPlayState (Mirror)", aDecoder->CanonicalNextPlayState()); - // Skip the initial notification we get when we Watch the value, since we're - // not on the right thread yet. - mNextFrameStatusUpdater->Watch(mState); - mNextFrameStatusUpdater->Watch(mAudioCompleted); - mNextFrameStatusUpdater->AddWeakCallback(this, &MediaDecoderStateMachine::UpdateNextFrameStatus); + mWatchManager.Watch(mState, &MediaDecoderStateMachine::UpdateNextFrameStatus); + mWatchManager.Watch(mAudioCompleted, &MediaDecoderStateMachine::UpdateNextFrameStatus); static bool sPrefCacheInit = false; if (!sPrefCacheInit) { diff --git a/dom/media/MediaDecoderStateMachine.h b/dom/media/MediaDecoderStateMachine.h index 8d137e2915d0..5374e1a14672 100644 --- a/dom/media/MediaDecoderStateMachine.h +++ b/dom/media/MediaDecoderStateMachine.h @@ -775,6 +775,9 @@ public: // Task queue for running the state machine. nsRefPtr mTaskQueue; + // State-watching manager. + WatchManager mWatchManager; + // True is we are decoding a realtime stream, like a camera stream. bool mRealTime; @@ -903,7 +906,6 @@ public: // The status of our next frame. Mirrored on the main thread and used to // compute ready state. - WatcherHolder mNextFrameStatusUpdater; Canonical::Holder mNextFrameStatus; public: AbstractCanonical* CanonicalNextFrameStatus() { return &mNextFrameStatus; } diff --git a/dom/media/StateWatching.h b/dom/media/StateWatching.h index 6dfeab11dc47..e0bcebfb240e 100644 --- a/dom/media/StateWatching.h +++ b/dom/media/StateWatching.h @@ -43,8 +43,8 @@ * There are two basic pieces: * (1) Objects that can be watched for updates. These inherit WatchTarget. * (2) Objects that receive objects and trigger processing. These inherit - * AbstractWatcher. Note that some watchers may themselves be watched, - * allowing watchers to be composed together. + * AbstractWatcher. In the current machinery, these exist only internally + * within the WatchManager, though that could change. * * Note that none of this machinery is thread-safe - it must all happen on the * same owning thread. To solve multi-threaded use-cases, use state mirroring @@ -76,18 +76,7 @@ public: virtual void Notify() = 0; protected: - virtual ~AbstractWatcher() {} - virtual void CustomDestroy() {} - -private: - // Only the holder is allowed to invoke Destroy(). - friend class WatcherHolder; - void Destroy() - { - mDestroyed = true; - CustomDestroy(); - } - + virtual ~AbstractWatcher() { MOZ_ASSERT(mDestroyed); } bool mDestroyed; }; @@ -174,87 +163,128 @@ private: T mValue; }; -/* - * Watcher is the concrete AbstractWatcher implementation. It registers itself with - * various WatchTargets, and accepts any number of callbacks that will be - * invoked whenever any WatchTarget notifies of a change. It may also be watched - * by other watchers. - * - * When a notification is received, a runnable is passed as "direct" to the - * thread's tail dispatcher, which run directly (rather than via dispatch) when - * the tail dispatcher fires. All subsequent notifications are ignored until the - * runnable executes, triggering the updates and resetting the flags. - */ -class Watcher : public AbstractWatcher, public WatchTarget +// Manager class for state-watching. Declare one of these in any class for which +// you want to invoke method callbacks. +// +// Internally, WatchManager maintains one AbstractWatcher per callback method. +// Consumers invoke Watch/Unwatch on a particular (WatchTarget, Callback) tuple. +// This causes an AbstractWatcher for |Callback| to be instantiated if it doesn't +// already exist, and registers it with |WatchTarget|. +// +// Using Direct Tasks on the TailDispatcher, WatchManager ensures that we fire +// watch callbacks no more than once per task, once all other operations for that +// task have been completed. +// +// WatchManager is intended to be declared as a member of |OwnerType| +// objects. Given that, it and its owned objects can't hold permanent strong refs to +// the owner, since that would keep the owner alive indefinitely. Instead, it +// _only_ holds strong refs while waiting for Direct Tasks to fire. This ensures +// that everything is kept alive just long enough. +template +class WatchManager { public: - explicit Watcher(const char* aName) - : WatchTarget(aName), mNotifying(false) {} + typedef void(OwnerType::*CallbackMethod)(); + explicit WatchManager(OwnerType* aOwner) + : mOwner(aOwner) {} - void Notify() override + ~WatchManager() { - if (mNotifying) { - return; - } - mNotifying = true; - - // Queue up our notification jobs to run in a stable state. - nsCOMPtr r = NS_NewRunnableMethod(this, &Watcher::DoNotify); - AbstractThread::GetCurrent()->TailDispatcher().AddDirectTask(r.forget()); - } - - void Watch(WatchTarget& aTarget) { aTarget.AddWatcher(this); } - void Unwatch(WatchTarget& aTarget) { aTarget.RemoveWatcher(this); } - - template - void AddWeakCallback(ThisType* aThisVal, void(ThisType::*aMethod)()) - { - mCallbacks.AppendElement(NS_NewNonOwningRunnableMethod(aThisVal, aMethod)); - } - -protected: - void CustomDestroy() override { mCallbacks.Clear(); } - - void DoNotify() - { - MOZ_ASSERT(mNotifying); - mNotifying = false; - - // Notify dependent watchers. - NotifyWatchers(); - - for (size_t i = 0; i < mCallbacks.Length(); ++i) { - mCallbacks[i]->Run(); + for (size_t i = 0; i < mWatchers.Length(); ++i) { + mWatchers[i]->Destroy(); } } -private: - nsTArray> mCallbacks; - - bool mNotifying; -}; - -/* - * WatcherHolder encapsulates a Watcher and handles lifetime issues. Use it to - * holder watcher members. - */ -class WatcherHolder -{ -public: - explicit WatcherHolder(const char* aName) { mWatcher = new Watcher(aName); } - operator Watcher*() { return mWatcher; } - Watcher* operator->() { return mWatcher; } - - ~WatcherHolder() + void Watch(WatchTarget& aTarget, CallbackMethod aMethod) { - mWatcher->Destroy(); - mWatcher = nullptr; + aTarget.AddWatcher(&EnsureWatcher(aMethod)); + } + + void Unwatch(WatchTarget& aTarget, CallbackMethod aMethod) + { + PerCallbackWatcher* watcher = GetWatcher(aMethod); + MOZ_ASSERT(watcher); + aTarget.RemoveWatcher(watcher); + } + + void ManualNotify(CallbackMethod aMethod) + { + PerCallbackWatcher* watcher = GetWatcher(aMethod); + MOZ_ASSERT(watcher); + watcher->Notify(); } private: - nsRefPtr mWatcher; -}; + class PerCallbackWatcher : public AbstractWatcher + { + public: + PerCallbackWatcher(OwnerType* aOwner, CallbackMethod aMethod) + : mOwner(aOwner), mCallbackMethod(aMethod) {} + void Destroy() + { + mDestroyed = true; + mOwner = nullptr; + } + + void Notify() override + { + MOZ_DIAGNOSTIC_ASSERT(mOwner, "mOwner is only null after destruction, " + "at which point we shouldn't be notified"); + if (mStrongRef) { + // We've already got a notification job in the pipe. + return; + } + mStrongRef = mOwner; // Hold the owner alive while notifying. + + // Queue up our notification jobs to run in a stable state. + nsCOMPtr r = NS_NewRunnableMethod(this, &PerCallbackWatcher::DoNotify); + AbstractThread::GetCurrent()->TailDispatcher().AddDirectTask(r.forget()); + } + + bool CallbackMethodIs(CallbackMethod aMethod) const + { + return mCallbackMethod == aMethod; + } + + private: + ~PerCallbackWatcher() {} + + void DoNotify() + { + MOZ_ASSERT(mStrongRef); + nsRefPtr ref = mStrongRef.forget(); + ((*ref).*mCallbackMethod)(); + } + + OwnerType* mOwner; // Never null. + nsRefPtr mStrongRef; // Only non-null when notifying. + CallbackMethod mCallbackMethod; + }; + + PerCallbackWatcher* GetWatcher(CallbackMethod aMethod) + { + for (size_t i = 0; i < mWatchers.Length(); ++i) { + if (mWatchers[i]->CallbackMethodIs(aMethod)) { + return mWatchers[i]; + } + } + return nullptr; + } + + PerCallbackWatcher& EnsureWatcher(CallbackMethod aMethod) + { + PerCallbackWatcher* watcher = GetWatcher(aMethod); + if (watcher) { + return *watcher; + } + watcher = mWatchers.AppendElement(new PerCallbackWatcher(mOwner, aMethod))->get(); + return *watcher; + } + + nsTArray> mWatchers; + OwnerType* mOwner; +}; #undef WATCH_LOG