From 84171dfe8ca17dac86643dd1e1ebf64b0a6e5b63 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 3 Dec 2015 15:59:44 +0800 Subject: [PATCH] Bug 1230004. Part 2 - have MDSM::BeginShutdown return a promise and remove MDSM::mDecoder. r=cpearce. --- dom/media/MediaDecoder.cpp | 20 +++++--- dom/media/MediaDecoder.h | 5 +- dom/media/MediaDecoderStateMachine.cpp | 67 ++++++-------------------- dom/media/MediaDecoderStateMachine.h | 19 ++------ 4 files changed, 32 insertions(+), 79 deletions(-) diff --git a/dom/media/MediaDecoder.cpp b/dom/media/MediaDecoder.cpp index c4cd28e29b2c..49113f7c3bdb 100644 --- a/dom/media/MediaDecoder.cpp +++ b/dom/media/MediaDecoder.cpp @@ -615,13 +615,16 @@ MediaDecoder::Shutdown() // necessary to unblock the state machine thread if it's blocked, so // the asynchronous shutdown in nsDestroyStateMachine won't deadlock. if (mDecoderStateMachine) { - mDecoderStateMachine->DispatchShutdown(); mTimedMetadataListener.Disconnect(); mMetadataLoadedListener.Disconnect(); mFirstFrameLoadedListener.Disconnect(); mOnPlaybackEvent.Disconnect(); mOnSeekingStart.Disconnect(); mOnMediaNotSeekable.Disconnect(); + + mDecoderStateMachine->BeginShutdown()->Then( + AbstractThread::MainThread(), __func__, this, + &MediaDecoder::FinishShutdown, &MediaDecoder::FinishShutdown); } // Force any outstanding seek and byterange requests to complete @@ -668,6 +671,14 @@ MediaDecoder::OnPlaybackEvent(MediaEventType aEvent) } } +void +MediaDecoder::FinishShutdown() +{ + MOZ_ASSERT(NS_IsMainThread()); + mDecoderStateMachine->BreakCycles(); + SetStateMachine(nullptr); +} + MediaResourceCallback* MediaDecoder::GetResourceCallback() const { @@ -1539,13 +1550,6 @@ MediaDecoder::GetStateMachine() const { return mDecoderStateMachine; } -// Drop reference to state machine. Only called during shutdown dance. -void -MediaDecoder::BreakCycles() { - MOZ_ASSERT(NS_IsMainThread()); - SetStateMachine(nullptr); -} - void MediaDecoder::FireTimeUpdate() { diff --git a/dom/media/MediaDecoder.h b/dom/media/MediaDecoder.h index ed0846475162..5dcd07454ced 100644 --- a/dom/media/MediaDecoder.h +++ b/dom/media/MediaDecoder.h @@ -636,9 +636,6 @@ private: // position. int64_t GetDownloadPosition(); - // Drop reference to state machine. Only called during shutdown dance. - virtual void BreakCycles(); - // Notifies the element that decoding has failed. void DecodeError(); @@ -815,6 +812,8 @@ private: SetMediaSeekable(false); } + void FinishShutdown(); + MediaEventProducer mDataArrivedEvent; // The state machine object for handling the decoding. It is safe to diff --git a/dom/media/MediaDecoderStateMachine.cpp b/dom/media/MediaDecoderStateMachine.cpp index 0b9b996a9af4..7e3407970269 100644 --- a/dom/media/MediaDecoderStateMachine.cpp +++ b/dom/media/MediaDecoderStateMachine.cpp @@ -199,7 +199,6 @@ static void InitVideoQueuePrefs() { MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder, MediaDecoderReader* aReader, bool aRealTime) : - mDecoder(aDecoder), mDecoderID(aDecoder), mFrameStats(&aDecoder->GetFrameStatistics()), mVideoFrameContainer(aDecoder->GetVideoFrameContainer()), @@ -1304,7 +1303,8 @@ MediaDecoderStateMachine::SetDormant(bool aDormant) } } -void MediaDecoderStateMachine::Shutdown() +RefPtr +MediaDecoderStateMachine::Shutdown() { MOZ_ASSERT(OnTaskQueue()); @@ -1335,13 +1335,16 @@ void MediaDecoderStateMachine::Shutdown() mStartTimeRendezvous->Destroy(); } + DECODER_LOG("Shutdown started"); + // Put a task in the decode queue to shutdown the reader. // the queue to spin down. - InvokeAsync(DecodeTaskQueue(), mReader.get(), __func__, &MediaDecoderReader::Shutdown) + return InvokeAsync(DecodeTaskQueue(), mReader.get(), __func__, + &MediaDecoderReader::Shutdown) ->Then(OwnerThread(), __func__, this, &MediaDecoderStateMachine::FinishShutdown, - &MediaDecoderStateMachine::FinishShutdown); - DECODER_LOG("Shutdown started"); + &MediaDecoderStateMachine::FinishShutdown) + ->CompletionPromise(); } void MediaDecoderStateMachine::StartDecoding() @@ -2159,40 +2162,15 @@ MediaDecoderStateMachine::SeekCompleted() } } -class DecoderDisposer -{ -public: - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecoderDisposer) - DecoderDisposer(MediaDecoder* aDecoder, MediaDecoderStateMachine* aStateMachine) - : mDecoder(aDecoder), mStateMachine(aStateMachine) {} - - void OnTaskQueueShutdown() - { - MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(mStateMachine); - MOZ_ASSERT(mDecoder); - mStateMachine->BreakCycles(); - mDecoder->BreakCycles(); - mStateMachine = nullptr; - mDecoder = nullptr; - } - -private: - virtual ~DecoderDisposer() {} - RefPtr mDecoder; - RefPtr mStateMachine; -}; - -void -MediaDecoderStateMachine::DispatchShutdown() +RefPtr +MediaDecoderStateMachine::BeginShutdown() { mStreamSink->BeginShutdown(); - nsCOMPtr runnable = - NS_NewRunnableMethod(this, &MediaDecoderStateMachine::Shutdown); - OwnerThread()->Dispatch(runnable.forget()); + return InvokeAsync(OwnerThread(), this, __func__, + &MediaDecoderStateMachine::Shutdown); } -void +RefPtr MediaDecoderStateMachine::FinishShutdown() { MOZ_ASSERT(OnTaskQueue()); @@ -2233,25 +2211,8 @@ MediaDecoderStateMachine::FinishShutdown() MOZ_ASSERT(mState == DECODER_STATE_SHUTDOWN, "How did we escape from the shutdown state?"); - // We must daisy-chain these events to destroy the decoder. We must - // destroy the decoder on the main thread, but we can't destroy the - // decoder while this thread holds the decoder monitor. We can't - // dispatch an event to the main thread to destroy the decoder from - // here, as the event may run before the dispatch returns, and we - // hold the decoder monitor here. We also want to guarantee that the - // state machine is destroyed on the main thread, and so the - // event runner running this function (which holds a reference to the - // state machine) needs to finish and be released in order to allow - // that. So we dispatch an event to run after this event runner has - // finished and released its monitor/references. That event then will - // dispatch an event to the main thread to release the decoder and - // state machine. DECODER_LOG("Shutting down state machine task queue"); - RefPtr disposer = new DecoderDisposer(mDecoder, this); - OwnerThread()->BeginShutdown()->Then(AbstractThread::MainThread(), __func__, - disposer.get(), - &DecoderDisposer::OnTaskQueueShutdown, - &DecoderDisposer::OnTaskQueueShutdown); + return OwnerThread()->BeginShutdown(); } nsresult MediaDecoderStateMachine::RunStateMachine() diff --git a/dom/media/MediaDecoderStateMachine.h b/dom/media/MediaDecoderStateMachine.h index f9ed196cee1b..e677c73a4eca 100644 --- a/dom/media/MediaDecoderStateMachine.h +++ b/dom/media/MediaDecoderStateMachine.h @@ -167,7 +167,7 @@ public: // Set/Unset dormant state. void DispatchSetDormant(bool aDormant); - void DispatchShutdown(); + RefPtr BeginShutdown(); void DispatchStartBuffering() { @@ -218,14 +218,13 @@ public: OwnerThread()->Dispatch(r.forget()); } - // Drop reference to decoder. Only called during shutdown dance. + // Drop reference to mReader and mResource. Only called during shutdown dance. void BreakCycles() { MOZ_ASSERT(NS_IsMainThread()); if (mReader) { mReader->BreakCycles(); } mResource = nullptr; - mDecoder = nullptr; } TimedMetadataEventSource& TimedMetadataEvent() { @@ -284,9 +283,9 @@ private: RefPtr Seek(SeekTarget aTarget); - void Shutdown(); + RefPtr Shutdown(); - void FinishShutdown(); + RefPtr FinishShutdown(); // Update the playback position. This can result in a timeupdate event // and an invalidate of the frame being dispatched asynchronously if @@ -666,16 +665,6 @@ private: void AdjustAudioThresholds(); - // The decoder object that created this state machine. The state machine - // holds a strong reference to the decoder to ensure that the decoder stays - // alive once media element has started the decoder shutdown process, and has - // dropped its reference to the decoder. This enables the state machine to - // keep using the decoder's monitor until the state machine has finished - // shutting down, without fear of the monitor being destroyed. After - // shutting down, the state machine will then release this reference, - // causing the decoder to be destroyed. This is accessed on the decode, - // state machine, audio and main threads. - RefPtr mDecoder; void* const mDecoderID; const RefPtr mFrameStats; const RefPtr mVideoFrameContainer;