Bug 479863. Add ability to close and reopen suspended streams, and recover from server-side disconnect of suspended streams. r=doublec

--HG--
extra : rebase_source : 5ed7825cd919ad3f997f58dafb61c2d4249b60d4

content/media/video/public/nsMediaCache.h

@@ -268,7 +268,8 @@ public:
   // requested. In particular we might unexpectedly start providing
   // data at offset 0. This need not be called if the offset is the
   // offset that the cache requested in
-  // nsMediaChannelStream::CacheClientSeek.
+  // nsMediaChannelStream::CacheClientSeek. This can be called at any
+  // time by the client, not just after a CacheClientSeek.
   void NotifyDataStarted(PRInt64 aOffset);
   // Notifies the cache that data has been received. The stream already
   // knows the offset because data is received in sequence and
@@ -320,6 +321,8 @@ public:
   // because it doesn't know when the decoder was paused, buffering, etc.
   // Do not pass zero.
   void SetPlaybackRate(PRUint32 aBytesPerSecond);
+  // Returns the last set value of SetSeekable.
+  PRBool IsSeekable();
 
   // These methods must be called on a different thread from the main
   // thread. They should always be called on the same thread for a given

content/media/video/public/nsMediaStream.h

@@ -158,7 +158,11 @@ public:
   // return an error.
   virtual nsresult Close() = 0;
   // Suspend any downloads that are in progress.
-  virtual void Suspend() = 0;
+  // If aCloseImmediately is set, resources should be released immediately
+  // since we don't expect to resume again any time soon. Otherwise we
+  // may resume again soon so resources should be held for a little
+  // while.
+  virtual void Suspend(PRBool aCloseImmediately) = 0;
   // Resume any downloads that have been suspended.
   virtual void Resume() = 0;
   // Get the current principal for the channel
@@ -311,7 +315,7 @@ public:
   // and no more data from the old load will be notified via
   // nsMediaCacheStream::NotifyDataReceived/Ended.
   // This can fail.
-  nsresult CacheClientSeek(PRInt64 aOffset);
+  nsresult CacheClientSeek(PRInt64 aOffset, PRBool aResume);
   // Suspend the current load since data is currently not wanted
   nsresult CacheClientSuspend();
   // Resume the current load since data is wanted again
@@ -320,7 +324,7 @@ public:
   // Main thread
   virtual nsresult Open(nsIStreamListener** aStreamListener);
   virtual nsresult Close();
-  virtual void     Suspend();
+  virtual void     Suspend(PRBool aCloseImmediately);
   virtual void     Resume();
   virtual already_AddRefed<nsIPrincipal> GetCurrentPrincipal();
   // Return PR_TRUE if the stream has been closed.
@@ -372,9 +376,9 @@ protected:
                            PRUint32 aCount);
   nsresult OnChannelRedirect(nsIChannel* aOld, nsIChannel* aNew, PRUint32 aFlags);
 
-  // Opens the channel, using an HTTP byte range request to start at aOffset
+  // Opens the channel, using an HTTP byte range request to start at mOffset
   // if possible. Main thread only.
-  nsresult OpenChannel(nsIStreamListener** aStreamListener, PRInt64 aOffset);
+  nsresult OpenChannel(nsIStreamListener** aStreamListener);
   void SetupChannelHeaders();
   // Closes the channel. Main thread only.
   void CloseChannel();
@@ -387,9 +391,15 @@ protected:
                                       PRUint32 *aWriteCount);
 
   // Main thread access only
-  PRInt64            mLastSeekOffset;
+  PRInt64            mOffset;
   nsRefPtr<Listener> mListener;
   PRUint32           mSuspendCount;
+  // When this flag is set, if we get a network error we should silently
+  // reopen the stream.
+  PRPackedBool       mReopenOnError;
+  // When this flag is set, we should not report the next close of the
+  // channel.
+  PRPackedBool       mIgnoreClose;
 
   // Any thread access
   nsMediaCacheStream mCacheStream;

content/media/video/src/nsMediaCache.cpp

@@ -1031,18 +1031,13 @@ nsMediaCache::Update()
       // We need to seek now.
       NS_ASSERTION(stream->mIsSeekable || desiredOffset == 0,
                    "Trying to seek in a non-seekable stream!");
-      if (stream->mCacheSuspended) {
-        LOG(PR_LOG_DEBUG, ("Stream %p Resumed", stream));
-        rv = stream->mClient->CacheClientResume();
-        stream->mCacheSuspended = PR_FALSE;
-      }
-      if (NS_SUCCEEDED(rv)) {
-        // Round seek offset down to the start of the block
-        stream->mChannelOffset = (desiredOffset/BLOCK_SIZE)*BLOCK_SIZE;
-        LOG(PR_LOG_DEBUG, ("Stream %p CacheSeek to %lld", stream,
-            (long long)stream->mChannelOffset));
-        rv = stream->mClient->CacheClientSeek(stream->mChannelOffset);
-      }
+      // Round seek offset down to the start of the block
+      stream->mChannelOffset = (desiredOffset/BLOCK_SIZE)*BLOCK_SIZE;
+      LOG(PR_LOG_DEBUG, ("Stream %p CacheSeek to %lld (resume=%d)", stream,
+          (long long)stream->mChannelOffset, stream->mCacheSuspended));
+      rv = stream->mClient->CacheClientSeek(stream->mChannelOffset,
+                                            stream->mCacheSuspended);
+      stream->mCacheSuspended = PR_FALSE;
     } else if (enableReading && stream->mCacheSuspended) {
       LOG(PR_LOG_DEBUG, ("Stream %p Resumed", stream));
       rv = stream->mClient->CacheClientResume();
@@ -1534,6 +1529,13 @@ nsMediaCacheStream::SetSeekable(PRBool aIsSeekable)
   gMediaCache->QueueUpdate();
 }
 
+PRBool
+nsMediaCacheStream::IsSeekable()
+{
+  nsAutoMonitor mon(gMediaCache->Monitor());
+  return mIsSeekable;
+}
+
 void
 nsMediaCacheStream::Close()
 {

content/media/video/src/nsMediaStream.cpp

@@ -65,10 +65,11 @@ using mozilla::TimeStamp;
 nsMediaChannelStream::nsMediaChannelStream(nsMediaDecoder* aDecoder,
     nsIChannel* aChannel, nsIURI* aURI)
   : nsMediaStream(aDecoder, aChannel, aURI),
-    mLastSeekOffset(0), mSuspendCount(0),
+    mOffset(0), mSuspendCount(0), 
+    mReopenOnError(PR_FALSE), mIgnoreClose(PR_FALSE),
     mCacheStream(this),
     mLock(nsAutoLock::NewLock("media.channel.stream")),
-    mCacheSuspendCount(0)
+    mCacheSuspendCount(0)    
 {
 }
 
@@ -166,7 +167,7 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest)
                           ranges);
     PRBool acceptsRanges = ranges.EqualsLiteral("bytes"); 
 
-    if (mLastSeekOffset == 0) {
+    if (mOffset == 0) {
       // Look for duration headers from known Ogg content systems. In the case
       // of multiple options for obtaining the duration the order of precedence is;
       // 1) The Media resource metadata if possible (done by the decoder itself).
@@ -190,12 +191,13 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest)
  
     PRUint32 responseStatus = 0; 
     hc->GetResponseStatus(&responseStatus);
-    if (mLastSeekOffset > 0 && responseStatus == HTTP_OK_CODE) {
+    if (mOffset > 0 && responseStatus == HTTP_OK_CODE) {
       // If we get an OK response but we were seeking, we have to assume
       // that seeking doesn't work. We also need to tell the cache that
       // it's getting data for the start of the stream.
       mCacheStream.NotifyDataStarted(0);
-    } else if (mLastSeekOffset == 0 && 
+      mOffset = 0;
+    } else if (mOffset == 0 && 
                (responseStatus == HTTP_OK_CODE ||
                 responseStatus == HTTP_PARTIAL_RESPONSE_CODE)) {
       // We weren't seeking and got a valid response status,
@@ -206,6 +208,8 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest)
         mCacheStream.NotifyDataLength(cl);
       }
     }
+    // XXX we probably should examine the Content-Range header in case
+    // the server gave us a range which is not quite what we asked for
 
     // If we get an HTTP_OK_CODE response to our byte range request,
     // and the server isn't sending Accept-Ranges:bytes then we don't
@@ -230,6 +234,8 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest)
     mChannelStatistics.Start(TimeStamp::Now());
   }
 
+  mReopenOnError = PR_FALSE;
+  mIgnoreClose = PR_FALSE;
   if (mSuspendCount > 0) {
     // Re-suspend the channel if it needs to be suspended
     mChannel->Suspend();
@@ -246,18 +252,30 @@ nsresult
 nsMediaChannelStream::OnStopRequest(nsIRequest* aRequest, nsresult aStatus)
 {
   NS_ASSERTION(mChannel.get() == aRequest, "Wrong channel!");
+  NS_ASSERTION(mSuspendCount == 0,
+               "How can OnStopRequest fire while we're suspended?");
 
   {
     nsAutoLock lock(mLock);
     mChannelStatistics.Stop(TimeStamp::Now());
   }
 
-  mCacheStream.NotifyDataEnded(aStatus);
-  if (mDecoder) {
-    mDecoder->NotifyDownloadEnded(aStatus);
+  if (NS_FAILED(aStatus) && aStatus != NS_ERROR_PARSED_DATA_CACHED &&
+      mReopenOnError) {
+    nsresult rv = CacheClientSeek(mOffset, PR_FALSE);
+    if (NS_SUCCEEDED(rv))
+      return rv;
+    // If the reopen/reseek fails, just fall through and treat this
+    // error as fatal.
   }
-  NS_ASSERTION(mSuspendCount == 0,
-               "How can OnStopRequest fire while we're suspended?");
+
+  if (!mIgnoreClose) {
+    mCacheStream.NotifyDataEnded(aStatus);
+    if (mDecoder) {
+      mDecoder->NotifyDownloadEnded(aStatus);
+    }
+  }
+
   return NS_OK;
 }
 
@@ -284,6 +302,8 @@ nsMediaChannelStream::CopySegmentToCache(nsIInputStream *aInStream,
                                          PRUint32 *aWriteCount)
 {
   CopySegmentClosure* closure = static_cast<CopySegmentClosure*>(aClosure);
+  // Keep track of where we're up to
+  closure->mStream->mOffset += aCount;
   closure->mStream->mCacheStream.NotifyDataReceived(aCount, aFromSegment,
                                                     closure->mPrincipal);
   *aWriteCount = aCount;
@@ -336,11 +356,11 @@ nsresult nsMediaChannelStream::Open(nsIStreamListener **aStreamListener)
   nsresult rv = mCacheStream.Init();
   if (NS_FAILED(rv))
     return rv;
-  return OpenChannel(aStreamListener, 0);
+  NS_ASSERTION(mOffset == 0, "Who set mOffset already?");
+  return OpenChannel(aStreamListener);
 }
 
-nsresult nsMediaChannelStream::OpenChannel(nsIStreamListener** aStreamListener,
-                                           PRInt64 aOffset)
+nsresult nsMediaChannelStream::OpenChannel(nsIStreamListener** aStreamListener)
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   NS_ENSURE_TRUE(mChannel, NS_ERROR_NULL_POINTER);
@@ -350,8 +370,6 @@ nsresult nsMediaChannelStream::OpenChannel(nsIStreamListener** aStreamListener,
     *aStreamListener = nsnull;
   }
 
-  mLastSeekOffset = aOffset;
-
   mListener = new Listener(this);
   NS_ENSURE_TRUE(mListener, NS_ERROR_OUT_OF_MEMORY);
 
@@ -402,11 +420,11 @@ void nsMediaChannelStream::SetupChannelHeaders()
   nsCOMPtr<nsIHttpChannel> hc = do_QueryInterface(mChannel);
   if (hc) {
     nsCAutoString rangeString("bytes=");
-    rangeString.AppendInt(mLastSeekOffset);
+    rangeString.AppendInt(mOffset);
     rangeString.Append("-");
     hc->SetRequestHeader(NS_LITERAL_CSTRING("Range"), rangeString, PR_FALSE);
   } else {
-    NS_ASSERTION(mLastSeekOffset == 0, "Don't know how to seek on this channel type");
+    NS_ASSERTION(mOffset == 0, "Don't know how to seek on this channel type");
   }
 }
 
@@ -479,38 +497,53 @@ PRInt64 nsMediaChannelStream::Tell()
   return mCacheStream.Tell();
 }
 
-void nsMediaChannelStream::Suspend()
+void nsMediaChannelStream::Suspend(PRBool aCloseImmediately)
 {
   NS_ASSERTION(NS_IsMainThread(), "Don't call on main thread");
 
-  if (mSuspendCount == 0 && mChannel) {
-    {
-      nsAutoLock lock(mLock);
-      mChannelStatistics.Stop(TimeStamp::Now());
+  if (mChannel) {
+    if (aCloseImmediately && mCacheStream.IsSeekable()) {
+      // Kill off our channel right now, but don't tell anyone about it.
+      mIgnoreClose = PR_TRUE;
+      CloseChannel();
+    } else if (mSuspendCount == 0) {
+      {
+        nsAutoLock lock(mLock);
+        mChannelStatistics.Stop(TimeStamp::Now());
+      }
+      mChannel->Suspend();
     }
-    mChannel->Suspend();
   }
+
   ++mSuspendCount;
 }
 
 void nsMediaChannelStream::Resume()
 {
   NS_ASSERTION(NS_IsMainThread(), "Don't call on main thread");
+  NS_ASSERTION(mSuspendCount > 0, "Too many resumes!");
 
   --mSuspendCount;
-  if (mSuspendCount == 0 && mChannel) {
-    {
-      nsAutoLock lock(mLock);
-      mChannelStatistics.Start(TimeStamp::Now());
+  if (mSuspendCount == 0) {
+    if (mChannel) {
+      // Just wake up our existing channel
+      {
+        nsAutoLock 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();
+    } else {
+      // Need to recreate the channel
+      CacheClientSeek(mOffset, PR_FALSE);
     }
-    mChannel->Resume();
-    // XXX need to do something fancier here because we often won't
-    // be able to resume cleanly
   }
 }
 
 nsresult
-nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset)
+nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset, PRBool aResume)
 {
   NS_ASSERTION(NS_IsMainThread(), "Don't call on main thread");
 
@@ -528,6 +561,12 @@ nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset)
   nsCOMPtr<nsILoadGroup> loadGroup = element->GetDocumentLoadGroup();
   NS_ENSURE_TRUE(loadGroup, NS_ERROR_NULL_POINTER);
 
+  if (aResume) {
+    NS_ASSERTION(mSuspendCount > 0, "Too many resumes!");
+    // No need to mess with the channel, since we're making a new one
+    --mSuspendCount;
+  }
+
   nsresult rv =
     NS_NewChannel(getter_AddRefs(mChannel),
                   mURI,
@@ -537,7 +576,8 @@ nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset)
                   loadFlags);
   if (NS_FAILED(rv))
     return rv;
-  return OpenChannel(nsnull, aOffset);
+  mOffset = aOffset;
+  return OpenChannel(nsnull);
 }
 
 class SuspendedStatusChanged : public nsRunnable 
@@ -569,7 +609,7 @@ nsMediaChannelStream::CacheClientSuspend()
     nsAutoLock lock(mLock);
     ++mCacheSuspendCount;
   }
-  Suspend();
+  Suspend(PR_FALSE);
 
   // We have to spawn an event here since we're being called back from
   // a sensitive place in nsMediaCache, which doesn't want us to reenter
@@ -676,7 +716,7 @@ public:
   // Main thread
   virtual nsresult Open(nsIStreamListener** aStreamListener);
   virtual nsresult Close();
-  virtual void     Suspend() {}
+  virtual void     Suspend(PRBool aCloseImmediately) {}
   virtual void     Resume() {}
   virtual already_AddRefed<nsIPrincipal> GetCurrentPrincipal();
 

content/media/video/src/nsOggDecoder.cpp

@@ -2232,7 +2232,7 @@ PRBool nsOggDecoder::GetSeekable()
 void nsOggDecoder::Suspend()
 {
   if (mReader) {
-    mReader->Stream()->Suspend();
+    mReader->Stream()->Suspend(PR_TRUE);
   }
 }
 

content/media/video/src/nsWaveDecoder.cpp

@@ -1616,7 +1616,7 @@ void
 nsWaveDecoder::Suspend()
 {
   if (mStream) {
-    mStream->Suspend();
+    mStream->Suspend(PR_TRUE);
   }
 }
 

content/media/video/test/Makefile.in

@@ -71,6 +71,7 @@ _TEST_FILES += \
 		test_bug468190.html \
 		test_bug482461.html \
 		test_can_play_type_ogg.html \
+		test_closing_connections.html \
 		test_contentDuration1.html \
 		test_contentDuration2.html \
 		test_contentDuration3.html \

content/media/video/test/test_closing_connections.html

@@ -0,0 +1,61 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=479863
+-->
+<head>
+  <title>Test for Bug 479863 --- loading many connections</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript" src="use_large_cache.js"></script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=479863">Mozilla Bug 479863</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+
+<script type="application/javascript">
+window.onload = function() {
+  ok(true, "loaded metadata for all videos");
+  SimpleTest.finish();
+}
+</script>
+
+<!-- With normal per-domain connection limits and a naive implementation, we
+     won't ever be able to load all these videos because the first 15 (or whatever)
+     will each take up one HTTP connection (which will be suspended) and then
+     the others will be blocked by the per-domain HTTP connection limit. We
+     pass this test by closing the connection for non-buffered videos after
+     we've got the first frame. -->
+
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+<video src="seek.ogv"></video>
+
+<pre id="test">
+<script type="application/javascript">
+SimpleTest.waitForExplicitFinish();
+</script>
+</pre>
+</body>
+</html>