Bug 1411808. P3 - InitAsClone() shouldn't call |mMediaCache->OpenStream(this)| until initialization is done. r=gerald
authorJW Wang <jwwang@mozilla.com>
Thu, 26 Oct 2017 11:13:38 +0800
changeset 443225 efbfeacfdb9621ab2c67b2400cb89d92669b6c53
parent 443224 875d91b29c311400cc3108f959da53d09bab66b3
child 443226 e15cf10f369acd8129126525c349ade395abf876
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1411808
milestone58.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 1411808. P3 - InitAsClone() shouldn't call |mMediaCache->OpenStream(this)| until initialization is done. r=gerald We don't want MediaCache to use a half-initialized stream. MozReview-Commit-ID: LjPLOYwy0Wd
dom/media/MediaCache.cpp
dom/media/MediaCache.h
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -167,27 +167,28 @@ public:
   // Cache-file access methods. These are the lowest-level cache methods.
   // mReentrantMonitor must be held; these can be called on any thread.
   // This can return partial reads.
   // Note mReentrantMonitor will be dropped while doing IO. The caller need
   // to handle changes happening when the monitor is not held.
   nsresult ReadCacheFile(int64_t aOffset, void* aData, int32_t aLength,
                          int32_t* aBytes);
 
+  // The generated IDs are always positive.
   int64_t AllocateResourceID()
   {
     mReentrantMonitor.AssertCurrentThreadIn();
-    return mNextResourceID++;
+    return ++mNextResourceID;
   }
 
   // mReentrantMonitor must be held, called on main thread.
   // These methods are used by the stream to set up and tear down streams,
   // and to handle reads and writes.
   // Add aStream to the list of streams.
-  void OpenStream(MediaCacheStream* aStream);
+  void OpenStream(MediaCacheStream* aStream, bool aIsClone = false);
   // Remove aStream from the list of streams.
   void ReleaseStream(MediaCacheStream* aStream);
   // Free all blocks belonging to aStream.
   void ReleaseStreamBlocks(MediaCacheStream* aStream);
   // Find a cache entry for this data, and write the data into it
   void AllocateAndWriteBlock(
     MediaCacheStream* aStream,
     int32_t aStreamBlockIndex,
@@ -266,18 +267,17 @@ public:
   private:
     MediaCache* mMediaCache;
     int64_t  mResourceID;
     uint32_t mNext;
   };
 
 protected:
   explicit MediaCache(MediaBlockCacheBase* aCache)
-    : mNextResourceID(1)
-    , mReentrantMonitor("MediaCache.mReentrantMonitor")
+    : mReentrantMonitor("MediaCache.mReentrantMonitor")
     , mBlockCache(aCache)
     , mUpdateQueued(false)
 #ifdef DEBUG
     , mInUpdate(false)
 #endif
   {
     NS_ASSERTION(NS_IsMainThread(), "Only construct MediaCache on main thread");
     MOZ_COUNT_CTOR(MediaCache);
@@ -410,17 +410,17 @@ protected:
   // There is at most one file-backed media cache.
   // It is owned by all MediaCacheStreams that use it.
   // This is a raw pointer set by GetMediaCache(), and reset by ~MediaCache(),
   // both on the main thread; and is not accessed anywhere else.
   static MediaCache* gMediaCache;
 
   // This member is main-thread only. It's used to allocate unique
   // resource IDs to streams.
-  int64_t                       mNextResourceID;
+  int64_t mNextResourceID = 0;
 
   // The monitor protects all the data members here. Also, off-main-thread
   // readers that need to block will Wait() on this monitor. When new
   // data becomes available in the cache, we NotifyAll() on this monitor.
   ReentrantMonitor         mReentrantMonitor;
   // This is only written while on the main thread and the monitor is held.
   // Thus, it can be safely read from the main thread or while holding the monitor.
   nsTArray<MediaCacheStream*> mStreams;
@@ -475,17 +475,16 @@ MediaCacheFlusher::Observe(nsISupports *
   return NS_OK;
 }
 
 MediaCacheStream::MediaCacheStream(ChannelMediaResource* aClient,
                                    bool aIsPrivateBrowsing)
   : mMediaCache(nullptr)
   , mClient(aClient)
   , mDidNotifyDataEnded(false)
-  , mResourceID(0)
   , mIsTransportSeekable(false)
   , mCacheSuspended(false)
   , mChannelEnded(false)
   , mStreamOffset(0)
   , mPlaybackBytesPerSecond(10000)
   , mPinCount(0)
   , mCurrentMode(MODE_PLAYBACK)
   , mMetadataInPartialBlockBuffer(false)
@@ -1735,24 +1734,32 @@ MediaCache::AllocateAndWriteBlock(MediaC
   }
 
   // Queue an Update since the cache state has changed (for example
   // we might want to stop loading because the cache is full)
   QueueUpdate();
 }
 
 void
-MediaCache::OpenStream(MediaCacheStream* aStream)
+MediaCache::OpenStream(MediaCacheStream* aStream, bool aIsClone)
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
   LOG("Stream %p opened", aStream);
   mStreams.AppendElement(aStream);
-  aStream->mResourceID = AllocateResourceID();
+
+  // A cloned stream should've got the ID from its original.
+  if (!aIsClone) {
+    MOZ_ASSERT(aStream->mResourceID == 0, "mResourceID has been initialized.");
+    aStream->mResourceID = AllocateResourceID();
+  }
+
+  // We should have a valid ID now no matter it is cloned or not.
+  MOZ_ASSERT(aStream->mResourceID > 0, "mResourceID is invalid");
 
   // Queue an update since a new stream has been opened.
   QueueUpdate();
 }
 
 void
 MediaCache::ReleaseStream(MediaCacheStream* aStream)
 {
@@ -2678,18 +2685,16 @@ MediaCacheStream::InitAsClone(MediaCache
   MOZ_ASSERT(aOriginal->mMediaCache, "Don't clone an uninitialized stream.");
 
   // This needs to be done before OpenStream() to avoid data race.
   mClientSuspended = true;
 
   // Use the same MediaCache as our clone.
   mMediaCache = aOriginal->mMediaCache;
 
-  mMediaCache->OpenStream(this);
-
   mResourceID = aOriginal->mResourceID;
 
   // Grab cache blocks from aOriginal as readahead blocks for our stream
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
 
   mPrincipal = aOriginal->mPrincipal;
   mStreamLength = aOriginal->mStreamLength;
   mIsTransportSeekable = aOriginal->mIsTransportSeekable;
@@ -2712,16 +2717,18 @@ 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(cacheBlockIndex, this, i);
   }
+
+  mMediaCache->OpenStream(this, true /* aIsClone */);
 }
 
 nsIEventTarget*
 MediaCacheStream::OwnerThread() const
 {
   return mMediaCache->OwnerThread();
 }
 
--- a/dom/media/MediaCache.h
+++ b/dom/media/MediaCache.h
@@ -455,17 +455,19 @@ private:
   // or while holding the cache's monitor.
 
   // Set to true when the stream has been closed either explicitly or
   // due to an internal cache error
   bool mClosed = false;
   // This is a unique ID representing the resource we're loading.
   // All streams with the same mResourceID are loading the same
   // underlying resource and should share data.
-  int64_t mResourceID;
+  // Initialized to 0 as invalid. Will be allocated a valid ID (always positive)
+  // from the cache.
+  int64_t mResourceID = 0;
   // The last reported seekability state for the underlying channel
   bool mIsTransportSeekable;
   // True if the cache has suspended our channel because the cache is
   // full and the priority of the data that would be received is lower
   // than the priority of the data already in the cache
   bool mCacheSuspended;
   // True if the channel ended and we haven't seeked it again.
   bool mChannelEnded;