nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1859200 permit AppendMessage() with other ControlMessageInterface objects r=pehrsons

Browse source 
A similar assertion will be added to new methods for other
ControlMessageInterface objects in a subsequent patch.

MediaTrackGraphImpl::OpenAudioInput() and
AudioListener::SendListenerEngineEvent() are two of a number of cases that
construct a ControlMessage with a null track.

Differential Revision: https://phabricator.services.mozilla.com/D191061
This commit is contained in:
Karl Tomlinson 2023-10-17 00:02:50 +00:00
parent ce078b9839
commit 8a25ccdeef
4 changed files with 28 additions and 23 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

12
dom/media/MediaTrackGraph.cpp
View file

@ -1284,7 +1284,7 @@ void MediaTrackGraphImpl::ProduceDataForTracksBlockByBlock(
} }
void MediaTrackGraphImpl::RunMessageAfterProcessing( void MediaTrackGraphImpl::RunMessageAfterProcessing(
UniquePtr<ControlMessage> aMessage) { UniquePtr<ControlMessageInterface> aMessage) {
MOZ_ASSERT(OnGraphThread()); MOZ_ASSERT(OnGraphThread());
if (mFrontMessageQueue.IsEmpty()) { if (mFrontMessageQueue.IsEmpty()) {
@ -1303,7 +1303,7 @@ void MediaTrackGraphImpl::RunMessagesInQueue() {
// batch corresponding to an event loop task). This isolates the performance // batch corresponding to an event loop task). This isolates the performance
// of different scripts to some extent. // of different scripts to some extent.
for (uint32_t i = 0; i < mFrontMessageQueue.Length(); ++i) { for (uint32_t i = 0; i < mFrontMessageQueue.Length(); ++i) {
nsTArray<UniquePtr<ControlMessage>>& messages = nsTArray<UniquePtr<ControlMessageInterface>>& messages =
mFrontMessageQueue[i].mMessages; mFrontMessageQueue[i].mMessages;
for (uint32_t j = 0; j < messages.Length(); ++j) { for (uint32_t j = 0; j < messages.Length(); ++j) {
@ -1868,7 +1868,8 @@ void MediaTrackGraphImpl::RunInStableState(bool aSourceIsMTG) {
// When we're doing a forced shutdown, pending control messages may be // When we're doing a forced shutdown, pending control messages may be
// run on the main thread via RunDuringShutdown. Those messages must // run on the main thread via RunDuringShutdown. Those messages must
// run without the graph monitor being held. So, we collect them here. // run without the graph monitor being held. So, we collect them here.
nsTArray<UniquePtr<ControlMessage>> controlMessagesToRunDuringShutdown; nsTArray<UniquePtr<ControlMessageInterface>>
controlMessagesToRunDuringShutdown;
{ {
MonitorAutoLock lock(mMonitor); MonitorAutoLock lock(mMonitor);
@ -2031,10 +2032,9 @@ void MediaTrackGraphImpl::SignalMainThreadCleanup() {
EnsureStableStateEventPosted(); EnsureStableStateEventPosted();
} }
void MediaTrackGraphImpl::AppendMessage(UniquePtr<ControlMessage> aMessage) { void MediaTrackGraphImpl::AppendMessage(
UniquePtr<ControlMessageInterface> aMessage) {
MOZ_ASSERT(NS_IsMainThread(), "main thread only"); MOZ_ASSERT(NS_IsMainThread(), "main thread only");
MOZ_RELEASE_ASSERT(!aMessage->GetTrack() ||
!aMessage->GetTrack()->IsDestroyed());
MOZ_DIAGNOSTIC_ASSERT(mMainThreadTrackCount > 0 || mMainThreadPortCount > 0); MOZ_DIAGNOSTIC_ASSERT(mMainThreadTrackCount > 0 || mMainThreadPortCount > 0);
if (!mGraphDriverRunning && if (!mGraphDriverRunning &&

24
dom/media/MediaTrackGraphImpl.h
View file

@ -72,12 +72,14 @@ struct TrackUpdate {
* This represents a message run on the graph thread to modify track or graph * 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 * state. These are passed from main thread to graph thread through
* AppendMessage(), or scheduled on the graph thread with * AppendMessage(), or scheduled on the graph thread with
* RunMessageAfterProcessing(). A ControlMessage * RunMessageAfterProcessing(). A ControlMessage often has
* always has a weak reference to a particular affected track. * a weak reference to a particular affected track.
*/ */
class ControlMessage : public MediaTrack::ControlMessageInterface { class ControlMessage : public MediaTrack::ControlMessageInterface {
public: public:
explicit ControlMessage(MediaTrack* aTrack) : mTrack(aTrack) {} explicit ControlMessage(MediaTrack* aTrack) : mTrack(aTrack) {
MOZ_RELEASE_ASSERT(!aTrack || !NS_IsMainThread() || !aTrack->IsDestroyed());
}
MediaTrack* GetTrack() { return mTrack; } MediaTrack* GetTrack() { return mTrack; }
@ -90,7 +92,7 @@ class ControlMessage : public MediaTrack::ControlMessageInterface {
class MessageBlock { class MessageBlock {
public: public:
nsTArray<UniquePtr<ControlMessage>> mMessages; nsTArray<UniquePtr<MediaTrack::ControlMessageInterface>> mMessages;
}; };
/** /**
@ -180,10 +182,10 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
*/ */
void ApplyTrackUpdate(TrackUpdate* aUpdate) MOZ_REQUIRES(mMonitor); void ApplyTrackUpdate(TrackUpdate* aUpdate) MOZ_REQUIRES(mMonitor);
/** /**
* Append a ControlMessage to the message queue. This queue is drained * Append a control message to the message queue. This queue is drained
* during RunInStableState; the messages will run on the graph thread. * during RunInStableState; the messages will run on the graph thread.
*/ */
virtual void AppendMessage(UniquePtr<ControlMessage> aMessage); virtual void AppendMessage(UniquePtr<ControlMessageInterface> aMessage);
/** /**
* Dispatches a runnable from any thread to the correct main thread for this * Dispatches a runnable from any thread to the correct main thread for this
@ -330,7 +332,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
* Schedules |aMessage| to run after processing, at a time when graph state * Schedules |aMessage| to run after processing, at a time when graph state
* can be changed. Graph thread. * can be changed. Graph thread.
*/ */
void RunMessageAfterProcessing(UniquePtr<ControlMessage> aMessage); void RunMessageAfterProcessing(UniquePtr<ControlMessageInterface> aMessage);
/** /**
* Resolve the GraphStartedPromise when the driver has started processing on * Resolve the GraphStartedPromise when the driver has started processing on
@ -664,7 +666,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
* Main-thread view of the number of ports in this graph, to catch bugs. * Main-thread view of the number of ports in this graph, to catch bugs.
* *
* When this becomes zero, and mMainThreadTrackCount is 0, the graph is * When this becomes zero, and mMainThreadTrackCount is 0, the graph is
* marked as forbidden to add more ControlMessages to. It will be shut down * marked as forbidden to add more control messages to. It will be shut down
* shortly after. * shortly after.
*/ */
size_t mMainThreadPortCount = 0; size_t mMainThreadPortCount = 0;
@ -772,7 +774,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
// MediaTrackGraph normally does its work without holding mMonitor, so it is // MediaTrackGraph normally does its work without holding mMonitor, so it is
// not safe to just grab mMonitor from some thread and start monkeying with // not safe to just grab mMonitor from some thread and start monkeying with
// the graph. Instead, communicate with the graph thread using provided // the graph. Instead, communicate with the graph thread using provided
// mechanisms such as the ControlMessage queue. // mechanisms such as the control message queue.
Monitor mMonitor; Monitor mMonitor;
// Data guarded by mMonitor (must always be accessed with mMonitor held, // Data guarded by mMonitor (must always be accessed with mMonitor held,
@ -925,7 +927,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
* immediately because we want all messages between stable states to be * immediately because we want all messages between stable states to be
* processed as an atomic batch. * processed as an atomic batch.
*/ */
nsTArray<UniquePtr<ControlMessage>> mCurrentTaskMessageQueue; nsTArray<UniquePtr<ControlMessageInterface>> mCurrentTaskMessageQueue;
/** /**
* True from when RunInStableState sets mLifecycleState to LIFECYCLE_RUNNING, * True from when RunInStableState sets mLifecycleState to LIFECYCLE_RUNNING,
* until RunInStableState has determined that mLifecycleState is > * until RunInStableState has determined that mLifecycleState is >
@ -995,7 +997,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
#ifdef DEBUG #ifdef DEBUG
/** /**
* Used to assert when AppendMessage() runs ControlMessages synchronously. * Used to assert when AppendMessage() runs control messages synchronously.
*/ */
bool mCanRunMessagesSynchronously; bool mCanRunMessagesSynchronously;
#endif #endif

13
dom/media/gtest/TestAudioDecoderInputTrack.cpp
View file

@ -21,6 +21,7 @@ using namespace mozilla::media;
using testing::AssertionResult; using testing::AssertionResult;
using testing::NiceMock; using testing::NiceMock;
using testing::Return; using testing::Return;
using ControlMessageInterface = MediaTrack::ControlMessageInterface;
constexpr uint32_t kNoFlags = 0; constexpr uint32_t kNoFlags = 0;
constexpr TrackRate kRate = 44100; constexpr TrackRate kRate = 44100;
@ -40,7 +41,7 @@ class MockTestGraph : public MediaTrackGraphImpl {
} }
MOCK_CONST_METHOD0(OnGraphThread, bool()); MOCK_CONST_METHOD0(OnGraphThread, bool());
MOCK_METHOD1(AppendMessage, void(UniquePtr<ControlMessage>)); MOCK_METHOD1(AppendMessage, void(UniquePtr<ControlMessageInterface>));
protected: protected:
~MockTestGraph() = default; ~MockTestGraph() = default;
@ -320,8 +321,9 @@ TEST_F(TestAudioDecoderInputTrack, VolumeChange) {
// one for setting the track's volume, another for the track destruction. // one for setting the track's volume, another for the track destruction.
EXPECT_CALL(*mGraph, AppendMessage) EXPECT_CALL(*mGraph, AppendMessage)
.Times(2) .Times(2)
.WillOnce([](UniquePtr<ControlMessage> aMessage) { aMessage->Run(); }) .WillOnce(
.WillOnce([](UniquePtr<ControlMessage> aMessage) {}); [](UniquePtr<ControlMessageInterface> aMessage) { aMessage->Run(); })
.WillOnce([](UniquePtr<ControlMessageInterface> aMessage) {});
// The default volume is 1.0. // The default volume is 1.0.
float expectedVolume = 1.0; float expectedVolume = 1.0;
@ -419,8 +421,9 @@ TEST_F(TestAudioDecoderInputTrack, PlaybackRateChange) {
// one for setting the track's playback, another for the track destruction. // one for setting the track's playback, another for the track destruction.
EXPECT_CALL(*mGraph, AppendMessage) EXPECT_CALL(*mGraph, AppendMessage)
.Times(2) .Times(2)
.WillOnce([](UniquePtr<ControlMessage> aMessage) { aMessage->Run(); }) .WillOnce(
.WillOnce([](UniquePtr<ControlMessage> aMessage) {}); [](UniquePtr<ControlMessageInterface> aMessage) { aMessage->Run(); })
.WillOnce([](UniquePtr<ControlMessageInterface> aMessage) {});
// Changing the playback rate. // Changing the playback rate.
float expectedPlaybackRate = 2.0; float expectedPlaybackRate = 2.0;

2
dom/media/gtest/TestDeviceInputTrack.cpp
View file

@ -39,7 +39,7 @@ class MockGraphImpl : public MediaTrackGraphImpl {
} }
MOCK_CONST_METHOD0(OnGraphThread, bool()); MOCK_CONST_METHOD0(OnGraphThread, bool());
MOCK_METHOD1(AppendMessage, void(UniquePtr<ControlMessage>)); MOCK_METHOD1(AppendMessage, void(UniquePtr<ControlMessageInterface>));
protected: protected:
~MockGraphImpl() = default; ~MockGraphImpl() = default;