Bug 1724106 - Clarify threading usage when getting media debug info. r=alwu

Differential Revision: https://phabricator.services.mozilla.com/D124104

dom/html/HTMLMediaElement.cpp

@@ -7385,6 +7385,8 @@ void HTMLMediaElement::AsyncRejectPendingPlayPromises(nsresult aError) {
 }
 
 void HTMLMediaElement::GetEMEInfo(dom::EMEDebugInfo& aInfo) {
+  MOZ_ASSERT(NS_IsMainThread(),
+             "MediaKeys expects to be interacted with on main thread!");
   if (!mMediaKeys) {
     return;
   }

dom/media/MediaDecoder.cpp

@@ -1376,6 +1376,7 @@ MediaDecoderOwner::NextFrameStatus MediaDecoder::NextFrameBufferedStatus() {
 }
 
 void MediaDecoder::GetDebugInfo(dom::MediaDecoderDebugInfo& aInfo) {
+  MOZ_ASSERT(NS_IsMainThread());
   CopyUTF8toUTF16(nsPrintfCString("%p", this), aInfo.mInstance);
   aInfo.mChannels = mInfo ? mInfo->mAudio.mChannels : 0;
   aInfo.mRate = mInfo ? mInfo->mAudio.mRate : 0;
@@ -1384,27 +1385,28 @@ void MediaDecoder::GetDebugInfo(dom::MediaDecoderDebugInfo& aInfo) {
   CopyUTF8toUTF16(MakeStringSpan(PlayStateStr()), aInfo.mPlayState);
   aInfo.mContainerType =
       NS_ConvertUTF8toUTF16(ContainerType().Type().AsString());
-  mReader->GetDebugInfo(aInfo.mReader);
 }
 
 RefPtr<GenericPromise> MediaDecoder::RequestDebugInfo(
     MediaDecoderDebugInfo& aInfo) {
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
+  if (!NS_IsMainThread()) {
+    // Run the request on the main thread if it's not already.
+    return InvokeAsync(AbstractThread::MainThread(), __func__,
+                       [this, self = RefPtr{this}, &aInfo]() {
+                         return RequestDebugInfo(aInfo);
+                       });
+  }
   GetDebugInfo(aInfo);
 
-  if (!GetStateMachine()) {
+  return mReader->RequestDebugInfo(aInfo.mReader)
-    return GenericPromise::CreateAndResolve(true, __func__);
+      ->Then(AbstractThread::MainThread(), __func__,
-  }
+             [this, self = RefPtr{this}, &aInfo] {
-
+               if (!GetStateMachine()) {
-  return GetStateMachine()
+                 return GenericPromise::CreateAndResolve(true, __func__);
-      ->RequestDebugInfo(aInfo.mStateMachine)
+               }
-      ->Then(
+               return GetStateMachine()->RequestDebugInfo(aInfo.mStateMachine);
-          AbstractThread::MainThread(), __func__,
+             });
-          []() { return GenericPromise::CreateAndResolve(true, __func__); },
-          []() {
-            MOZ_ASSERT_UNREACHABLE("Unexpected RequestDebugInfo() rejection");
-            return GenericPromise::CreateAndResolve(false, __func__);
-          });
 }
 
 void MediaDecoder::NotifyAudibleStateChanged() {

dom/media/MediaFormatReader.cpp

@@ -3096,7 +3096,23 @@ layers::ImageContainer* MediaFormatReader::GetImageContainer() {
                               : nullptr;
 }
 
+RefPtr<GenericPromise> MediaFormatReader::RequestDebugInfo(
+    dom::MediaFormatReaderDebugInfo& aInfo) {
+  if (!OnTaskQueue()) {
+    // Run the request on the task queue if it's not already.
+    return InvokeAsync(mTaskQueue, __func__,
+                       [this, self = RefPtr{this}, &aInfo] {
+                         return RequestDebugInfo(aInfo);
+                       });
+  }
+  GetDebugInfo(aInfo);
+  return GenericPromise::CreateAndResolve(true, __func__);
+}
+
 void MediaFormatReader::GetDebugInfo(dom::MediaFormatReaderDebugInfo& aInfo) {
+  MOZ_ASSERT(OnTaskQueue(),
+             "Don't call this off the task queue, it's going to touch a lot of "
+             "data members");
   nsCString result;
   nsAutoCString audioDecoderName("unavailable");
   nsAutoCString videoDecoderName = audioDecoderName;
@@ -3104,34 +3120,11 @@ void MediaFormatReader::GetDebugInfo(dom::MediaFormatReaderDebugInfo& aInfo) {
   nsAutoCString videoType("none");
 
   AudioInfo audioInfo;
-  {
-    MutexAutoLock lock(mAudio.mMutex);
-    if (HasAudio()) {
-      audioInfo = *mAudio.GetWorkingInfo()->GetAsAudioInfo();
-      audioDecoderName = mAudio.mDecoder ? mAudio.mDecoder->GetDescriptionName()
-                                         : mAudio.mDescription;
-      audioType = audioInfo.mMimeType;
-    }
-  }
-
-  VideoInfo videoInfo;
-  {
-    MutexAutoLock lock(mVideo.mMutex);
-    if (HasVideo()) {
-      videoInfo = *mVideo.GetWorkingInfo()->GetAsVideoInfo();
-      videoDecoderName = mVideo.mDecoder ? mVideo.mDecoder->GetDescriptionName()
-                                         : mVideo.mDescription;
-      videoType = videoInfo.mMimeType;
-    }
-  }
-
-  CopyUTF8toUTF16(audioDecoderName, aInfo.mAudioDecoderName);
-  CopyUTF8toUTF16(audioType, aInfo.mAudioType);
-  aInfo.mAudioChannels = audioInfo.mChannels;
-  aInfo.mAudioRate = audioInfo.mRate / 1000.0f;
-  aInfo.mAudioFramesDecoded = mAudio.mNumSamplesOutputTotal;
-
   if (HasAudio()) {
+    audioInfo = *mAudio.GetWorkingInfo()->GetAsAudioInfo();
+    audioDecoderName = mAudio.mDecoder ? mAudio.mDecoder->GetDescriptionName()
+                                       : mAudio.mDescription;
+    audioType = audioInfo.mMimeType;
     aInfo.mAudioState.mNeedInput = NeedInput(mAudio);
     aInfo.mAudioState.mHasPromise = mAudio.HasPromise();
     aInfo.mAudioState.mWaitingPromise = !mAudio.mWaitingPromise.IsEmpty();
@@ -3155,18 +3148,18 @@ void MediaFormatReader::GetDebugInfo(dom::MediaFormatReaderDebugInfo& aInfo) {
     aInfo.mAudioState.mLastStreamSourceID = mAudio.mLastStreamSourceID;
   }
 
-  CopyUTF8toUTF16(videoDecoderName, aInfo.mVideoDecoderName);
+  CopyUTF8toUTF16(audioDecoderName, aInfo.mAudioDecoderName);
-  CopyUTF8toUTF16(videoType, aInfo.mVideoType);
+  CopyUTF8toUTF16(audioType, aInfo.mAudioType);
-  aInfo.mVideoWidth =
+  aInfo.mAudioChannels = audioInfo.mChannels;
-      videoInfo.mDisplay.width < 0 ? 0 : videoInfo.mDisplay.width;
+  aInfo.mAudioRate = audioInfo.mRate / 1000.0f;
-  aInfo.mVideoHeight =
+  aInfo.mAudioFramesDecoded = mAudio.mNumSamplesOutputTotal;
-      videoInfo.mDisplay.height < 0 ? 0 : videoInfo.mDisplay.height;
-  aInfo.mVideoRate = mVideo.mMeanRate.Mean();
-  aInfo.mVideoHardwareAccelerated = VideoIsHardwareAccelerated();
-  aInfo.mVideoNumSamplesOutputTotal = mVideo.mNumSamplesOutputTotal;
-  aInfo.mVideoNumSamplesSkippedTotal = mVideo.mNumSamplesSkippedTotal;
 
+  VideoInfo videoInfo;
   if (HasVideo()) {
+    videoInfo = *mVideo.GetWorkingInfo()->GetAsVideoInfo();
+    videoDecoderName = mVideo.mDecoder ? mVideo.mDecoder->GetDescriptionName()
+                                       : mVideo.mDescription;
+    videoType = videoInfo.mMimeType;
     aInfo.mVideoState.mNeedInput = NeedInput(mVideo);
     aInfo.mVideoState.mHasPromise = mVideo.HasPromise();
     aInfo.mVideoState.mWaitingPromise = !mVideo.mWaitingPromise.IsEmpty();
@@ -3190,6 +3183,17 @@ void MediaFormatReader::GetDebugInfo(dom::MediaFormatReaderDebugInfo& aInfo) {
     aInfo.mVideoState.mLastStreamSourceID = mVideo.mLastStreamSourceID;
   }
 
+  CopyUTF8toUTF16(videoDecoderName, aInfo.mVideoDecoderName);
+  CopyUTF8toUTF16(videoType, aInfo.mVideoType);
+  aInfo.mVideoWidth =
+      videoInfo.mDisplay.width < 0 ? 0 : videoInfo.mDisplay.width;
+  aInfo.mVideoHeight =
+      videoInfo.mDisplay.height < 0 ? 0 : videoInfo.mDisplay.height;
+  aInfo.mVideoRate = mVideo.mMeanRate.Mean();
+  aInfo.mVideoHardwareAccelerated = VideoIsHardwareAccelerated();
+  aInfo.mVideoNumSamplesOutputTotal = mVideo.mNumSamplesOutputTotal;
+  aInfo.mVideoNumSamplesSkippedTotal = mVideo.mNumSamplesSkippedTotal;
+
   // Looking at dropped frames
   FrameStatisticsData stats = mFrameStats->GetFrameStatisticsData();
   aInfo.mFrameStats.mDroppedDecodedFrames = stats.mDroppedDecodedFrames;

dom/media/MediaFormatReader.h

@@ -16,6 +16,7 @@
 #  include "SeekTarget.h"
 #  include "mozilla/Atomics.h"
 #  include "mozilla/Maybe.h"
+#  include "mozilla/MozPromise.h"
 #  include "mozilla/Mutex.h"
 #  include "mozilla/StateMirroring.h"
 #  include "mozilla/StaticPrefs_media.h"
@@ -182,9 +183,11 @@ class MediaFormatReader final
 
   RefPtr<SetCDMPromise> SetCDMProxy(CDMProxy* aProxy);
 
-  // Returns a MediaDebugInfo structure
+  // Requests that the MediaFormatReader populates aInfo with debug information.
-  // Used for debugging purposes.
+  // This may be done asynchronously, and aInfo should *not* be accessed by the
-  void GetDebugInfo(dom::MediaFormatReaderDebugInfo& aInfo);
+  // caller until the returned promise is resolved or rejected.
+  RefPtr<GenericPromise> RequestDebugInfo(
+      dom::MediaFormatReaderDebugInfo& aInfo);
 
   // Switch the video decoder to NullDecoderModule. It might takes effective
   // since a few samples later depends on how much demuxed samples are already
@@ -813,6 +816,8 @@ class MediaFormatReader final
   MozPromiseHolder<SetCDMPromise> mSetCDMPromise;
   TrackSet mSetCDMForTracks{};
   bool IsDecoderWaitingForCDM(TrackType aTrack);
+
+  void GetDebugInfo(dom::MediaFormatReaderDebugInfo& aInfo);
 };
 
 }  // namespace mozilla

dom/media/mediasource/MediaSource.cpp

@@ -629,9 +629,17 @@ already_AddRefed<Promise> MediaSource::MozDebugReaderData(ErrorResult& aRv) {
     return nullptr;
   }
   MOZ_ASSERT(domPromise);
-  MediaSourceDecoderDebugInfo info;
+  UniquePtr<MediaSourceDecoderDebugInfo> info =
-  mDecoder->GetDebugInfo(info);
+      MakeUnique<MediaSourceDecoderDebugInfo>();
-  domPromise->MaybeResolve(info);
+  mDecoder->RequestDebugInfo(*info)->Then(
+      mAbstractMainThread, __func__,
+      [domPromise, infoPtr = std::move(info)] {
+        domPromise->MaybeResolve(infoPtr.get());
+      },
+      [] {
+        MOZ_ASSERT_UNREACHABLE("Unexpected rejection while getting debug data");
+      });
+
   return domPromise.forget();
 }
 

dom/media/mediasource/MediaSourceDecoder.cpp

@@ -223,11 +223,22 @@ void MediaSourceDecoder::SetMediaSourceDuration(double aDuration) {
   }
 }
 
-void MediaSourceDecoder::GetDebugInfo(dom::MediaSourceDecoderDebugInfo& aInfo) {
+RefPtr<GenericPromise> MediaSourceDecoder::RequestDebugInfo(
-  if (mReader && mDemuxer) {
+    dom::MediaSourceDecoderDebugInfo& aInfo) {
-    mReader->GetDebugInfo(aInfo.mReader);
+  // This should be safe to call off main thead, but there's no such usage at
-    mDemuxer->GetDebugInfo(aInfo.mDemuxer);
+  // time of writing. Can be carefully relaxed if needed.
+  MOZ_ASSERT(NS_IsMainThread(), "Expects to be called on main thread.");
+  if (mReader) {
+    return mReader->RequestDebugInfo(aInfo.mReader)
+        ->Then(GetCurrentSerialEventTarget(), __func__,
+               [this, self = RefPtr<MediaSourceDecoder>{this}, &aInfo] {
+                 if (mDemuxer) {
+                   mDemuxer->GetDebugInfo(aInfo.mDemuxer);
+                 }
+                 return GenericPromise::CreateAndResolve(true, __func__);
+               });
   }
+  return GenericPromise::CreateAndResolve(true, __func__);
 }
 
 double MediaSourceDecoder::GetDuration() {

dom/media/mediasource/MediaSourceDecoder.h

@@ -54,9 +54,11 @@ class MediaSourceDecoder : public MediaDecoder,
 
   bool IsTransportSeekable() override { return true; }
 
-  // Returns a structure describing the state of the MediaSource internal
+  // Requests that the MediaSourceDecoder populates aInfo with debug
-  // buffered data. Used for debugging purposes.
+  // information. This may be done asynchronously, and aInfo should *not* be
-  void GetDebugInfo(dom::MediaSourceDecoderDebugInfo& aInfo);
+  // accessed by the caller until the returned promise is resolved or rejected.
+  RefPtr<GenericPromise> RequestDebugInfo(
+      dom::MediaSourceDecoderDebugInfo& aInfo);
 
   void AddSizeOfResources(ResourceSizes* aSizes) override;