Bug 1417305. P1 - let callers of MediaResourceCallback::NotifyDataEnded() decide whether to call it synchronously. draft
authorJW Wang <jwwang@mozilla.com>
Fri, 10 Nov 2017 14:27:59 +0800
changeset 698004 8dd01cf049c7dfae898075b51bda1f3d80932f68
parent 698003 39a5baaa742e64bb15202cb05cf5bce027d2cb70
child 698005 50dfc5450770d50e2c31835adce4c7adf068751c
push id89171
push userjwwang@mozilla.com
push dateWed, 15 Nov 2017 05:14:20 +0000
bugs1417305
milestone59.0a1
Bug 1417305. P1 - let callers of MediaResourceCallback::NotifyDataEnded() decide whether to call it synchronously. Instead of doing dispatch inside MediaResourceCallback::NotifyDataEnded(), we let the callers decide whether to call the function synchronously. This is required by ChannelMediaResource::CacheClientNotifyDataEnded() which will run off the main thread. MozReview-Commit-ID: IzUYw8QMZbD
dom/media/ChannelMediaDecoder.cpp
dom/media/ChannelMediaResource.cpp
dom/media/CloneableWithRangeMediaResource.cpp
dom/media/FileMediaResource.cpp
--- a/dom/media/ChannelMediaDecoder.cpp
+++ b/dom/media/ChannelMediaDecoder.cpp
@@ -98,36 +98,31 @@ ChannelMediaDecoder::ResourceCallback::N
   mTimer->InitWithNamedFuncCallback(
     TimerCallback, this, sDelay, nsITimer::TYPE_ONE_SHOT,
     "ChannelMediaDecoder::ResourceCallback::TimerCallback");
 }
 
 void
 ChannelMediaDecoder::ResourceCallback::NotifyDataEnded(nsresult aStatus)
 {
-  RefPtr<ResourceCallback> self = this;
-  nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
-    "ChannelMediaDecoder::ResourceCallback::NotifyDataEnded",
-    [=]() {
-    if (!self->mDecoder) {
-      return;
-    }
-    self->mDecoder->NotifyDownloadEnded(aStatus);
-    if (NS_SUCCEEDED(aStatus)) {
-      MediaDecoderOwner* owner = self->GetMediaOwner();
-      MOZ_ASSERT(owner);
-      owner->DownloadSuspended();
+  MOZ_ASSERT(NS_IsMainThread());
+  if (!mDecoder) {
+    return;
+  }
+  mDecoder->NotifyDownloadEnded(aStatus);
+  if (NS_SUCCEEDED(aStatus)) {
+    MediaDecoderOwner* owner = GetMediaOwner();
+    MOZ_ASSERT(owner);
+    owner->DownloadSuspended();
 
-      // NotifySuspendedStatusChanged will tell the element that download
-      // has been suspended "by the cache", which is true since we never
-      // download anything. The element can then transition to HAVE_ENOUGH_DATA.
-      owner->NotifySuspendedByCache(true);
-    }
-  });
-  mAbstractMainThread->Dispatch(r.forget());
+    // NotifySuspendedStatusChanged will tell the element that download
+    // has been suspended "by the cache", which is true since we never
+    // download anything. The element can then transition to HAVE_ENOUGH_DATA.
+    owner->NotifySuspendedByCache(true);
+  }
 }
 
 void
 ChannelMediaDecoder::ResourceCallback::NotifyPrincipalChanged()
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (mDecoder) {
     mDecoder->NotifyPrincipalChanged();
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -832,18 +832,21 @@ ChannelMediaResource::CacheClientNotifyD
     NewRunnableMethod("MediaResourceCallback::NotifyDataArrived",
                       mCallback.get(),
                       &MediaResourceCallback::NotifyDataArrived));
 }
 
 void
 ChannelMediaResource::CacheClientNotifyDataEnded(nsresult aStatus)
 {
-  MOZ_ASSERT(NS_IsMainThread());
-  mCallback->NotifyDataEnded(aStatus);
+  mCallback->AbstractMainThread()->Dispatch(
+    NewRunnableMethod<nsresult>("MediaResourceCallback::NotifyDataEnded",
+                                mCallback.get(),
+                                &MediaResourceCallback::NotifyDataEnded,
+                                aStatus));
 }
 
 void
 ChannelMediaResource::CacheClientNotifyPrincipalChanged()
 {
   NS_ASSERTION(NS_IsMainThread(), "Don't call on non-main thread");
 
   mCallback->NotifyPrincipalChanged();
--- a/dom/media/CloneableWithRangeMediaResource.cpp
+++ b/dom/media/CloneableWithRangeMediaResource.cpp
@@ -128,17 +128,21 @@ NS_INTERFACE_MAP_END
 
 } // anonymous
 
 void
 CloneableWithRangeMediaResource::MaybeInitialize()
 {
   if (!mInitialized) {
     mInitialized = true;
-    mCallback->NotifyDataEnded(NS_OK);
+    mCallback->AbstractMainThread()->Dispatch(
+      NewRunnableMethod<nsresult>("MediaResourceCallback::NotifyDataEnded",
+                                  mCallback.get(),
+                                  &MediaResourceCallback::NotifyDataEnded,
+                                  NS_OK));
   }
 }
 
 nsresult
 CloneableWithRangeMediaResource::GetCachedRanges(MediaByteRangeSet& aRanges)
 {
   MaybeInitialize();
   aRanges += MediaByteRange(0, (int64_t)mSize);
--- a/dom/media/FileMediaResource.cpp
+++ b/dom/media/FileMediaResource.cpp
@@ -22,17 +22,21 @@ FileMediaResource::EnsureSizeInitialized
     return;
   }
   mSizeInitialized = true;
   // Get the file size and inform the decoder.
   uint64_t size;
   nsresult res = mInput->Available(&size);
   if (NS_SUCCEEDED(res) && size <= INT64_MAX) {
     mSize = (int64_t)size;
-    mCallback->NotifyDataEnded(NS_OK);
+    mCallback->AbstractMainThread()->Dispatch(
+      NewRunnableMethod<nsresult>("MediaResourceCallback::NotifyDataEnded",
+                                  mCallback.get(),
+                                  &MediaResourceCallback::NotifyDataEnded,
+                                  NS_OK));
   }
 }
 
 nsresult
 FileMediaResource::GetCachedRanges(MediaByteRangeSet& aRanges)
 {
   MutexAutoLock lock(mLock);