Bug 1407384 - P1: Notify "http-on-before-connect" and "http-on-modify-request" observers in DoAuthRetry r=mayhemer
authorKershaw Chang <kershaw@mozilla.com>
Tue, 15 Jan 2019 16:04:44 +0000
changeset 453948 7e8129336381f8bf33ecd89558b29effe9c3d864
parent 453947 d7d3b47d3a775c7fa45d5e0c3a6eba850cc394e8
child 453949 18abdcc812a606114c3b761c5134699bd385c703
push id35380
push userdluca@mozilla.com
push dateTue, 15 Jan 2019 22:13:12 +0000
treeherdermozilla-central@a51d26029042 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1407384
milestone66.0a1
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 1407384 - P1: Notify "http-on-before-connect" and "http-on-modify-request" observers in DoAuthRetry r=mayhemer The goal in this patch is to notify "http-on-before-connect" and "http-on-modify-request" observers in DoAuthRetry and also handle the case when the channel is canceled or suspended by observer. Differential Revision: https://phabricator.services.mozilla.com/D10741
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -788,17 +788,18 @@ NS_DEFINE_STATIC_IID_ACCESSOR(HttpBaseCh
 // - We want to store member function pointer to call at resume time, but one
 //   such function--HandleAsyncAbort--we want to share between the
 //   nsHttpChannel/HttpChannelChild.  Can't define it in base class, because
 //   then we'd have to cast member function ptr between base/derived class
 //   types.  Sigh...
 template <class T>
 class HttpAsyncAborter {
  public:
-  explicit HttpAsyncAborter(T *derived) : mThis(derived), mCallOnResume(0) {}
+  explicit HttpAsyncAborter(T *derived)
+      : mThis(derived), mCallOnResume(nullptr) {}
 
   // Aborts channel: calls OnStart/Stop with provided status, removes channel
   // from loadGroup.
   MOZ_MUST_USE nsresult AsyncAbort(nsresult status);
 
   // Does most the actual work.
   void HandleAsyncAbort();
 
@@ -808,17 +809,17 @@ class HttpAsyncAborter {
   MOZ_MUST_USE virtual nsresult AsyncCall(
       void (T::*funcPtr)(), nsRunnableMethod<T> **retval = nullptr);
 
  private:
   T *mThis;
 
  protected:
   // Function to be called at resume time
-  void (T::*mCallOnResume)(void);
+  std::function<nsresult(T *)> mCallOnResume;
 };
 
 template <class T>
 MOZ_MUST_USE nsresult HttpAsyncAborter<T>::AsyncAbort(nsresult status) {
   MOZ_LOG(gHttpLog, LogLevel::Debug,
           ("HttpAsyncAborter::AsyncAbort [this=%p status=%" PRIx32 "]\n", mThis,
            static_cast<uint32_t>(status)));
 
@@ -833,17 +834,20 @@ MOZ_MUST_USE nsresult HttpAsyncAborter<T
 template <class T>
 inline void HttpAsyncAborter<T>::HandleAsyncAbort() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
 
   if (mThis->mSuspendCount) {
     MOZ_LOG(
         gHttpLog, LogLevel::Debug,
         ("Waiting until resume to do async notification [this=%p]\n", mThis));
-    mCallOnResume = &T::HandleAsyncAbort;
+    mCallOnResume = [](T *self) {
+      self->HandleAsyncAbort();
+      return NS_OK;
+    };
     return;
   }
 
   mThis->DoNotifyListener();
 
   // finally remove ourselves from the load group.
   if (mThis->mLoadGroup)
     mThis->mLoadGroup->RemoveRequest(mThis, nullptr, mThis->mStatus);
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -2342,19 +2342,27 @@ HttpChannelChild::Resume() {
   // Don't SendResume at all if we're diverting callbacks to the parent (unless
   // suspend was sent earlier); otherwise, resume will be called at the correct
   // time in the parent itself.
   if (!--mSuspendCount && (!mDivertingToParent || mSuspendSent)) {
     if (RemoteChannelExists()) {
       SendResume();
     }
     if (mCallOnResume) {
-      rv = AsyncCall(mCallOnResume);
-      NS_ENSURE_SUCCESS(rv, rv);
-      mCallOnResume = nullptr;
+      nsCOMPtr<nsIEventTarget> neckoTarget = GetNeckoTarget();
+      MOZ_ASSERT(neckoTarget);
+
+      RefPtr<HttpChannelChild> self = this;
+      std::function<nsresult(HttpChannelChild*)> callOnResume = nullptr;
+      std::swap(callOnResume, mCallOnResume);
+      rv = neckoTarget->Dispatch(
+          NS_NewRunnableFunction(
+              "net::HttpChannelChild::mCallOnResume",
+              [callOnResume, self{std::move(self)}]() { callOnResume(self); }),
+          NS_DISPATCH_NORMAL);
     }
   }
   if (mSynthesizedResponsePump) {
     mSynthesizedResponsePump->Resume();
   }
   mEventQ->Resume();
 
   return rv;
@@ -3808,17 +3816,17 @@ HttpChannelChild::LogMimeTypeMismatch(co
                                       const nsAString& aContentType) {
   RefPtr<Document> doc;
   if (mLoadInfo) {
     mLoadInfo->GetLoadingDocument(getter_AddRefs(doc));
   }
 
   nsAutoString url(aURL);
   nsAutoString contentType(aContentType);
-  const char16_t* params[] = { url.get(), contentType.get() };
+  const char16_t* params[] = {url.get(), contentType.get()};
   nsContentUtils::ReportToConsole(
       aWarning ? nsIScriptError::warningFlag : nsIScriptError::errorFlag,
       NS_LITERAL_CSTRING("MIMEMISMATCH"), doc,
       nsContentUtils::eSECURITY_PROPERTIES, nsCString(aMessageName).get(),
       params, ArrayLength(params));
   return NS_OK;
 }
 
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -434,47 +434,56 @@ nsresult nsHttpChannel::PrepareToConnect
   if (mCanceled) {
     return mStatus;
   }
 
   if (mSuspendCount) {
     // We abandon the connection here if there was one.
     LOG(("Waiting until resume OnBeforeConnect [this=%p]\n", this));
     MOZ_ASSERT(!mCallOnResume);
-    mCallOnResume = &nsHttpChannel::HandleOnBeforeConnect;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleOnBeforeConnect();
+      return NS_OK;
+    };
     return NS_OK;
   }
 
   return OnBeforeConnect();
 }
 
 void nsHttpChannel::HandleContinueCancelledByTrackingProtection() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
 
   if (mSuspendCount) {
     LOG(
         ("Waiting until resume HandleContinueCancelledByTrackingProtection "
          "[this=%p]\n",
          this));
-    mCallOnResume = &nsHttpChannel::HandleContinueCancelledByTrackingProtection;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleContinueCancelledByTrackingProtection();
+      return NS_OK;
+    };
     return;
   }
 
   LOG(("nsHttpChannel::HandleContinueCancelledByTrackingProtection [this=%p]\n",
        this));
   ContinueCancelledByTrackingProtection();
 }
 
 void nsHttpChannel::HandleOnBeforeConnect() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
   nsresult rv;
 
   if (mSuspendCount) {
     LOG(("Waiting until resume OnBeforeConnect [this=%p]\n", this));
-    mCallOnResume = &nsHttpChannel::HandleOnBeforeConnect;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleOnBeforeConnect();
+      return NS_OK;
+    };
     return;
   }
 
   LOG(("nsHttpChannel::HandleOnBeforeConnect [this=%p]\n", this));
   rv = OnBeforeConnect();
   if (NS_FAILED(rv)) {
     CloseCacheEntry(false);
     Unused << AsyncAbort(rv);
@@ -588,30 +597,36 @@ nsresult nsHttpChannel::OnBeforeConnect(
   if (mCanceled) {
     return mStatus;
   }
 
   if (mSuspendCount) {
     // We abandon the connection here if there was one.
     LOG(("Waiting until resume OnBeforeConnect [this=%p]\n", this));
     MOZ_ASSERT(!mCallOnResume);
-    mCallOnResume = &nsHttpChannel::OnBeforeConnectContinue;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->OnBeforeConnectContinue();
+      return NS_OK;
+    };
     return NS_OK;
   }
 
   return Connect();
 }
 
 void nsHttpChannel::OnBeforeConnectContinue() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
   nsresult rv;
 
   if (mSuspendCount) {
     LOG(("Waiting until resume OnBeforeConnect [this=%p]\n", this));
-    mCallOnResume = &nsHttpChannel::OnBeforeConnectContinue;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->OnBeforeConnectContinue();
+      return NS_OK;
+    };
     return;
   }
 
   LOG(("nsHttpChannel::OnBeforeConnectContinue [this=%p]\n", this));
   rv = Connect();
   if (NS_FAILED(rv)) {
     CloseCacheEntry(false);
     Unused << AsyncAbort(rv);
@@ -769,24 +784,41 @@ nsresult nsHttpChannel::ContinueConnect(
   }
 
   if (mLoadFlags & LOAD_NO_NETWORK_IO) {
     LOG(("  mLoadFlags & LOAD_NO_NETWORK_IO"));
     return NS_ERROR_DOCUMENT_NOT_CACHED;
   }
 
   // hit the net...
+  return DoConnect();
+}
+
+nsresult nsHttpChannel::DoConnect(nsAHttpConnection *aConn) {
+  LOG(("nsHttpChannel::DoConnect [this=%p]\n", this));
+
   nsresult rv = SetupTransaction();
-  if (NS_FAILED(rv)) return rv;
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  // transfer ownership of connection to transaction
+  if (aConn) {
+    mTransaction->SetConnection(aConn);
+  }
 
   rv = gHttpHandler->InitiateTransaction(mTransaction, mPriority);
-  if (NS_FAILED(rv)) return rv;
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   rv = mTransactionPump->AsyncRead(this, nullptr);
-  if (NS_FAILED(rv)) return rv;
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   uint32_t suspendCount = mSuspendCount;
   if (mAsyncResumePending) {
     LOG(
         ("  Suspend()'ing transaction pump once because of async resume pending"
          ", sc=%u, pump=%p, this=%p",
          suspendCount, mTransactionPump.get(), this));
     ++suspendCount;
@@ -847,17 +879,20 @@ void nsHttpChannel::DoAsyncAbort(nsresul
   Unused << AsyncAbort(aStatus);
 }
 
 void nsHttpChannel::HandleAsyncRedirect() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to do async redirect [this=%p]\n", this));
-    mCallOnResume = &nsHttpChannel::HandleAsyncRedirect;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleAsyncRedirect();
+      return NS_OK;
+    };
     return;
   }
 
   nsresult rv = NS_OK;
 
   LOG(("nsHttpChannel::HandleAsyncRedirect [this=%p]\n", this));
 
   // since this event is handled asynchronously, it is possible that this
@@ -913,17 +948,20 @@ nsresult nsHttpChannel::ContinueHandleAs
   return NS_OK;
 }
 
 void nsHttpChannel::HandleAsyncNotModified() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to do async not-modified [this=%p]\n", this));
-    mCallOnResume = &nsHttpChannel::HandleAsyncNotModified;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleAsyncNotModified();
+      return NS_OK;
+    };
     return;
   }
 
   LOG(("nsHttpChannel::HandleAsyncNotModified [this=%p]\n", this));
 
   DoNotifyListener();
 
   CloseCacheEntry(false);
@@ -933,17 +971,20 @@ void nsHttpChannel::HandleAsyncNotModifi
   if (mLoadGroup) mLoadGroup->RemoveRequest(this, nullptr, mStatus);
 }
 
 void nsHttpChannel::HandleAsyncFallback() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to do async fallback [this=%p]\n", this));
-    mCallOnResume = &nsHttpChannel::HandleAsyncFallback;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleAsyncFallback();
+      return NS_OK;
+    };
     return;
   }
 
   nsresult rv = NS_OK;
 
   LOG(("nsHttpChannel::HandleAsyncFallback [this=%p]\n", this));
 
   // since this event is handled asynchronously, it is possible that this
@@ -2317,17 +2358,20 @@ void nsHttpChannel::AsyncContinueProcess
 
 nsresult nsHttpChannel::ContinueProcessResponse1() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
   nsresult rv;
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to finish processing response [this=%p]\n",
          this));
-    mCallOnResume = &nsHttpChannel::AsyncContinueProcessResponse;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->AsyncContinueProcessResponse();
+      return NS_OK;
+    };
     return NS_OK;
   }
 
   // Check if request was cancelled during http-on-examine-response.
   if (mCanceled) {
     return CallOnStartRequest();
   }
 
@@ -2812,17 +2856,20 @@ nsresult nsHttpChannel::ProxyFailover() 
 }
 
 void nsHttpChannel::HandleAsyncRedirectChannelToHttps() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to do async redirect to https [this=%p]\n",
          this));
-    mCallOnResume = &nsHttpChannel::HandleAsyncRedirectChannelToHttps;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleAsyncRedirectChannelToHttps();
+      return NS_OK;
+    };
     return;
   }
 
   nsresult rv = StartRedirectChannelToHttps();
   if (NS_FAILED(rv)) {
     rv = ContinueAsyncRedirectChannelToURI(rv);
     if (NS_FAILED(rv)) {
       LOG(("ContinueAsyncRedirectChannelToURI failed (%08x) [this=%p]\n",
@@ -2844,17 +2891,20 @@ nsresult nsHttpChannel::StartRedirectCha
 }
 
 void nsHttpChannel::HandleAsyncAPIRedirect() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
   MOZ_ASSERT(mAPIRedirectToURI, "How did that happen?");
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to do async API redirect [this=%p]\n", this));
-    mCallOnResume = &nsHttpChannel::HandleAsyncAPIRedirect;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleAsyncAPIRedirect();
+      return NS_OK;
+    };
     return;
   }
 
   nsresult rv = StartRedirectChannelToURI(
       mAPIRedirectToURI, nsIChannelEventSink::REDIRECT_PERMANENT);
   if (NS_FAILED(rv)) {
     rv = ContinueAsyncRedirectChannelToURI(rv);
     if (NS_FAILED(rv)) {
@@ -5910,17 +5960,20 @@ nsHttpChannel::CancelForTrackingProtecti
   if (mCanceled) {
     return mStatus;
   }
 
   if (mSuspendCount) {
     LOG(("Waiting until resume in Cancel [this=%p]\n", this));
     MOZ_ASSERT(!mCallOnResume);
     mTrackingProtectionCancellationPending = 1;
-    mCallOnResume = &nsHttpChannel::HandleContinueCancelledByTrackingProtection;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->HandleContinueCancelledByTrackingProtection();
+      return NS_OK;
+    };
     return NS_OK;
   }
 
   // Check to see if we should redirect this channel elsewhere by
   // nsIHttpChannel.redirectTo API request
   if (mAPIRedirectToURI) {
     mTrackingProtectionCancellationPending = 1;
     return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
@@ -6493,22 +6546,16 @@ nsresult nsHttpChannel::BeginConnectActu
     return mStatus;
   }
 
   if (mTrackingProtectionCancellationPending) {
     LOG(
         ("Waiting for tracking protection cancellation in BeginConnectActual "
          "[this=%p]\n",
          this));
-    MOZ_ASSERT(
-        !mCallOnResume ||
-            mCallOnResume ==
-                &nsHttpChannel::HandleContinueCancelledByTrackingProtection,
-        "We should be paused waiting for cancellation from tracking "
-        "protection");
     return NS_OK;
   }
 
   MaybeStartDNSPrefetch();
 
   nsresult rv = ContinueBeginConnectWithResult();
   if (NS_FAILED(rv)) {
     return rv;
@@ -6640,17 +6687,20 @@ nsHttpChannel::SetPriority(int32_t value
 nsresult nsHttpChannel::ContinueBeginConnectWithResult() {
   LOG(("nsHttpChannel::ContinueBeginConnectWithResult [this=%p]", this));
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
 
   nsresult rv;
 
   if (mSuspendCount) {
     LOG(("Waiting until resume to do async connect [this=%p]\n", this));
-    mCallOnResume = &nsHttpChannel::ContinueBeginConnect;
+    mCallOnResume = [](nsHttpChannel *self) {
+      self->ContinueBeginConnect();
+      return NS_OK;
+    };
     rv = NS_OK;
   } else if (mCanceled) {
     // We may have been cancelled already, by nsChannelClassifier in that
     // case, we should not send the request to the server
     rv = mStatus;
   } else {
     rv = PrepareToConnect();
   }
@@ -7409,65 +7459,102 @@ nsHttpChannel::OnStopRequest(nsIRequest 
           mLastStatusReported, TimeStamp::Now(), mLogicalOffset,
           mCacheDisposition, &mTransactionTimings, nullptr);
     }
 #endif
 
     // handle auth retry...
     if (authRetry) {
       mAuthRetryPending = false;
-      status = DoAuthRetry(conn);
-      if (NS_SUCCEEDED(status)) return NS_OK;
-    }
-
-    // If DoAuthRetry failed, or if we have been cancelled since showing
-    // the auth. dialog, then we need to send OnStartRequest now
-    if (authRetry || (mAuthRetryPending && NS_FAILED(status))) {
-      MOZ_ASSERT(NS_FAILED(status), "should have a failure code here");
-      // NOTE: since we have a failure status, we can ignore the return
-      // value from onStartRequest.
-      LOG(("  calling mListener->OnStartRequest [this=%p, listener=%p]\n", this,
-           mListener.get()));
-      if (mListener) {
-        MOZ_ASSERT(!mOnStartRequestCalled,
-                   "We should not call OnStartRequest twice.");
-        mListener->OnStartRequest(this, mListenerContext);
-        mOnStartRequestCalled = true;
-      } else {
-        NS_WARNING("OnStartRequest skipped because of null listener");
+      auto continueOSR = [authRetry, isFromNet, contentComplete,
+                          stickyConn{std::move(stickyConn)}](auto *self,
+                                                             nsresult aStatus) {
+        return self->ContinueOnStopRequestAfterAuthRetry(
+            aStatus, authRetry, isFromNet, contentComplete, stickyConn);
+      };
+      status = DoAuthRetry(conn, continueOSR);
+      if (NS_SUCCEEDED(status)) {
+        return NS_OK;
       }
     }
-
-    // if this transaction has been replaced, then bail.
-    if (mTransactionReplaced) {
-      LOG(("Transaction replaced\n"));
-      // This was just the network check for a 304 response.
-      mFirstResponseSource = RESPONSE_PENDING;
-      return NS_OK;
-    }
-
-    bool upgradeWebsocket = mUpgradeProtocolCallback && stickyConn &&
-                            mResponseHead &&
-                            ((mResponseHead->Status() == 101 &&
-                              mResponseHead->Version() == HttpVersion::v1_1) ||
-                             (mResponseHead->Status() == 200 &&
-                              mResponseHead->Version() == HttpVersion::v2_0));
-
-    bool upgradeConnect = mUpgradeProtocolCallback && stickyConn &&
-                          (mCaps & NS_HTTP_CONNECT_ONLY) && mResponseHead &&
-                          mResponseHead->Status() == 200;
-
-    if (upgradeWebsocket || upgradeConnect) {
-      nsresult rv = gHttpHandler->ConnMgr()->CompleteUpgrade(
-          stickyConn, mUpgradeProtocolCallback);
-      if (NS_FAILED(rv)) {
-        LOG(("  CompleteUpgrade failed with %08x", static_cast<uint32_t>(rv)));
-      }
-    }
-  }
+    return ContinueOnStopRequestAfterAuthRetry(status, authRetry, isFromNet,
+                                               contentComplete, stickyConn);
+  }
+
+  return ContinueOnStopRequest(status, isFromNet, contentComplete);
+}
+
+nsresult nsHttpChannel::ContinueOnStopRequestAfterAuthRetry(
+    nsresult aStatus, bool aAuthRetry, bool aIsFromNet, bool aContentComplete,
+    nsAHttpConnection *aStickyConn) {
+  LOG(
+      ("nsHttpChannel::ContinueOnStopRequestAfterAuthRetry "
+       "[this=%p, aStatus=%" PRIx32
+       " aAuthRetry=%d, aIsFromNet=%d, aStickyConn=%p]\n",
+       this, static_cast<uint32_t>(aStatus), aAuthRetry, aIsFromNet,
+       aStickyConn));
+
+  if (aAuthRetry && NS_SUCCEEDED(aStatus)) {
+    return NS_OK;
+  }
+
+  // If DoAuthRetry failed, or if we have been cancelled since showing
+  // the auth. dialog, then we need to send OnStartRequest now
+  if (aAuthRetry || (mAuthRetryPending && NS_FAILED(aStatus))) {
+    MOZ_ASSERT(NS_FAILED(aStatus), "should have a failure code here");
+    // NOTE: since we have a failure status, we can ignore the return
+    // value from onStartRequest.
+    LOG(("  calling mListener->OnStartRequest [this=%p, listener=%p]\n", this,
+         mListener.get()));
+    if (mListener) {
+      MOZ_ASSERT(!mOnStartRequestCalled,
+                 "We should not call OnStartRequest twice.");
+      mListener->OnStartRequest(this, mListenerContext);
+      mOnStartRequestCalled = true;
+    } else {
+      NS_WARNING("OnStartRequest skipped because of null listener");
+    }
+  }
+
+  // if this transaction has been replaced, then bail.
+  if (mTransactionReplaced) {
+    LOG(("Transaction replaced\n"));
+    // This was just the network check for a 304 response.
+    mFirstResponseSource = RESPONSE_PENDING;
+    return NS_OK;
+  }
+
+  bool upgradeWebsocket = mUpgradeProtocolCallback && aStickyConn &&
+                          mResponseHead &&
+                          ((mResponseHead->Status() == 101 &&
+                            mResponseHead->Version() == HttpVersion::v1_1) ||
+                           (mResponseHead->Status() == 200 &&
+                            mResponseHead->Version() == HttpVersion::v2_0));
+
+  bool upgradeConnect = mUpgradeProtocolCallback && aStickyConn &&
+                        (mCaps & NS_HTTP_CONNECT_ONLY) && mResponseHead &&
+                        mResponseHead->Status() == 200;
+
+  if (upgradeWebsocket || upgradeConnect) {
+    nsresult rv = gHttpHandler->ConnMgr()->CompleteUpgrade(
+        aStickyConn, mUpgradeProtocolCallback);
+    if (NS_FAILED(rv)) {
+      LOG(("  CompleteUpgrade failed with %08x", static_cast<uint32_t>(rv)));
+    }
+  }
+
+  return ContinueOnStopRequest(aStatus, aIsFromNet, aContentComplete);
+}
+
+nsresult nsHttpChannel::ContinueOnStopRequest(nsresult aStatus, bool aIsFromNet,
+                                              bool aContentComplete) {
+  LOG(
+      ("nsHttpChannel::ContinueOnStopRequest "
+       "[this=%p aStatus=%" PRIx32 ", aIsFromNet=%d]\n",
+       this, static_cast<uint32_t>(aStatus), aIsFromNet));
 
   // HTTP_CHANNEL_DISPOSITION TELEMETRY
   enum ChannelDisposition {
     kHttpCanceled = 0,
     kHttpDisk = 1,
     kHttpNetOK = 2,
     kHttpNetEarlyFail = 3,
     kHttpNetLateFail = 4,
@@ -7487,17 +7574,17 @@ nsHttpChannel::OnStopRequest(nsIRequest 
     chanDisposition = kHttpCanceled;
     upgradeChanDisposition =
         Telemetry::LABELS_HTTP_CHANNEL_DISPOSITION_UPGRADE::cancel;
   } else if (!mUsedNetwork || (mRaceCacheWithNetwork &&
                                mFirstResponseSource == RESPONSE_FROM_CACHE)) {
     chanDisposition = kHttpDisk;
     upgradeChanDisposition =
         Telemetry::LABELS_HTTP_CHANNEL_DISPOSITION_UPGRADE::disk;
-  } else if (NS_SUCCEEDED(status) && mResponseHead &&
+  } else if (NS_SUCCEEDED(aStatus) && mResponseHead &&
              mResponseHead->Version() != HttpVersion::v0_9) {
     chanDisposition = kHttpNetOK;
     upgradeChanDisposition =
         Telemetry::LABELS_HTTP_CHANNEL_DISPOSITION_UPGRADE::netOk;
   } else if (!mTransferSize) {
     chanDisposition = kHttpNetEarlyFail;
     upgradeChanDisposition =
         Telemetry::LABELS_HTTP_CHANNEL_DISPOSITION_UPGRADE::netEarlyFail;
@@ -7535,17 +7622,17 @@ nsHttpChannel::OnStopRequest(nsIRequest 
                      : NS_LITERAL_CSTRING("disabledWont");
   }
   Telemetry::AccumulateCategoricalKeyed(upgradeKey, upgradeChanDisposition);
   LOG(("  nsHttpChannel::OnStopRequest ChannelDisposition %d\n",
        chanDisposition));
   Telemetry::Accumulate(Telemetry::HTTP_CHANNEL_DISPOSITION, chanDisposition);
 
   // if needed, check cache entry has all data we expect
-  if (mCacheEntry && mCachePump && mConcurrentCacheAccess && contentComplete) {
+  if (mCacheEntry && mCachePump && mConcurrentCacheAccess && aContentComplete) {
     int64_t size, contentLength;
     nsresult rv = CheckPartial(mCacheEntry, &size, &contentLength);
     if (NS_SUCCEEDED(rv)) {
       if (size == int64_t(-1)) {
         // mayhemer TODO - we have to restart read from cache here at the size
         // offset
         MOZ_ASSERT(false);
         LOG(
@@ -7566,75 +7653,77 @@ nsHttpChannel::OnStopRequest(nsIRequest 
           rv = ContinueConnect();
           if (NS_SUCCEEDED(rv)) {
             LOG(("  performing range request"));
             mCachePump = nullptr;
             return NS_OK;
           }
           LOG(("  but range request perform failed 0x%08" PRIx32,
                static_cast<uint32_t>(rv)));
-          status = NS_ERROR_NET_INTERRUPT;
+          aStatus = NS_ERROR_NET_INTERRUPT;
         } else {
           LOG(("  but range request setup failed rv=0x%08" PRIx32
                ", failing load",
                static_cast<uint32_t>(rv)));
         }
       }
     }
   }
 
   mIsPending = false;
-  mStatus = status;
+  mStatus = aStatus;
 
   // perform any final cache operations before we close the cache entry.
   if (mCacheEntry && mRequestTimeInitialized) {
     bool writeAccess;
     // New implementation just returns value of the !mCacheEntryIsReadOnly flag
     // passed in. Old implementation checks on nsICache::ACCESS_WRITE flag.
     mCacheEntry->HasWriteAccess(!mCacheEntryIsReadOnly, &writeAccess);
     if (writeAccess) {
       nsresult rv = FinalizeCacheEntry();
       if (NS_FAILED(rv)) {
         LOG(("FinalizeCacheEntry failed (%08x)", static_cast<uint32_t>(rv)));
       }
     }
   }
 
-  ReportRcwnStats(isFromNet);
+  ReportRcwnStats(aIsFromNet);
 
   // Register entry to the PerformanceStorage resource timing
   MaybeReportTimingData();
 
   if (mListener) {
     LOG(("nsHttpChannel %p calling OnStopRequest\n", this));
     MOZ_ASSERT(mOnStartRequestCalled,
                "OnStartRequest should be called before OnStopRequest");
     MOZ_ASSERT(!mOnStopRequestCalled, "We should not call OnStopRequest twice");
-    mListener->OnStopRequest(this, mListenerContext, status);
+    mListener->OnStopRequest(this, mListenerContext, aStatus);
     mOnStopRequestCalled = true;
   }
 
   // notify "http-on-stop-connect" observers
   gHttpHandler->OnStopRequest(this);
 
   RemoveAsNonTailRequest();
 
   // If a preferred alt-data type was set, this signals the consumer is
   // interested in reading and/or writing the alt-data representation.
   // We need to hold a reference to the cache entry in case the listener calls
   // openAlternativeOutputStream() after CloseCacheEntry() clears mCacheEntry.
   if (!mPreferredCachedAltDataTypes.IsEmpty()) {
     mAltDataCacheEntry = mCacheEntry;
   }
 
-  CloseCacheEntry(!contentComplete);
+  CloseCacheEntry(!aContentComplete);
 
   if (mOfflineCacheEntry) CloseOfflineCacheEntry();
 
-  if (mLoadGroup) mLoadGroup->RemoveRequest(this, nullptr, status);
+  if (mLoadGroup) {
+    mLoadGroup->RemoveRequest(this, nullptr, aStatus);
+  }
 
   // We don't need this info anymore
   CleanRedirectCacheChainIfNecessary();
 
   ReleaseListeners();
 
   return NS_OK;
 }
@@ -8190,88 +8279,90 @@ nsHttpChannel::ResumeAt(uint64_t aStartP
   LOG(("nsHttpChannel::ResumeAt [this=%p startPos=%" PRIu64 " id='%s']\n", this,
        aStartPos, PromiseFlatCString(aEntityID).get()));
   mEntityID = aEntityID;
   mStartPos = aStartPos;
   mResuming = true;
   return NS_OK;
 }
 
-nsresult nsHttpChannel::DoAuthRetry(nsAHttpConnection *conn) {
+nsresult nsHttpChannel::DoAuthRetry(
+    nsAHttpConnection *conn,
+    const std::function<nsresult(nsHttpChannel *, nsresult)>
+        &aContinueOnStopRequestFunc) {
   LOG(("nsHttpChannel::DoAuthRetry [this=%p]\n", this));
 
   MOZ_ASSERT(!mTransaction, "should not have a transaction");
-  nsresult rv;
-
-  // toggle mIsPending to allow nsIObserver implementations to modify
-  // the request headers (bug 95044).
-  mIsPending = false;
+
+  // Note that we don't have to toggle |mIsPending| anymore. See the reasons
+  // below.
+  // 1. We can't suspend the channel during "http-on-modify-request"
+  // when |mIsPending| is false.
+  // 2. We don't check |mIsPending| in SetRequestHeader now.
 
   // Reset mRequestObserversCalled because we've probably called the request
   // observers once already.
   mRequestObserversCalled = false;
 
   // fetch cookies, and add them to the request header.
   // the server response could have included cookies that must be sent with
   // this authentication attempt (bug 84794).
   // TODO: save cookies from auth response and send them here (bug 572151).
   AddCookiesToRequest();
 
   // notify "http-on-modify-request" observers
   CallOnModifyRequestObservers();
 
+  RefPtr<nsAHttpConnection> connRef(conn);
+  return CallOrWaitForResume(
+      [conn{std::move(connRef)}, aContinueOnStopRequestFunc](auto *self) {
+        return self->ContinueDoAuthRetry(conn, aContinueOnStopRequestFunc);
+      });
+}
+
+nsresult nsHttpChannel::ContinueDoAuthRetry(
+    nsAHttpConnection *aConn,
+    const std::function<nsresult(nsHttpChannel *, nsresult)>
+        &aContinueOnStopRequestFunc) {
+  LOG(("nsHttpChannel::ContinueDoAuthRetry [this=%p]\n", this));
+
   mIsPending = true;
 
   // get rid of the old response headers
   mResponseHead = nullptr;
 
   // rewind the upload stream
   if (mUploadStream) {
     nsCOMPtr<nsISeekableStream> seekable = do_QueryInterface(mUploadStream);
-    if (seekable) seekable->Seek(nsISeekableStream::NS_SEEK_SET, 0);
+    if (seekable) {
+      seekable->Seek(nsISeekableStream::NS_SEEK_SET, 0);
+    }
   }
 
   // always set sticky connection flag
   mCaps |= NS_HTTP_STICKY_CONNECTION;
   // and when needed, allow restart regardless the sticky flag
   if (mAuthConnectionRestartable) {
     LOG(("  connection made restartable"));
     mCaps |= NS_HTTP_CONNECTION_RESTARTABLE;
     mAuthConnectionRestartable = false;
   } else {
     LOG(("  connection made non-restartable"));
     mCaps &= ~NS_HTTP_CONNECTION_RESTARTABLE;
   }
 
-  // and create a new one...
-  rv = SetupTransaction();
-  if (NS_FAILED(rv)) return rv;
-
-  // transfer ownership of connection to transaction
-  if (conn) mTransaction->SetConnection(conn);
-
-  rv = gHttpHandler->InitiateTransaction(mTransaction, mPriority);
-  if (NS_FAILED(rv)) return rv;
-
-  rv = mTransactionPump->AsyncRead(this, nullptr);
-  if (NS_FAILED(rv)) return rv;
-
-  uint32_t suspendCount = mSuspendCount;
-  if (mAsyncResumePending) {
-    LOG(
-        ("  Suspend()'ing transaction pump once because of async resume pending"
-         ", sc=%u, pump=%p, this=%p",
-         suspendCount, mTransactionPump.get(), this));
-    ++suspendCount;
-  }
-  while (suspendCount--) {
-    mTransactionPump->Suspend();
-  }
-
-  return NS_OK;
+  // notify "http-on-before-connect" observers
+  gHttpHandler->OnBeforeConnect(this);
+
+  RefPtr<nsAHttpConnection> connRef(aConn);
+  return CallOrWaitForResume(
+      [conn{std::move(connRef)}, aContinueOnStopRequestFunc](auto *self) {
+        nsresult rv = self->DoConnect(conn);
+        return aContinueOnStopRequestFunc(self, rv);
+      });
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIApplicationCacheChannel
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsHttpChannel::GetApplicationCache(nsIApplicationCache **out) {
@@ -8804,16 +8895,33 @@ nsHttpChannel::SuspendInternal() {
   nsresult rvCache = NS_OK;
   if (mCachePump) {
     rvCache = mCachePump->Suspend();
   }
 
   return NS_FAILED(rvTransaction) ? rvTransaction : rvCache;
 }
 
+nsresult nsHttpChannel::CallOrWaitForResume(
+    const std::function<nsresult(nsHttpChannel *)> &aFunc) {
+  if (mCanceled) {
+    MOZ_ASSERT(NS_FAILED(mStatus));
+    return mStatus;
+  }
+
+  if (mSuspendCount) {
+    LOG(("Waiting until resume [this=%p]\n", this));
+    MOZ_ASSERT(!mCallOnResume);
+    mCallOnResume = aFunc;
+    return NS_OK;
+  }
+
+  return aFunc(this);
+}
+
 NS_IMETHODIMP
 nsHttpChannel::ResumeInternal() {
   NS_ENSURE_TRUE(mSuspendCount > 0, NS_ERROR_UNEXPECTED);
 
   LOG(("nsHttpChannel::ResumeInternal [this=%p]\n", this));
 
   if (--mSuspendCount == 0) {
     mSuspendTotalTime +=
@@ -8823,30 +8931,34 @@ nsHttpChannel::ResumeInternal() {
       // Resume the interrupted procedure first, then resume
       // the pump to continue process the input stream.
       // Any newly created pump MUST be suspended to prevent calling
       // its OnStartRequest before OnStopRequest of any pre-existing
       // pump.  mAsyncResumePending ensures that.
       MOZ_ASSERT(!mAsyncResumePending);
       mAsyncResumePending = 1;
 
-      auto const callOnResume = mCallOnResume;
-      mCallOnResume = nullptr;
+      std::function<nsresult(nsHttpChannel *)> callOnResume = nullptr;
+      std::swap(callOnResume, mCallOnResume);
 
       RefPtr<nsHttpChannel> self(this);
       RefPtr<nsInputStreamPump> transactionPump = mTransactionPump;
       RefPtr<nsInputStreamPump> cachePump = mCachePump;
 
       nsresult rv = NS_DispatchToCurrentThread(NS_NewRunnableFunction(
           "nsHttpChannel::CallOnResume",
-          [callOnResume, self{std::move(self)},
+          [callOnResume{std::move(callOnResume)}, self{std::move(self)},
            transactionPump{std::move(transactionPump)},
            cachePump{std::move(cachePump)}]() {
             MOZ_ASSERT(self->mAsyncResumePending);
-            (self->*callOnResume)();
+            nsresult rv = self->CallOrWaitForResume(callOnResume);
+            if (NS_FAILED(rv)) {
+              self->CloseCacheEntry(false);
+              Unused << self->AsyncAbort(rv);
+            }
             MOZ_ASSERT(self->mAsyncResumePending);
 
             self->mAsyncResumePending = 0;
 
             // And now actually resume the previously existing pumps.
             if (transactionPump) {
               LOG(
                   ("nsHttpChannel::CallOnResume resuming previous transaction "
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -296,16 +296,22 @@ class nsHttpChannel final : public HttpB
   CacheDisposition mCacheDisposition;
 
  protected:
   virtual ~nsHttpChannel();
 
  private:
   typedef nsresult (nsHttpChannel::*nsContinueRedirectionFunc)(nsresult result);
 
+  // Directly call |aFunc| if the channel is not canceled and not suspended.
+  // Otherwise, set |aFunc| to |mCallOnResume| and wait until the channel
+  // resumes.
+  nsresult CallOrWaitForResume(
+      const std::function<nsresult(nsHttpChannel *)> &aFunc);
+
   bool RequestIsConditional();
   void HandleContinueCancelledByTrackingProtection();
   nsresult CancelInternal(nsresult status);
   void ContinueCancelledByTrackingProtection();
 
   // Connections will only be established in this function.
   // (including DNS prefetch and speculative connection.)
   nsresult BeginConnectActual();
@@ -406,17 +412,28 @@ class nsHttpChannel final : public HttpB
 
   // Handle the bogus Content-Encoding Apache sometimes sends
   void ClearBogusContentEncodingIfNeeded();
 
   // byte range request specific methods
   MOZ_MUST_USE nsresult ProcessPartialContent();
   MOZ_MUST_USE nsresult OnDoneReadingPartialCacheEntry(bool *streamDone);
 
-  MOZ_MUST_USE nsresult DoAuthRetry(nsAHttpConnection *);
+  MOZ_MUST_USE nsresult
+  DoAuthRetry(nsAHttpConnection *,
+              const std::function<nsresult(nsHttpChannel *, nsresult)> &aOuter);
+  MOZ_MUST_USE nsresult ContinueDoAuthRetry(
+      nsAHttpConnection *aConn,
+      const std::function<nsresult(nsHttpChannel *, nsresult)> &aOuter);
+  MOZ_MUST_USE nsresult DoConnect(nsAHttpConnection *aConn = nullptr);
+  MOZ_MUST_USE nsresult ContinueOnStopRequestAfterAuthRetry(
+      nsresult aStatus, bool aAuthRetry, bool aIsFromNet, bool aContentComplete,
+      nsAHttpConnection *aStickyConn);
+  MOZ_MUST_USE nsresult ContinueOnStopRequest(nsresult status, bool aIsFromNet,
+                                              bool aContentComplete);
 
   void HandleAsyncRedirectChannelToHttps();
   MOZ_MUST_USE nsresult StartRedirectChannelToHttps();
   MOZ_MUST_USE nsresult ContinueAsyncRedirectChannelToURI(nsresult rv);
   MOZ_MUST_USE nsresult OpenRedirectChannel(nsresult rv);
 
   /**
    * A function that takes care of reading STS and PKP headers and enforcing