Bug 1427667 - move the call to mMediaCache->ReleaseStream() from the destructor to CloseInternal(). draft
authorJW Wang <jwwang@mozilla.com>
Tue, 19 Dec 2017 17:51:51 +0800
changeset 715194 eb3c9895e50cfb31bc0403a9e44af069e2f1f1ec
parent 715193 2deb61fab45c861a81c48cca946ecb9f8295134d
child 715210 06c0fe6ea08fc66c2177b3dd78e96e4475d0e022
push id94089
push userjwwang@mozilla.com
push dateWed, 03 Jan 2018 02:48:57 +0000
bugs1427667
milestone59.0a1
Bug 1427667 - move the call to mMediaCache->ReleaseStream() from the destructor to CloseInternal(). 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