Bug 1418213 - always run MediaCacheStream::NotifyDataReceived() off the main thread. r=bechen,gerald
authorJW Wang <jwwang@mozilla.com>
Wed, 15 Nov 2017 17:56:10 +0800
changeset 392834 ed68d2d0cb05190be93afb1064a968542fa885f2
parent 392833 3f7025b5d57e90414f756d238a6a22f3a48a7775
child 392835 71b0dd5f14d370350784675bbef514dc5a5c0422
push id55727
push userjwwang@mozilla.com
push dateTue, 21 Nov 2017 07:43:43 +0000
treeherderautoland@ed68d2d0cb05 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbechen, gerald
bugs1418213
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1418213 - always run MediaCacheStream::NotifyDataReceived() off the main thread. r=bechen,gerald MozReview-Commit-ID: GBQ0lEf8rVI
dom/media/ChannelMediaResource.cpp
dom/media/MediaCache.cpp
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -298,17 +298,16 @@ ChannelMediaResource::OnStartRequest(nsI
   mIsTransportSeekable = seekable;
   mChannelStatistics.Start();
 
   mSuspendAgent.Delegate(mChannel);
 
   // Fires an initial progress event.
   owner->DownloadProgressed();
 
-  // TODO: Don't turn this on until we fix all data races.
   nsCOMPtr<nsIThreadRetargetableRequest> retarget;
   if (Preferences::GetBool("media.omt_data_delivery.enabled", false) &&
       (retarget = do_QueryInterface(aRequest))) {
     // Note this will not always succeed. We need to handle the case where
     // all resources sharing the same cache might run their data callbacks
     // on different threads.
     retarget->RetargetDeliveryTo(mCacheStream.OwnerThread());
   }
@@ -412,22 +411,35 @@ ChannelMediaResource::OnChannelRedirect(
 nsresult
 ChannelMediaResource::CopySegmentToCache(nsIInputStream* aInStream,
                                          void* aClosure,
                                          const char* aFromSegment,
                                          uint32_t aToOffset,
                                          uint32_t aCount,
                                          uint32_t* aWriteCount)
 {
+  *aWriteCount = aCount;
   Closure* closure = static_cast<Closure*>(aClosure);
-  closure->mResource->mCacheStream.NotifyDataReceived(
-    closure->mLoadID,
-    aCount,
-    reinterpret_cast<const uint8_t*>(aFromSegment));
-  *aWriteCount = aCount;
+  MediaCacheStream* cacheStream = &closure->mResource->mCacheStream;
+  if (cacheStream->OwnerThread()->IsOnCurrentThread()) {
+    cacheStream->NotifyDataReceived(
+      closure->mLoadID, aCount, reinterpret_cast<const uint8_t*>(aFromSegment));
+    return NS_OK;
+  }
+
+  RefPtr<ChannelMediaResource> self = closure->mResource;
+  uint32_t loadID = closure->mLoadID;
+  UniquePtr<uint8_t[]> data = MakeUnique<uint8_t[]>(aCount);
+  memcpy(data.get(), aFromSegment, aCount);
+  cacheStream->OwnerThread()->Dispatch(NS_NewRunnableFunction(
+    "MediaCacheStream::NotifyDataReceived",
+    [ self, loadID, data = Move(data), aCount ]() {
+      self->mCacheStream.NotifyDataReceived(loadID, aCount, data.get());
+    }));
+
   return NS_OK;
 }
 
 nsresult
 ChannelMediaResource::OnDataAvailable(uint32_t aLoadID,
                                       nsIInputStream* aStream,
                                       uint32_t aCount)
 {
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2001,18 +2001,18 @@ MediaCacheStream::UpdatePrincipal(nsIPri
   }
 }
 
 void
 MediaCacheStream::NotifyDataReceived(uint32_t aLoadID,
                                      uint32_t aCount,
                                      const uint8_t* aData)
 {
+  MOZ_ASSERT(OwnerThread()->IsOnCurrentThread());
   MOZ_ASSERT(aLoadID > 0);
-  // This might happen off the main thread.
 
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
   if (mClosed) {
     // Nothing to do if the stream is closed.
     return;
   }
 
   LOG("Stream %p DataReceived at %" PRId64 " count=%u aLoadID=%u",