Bug 1427667 - move the call to mMediaCache->ReleaseStream() from the destructor to CloseInternal(). r=bechen,gerald
authorJW Wang <jwwang@mozilla.com>
Tue, 19 Dec 2017 17:51:51 +0800
changeset 449338 a054313a69b76a57cf174348dca63e6ee4bc2006
parent 449337 825f84c6efc2ac4d75a9eee28ab8d0ef91554211
child 449339 41e4dd8f635d59d3e5211b627ab125642035583b
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
bugs1427667
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 1427667 - move the call to mMediaCache->ReleaseStream() from the destructor to CloseInternal(). r=bechen,gerald So we won't need to take the cache monitor on the main thread. MozReview-Commit-ID: FavZKcfaHn8
dom/media/MediaCache.cpp
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -184,17 +184,17 @@ public:
   int64_t AllocateResourceID(AutoLock&) { 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(AutoLock&, MediaCacheStream* aStream, bool aIsClone = false);
   // Remove aStream from the list of streams.
-  void ReleaseStream(MediaCacheStream* aStream);
+  void ReleaseStream(AutoLock&, MediaCacheStream* aStream);
   // Free all blocks belonging to aStream.
   void ReleaseStreamBlocks(AutoLock&, MediaCacheStream* aStream);
   // Find a cache entry for this data, and write the data into it
   void AllocateAndWriteBlock(
     AutoLock&,
     MediaCacheStream* aStream,
     int32_t aStreamBlockIndex,
     MediaCacheStream::ReadMode aMode,
@@ -1816,28 +1816,22 @@ MediaCache::OpenStream(AutoLock& aLock,
   // 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(aLock);
 }
 
 void
-MediaCache::ReleaseStream(MediaCacheStream* aStream)
+MediaCache::ReleaseStream(AutoLock&, MediaCacheStream* aStream)
 {
-  NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
-
-  AutoLock lock(mMonitor);
+  MOZ_ASSERT(OwnerThread()->IsOnCurrentThread());
   LOG("Stream %p closed", aStream);
   mStreams.RemoveElement(aStream);
-
-  // Update MediaCache again for |mStreams| is changed.
-  // We need to re-run Update() to ensure streams reading from the same resource
-  // as the removed stream get a chance to continue reading.
-  QueueUpdate(lock);
+  // The caller needs to call QueueUpdate() to re-run Update().
 }
 
 void
 MediaCache::ReleaseStreamBlocks(AutoLock& aLock, MediaCacheStream* aStream)
 {
   // XXX scanning the entire stream doesn't seem great, if not much of it
   // is cached, but the only easy alternative is to scan the entire cache
   // which isn't better
@@ -2325,23 +2319,19 @@ MediaCacheStream::NotifyResume()
       // The channel remains dead. If we want to read some other data in the
       // future, CacheClientSeek() will be called to reopen the channel.
     });
   OwnerThread()->Dispatch(r.forget());
 }
 
 MediaCacheStream::~MediaCacheStream()
 {
-  NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
-  NS_ASSERTION(!mPinCount, "Unbalanced Pin");
-
-  if (mMediaCache) {
-    NS_ASSERTION(mClosed, "Stream was not closed");
-    mMediaCache->ReleaseStream(this);
-  }
+  MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
+  MOZ_ASSERT(!mPinCount, "Unbalanced Pin");
+  MOZ_ASSERT(!mMediaCache || mClosed);
 
   uint32_t lengthKb = uint32_t(
     std::min(std::max(mStreamLength, int64_t(0)) / 1024, int64_t(UINT32_MAX)));
   LOG("MediaCacheStream::~MediaCacheStream(this=%p) "
       "MEDIACACHESTREAM_LENGTH_KB=%" PRIu32,
       this,
       lengthKb);
   Telemetry::Accumulate(Telemetry::HistogramID::MEDIACACHESTREAM_LENGTH_KB,
@@ -2399,16 +2389,17 @@ MediaCacheStream::CloseInternal(AutoLock
 
   // Closing a stream will change the return value of
   // MediaCacheStream::AreAllStreamsForResourceSuspended as well as
   // ChannelMediaResource::IsSuspendedByCache. Let's notify it.
   mMediaCache->QueueSuspendedStatusUpdate(aLock, mResourceID);
 
   mClosed = true;
   mMediaCache->ReleaseStreamBlocks(aLock, this);
+  mMediaCache->ReleaseStream(aLock, this);
   // Wake up any blocked readers
   aLock.NotifyAll();
 
   // Queue an Update since we may have created more free space.
   mMediaCache->QueueUpdate(aLock);
 }
 
 void