From ce169c07d139933ee7b02f7a7b8d36a648f44e8a Mon Sep 17 00:00:00 2001 From: Karl Tomlinson Date: Tue, 17 Oct 2023 01:40:42 +0000 Subject: [PATCH] Bug 1859200 add MediaTrack methods to queue a graph control message to execute a provided lambda expression r=pehrsons and use them in MediaTrack. Having the public API on the MediaTrack ensures that the caller has a track to keep the graph alive. The only behavior change is that "MediaTrack::DispatchToMainThreadStableState ControlMessage" is now traced even if in shutdown. Strong MediaTrack references are now held for refcounted-inside-lambda static analysis. Differential Revision: https://phabricator.services.mozilla.com/D191062 --- dom/media/MediaTrackGraph.cpp | 252 ++++++++++++---------------------- dom/media/MediaTrackGraph.h | 81 ++++++++++- 2 files changed, 164 insertions(+), 169 deletions(-) diff --git a/dom/media/MediaTrackGraph.cpp b/dom/media/MediaTrackGraph.cpp index 66319edbe819..88b659a40b9c 100644 --- a/dom/media/MediaTrackGraph.cpp +++ b/dom/media/MediaTrackGraph.cpp @@ -2219,26 +2219,21 @@ void MediaTrack::DestroyImpl() { void MediaTrack::Destroy() { // Keep this track alive until we leave this method RefPtr kungFuDeathGrip = this; - - class Message : public ControlMessage { - public: - explicit Message(MediaTrack* aTrack) : ControlMessage(aTrack) {} - void RunDuringShutdown() override { - TRACE("MediaTrack::Destroy ControlMessage"); - mTrack->RemoveAllResourcesAndListenersImpl(); - auto graph = mTrack->GraphImpl(); - mTrack->DestroyImpl(); - graph->RemoveTrackGraphThread(mTrack); - } - void Run() override { - mTrack->OnGraphThreadDone(); - RunDuringShutdown(); - } - }; // Keep a reference to the graph, since Message might RunDuringShutdown() // synchronously and make GraphImpl() invalid. RefPtr graph = GraphImpl(); - graph->AppendMessage(MakeUnique(this)); + + QueueControlOrShutdownMessage( + [self = RefPtr{this}, this](IsInShutdown aInShutdown) { + if (aInShutdown == IsInShutdown::No) { + OnGraphThreadDone(); + } + TRACE("MediaTrack::Destroy ControlMessage"); + RemoveAllResourcesAndListenersImpl(); + auto* graph = GraphImpl(); + DestroyImpl(); + graph->RemoveTrackGraphThread(this); + }); graph->RemoveTrack(this); // Message::RunDuringShutdown may have removed this track from the graph, // but our kungFuDeathGrip above will have kept this track alive if @@ -2251,20 +2246,13 @@ TrackTime MediaTrack::GetEnd() const { } void MediaTrack::AddAudioOutput(void* aKey) { - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, void* aKey) - : ControlMessage(aTrack), mKey(aKey) {} - void Run() override { - TRACE("MediaTrack::AddAudioOutputImpl ControlMessage"); - mTrack->AddAudioOutputImpl(mKey); - } - void* mKey; - }; if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this, aKey)); + QueueControlMessageWithNoShutdown([self = RefPtr{this}, this, aKey] { + TRACE("MediaTrack::AddAudioOutputImpl ControlMessage"); + AddAudioOutputImpl(aKey); + }); } void MediaTrackGraphImpl::SetAudioOutputVolume(MediaTrack* aTrack, void* aKey, @@ -2284,21 +2272,13 @@ void MediaTrack::SetAudioOutputVolumeImpl(void* aKey, float aVolume) { } void MediaTrack::SetAudioOutputVolume(void* aKey, float aVolume) { - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, void* aKey, float aVolume) - : ControlMessage(aTrack), mKey(aKey), mVolume(aVolume) {} - void Run() override { - TRACE("MediaTrack::SetAudioOutputVolumeImpl ControlMessage"); - mTrack->SetAudioOutputVolumeImpl(mKey, mVolume); - } - void* mKey; - float mVolume; - }; if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this, aKey, aVolume)); + QueueControlMessageWithNoShutdown([self = RefPtr{this}, this, aKey, aVolume] { + TRACE("MediaTrack::SetAudioOutputVolumeImpl ControlMessage"); + SetAudioOutputVolumeImpl(aKey, aVolume); + }); } void MediaTrack::AddAudioOutputImpl(void* aKey) { @@ -2312,56 +2292,37 @@ void MediaTrack::RemoveAudioOutputImpl(void* aKey) { } void MediaTrack::RemoveAudioOutput(void* aKey) { - class Message : public ControlMessage { - public: - explicit Message(MediaTrack* aTrack, void* aKey) - : ControlMessage(aTrack), mKey(aKey) {} - void Run() override { - TRACE("MediaTrack::RemoveAudioOutputImpl ControlMessage"); - mTrack->RemoveAudioOutputImpl(mKey); - } - void* mKey; - }; if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this, aKey)); + QueueControlMessageWithNoShutdown([self = RefPtr{this}, this, aKey] { + TRACE("MediaTrack::RemoveAudioOutputImpl ControlMessage"); + RemoveAudioOutputImpl(aKey); + }); } void MediaTrack::Suspend() { - class Message : public ControlMessage { - public: - explicit Message(MediaTrack* aTrack) : ControlMessage(aTrack) {} - void Run() override { - TRACE("MediaTrack::IncrementSuspendCount ControlMessage"); - mTrack->IncrementSuspendCount(); - } - }; - // This can happen if this method has been called asynchronously, and the // track has been destroyed since then. if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this)); + QueueControlMessageWithNoShutdown([self = RefPtr{this}, this] { + TRACE("MediaTrack::IncrementSuspendCount ControlMessage"); + IncrementSuspendCount(); + }); } void MediaTrack::Resume() { - class Message : public ControlMessage { - public: - explicit Message(MediaTrack* aTrack) : ControlMessage(aTrack) {} - void Run() override { - TRACE("MediaTrack::DecrementSuspendCount ControlMessage"); - mTrack->DecrementSuspendCount(); - } - }; - // This can happen if this method has been called asynchronously, and the // track has been destroyed since then. if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this)); + QueueControlMessageWithNoShutdown([self = RefPtr{this}, this] { + TRACE("MediaTrack::DecrementSuspendCount ControlMessage"); + DecrementSuspendCount(); + }); } void MediaTrack::AddListenerImpl( @@ -2381,21 +2342,15 @@ void MediaTrack::AddListenerImpl( } void MediaTrack::AddListener(MediaTrackListener* aListener) { - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, MediaTrackListener* aListener) - : ControlMessage(aTrack), mListener(aListener) {} - void Run() override { - TRACE("MediaTrack::AddListenerImpl ControlMessage"); - mTrack->AddListenerImpl(mListener.forget()); - } - RefPtr mListener; - }; MOZ_ASSERT(mSegment, "Segment-less tracks do not support listeners"); if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this, aListener)); + QueueControlMessageWithNoShutdown( + [self = RefPtr{this}, this, listener = RefPtr{aListener}]() mutable { + TRACE("MediaTrack::AddListenerImpl ControlMessage"); + AddListenerImpl(listener.forget()); + }); } void MediaTrack::RemoveListenerImpl(MediaTrackListener* aListener) { @@ -2410,31 +2365,21 @@ void MediaTrack::RemoveListenerImpl(MediaTrackListener* aListener) { RefPtr MediaTrack::RemoveListener( MediaTrackListener* aListener) { - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, MediaTrackListener* aListener) - : ControlMessage(aTrack), mListener(aListener) {} - void Run() override { - TRACE("MediaTrack::RemoveListenerImpl ControlMessage"); - mTrack->RemoveListenerImpl(mListener); - mRemovedPromise.Resolve(true, __func__); - } - void RunDuringShutdown() override { - // During shutdown we still want the listener's NotifyRemoved to be - // called, since not doing that might block shutdown of other modules. - Run(); - } - RefPtr mListener; - MozPromiseHolder mRemovedPromise; - }; - - UniquePtr message = MakeUnique(this, aListener); - RefPtr p = message->mRemovedPromise.Ensure(__func__); + MozPromiseHolder promiseHolder; + RefPtr p = promiseHolder.Ensure(__func__); if (mMainThreadDestroyed) { - message->mRemovedPromise.Reject(NS_ERROR_FAILURE, __func__); + promiseHolder.Reject(NS_ERROR_FAILURE, __func__); return p; } - GraphImpl()->AppendMessage(std::move(message)); + QueueControlOrShutdownMessage( + [self = RefPtr{this}, this, listener = RefPtr{aListener}, + promiseHolder = std::move(promiseHolder)](IsInShutdown) mutable { + TRACE("MediaTrack::RemoveListenerImpl ControlMessage"); + // During shutdown we still want the listener's NotifyRemoved to be + // called, since not doing that might block shutdown of other modules. + RemoveListenerImpl(listener); + promiseHolder.Resolve(true, __func__); + }); return p; } @@ -2448,20 +2393,14 @@ void MediaTrack::AddDirectListenerImpl( } void MediaTrack::AddDirectListener(DirectMediaTrackListener* aListener) { - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, DirectMediaTrackListener* aListener) - : ControlMessage(aTrack), mListener(aListener) {} - void Run() override { - TRACE("MediaTrack::AddDirectListenerImpl ControlMessage"); - mTrack->AddDirectListenerImpl(mListener.forget()); - } - RefPtr mListener; - }; if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this, aListener)); + QueueControlMessageWithNoShutdown( + [self = RefPtr{this}, this, listener = RefPtr{aListener}]() mutable { + TRACE("MediaTrack::AddDirectListenerImpl ControlMessage"); + AddDirectListenerImpl(listener.forget()); + }); } void MediaTrack::RemoveDirectListenerImpl(DirectMediaTrackListener* aListener) { @@ -2469,57 +2408,39 @@ void MediaTrack::RemoveDirectListenerImpl(DirectMediaTrackListener* aListener) { } void MediaTrack::RemoveDirectListener(DirectMediaTrackListener* aListener) { - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, DirectMediaTrackListener* aListener) - : ControlMessage(aTrack), mListener(aListener) {} - void Run() override { - TRACE("MediaTrack::RemoveDirectListenerImpl ControlMessage"); - mTrack->RemoveDirectListenerImpl(mListener); - } - void RunDuringShutdown() override { - // During shutdown we still want the listener's - // NotifyDirectListenerUninstalled to be called, since not doing that - // might block shutdown of other modules. - Run(); - } - RefPtr mListener; - }; if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this, aListener)); + QueueControlOrShutdownMessage( + [self = RefPtr{this}, this, listener = RefPtr{aListener}](IsInShutdown) { + TRACE("MediaTrack::RemoveDirectListenerImpl ControlMessage"); + // During shutdown we still want the listener's + // NotifyDirectListenerUninstalled to be called, since not doing that + // might block shutdown of other modules. + RemoveDirectListenerImpl(listener); + }); } void MediaTrack::RunAfterPendingUpdates( already_AddRefed aRunnable) { MOZ_ASSERT(NS_IsMainThread()); - MediaTrackGraphImpl* graph = GraphImpl(); - nsCOMPtr runnable(aRunnable); - - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, already_AddRefed aRunnable) - : ControlMessage(aTrack), mRunnable(aRunnable) {} - void Run() override { - TRACE("MediaTrack::DispatchToMainThreadStableState ControlMessage"); - mTrack->Graph()->DispatchToMainThreadStableState(mRunnable.forget()); - } - void RunDuringShutdown() override { - // Don't run mRunnable now as it may call AppendMessage() which would - // assume that there are no remaining controlMessagesToRunDuringShutdown. - MOZ_ASSERT(NS_IsMainThread()); - mTrack->GraphImpl()->Dispatch(mRunnable.forget()); - } - - private: - nsCOMPtr mRunnable; - }; - if (mMainThreadDestroyed) { return; } - graph->AppendMessage(MakeUnique(this, runnable.forget())); + QueueControlOrShutdownMessage( + [self = RefPtr{this}, this, + runnable = nsCOMPtr{aRunnable}](IsInShutdown aInShutdown) mutable { + TRACE("MediaTrack::DispatchToMainThreadStableState ControlMessage"); + if (aInShutdown == IsInShutdown::No) { + Graph()->DispatchToMainThreadStableState(runnable.forget()); + } else { + // Don't run mRunnable now as it may call AppendMessage() which would + // assume that there are no remaining + // controlMessagesToRunDuringShutdown. + MOZ_ASSERT(NS_IsMainThread()); + GraphImpl()->Dispatch(runnable.forget()); + } + }); } void MediaTrack::SetDisabledTrackModeImpl(DisabledTrackMode aMode) { @@ -2534,20 +2455,13 @@ void MediaTrack::SetDisabledTrackModeImpl(DisabledTrackMode aMode) { } void MediaTrack::SetDisabledTrackMode(DisabledTrackMode aMode) { - class Message : public ControlMessage { - public: - Message(MediaTrack* aTrack, DisabledTrackMode aMode) - : ControlMessage(aTrack), mMode(aMode) {} - void Run() override { - TRACE("MediaTrack::SetDisabledTrackModeImpl ControlMessage"); - mTrack->SetDisabledTrackModeImpl(mMode); - } - DisabledTrackMode mMode; - }; if (mMainThreadDestroyed) { return; } - GraphImpl()->AppendMessage(MakeUnique(this, aMode)); + QueueControlMessageWithNoShutdown([self = RefPtr{this}, this, aMode]() { + TRACE("MediaTrack::SetDisabledTrackModeImpl ControlMessage"); + SetDisabledTrackModeImpl(aMode); + }); } void MediaTrack::ApplyTrackDisabling(MediaSegment* aSegment, @@ -2630,6 +2544,12 @@ void MediaTrack::NotifyIfDisabledModeChangedFrom(DisabledTrackMode aOldMode) { } } +void MediaTrack::QueueMessage(UniquePtr aMessage) { + MOZ_ASSERT(NS_IsMainThread(), "Main thread only"); + MOZ_RELEASE_ASSERT(!IsDestroyed()); + GraphImpl()->AppendMessage(std::move(aMessage)); +} + SourceMediaTrack::SourceMediaTrack(MediaSegment::Type aType, TrackRate aSampleRate) : MediaTrack(aSampleRate, aType, diff --git a/dom/media/MediaTrackGraph.h b/dom/media/MediaTrackGraph.h index 85fb06d39033..b1114053eb26 100644 --- a/dom/media/MediaTrackGraph.h +++ b/dom/media/MediaTrackGraph.h @@ -315,6 +315,33 @@ class MediaTrack : public mozilla::LinkedListElement { mMainThreadListeners.RemoveElement(aListener); } + /** + * Append to the message queue a control message to execute a given lambda + * function with no parameters. The queue is drained during + * RunInStableState(). The lambda will be executed on the graph thread. + * The lambda will not be executed if the graph has been forced to shut + * down. + **/ + template + void QueueControlMessageWithNoShutdown(Function&& aFunction) { + QueueMessage(WrapUnique( + new ControlMessageWithNoShutdown(std::forward(aFunction)))); + } + + enum class IsInShutdown { No, Yes }; + /** + * Append to the message queue a control message to execute a given lambda + * function with a single IsInShutdown parameter. A No argument indicates + * execution on the thread of a graph that is still running. A Yes argument + * indicates execution on the main thread when the graph has been forced to + * shut down. + **/ + template + void QueueControlOrShutdownMessage(Function&& aFunction) { + QueueMessage(WrapUnique( + new ControlOrShutdownMessage(std::forward(aFunction)))); + } + /** * Ensure a runnable will run on the main thread after running all pending * updates that were sent from the graph thread or will be sent before the @@ -498,6 +525,14 @@ class MediaTrack : public mozilla::LinkedListElement { virtual void AdvanceTimeVaryingValuesToCurrentTime(GraphTime aCurrentTime, GraphTime aBlockedTime); + private: + template + class ControlMessageWithNoShutdown; + template + class ControlOrShutdownMessage; + + void QueueMessage(UniquePtr aMessage); + void NotifyMainThreadListeners() { NS_ASSERTION(NS_IsMainThread(), "Call only on main thread"); @@ -517,6 +552,7 @@ class MediaTrack : public mozilla::LinkedListElement { return true; } + protected: // Notifies listeners and consumers of the change in disabled mode when the // current combined mode is different from aMode. void NotifyIfDisabledModeChangedFrom(DisabledTrackMode aOldMode); @@ -1178,8 +1214,9 @@ class MediaTrackGraph { /** * This represents a message run on the graph thread to modify track or graph - * state. These are passed from main thread to graph thread through - * AppendMessage(), or scheduled on the graph thread with + * state. These are passed from main thread to graph thread by + * QueueControlMessageWithNoShutdown() or QueueControlOrShutdownMessage() + * through AppendMessage(), or scheduled on the graph thread with * RunMessageAfterProcessing(). */ class MediaTrack::ControlMessageInterface { @@ -1194,13 +1231,51 @@ class MediaTrack::ControlMessageInterface { // computed. virtual void Run() = 0; // RunDuringShutdown() is only relevant to messages generated on the main - // thread (for AppendMessage()). + // thread by QueueControlOrShutdownMessage() or for AppendMessage(). // When we're shutting down the application, most messages are ignored but // some cleanup messages should still be processed (on the main thread). // This must not add new control messages to the graph. virtual void RunDuringShutdown() {} }; +template +class MediaTrack::ControlMessageWithNoShutdown + : public ControlMessageInterface { + public: + explicit ControlMessageWithNoShutdown(Function&& aFunction) + : mFunction(std::forward(aFunction)) {} + + void Run() override { + static_assert(std::is_void_v, + "The lambda must return void!"); + mFunction(); + } + + private: + using StoredFunction = std::decay_t; + StoredFunction mFunction; +}; + +template +class MediaTrack::ControlOrShutdownMessage : public ControlMessageInterface { + public: + explicit ControlOrShutdownMessage(Function&& aFunction) + : mFunction(std::forward(aFunction)) {} + + void Run() override { + static_assert(std::is_void_v, + "The lambda must return void!"); + mFunction(IsInShutdown::No); + } + void RunDuringShutdown() override { mFunction(IsInShutdown::Yes); } + + private: + // The same lambda is used whether or not in shutdown so that captured + // variables are available in both cases. + using StoredFunction = std::decay_t; + StoredFunction mFunction; +}; + } // namespace mozilla #endif /* MOZILLA_MEDIATRACKGRAPH_H_ */