Bug 1426578. P3 - make MediaCacheStream::InitAsClone() infallible. draft
authorJW Wang <jwwang@mozilla.com>
Sat, 16 Dec 2017 23:50:07 +0800
changeset 713858 c77249307248f65f7c9902a8068a70d8548368fd
parent 713857 1a2469069657433d4752c6f88a32d5df74675d03
child 713859 8953364cdf06b5a85f6740cfa1f41bab0eb5e243
push id93781
push userjwwang@mozilla.com
push dateThu, 21 Dec 2017 03:28:43 +0000
bugs1426578
milestone59.0a1
Bug 1426578. P3 - make MediaCacheStream::InitAsClone() infallible. 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.