Bug 1418430. P1 - always check "reopen on error" when a connection is closed. draft
authorJW Wang <jwwang@mozilla.com>
Wed, 22 Nov 2017 10:35:48 +0800
changeset 701670 1ad9f57e48da14be376174aa2a2c3eb123b2cc03
parent 701669 a768b2af57f80b1a91ab781f139542501dd80569
child 701671 19b4ab022ab7b68b66f50e61afd313137f8baec4
push id90231
push userjwwang@mozilla.com
push dateWed, 22 Nov 2017 03:44:14 +0000
bugs1418430
milestone59.0a1
Bug 1418430. P1 - always check "reopen on error" when a connection is closed. The server might send us fewer bytes than requested. So we also need "reopen on error" in this case as well. MozReview-Commit-ID: Fi82x4h1TZ0
dom/media/ChannelMediaResource.cpp
dom/media/ChannelMediaResource.h
dom/media/MediaCache.cpp
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -79,17 +79,17 @@ ChannelMediaResource::Listener::OnStartR
 nsresult
 ChannelMediaResource::Listener::OnStopRequest(nsIRequest* aRequest,
                                               nsISupports* aContext,
                                               nsresult aStatus)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mResource)
     return NS_OK;
-  return mResource->OnStopRequest(aRequest, aStatus, mReopenOnError);
+  return mResource->OnStopRequest(aRequest, aStatus);
 }
 
 nsresult
 ChannelMediaResource::Listener::OnDataAvailable(nsIRequest* aRequest,
                                                 nsISupports* aContext,
                                                 nsIInputStream* aStream,
                                                 uint64_t aOffset,
                                                 uint32_t aCount)
@@ -366,19 +366,17 @@ ChannelMediaResource::ParseContentRangeH
 
   LOG("Received bytes [%" PRId64 "] to [%" PRId64 "] of [%" PRId64 "] for decoder[%p]",
       aRangeStart, aRangeEnd, aRangeTotal, mCallback.get());
 
   return NS_OK;
 }
 
 nsresult
-ChannelMediaResource::OnStopRequest(nsIRequest* aRequest,
-                                    nsresult aStatus,
-                                    bool aReopenOnError)
+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(!mClosed);
 
   mChannelStatistics.Stop();
 
@@ -388,17 +386,17 @@ ChannelMediaResource::OnStopRequest(nsIR
   nsLoadFlags loadFlags;
   DebugOnly<nsresult> rv = mChannel->GetLoadFlags(&loadFlags);
   NS_ASSERTION(NS_SUCCEEDED(rv), "GetLoadFlags() failed!");
 
   if (loadFlags & nsIRequest::LOAD_BACKGROUND) {
     ModifyLoadFlags(loadFlags & ~nsIRequest::LOAD_BACKGROUND);
   }
 
-  mCacheStream.NotifyDataEnded(mLoadID, aStatus, aReopenOnError);
+  mCacheStream.NotifyDataEnded(mLoadID, aStatus, true /*aReopenOnError*/);
   return NS_OK;
 }
 
 nsresult
 ChannelMediaResource::OnChannelRedirect(nsIChannel* aOld,
                                         nsIChannel* aNew,
                                         uint32_t aFlags,
                                         int64_t aOffset)
@@ -714,19 +712,16 @@ ChannelMediaResource::Resume()
   MOZ_DIAGNOSTIC_ASSERT(owner);
   dom::HTMLMediaElement* element = owner->GetMediaElement();
   MOZ_DIAGNOSTIC_ASSERT(element);
 
   if (mSuspendAgent.Resume()) {
     if (mChannel) {
       // Just wake up our existing channel
       mChannelStatistics.Start();
-      // if an error occurs after Resume, assume it's because the server
-      // timed out the connection and we should reopen it.
-      mListener->SetReopenOnError();
       element->DownloadResumed();
     } else {
       int64_t totalLength = 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.
--- a/dom/media/ChannelMediaResource.h
+++ b/dom/media/ChannelMediaResource.h
@@ -169,44 +169,38 @@ public:
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSIREQUESTOBSERVER
     NS_DECL_NSISTREAMLISTENER
     NS_DECL_NSICHANNELEVENTSINK
     NS_DECL_NSIINTERFACEREQUESTOR
     NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
 
     void Revoke();
-    void SetReopenOnError() { mReopenOnError = true; }
 
   private:
     Mutex mMutex;
     // mResource should only be modified on the main thread with the lock.
     // So it can be read without lock on the main thread or on other threads
     // with the lock.
     RefPtr<ChannelMediaResource> mResource;
-    // When this flag is set, if we get a network error we should silently
-    // reopen the stream. Main thread only.
-    bool mReopenOnError = false;
 
     const int64_t mOffset;
     const uint32_t mLoadID;
   };
   friend class Listener;
 
   nsresult GetCachedRanges(MediaByteRangeSet& aRanges) override;
 
 protected:
   nsresult Seek(int64_t aOffset, bool aResume);
 
   bool IsSuspendedByCache();
   // These are called on the main thread by Listener.
   nsresult OnStartRequest(nsIRequest* aRequest, int64_t aRequestOffset);
-  nsresult OnStopRequest(nsIRequest* aRequest,
-                         nsresult aStatus,
-                         bool aReopenOnError);
+  nsresult OnStopRequest(nsIRequest* aRequest, nsresult aStatus);
   nsresult OnDataAvailable(uint32_t aLoadID,
                            nsIInputStream* aStream,
                            uint32_t aCount);
   nsresult OnChannelRedirect(nsIChannel* aOld,
                              nsIChannel* aNew,
                              uint32_t aFlags,
                              int64_t aOffset);
 
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2135,17 +2135,18 @@ MediaCacheStream::NotifyDataEndedInterna
 
   if (mClosed || aLoadID != mLoadID) {
     // Nothing to do if the stream is closed or a new load has begun.
     return;
   }
 
   // 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
+  // suspended. It is also possible that the server sends us fewer bytes than
+  // requested. 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
   // cause us to just re-read the stream, which would be really bad.
   if (aReopenOnError && aStatus != NS_ERROR_PARSED_DATA_CACHED &&
       aStatus != NS_BINDING_ABORTED &&
       (mChannelOffset == 0 ||
        (mStreamLength > 0 && mChannelOffset != mStreamLength &&
         mIsTransportSeekable))) {