Bug 1415461 - consolidate the life cycle of the channel managed by ChannelSuspendAgent. r=bechen,gerald
authorJW Wang <jwwang@mozilla.com>
Wed, 08 Nov 2017 16:16:24 +0800
changeset 444505 056e41a8b9821525eb5c84b63304d4b1dad9d1ab
parent 444504 f62b1c3fe4ff6a4198fce20796fab5c23d42d037
child 444506 476c4d0aadcc8ca5843736795685464c37db48c3
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)
reviewersbechen, gerald
bugs1415461
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 1415461 - consolidate the life cycle of the channel managed by ChannelSuspendAgent. r=bechen,gerald We want to suspend/resume a channel only after OnStartRequest() and before OnStopRequest(). MozReview-Commit-ID: 4QNRdsVvyAK
dom/media/ChannelMediaResource.cpp
dom/media/ChannelMediaResource.h
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -25,29 +25,29 @@ mozilla::LazyLogModule gMediaResourceLog
 namespace mozilla {
 
 ChannelMediaResource::ChannelMediaResource(MediaResourceCallback* aCallback,
                                            nsIChannel* aChannel,
                                            nsIURI* aURI,
                                            bool aIsPrivateBrowsing)
   : BaseMediaResource(aCallback, aChannel, aURI)
   , mCacheStream(this, aIsPrivateBrowsing)
-  , mSuspendAgent(mChannel, mCacheStream)
+  , mSuspendAgent(mCacheStream)
 {
 }
 
 ChannelMediaResource::ChannelMediaResource(
   MediaResourceCallback* aCallback,
   nsIChannel* aChannel,
   nsIURI* aURI,
   const MediaChannelStatistics& aStatistics)
   : BaseMediaResource(aCallback, aChannel, aURI)
   , mCacheStream(this, /* aIsPrivateBrowsing = */ false)
   , mChannelStatistics(aStatistics)
-  , mSuspendAgent(mChannel, mCacheStream)
+  , mSuspendAgent(mCacheStream)
 {
 }
 
 ChannelMediaResource::~ChannelMediaResource()
 {
   MOZ_ASSERT(mClosed);
   MOZ_ASSERT(!mChannel);
   MOZ_ASSERT(!mListener);
@@ -290,17 +290,17 @@ ChannelMediaResource::OnStartRequest(nsI
   // This is important, we want to make sure all principals are updated before
   // any consumer can see the new data.
   UpdatePrincipal();
 
   mCacheStream.NotifyDataStarted(mLoadID, startOffset, seekable);
   mIsTransportSeekable = seekable;
   mChannelStatistics.Start();
 
-  mSuspendAgent.UpdateSuspendedStatusIfNeeded();
+  mSuspendAgent.Delegate(mChannel);
 
   // Fires an initial progress event.
   owner->DownloadProgressed();
 
   // TODO: Don't turn this on until we fix all data races.
   nsCOMPtr<nsIThreadRetargetableRequest> retarget;
   if (Preferences::GetBool("media.omt_data_delivery.enabled", false) &&
       (retarget = do_QueryInterface(aRequest))) {
@@ -419,18 +419,19 @@ ChannelMediaResource::OnStopRequest(nsIR
 }
 
 nsresult
 ChannelMediaResource::OnChannelRedirect(nsIChannel* aOld,
                                         nsIChannel* aNew,
                                         uint32_t aFlags,
                                         int64_t aOffset)
 {
+  // OnChannelRedirect() is followed by OnStartRequest() where we will
+  // call mSuspendAgent.Delegate().
   mChannel = aNew;
-  mSuspendAgent.NotifyChannelOpened(mChannel);
   return SetupChannelHeaders(aOffset);
 }
 
 nsresult
 ChannelMediaResource::CopySegmentToCache(nsIInputStream* aInStream,
                                          void* aClosure,
                                          const char* aFromSegment,
                                          uint32_t aToOffset,
@@ -620,17 +621,17 @@ ChannelMediaResource::CloneData(MediaRes
 
 void ChannelMediaResource::CloseChannel()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   mChannelStatistics.Stop();
 
   if (mChannel) {
-    mSuspendAgent.NotifyChannelClosing();
+    mSuspendAgent.Revoke();
     // The status we use here won't be passed to the decoder, since
     // we've already revoked the listener. It can however be passed
     // to nsDocumentViewer::LoadComplete if our channel is the one
     // that kicked off creation of a video document. We don't want that
     // document load to think there was an error.
     // NS_ERROR_PARSED_DATA_CACHED is the best thing we have for that
     // at the moment.
     mChannel->Cancel(NS_ERROR_PARSED_DATA_CACHED);
@@ -812,18 +813,16 @@ ChannelMediaResource::RecreateChannel()
 
   nsCOMPtr<nsIClassOfService> cos(do_QueryInterface(mChannel));
   if (cos) {
     // Unconditionally disable throttling since we want the media to fluently
     // play even when we switch the tab to background.
     cos->AddClassFlags(nsIClassOfService::DontThrottle);
   }
 
-  mSuspendAgent.NotifyChannelOpened(mChannel);
-
   // Tell the cache to reset the download status when the channel is reopened.
   mCacheStream.NotifyChannelRecreated();
 
   return rv;
 }
 
 void
 ChannelMediaResource::CacheClientNotifyDataReceived()
@@ -1061,47 +1060,49 @@ ChannelSuspendAgent::Resume()
     }
     mCacheStream.NotifyClientSuspended(false);
     return true;
   }
   return false;
 }
 
 void
-ChannelSuspendAgent::UpdateSuspendedStatusIfNeeded()
+ChannelSuspendAgent::Delegate(nsIChannel* aChannel)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  if (!mIsChannelSuspended && IsSuspended()) {
+  MOZ_ASSERT(aChannel);
+  MOZ_ASSERT(!mChannel, "The previous channel not closed.");
+  MOZ_ASSERT(!mIsChannelSuspended);
+
+  mChannel = aChannel;
+  // Ensure the suspend status of the channel matches our suspend count.
+  if (IsSuspended()) {
     SuspendInternal();
   }
 }
 
 void
-ChannelSuspendAgent::NotifyChannelOpened(nsIChannel* aChannel)
+ChannelSuspendAgent::Revoke()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aChannel);
-  mChannel = aChannel;
-}
 
-void
-ChannelSuspendAgent::NotifyChannelClosing()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(mChannel);
-  // Before close the channel, it need to be resumed to make sure its internal
+  if (!mChannel) {
+    // Channel already revoked. Nothing to do.
+    return;
+  }
+
+  // Before closing the channel, it needs to be resumed to make sure its internal
   // state is correct. Besides, We need to suspend the channel after recreating.
   if (mIsChannelSuspended) {
     mChannel->Resume();
     mIsChannelSuspended = false;
   }
   mChannel = nullptr;
 }
 
 bool
 ChannelSuspendAgent::IsSuspended()
 {
   MOZ_ASSERT(NS_IsMainThread());
   return (mSuspendCount > 0);
 }
 
-
 } // mozilla namespace
--- a/dom/media/ChannelMediaResource.h
+++ b/dom/media/ChannelMediaResource.h
@@ -19,47 +19,41 @@ namespace mozilla {
 
 /**
  * This class is responsible for managing the suspend count and report suspend
  * status of channel.
  **/
 class ChannelSuspendAgent
 {
 public:
-  ChannelSuspendAgent(nsIChannel* aChannel, MediaCacheStream& aCacheStream)
-    : mChannel(aChannel)
-    , mCacheStream(aCacheStream)
+  explicit ChannelSuspendAgent(MediaCacheStream& aCacheStream)
+    : mCacheStream(aCacheStream)
   {
   }
 
   // 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.
   bool Suspend();
 
   // Return true only when the suspend count is equal to zero.
   bool Resume();
 
-  // Call after opening channel, set channel and check whether the channel
-  // needs to be suspended.
-  void NotifyChannelOpened(nsIChannel* aChannel);
-
-  // Call before closing channel, reset the channel internal status if needed.
-  void NotifyChannelClosing();
-
-  // Check whether we need to suspend the channel.
-  void UpdateSuspendedStatusIfNeeded();
+  // Tell the agent to manage the suspend status of the channel.
+  void Delegate(nsIChannel* aChannel);
+  // Stop the management of the suspend status of the channel.
+  void Revoke();
 
 private:
   // Only suspends channel but not changes the suspend count.
   void SuspendInternal();
 
-  nsIChannel* mChannel;
+  nsIChannel* mChannel = nullptr;
   MediaCacheStream& mCacheStream;
   uint32_t mSuspendCount = 0;
   bool mIsChannelSuspended = false;
 };
 
 /**
  * This is the MediaResource implementation that wraps Necko channels.
  * Much of its functionality is actually delegated to MediaCache via