Bug 1426578. P1 - tweak the constructor and init functions of ChannelMediaResource/MediaCacheStream. r=bechen,gerald
authorJW Wang <jwwang@mozilla.com>
Thu, 14 Dec 2017 16:08:17 +0800
changeset 397574 a29b9105722b07efd9202147d0ba4438b7151551
parent 397573 fa855df2e888bb6edbed237d7a6b78ab6476c96b
child 397575 fb2edb2d8dd1f0b40504733c08b1461ef0fcce0f
push id98555
push useraiakab@mozilla.com
push dateWed, 03 Jan 2018 09:58:33 +0000
treeherdermozilla-inbound@c5775459d615 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbechen, gerald
bugs1426578
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 1426578. P1 - tweak the constructor and init functions of ChannelMediaResource/MediaCacheStream. r=bechen,gerald We want to init as many members as possible before taking the cache monitor. This makes it easier to move part of the init functions to another thread. MozReview-Commit-ID: 6mmO356nCyQ
dom/media/ChannelMediaResource.cpp
dom/media/ChannelMediaResource.h
dom/media/MediaCache.cpp
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -25,17 +25,17 @@ mozilla::LazyLogModule gMediaResourceLog
 namespace mozilla {
 
 ChannelMediaResource::ChannelMediaResource(MediaResourceCallback* aCallback,
                                            nsIChannel* aChannel,
                                            nsIURI* aURI,
                                            bool aIsPrivateBrowsing)
   : BaseMediaResource(aCallback, aChannel, aURI)
   , mCacheStream(this, aIsPrivateBrowsing)
-  , mSuspendAgent(mCacheStream)
+  , mSuspendAgent(mCacheStream, !aChannel /*aSuspended*/)
 {
 }
 
 ChannelMediaResource::~ChannelMediaResource()
 {
   MOZ_ASSERT(mClosed);
   MOZ_ASSERT(!mChannel);
   MOZ_ASSERT(!mListener);
@@ -580,20 +580,16 @@ ChannelMediaResource::CloneData(MediaRes
   // 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;
   }
-  // mSuspendAgent.Suspend() accesses mCacheStream which is not ready
-  // until InitAsClone() is done.
-  resource->mSuspendAgent.Suspend();
-
   return resource.forget();
 }
 
 void ChannelMediaResource::CloseChannel()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   if (mChannel) {
--- a/dom/media/ChannelMediaResource.h
+++ b/dom/media/ChannelMediaResource.h
@@ -18,18 +18,19 @@ namespace mozilla {
 
 /**
  * This class is responsible for managing the suspend count and report suspend
  * status of channel.
  **/
 class ChannelSuspendAgent
 {
 public:
-  explicit ChannelSuspendAgent(MediaCacheStream& aCacheStream)
+  ChannelSuspendAgent(MediaCacheStream& aCacheStream, bool aSuspended)
     : mCacheStream(aCacheStream)
+    , mSuspendCount(aSuspended ? 1 : 0)
   {
   }
 
   // True when the channel has been suspended or needs to be suspended.
   bool IsSuspended();
 
   // Return true when the channel is logically suspended, i.e. the suspend
   // count goes from 0 to 1.
@@ -44,17 +45,17 @@ public:
   void Revoke();
 
 private:
   // Only suspends channel but not changes the suspend count.
   void SuspendInternal();
 
   nsIChannel* mChannel = nullptr;
   MediaCacheStream& mCacheStream;
-  uint32_t mSuspendCount = 0;
+  uint32_t mSuspendCount;
   bool mIsChannelSuspended = false;
 };
 
 DDLoggedTypeDeclNameAndBase(ChannelMediaResource, BaseMediaResource);
 
 /**
  * This is the MediaResource implementation that wraps Necko channels.
  * Much of its functionality is actually delegated to MediaCache via
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2871,43 +2871,41 @@ MediaCacheStream::Init(int64_t aContentL
 }
 
 nsresult
 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;
   }
 
-  // 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;
 
   // Grab cache blocks from aOriginal as readahead blocks for our stream
   mStreamLength = aOriginal->mStreamLength;
   mIsTransportSeekable = aOriginal->mIsTransportSeekable;
   mDownloadStatistics = aOriginal->mDownloadStatistics;
   mDownloadStatistics.Stop();
 
-  // Cloned streams are initially suspended, since there is no channel open
-  // initially for a clone.
-  mCacheSuspended = true;
-  mChannelEnded = true;
-
   if (aOriginal->mDidNotifyDataEnded) {
     mNotifyDataEndedStatus = aOriginal->mNotifyDataEndedStatus;
     mDidNotifyDataEnded = true;
     mClient->CacheClientNotifyDataEnded(mNotifyDataEndedStatus);
   }
 
   for (uint32_t i = 0; i < aOriginal->mBlocks.Length(); ++i) {
     int32_t cacheBlockIndex = aOriginal->mBlocks[i];