Bug 1412188 - ChannelMediaResource shouldn't create new channels when closed. draft
authorJW Wang <jwwang@mozilla.com>
Fri, 27 Oct 2017 12:10:23 +0800
changeset 688520 580cdd7b29624c7e5935ca05ee58a96d0b9603ed
parent 688392 927f660917377296f3e826df18e7086e58c1370f
child 738086 8e3e389e4f10da77efff086cd7a19a21fe9de7d7
push id86767
push userjwwang@mozilla.com
push dateMon, 30 Oct 2017 06:34:08 +0000
bugs1412188
milestone58.0a1
Bug 1412188 - ChannelMediaResource shouldn't create new channels when closed. 1. Add assertions to functions that shouldn't be called after Close(). 2. Early return if closed from Suspend/Resume/Seek which are called asynchronously by the cache. MozReview-Commit-ID: KZOJer36o77
dom/media/ChannelMediaResource.cpp
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -156,16 +156,17 @@ IsPayloadCompressed(nsIHttpChannel* aCha
   return encoding.Length() > 0;
 }
 
 nsresult
 ChannelMediaResource::OnStartRequest(nsIRequest* aRequest,
                                      int64_t aRequestOffset)
 {
   NS_ASSERTION(mChannel.get() == aRequest, "Wrong channel!");
+  MOZ_DIAGNOSTIC_ASSERT(!mCacheStream.IsClosed());
 
   MediaDecoderOwner* owner = mCallback->GetMediaOwner();
   NS_ENSURE_TRUE(owner, NS_ERROR_FAILURE);
   dom::HTMLMediaElement* element = owner->GetMediaElement();
   NS_ENSURE_TRUE(element, NS_ERROR_FAILURE);
   nsresult status;
   nsresult rv = aRequest->GetStatus(&status);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -366,16 +367,17 @@ ChannelMediaResource::ParseContentRangeH
 }
 
 nsresult
 ChannelMediaResource::OnStopRequest(nsIRequest* aRequest, nsresult aStatus)
 {
   NS_ASSERTION(mChannel.get() == aRequest, "Wrong channel!");
   NS_ASSERTION(!mSuspendAgent.IsSuspended(),
                "How can OnStopRequest fire while we're suspended?");
+  MOZ_DIAGNOSTIC_ASSERT(!mCacheStream.IsClosed());
 
   mChannelStatistics.Stop();
 
   // Note that aStatus might have succeeded --- this might be a normal close
   // --- even in situations where the server cut us off because we were
   // suspended. So we need to "reopen on error" in that case too. The only
   // cases where we don't need to reopen are when *we* closed the stream.
   // But don't reopen if we need to seek and we don't think we can... that would
@@ -498,16 +500,17 @@ ChannelMediaResource::Open(nsIStreamList
   NS_ADDREF(*aStreamListener);
   return NS_OK;
 }
 
 nsresult
 ChannelMediaResource::OpenChannel(int64_t aOffset)
 {
   MOZ_ASSERT(NS_IsMainThread());
+  MOZ_DIAGNOSTIC_ASSERT(!mCacheStream.IsClosed());
   MOZ_ASSERT(mChannel);
   MOZ_ASSERT(!mListener, "Listener should have been removed by now");
 
   mListener = new Listener(this, aOffset, ++mLoadID);
   nsresult rv = mChannel->SetNotificationCallbacks(mListener.get());
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = SetupChannelHeaders(aOffset);
@@ -523,16 +526,18 @@ ChannelMediaResource::OpenChannel(int64_
   element->DownloadResumed();
 
   return NS_OK;
 }
 
 nsresult
 ChannelMediaResource::SetupChannelHeaders(int64_t aOffset)
 {
+  MOZ_DIAGNOSTIC_ASSERT(!mCacheStream.IsClosed());
+
   // Always use a byte range request even if we're reading from the start
   // of the resource.
   // This enables us to detect if the stream supports byte range
   // requests, and therefore seeking, early.
   nsCOMPtr<nsIHttpChannel> hc = do_QueryInterface(mChannel);
   if (hc) {
     // Use |mOffset| if seeking in a complete file download.
     nsAutoCString rangeString("bytes=");
@@ -657,20 +662,26 @@ int64_t ChannelMediaResource::Tell()
   return mCacheStream.Tell();
 }
 
 nsresult ChannelMediaResource::GetCachedRanges(MediaByteRangeSet& aRanges)
 {
   return mCacheStream.GetCachedRanges(aRanges);
 }
 
-void ChannelMediaResource::Suspend(bool aCloseImmediately)
+void
+ChannelMediaResource::Suspend(bool aCloseImmediately)
 {
   NS_ASSERTION(NS_IsMainThread(), "Don't call on non-main thread");
 
+  if (mCacheStream.IsClosed()) {
+    // Nothing to do when we are closed.
+    return;
+  }
+
   MediaDecoderOwner* owner = mCallback->GetMediaOwner();
   if (!owner) {
     // Shutting down; do nothing.
     return;
   }
   dom::HTMLMediaElement* element = owner->GetMediaElement();
   if (!element) {
     // Shutting down; do nothing.
@@ -685,20 +696,26 @@ void ChannelMediaResource::Suspend(bool 
   if (mSuspendAgent.Suspend()) {
     if (mChannel) {
       mChannelStatistics.Stop();
       element->DownloadSuspended();
     }
   }
 }
 
-void ChannelMediaResource::Resume()
+void
+ChannelMediaResource::Resume()
 {
   NS_ASSERTION(NS_IsMainThread(), "Don't call on non-main thread");
 
+  if (mCacheStream.IsClosed()) {
+    // Nothing to do when we are closed.
+    return;
+  }
+
   MediaDecoderOwner* owner = mCallback->GetMediaOwner();
   if (!owner) {
     // Shutting down; do nothing.
     return;
   }
   dom::HTMLMediaElement* element = owner->GetMediaElement();
   if (!element) {
     // Shutting down; do nothing.
@@ -731,16 +748,18 @@ void ChannelMediaResource::Resume()
       }
     }
   }
 }
 
 nsresult
 ChannelMediaResource::RecreateChannel()
 {
+  MOZ_DIAGNOSTIC_ASSERT(!mCacheStream.IsClosed());
+
   nsLoadFlags loadFlags =
     nsICachingChannel::LOAD_BYPASS_LOCAL_CACHE_IF_BUSY |
     nsIChannel::LOAD_CLASSIFY_URI |
     (mLoadInBackground ? nsIRequest::LOAD_BACKGROUND : 0);
 
   MediaDecoderOwner* owner = mCallback->GetMediaOwner();
   if (!owner) {
     // The decoder is being shut down, so don't bother opening a new channel
@@ -854,16 +873,22 @@ ChannelMediaResource::CacheClientNotifyS
     &MediaResourceCallback::NotifySuspendedStatusChanged,
     IsSuspendedByCache()));
 }
 
 nsresult
 ChannelMediaResource::Seek(int64_t aOffset, bool aResume)
 {
   MOZ_ASSERT(NS_IsMainThread());
+
+  if (mCacheStream.IsClosed()) {
+    // Nothing to do when we are closed.
+    return NS_OK;
+  }
+
   LOG("Seek requested for aOffset [%" PRId64 "]", aOffset);
 
   CloseChannel();
 
   if (aResume) {
     mSuspendAgent.Resume();
   }