Bug 647178 - Don't call Suspend on channel if the channel has completed downloading - r=roc
authorChris Double <chris.double@double.co.nz>
Fri, 01 Apr 2011 22:43:29 +1300
changeset 67657 43cddadd2a45a039702673223d88197abb1a0d35
parent 67656 0dcdf1c5a7fadcf3a32ee7b6d29570d97787a299
child 67658 f6cb911d37da7a7968164be18da41cc64734a2b6
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs647178
milestone2.2a1pre
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 647178 - Don't call Suspend on channel if the channel has completed downloading - r=roc
content/media/nsMediaStream.cpp
content/media/nsMediaStream.h
--- a/content/media/nsMediaStream.cpp
+++ b/content/media/nsMediaStream.cpp
@@ -65,17 +65,18 @@ using namespace mozilla;
 
 nsMediaChannelStream::nsMediaChannelStream(nsMediaDecoder* aDecoder,
     nsIChannel* aChannel, nsIURI* aURI)
   : nsMediaStream(aDecoder, aChannel, aURI),
     mOffset(0), mSuspendCount(0),
     mReopenOnError(PR_FALSE), mIgnoreClose(PR_FALSE),
     mCacheStream(this),
     mLock("nsMediaChannelStream.mLock"),
-    mCacheSuspendCount(0)
+    mCacheSuspendCount(0),
+    mIgnoreResume(PR_FALSE)
 {
 }
 
 nsMediaChannelStream::~nsMediaChannelStream()
 {
   if (mListener) {
     // Kill its reference to us since we're going away
     mListener->Revoke();
@@ -271,17 +272,20 @@ nsMediaChannelStream::OnStartRequest(nsI
     MutexAutoLock lock(mLock);
     mChannelStatistics.Start(TimeStamp::Now());
   }
 
   mReopenOnError = PR_FALSE;
   mIgnoreClose = PR_FALSE;
   if (mSuspendCount > 0) {
     // Re-suspend the channel if it needs to be suspended
+    // No need to call PossiblySuspend here since the channel is
+    // definitely in the right state for us in OneStartRequest.
     mChannel->Suspend();
+    mIgnoreResume = PR_FALSE;
   }
 
   // Fires an initial progress event and sets up the stall counter so stall events
   // fire if no download occurs within the required time frame.
   mDecoder->Progress(PR_FALSE);
 
   return NS_OK;
 }
@@ -551,17 +555,17 @@ void nsMediaChannelStream::CloseChannel(
   if (mListener) {
     mListener->Revoke();
     mListener = nsnull;
   }
 
   if (mChannel) {
     if (mSuspendCount > 0) {
       // Resume the channel before we cancel it
-      mChannel->Resume();
+      PossiblyResume();
     }
     // 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 DocumentViewerImpl::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.
@@ -621,17 +625,17 @@ void nsMediaChannelStream::Suspend(PRBoo
       mIgnoreClose = PR_TRUE;
       CloseChannel();
       element->DownloadSuspended();
     } else if (mSuspendCount == 0) {
       {
         MutexAutoLock lock(mLock);
         mChannelStatistics.Stop(TimeStamp::Now());
       }
-      mChannel->Suspend();
+      PossiblySuspend();
       element->DownloadSuspended();
     }
   }
 
   ++mSuspendCount;
 }
 
 void nsMediaChannelStream::Resume()
@@ -652,17 +656,17 @@ void nsMediaChannelStream::Resume()
       // Just wake up our existing channel
       {
         MutexAutoLock lock(mLock);
         mChannelStatistics.Start(TimeStamp::Now());
       }
       // if an error occurs after Resume, assume it's because the server
       // timed out the connection and we should reopen it.
       mReopenOnError = PR_TRUE;
-      mChannel->Resume();
+      PossiblyResume();
       element->DownloadResumed();
     } else {
       PRInt64 totalLength = mCacheStream.GetLength();
       // If mOffset is at the end of the stream, then we shouldn't try to
       // seek to it. The seek will fail and be wasted anyway. We can leave
       // the channel dead; if the media cache wants to read some other data
       // in the future, it will call CacheClientSeek itself which will reopen the
       // channel.
@@ -862,16 +866,39 @@ nsMediaChannelStream::GetDownloadRate(PR
 }
 
 PRInt64
 nsMediaChannelStream::GetLength()
 {
   return mCacheStream.GetLength();
 }
 
+void
+nsMediaChannelStream::PossiblySuspend()
+{
+  PRBool isPending = PR_FALSE;
+  nsresult rv = mChannel->IsPending(&isPending);
+  if (NS_SUCCEEDED(rv) && isPending) {
+    mChannel->Suspend();
+    mIgnoreResume = PR_FALSE;
+  } else {
+    mIgnoreResume = PR_TRUE;
+  }
+}
+
+void
+nsMediaChannelStream::PossiblyResume()
+{
+  if (!mIgnoreResume) {
+    mChannel->Resume();
+  } else {
+    mIgnoreResume = PR_FALSE;
+  }
+}
+
 class nsMediaFileStream : public nsMediaStream
 {
 public:
   nsMediaFileStream(nsMediaDecoder* aDecoder, nsIChannel* aChannel, nsIURI* aURI) :
     nsMediaStream(aDecoder, aChannel, aURI), mSize(-1),
     mLock("nsMediaFileStream.mLock")
   {
   }
--- a/content/media/nsMediaStream.h
+++ b/content/media/nsMediaStream.h
@@ -447,16 +447,24 @@ protected:
 
   static NS_METHOD CopySegmentToCache(nsIInputStream *aInStream,
                                       void *aClosure,
                                       const char *aFromSegment,
                                       PRUint32 aToOffset,
                                       PRUint32 aCount,
                                       PRUint32 *aWriteCount);
 
+  // Suspend the channel only if the channels is currently downloading data.
+  // If it isn't we set a flag, mIgnoreResume, so that PossiblyResume knows
+  // whether to acutually resume or not.
+  void PossiblySuspend();
+
+  // Resume from a suspend if we actually suspended (See PossiblySuspend).
+  void PossiblyResume();
+
   // Main thread access only
   PRInt64            mOffset;
   nsRefPtr<Listener> mListener;
   // A data received event for the decoder that has been dispatched but has
   // not yet been processed.
   nsRevocableEventPtr<nsRunnableMethod<nsMediaChannelStream, void, false> > mDataReceivedEvent;
   PRUint32           mSuspendCount;
   // When this flag is set, if we get a network error we should silently
@@ -468,11 +476,16 @@ protected:
 
   // Any thread access
   nsMediaCacheStream mCacheStream;
 
   // This lock protects mChannelStatistics and mCacheSuspendCount
   Mutex               mLock;
   nsChannelStatistics mChannelStatistics;
   PRUint32            mCacheSuspendCount;
+
+  // PR_TRUE if we couldn't suspend the stream and we therefore don't want
+  // to resume later. This is usually due to the channel not being in the
+  // isPending state at the time of the suspend request.
+  PRPackedBool mIgnoreResume;
 };
 
 #endif