bug 742827 - fix problem with landing of bug 671591 r=honzab

netwerk/protocol/http/nsHttpTransaction.cpp

@@ -138,11 +138,10 @@ nsHttpTransaction::nsHttpTransaction()
     , mSSLConnectFailed(false)
     , mHttpResponseMatched(false)
     , mPreserveStream(false)
-    , mToReadBeforeRestart(0)
     , mReportedStart(false)
     , mReportedResponseHeader(false)
     , mForTakeResponseHead(nsnull)
-    , mTakenResponseHeader(false)
+    , mResponseHeadTaken(false)
 {
     LOG(("Creating nsHttpTransaction @%x\n", this));
     gHttpHandler->GetMaxPipelineObjectSize(mMaxPipelineObjectSize);
@@ -360,12 +359,12 @@ nsHttpTransaction::Connection()
 nsHttpResponseHead *
 nsHttpTransaction::TakeResponseHead()
 {
-    NS_ABORT_IF_FALSE(!mTakenResponseHeader, "TakeResponseHead called 2x");
+    NS_ABORT_IF_FALSE(!mResponseHeadTaken, "TakeResponseHead called 2x");
 
     // Lock RestartInProgress() and TakeResponseHead() against main thread
     MutexAutoLock lock(*nsHttp::GetLock());
 
-    mTakenResponseHeader = true;
+    mResponseHeadTaken = true;
 
     // Prefer mForTakeResponseHead over mResponseHead. It is always a complete
     // set of headers.
@@ -470,7 +469,7 @@ nsHttpTransaction::OnTransportStatus(nsITransport* transport,
                 PR_Now(), LL_ZERO, EmptyCString());
 
         // report the status and progress
-        if (!mRestartInProgressVerifier.Active())
+        if (!mRestartInProgressVerifier.IsDiscardingContent())
             mActivityDistributor->ObserveActivity(
                 mChannel,
                 NS_HTTP_ACTIVITY_TYPE_SOCKET_TRANSPORT,
@@ -841,30 +840,40 @@ nsHttpTransaction::RestartInProgress()
 {
     NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     
+    if ((mRestartCount + 1) >= gHttpHandler->MaxRequestAttempts()) {
+        LOG(("nsHttpTransaction::RestartInProgress() "
+             "reached max request attempts, failing transaction %p\n", this));
+        return NS_ERROR_NET_RESET;
+    }
+
     // Lock RestartInProgress() and TakeResponseHead() against main thread
     MutexAutoLock lock(*nsHttp::GetLock());
 
+    // Don't try and RestartInProgress() things that haven't gotten a response
+    // header yet. Those should be handled under the normal restart() path if
+    // they are eligible.
+    if (!mHaveAllHeaders)
+        return NS_ERROR_NET_RESET;
+
     // don't try and restart 0.9 or non 200/Get HTTP/1
-    if (mHaveAllHeaders && !mRestartInProgressVerifier.IsSetup())
+    if (!mRestartInProgressVerifier.IsSetup())
         return NS_ERROR_NET_RESET;
 
     LOG(("Will restart transaction %p and skip first %lld bytes, "
          "old Content-Length %lld",
          this, mContentRead, mContentLength));
 
-    if (mHaveAllHeaders) {
-        mRestartInProgressVerifier.SetAlreadyProcessed(
-            PR_MAX(mRestartInProgressVerifier.AlreadyProcessed(), mContentRead));
-        mToReadBeforeRestart = mRestartInProgressVerifier.AlreadyProcessed();
-        mRestartInProgressVerifier.SetActive(true);
+    mRestartInProgressVerifier.SetAlreadyProcessed(
+        PR_MAX(mRestartInProgressVerifier.AlreadyProcessed(), mContentRead));
 
-        if (!mTakenResponseHeader && !mForTakeResponseHead) {
-            // TakeResponseHeader() has not been called yet and this
-            // is the first restart. Store the resp headers exclusively
-            // for TakeResponseHeader()
-            mForTakeResponseHead = mResponseHead;
-            mResponseHead = nsnull;
-        }
+    if (!mResponseHeadTaken && !mForTakeResponseHead) {
+        // TakeResponseHeader() has not been called yet and this
+        // is the first restart. Store the resp headers exclusively
+        // for TakeResponseHead() which is called from the main thread and
+        // could happen at any time - so we can't continue to modify those
+        // headers (which restarting will effectively do)
+        mForTakeResponseHead = mResponseHead;
+        mResponseHead = nsnull;
     }
 
     if (mResponseHead) {
@@ -1253,7 +1262,7 @@ nsHttpTransaction::HandleContentStart()
                 LOG(("waiting for the server to close the connection.\n"));
 #endif
         }
-        if (mRestartInProgressVerifier.Active() &&
+        if (mRestartInProgressVerifier.IsSetup() &&
             !mRestartInProgressVerifier.Verify(mContentLength, mResponseHead)) {
             LOG(("Restart in progress subsequent transaction failed to match"));
             return NS_ERROR_ABORT;
@@ -1261,8 +1270,12 @@ nsHttpTransaction::HandleContentStart()
     }
 
     mDidContentStart = true;
+
+    // The verifier only initializes itself once (from the first iteration of
+    // a transaction that gets far enough to have response headers)
     if (mRequestHead->Method() == nsHttp::Get)
         mRestartInProgressVerifier.Set(mContentLength, mResponseHead);
+
     return NS_OK;
 }
 
@@ -1322,17 +1335,19 @@ nsHttpTransaction::HandleContent(char *buf,
         *contentRead = count;
     }
     
-    if (mRestartInProgressVerifier.Active() &&
-        mToReadBeforeRestart && *contentRead) {
-        PRUint32 ignore = PR_MIN(*contentRead, PRUint32(mToReadBeforeRestart));
+    PRInt64 toReadBeforeRestart =
+        mRestartInProgressVerifier.ToReadBeforeRestart();
+
+    if (toReadBeforeRestart && *contentRead) {
+        PRUint32 ignore =
+            PR_MIN(toReadBeforeRestart, PR_UINT32_MAX);
+        ignore = PR_MIN(*contentRead, ignore);
         LOG(("Due To Restart ignoring %d of remaining %ld",
-             ignore, mToReadBeforeRestart));
+             ignore, toReadBeforeRestart));
         *contentRead -= ignore;
         mContentRead += ignore;
-        mToReadBeforeRestart -= ignore;
+        mRestartInProgressVerifier.HaveReadBeforeRestart(ignore);
         memmove(buf, buf + ignore, *contentRead + *contentRemaining);
-        if (!mToReadBeforeRestart)
-            mRestartInProgressVerifier.SetActive(false);
     }
 
     if (*contentRead) {
@@ -1647,6 +1662,12 @@ nsHttpTransaction::RestartVerifier::Set(PRInt64 contentLength,
         val = head->PeekHeader(nsHttp::Transfer_Encoding);
         if (val)
             mTransferEncoding.Assign(val);
+
+        // We can only restart with any confidence if we have a stored etag or
+        // last-modified header
+        if (mETag.IsEmpty() && mLastModified.IsEmpty())
+            return;
+
         mSetup = true;
     }
 }

netwerk/protocol/http/nsHttpTransaction.h

@@ -231,13 +231,12 @@ private:
     // mResponseComplete := transaction ran to completion
 
     // For Restart-In-Progress Functionality
-    PRInt64                         mToReadBeforeRestart;
     bool                            mReportedStart;
     bool                            mReportedResponseHeader;
 
     // protected by nsHttp::GetLock()
     nsHttpResponseHead             *mForTakeResponseHead;
-    bool                            mTakenResponseHeader;
+    bool                            mResponseHeadTaken;
 
     class RestartVerifier 
     {
@@ -254,18 +253,27 @@ private:
         RestartVerifier()
             : mContentLength(-1)
             , mAlreadyProcessed(0)
-            , mActive(false)
+            , mToReadBeforeRestart(0)
             , mSetup(false)
         {}
         ~RestartVerifier() {}
         
         void Set(PRInt64 contentLength, nsHttpResponseHead *head);
         bool Verify(PRInt64 contentLength, nsHttpResponseHead *head);
-        bool Active() { return mActive; }
-        void SetActive(bool val) { mActive = val; }
+        bool IsDiscardingContent() { return mToReadBeforeRestart != 0; }
         bool IsSetup() { return mSetup; }
         PRInt64 AlreadyProcessed() { return mAlreadyProcessed; }
-        void SetAlreadyProcessed(PRInt64 val) { mAlreadyProcessed = val; }
+        void SetAlreadyProcessed(PRInt64 val) {
+            mAlreadyProcessed = val;
+            mToReadBeforeRestart = val;
+        }
+        PRInt64 ToReadBeforeRestart() { return mToReadBeforeRestart; }
+        void HaveReadBeforeRestart(PRUint32 amt)
+        {
+            NS_ABORT_IF_FALSE(amt <= mToReadBeforeRestart,
+                              "too large of a HaveReadBeforeRestart deduction");
+            mToReadBeforeRestart -= amt;
+        }
 
     private:
         // This is the data from the first complete response header
@@ -283,8 +291,10 @@ private:
         // be skipped in the new one.
         PRInt64                         mAlreadyProcessed;
 
-        // true when iteration > 0 has started
-        bool                            mActive;
+        // The amount of data that must be discarded in the current iteration
+        // (where iteration > 0) to reach the mAlreadyProcessed high water
+        // mark.
+        PRInt64                         mToReadBeforeRestart;
 
         // true when ::Set has been called with a response header
         bool                            mSetup;