Bug 1426578. P3 - make MediaCacheStream::InitAsClone() infallible. r=bechen,gerald
authorJW Wang <jwwang@mozilla.com>
Sat, 16 Dec 2017 23:50:07 +0800
changeset 449321 1f6caf89e3825360192a6d1dae284fc806e88649
parent 449320 fb2edb2d8dd1f0b40504733c08b1461ef0fcce0f
child 449322 d005226af73b702b34cb6419d211960722daf29b
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbechen, gerald
bugs1426578
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 1426578. P3 - make MediaCacheStream::InitAsClone() infallible. r=bechen,gerald It must be infallible for there is no way to propagate the error back to the main thread when part of the init functions run on another thread. It is OK to clone a stream that ends abnormally as long as we don't copy the error status of EOS. The cloned stream will open a new channel when necessary. Note we also copy the partial block from the original stream to get as much data as possible and thus reducing the chance of reopening the channel. MozReview-Commit-ID: 37iYQonFdBU
dom/media/ChannelMediaResource.cpp
dom/media/MediaCache.cpp
dom/media/MediaCache.h
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -575,21 +575,17 @@ ChannelMediaResource::CloneData(MediaRes
   mSharedInfo->mResources.AppendElement(resource.get());
 
   // Initially the clone is treated as suspended by the cache, because
   // we don't have a channel. If the cache needs to read data from the clone
   // it will call CacheClientResume (or CacheClientSeek with aResume true)
   // which will recreate the channel. This way, if all of the media data
   // is already in the cache we don't create an unnecessary HTTP channel
   // and perform a useless HTTP transaction.
-  nsresult rv = resource->mCacheStream.InitAsClone(&mCacheStream);
-  if (NS_FAILED(rv)) {
-    resource->Close();
-    return nullptr;
-  }
+  resource->mCacheStream.InitAsClone(&mCacheStream);
   return resource.forget();
 }
 
 void ChannelMediaResource::CloseChannel()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   if (mChannel) {
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2870,48 +2870,43 @@ MediaCacheStream::Init(int64_t aContentL
     return NS_ERROR_FAILURE;
   }
 
   AutoLock lock(mMediaCache->Monitor());
   mMediaCache->OpenStream(lock, this);
   return NS_OK;
 }
 
-nsresult
+void
 MediaCacheStream::InitAsClone(MediaCacheStream* aOriginal)
 {
   MOZ_ASSERT(!mMediaCache, "Has been initialized.");
   MOZ_ASSERT(aOriginal->mMediaCache, "Don't clone an uninitialized stream.");
 
   // Use the same MediaCache as our clone.
   mMediaCache = aOriginal->mMediaCache;
   // This needs to be done before OpenStream() to avoid data race.
   mClientSuspended = true;
   // Cloned streams are initially suspended, since there is no channel open
   // initially for a clone.
   mCacheSuspended = true;
   mChannelEnded = true;
 
   AutoLock lock(aOriginal->mMediaCache->Monitor());
 
-  if (aOriginal->mDidNotifyDataEnded &&
-      NS_FAILED(aOriginal->mNotifyDataEndedStatus)) {
-    // Streams that ended abnormally are ineligible for cloning.
-    return NS_ERROR_FAILURE;
-  }
-
   mResourceID = aOriginal->mResourceID;
 
   // Grab cache blocks from aOriginal as readahead blocks for our stream
   mStreamLength = aOriginal->mStreamLength;
   mIsTransportSeekable = aOriginal->mIsTransportSeekable;
   mDownloadStatistics = aOriginal->mDownloadStatistics;
   mDownloadStatistics.Stop();
 
-  if (aOriginal->mDidNotifyDataEnded) {
+  if (aOriginal->mDidNotifyDataEnded &&
+      NS_SUCCEEDED(aOriginal->mNotifyDataEndedStatus)) {
     mNotifyDataEndedStatus = aOriginal->mNotifyDataEndedStatus;
     mDidNotifyDataEnded = true;
     mClient->CacheClientNotifyDataEnded(mNotifyDataEndedStatus);
   }
 
   for (uint32_t i = 0; i < aOriginal->mBlocks.Length(); ++i) {
     int32_t cacheBlockIndex = aOriginal->mBlocks[i];
     if (cacheBlockIndex < 0)
@@ -2920,19 +2915,23 @@ MediaCacheStream::InitAsClone(MediaCache
     while (i >= mBlocks.Length()) {
       mBlocks.AppendElement(-1);
     }
     // Every block is a readahead block for the clone because the clone's initial
     // stream offset is zero
     mMediaCache->AddBlockOwnerAsReadahead(lock, cacheBlockIndex, this, i);
   }
 
-  mMediaCache->OpenStream(lock, this, true /* aIsClone */);
+  // Copy the partial block.
+  mChannelOffset = aOriginal->mChannelOffset;
+  memcpy(mPartialBlockBuffer.get(),
+         aOriginal->mPartialBlockBuffer.get(),
+         BLOCK_SIZE);
 
-  return NS_OK;
+  mMediaCache->OpenStream(lock, this, true /* aIsClone */);
 }
 
 nsIEventTarget*
 MediaCacheStream::OwnerThread() const
 {
   return mMediaCache->OwnerThread();
 }
 
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -213,17 +213,17 @@ public:
   // Exactly one of InitAsClone or Init must be called before any other method
   // on this class. Does nothing if already initialized.
   nsresult Init(int64_t aContentLength);
 
   // Set up this stream with the cache, assuming it's for the same data
   // as the aOriginal stream.
   // Exactly one of InitAsClone or Init must be called before any other method
   // on this class.
-  nsresult InitAsClone(MediaCacheStream* aOriginal);
+  void InitAsClone(MediaCacheStream* aOriginal);
 
   nsIEventTarget* OwnerThread() const;
 
   // These are called on the main thread.
   // This must be called (and return) before the ChannelMediaResource
   // used to create this MediaCacheStream is deleted.
   void Close();
   // This returns true when the stream has been closed.