Bug 1416643. P1 - remove checks for mDidNotifyDataEnded/mNotifyDataEndedStatus from IsAvailableForSharing(). draft
authorJW Wang <jwwang@mozilla.com>
Wed, 15 Nov 2017 15:14:21 +0800
changeset 698742 e5f5e3ba99e1f86b2953abe9cd6e325743518d9b
parent 698006 d17b8a45e6a280af6cf25ec95be4a2e09dfac6dc
child 698743 cf1a992a320237c7bb93c55e60cd911b91ee9cf2
push id89347
push userjwwang@mozilla.com
push dateThu, 16 Nov 2017 02:42:58 +0000
bugs1416643
milestone59.0a1
Bug 1416643. P1 - remove checks for mDidNotifyDataEnded/mNotifyDataEndedStatus from IsAvailableForSharing(). We will need to modify these members off the main thead while IsAvailableForSharing() is a main thread only function. InitAsClone() will return an error if the original stream ends abnormally. MozReview-Commit-ID: 1qRyboca2YZ
dom/media/ChannelMediaResource.cpp
dom/media/MediaCache.cpp
dom/media/MediaCache.h
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -605,17 +605,21 @@ ChannelMediaResource::CloneData(MediaRes
   resource->mIsTransportSeekable = mIsTransportSeekable;
 
   // 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.
-  resource->mCacheStream.InitAsClone(&mCacheStream);
+  nsresult rv = resource->mCacheStream.InitAsClone(&mCacheStream);
+  if (NS_FAILED(rv)) {
+    resource->Close();
+    return nullptr;
+  }
   // mSuspendAgent.Suspend() accesses mCacheStream which is not ready
   // until InitAsClone() is done.
   resource->mSuspendAgent.Suspend();
   resource->mChannelStatistics.Stop();
 
   return resource.forget();
 }
 
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2692,23 +2692,29 @@ MediaCacheStream::Init(int64_t aContentL
   mMediaCache = MediaCache::GetMediaCache(aContentLength);
   if (!mMediaCache) {
     return NS_ERROR_FAILURE;
   }
   mMediaCache->OpenStream(this);
   return NS_OK;
 }
 
-void
+nsresult
 MediaCacheStream::InitAsClone(MediaCacheStream* aOriginal)
 {
   MOZ_ASSERT(aOriginal->IsAvailableForSharing());
   MOZ_ASSERT(!mMediaCache, "Has been initialized.");
   MOZ_ASSERT(aOriginal->mMediaCache, "Don't clone an uninitialized stream.");
 
+  if (aOriginal->mDidNotifyDataEnded &&
+      NS_FAILED(aOriginal->mNotifyDataEndedStatus)) {
+    // Streams that ended abnormally are ineligible for cloning.
+    return NS_ERROR_FAILURE;
+  }
+
   // This needs to be done before OpenStream() to avoid data race.
   mClientSuspended = true;
 
   // Use the same MediaCache as our clone.
   mMediaCache = aOriginal->mMediaCache;
 
   mResourceID = aOriginal->mResourceID;
 
@@ -2739,16 +2745,18 @@ MediaCacheStream::InitAsClone(MediaCache
       mBlocks.AppendElement(-1);
     }
     // Every block is a readahead block for the clone because the clone's initial
     // stream offset is zero
     mMediaCache->AddBlockOwnerAsReadahead(cacheBlockIndex, this, i);
   }
 
   mMediaCache->OpenStream(this, true /* aIsClone */);
+
+  return NS_OK;
 }
 
 nsIEventTarget*
 MediaCacheStream::OwnerThread() const
 {
   return mMediaCache->OwnerThread();
 }
 
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -205,34 +205,30 @@ 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.
-  void InitAsClone(MediaCacheStream* aOriginal);
+  nsresult 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.
   // Must be used on the main thread or while holding the cache lock.
   bool IsClosed() const { return mClosed; }
   // Returns true when this stream is can be shared by a new resource load.
   // Called on the main thread only.
-  bool IsAvailableForSharing() const
-  {
-    return !mClosed && !mIsPrivateBrowsing &&
-      (!mDidNotifyDataEnded || NS_SUCCEEDED(mNotifyDataEndedStatus));
-  }
+  bool IsAvailableForSharing() const { return !mClosed && !mIsPrivateBrowsing; }
   // Get the principal for this stream. Anything accessing the contents of
   // this stream must have a principal that subsumes this principal.
   nsIPrincipal* GetCurrentPrincipal() { return mPrincipal; }
 
   // These callbacks are called on the main thread by the client
   // when data has been received via the channel.
   // Tells the cache what the server said the data length is going to be.
   // The actual data length may be greater (we receive more data than