Bug 1415461 - consolidate the life cycle of the channel managed by ChannelSuspendAgent. draft
authorJW Wang <jwwang@mozilla.com>
Wed, 08 Nov 2017 16:16:24 +0800
changeset 696102 971044a3ca5ec9f17041fd5d5005f9a682d5dcfa
parent 695965 a12053d9b5cba57b794be594c2d5fb7bd1ee3929
child 696934 f02f1542258668ea47203a51f807ece6429f0ace
push id88640
push userjwwang@mozilla.com
push dateFri, 10 Nov 2017 07:30:58 +0000
bugs1415461
milestone58.0a1
Bug 1415461 - consolidate the life cycle of the channel managed by ChannelSuspendAgent. 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