Bug 1407186 P1 Fix InterceptedHttpChannel::AsyncOpen() error handling. r=asuth
authorBen Kelly <ben@wanderview.com>
Wed, 11 Oct 2017 11:23:46 -0700
changeset 428229 9f3b5e007cd1982f334733e975735db974ccf5d8
parent 428228 fa88f9fbee7181b68286ba1753d33ea9032f3b75
child 428230 1a282a37fcb198320fabe17ef499d2e2629c8990
push id97
push userfmarier@mozilla.com
push dateSat, 14 Oct 2017 01:12:59 +0000
reviewersasuth
bugs1407186
milestone58.0a1
Bug 1407186 P1 Fix InterceptedHttpChannel::AsyncOpen() error handling. r=asuth
netwerk/protocol/http/InterceptedHttpChannel.cpp
netwerk/protocol/http/InterceptedHttpChannel.h
--- a/netwerk/protocol/http/InterceptedHttpChannel.cpp
+++ b/netwerk/protocol/http/InterceptedHttpChannel.cpp
@@ -81,16 +81,86 @@ InterceptedHttpChannel::SetupReplacement
     }
 
     resumable->ResumeAt(mResumeStartPos, mResumeEntityId);
   }
 
   return NS_OK;
 }
 
+void
+InterceptedHttpChannel::AsyncOpenInternal()
+{
+  // If an error occurs in this file we must ensure mListener callbacks are
+  // invoked in some way.  We either Cancel() or ResetInterception below
+  // depending on which path we take.
+  nsresult rv = NS_OK;
+
+  // We should have pre-set the AsyncOpen time based on the original channel if
+  // timings are enabled.
+  if (mTimingEnabled) {
+    MOZ_DIAGNOSTIC_ASSERT(!mAsyncOpenTime.IsNull());
+  }
+
+  mIsPending = true;
+  mResponseCouldBeSynthesized = true;
+
+  if (mLoadGroup) {
+    mLoadGroup->AddRequest(this, nullptr);
+  }
+
+  // If we already have a synthesized body then we are pre-synthesized.
+  // This can happen for two reasons:
+  //  1. We have a pre-synthesized redirect in e10s mode.  In this case
+  //     we should follow the redirect.
+  //  2. We are handling a "fake" redirect for an opaque response.  Here
+  //     we should just process the synthetic body.
+  if (mBodyReader) {
+    // If we fail in this path, then cancel the channel.  We don't want
+    // to ResetInterception() after a synthetic result has already been
+    // produced by the ServiceWorker.
+    auto autoCancel = MakeScopeExit([&] {
+      if (NS_FAILED(rv)) {
+        Cancel(rv);
+      }
+    });
+
+    if (ShouldRedirect()) {
+      rv = FollowSyntheticRedirect();
+      return;
+    }
+
+    rv = StartPump();
+    return;
+  }
+
+  // If we fail the initial interception, then attempt to ResetInterception
+  // to fall back to network.  We only cancel if the reset fails.
+  auto autoReset = MakeScopeExit([&] {
+    if (NS_FAILED(rv)) {
+      rv = ResetInterception();
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        Cancel(rv);
+      }
+    }
+  });
+
+  // Otherwise we need to trigger a FetchEvent in a ServiceWorker.
+  nsCOMPtr<nsINetworkInterceptController> controller;
+  GetCallback(controller);
+
+  if (NS_WARN_IF(!controller)) {
+    rv = NS_ERROR_DOM_INVALID_STATE_ERR;
+    return;
+  }
+
+  rv = controller->ChannelIntercepted(this);
+  NS_ENSURE_SUCCESS_VOID(rv);
+}
+
 bool
 InterceptedHttpChannel::ShouldRedirect() const
 {
   // Determine if the synthetic response requires us to perform a real redirect.
   return nsHttpChannel::WillRedirect(mResponseHead) &&
          !mLoadInfo->GetDontFollowRedirects();
 }
 
@@ -380,17 +450,35 @@ InterceptedHttpChannel::CreateForSynthes
   ref->mResponseHead = new nsHttpResponseHead(*aHead);
 
   return ref.forget();
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::Cancel(nsresult aStatus)
 {
-  return CancelInterception(aStatus);
+  // Note: This class has been designed to send all error results through
+  //       Cancel().  Don't add calls directly to AsyncAbort() or
+  //       DoNotifyListener().  Instead call Cancel().
+
+  if (mCanceled) {
+    return NS_OK;
+  }
+  mCanceled = true;
+
+  MOZ_DIAGNOSTIC_ASSERT(NS_FAILED(aStatus));
+  if (NS_SUCCEEDED(mStatus)) {
+    mStatus = aStatus;
+  }
+
+  if (mPump) {
+    return mPump->Cancel(mStatus);
+  }
+
+  return AsyncAbort(mStatus);
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::Suspend(void)
 {
   nsresult rv = SuspendInternal();
 
   nsresult rvParentChannel = NS_OK;
@@ -424,73 +512,32 @@ InterceptedHttpChannel::GetSecurityInfo(
 
 NS_IMETHODIMP
 InterceptedHttpChannel::AsyncOpen(nsIStreamListener* aListener, nsISupports* aContext)
 {
   if (mCanceled) {
     return mStatus;
   }
 
-  // We should have pre-set the AsyncOpen time based on the original channel if
-  // timings are enabled.
-  if (mTimingEnabled) {
-    MOZ_DIAGNOSTIC_ASSERT(!mAsyncOpenTime.IsNull());
-  }
-
-  mIsPending = true;
+  // After this point we should try to return NS_OK and notify the listener
+  // of the result.
   mListener = aListener;
 
-  mResponseCouldBeSynthesized = true;
-
-  if (mLoadGroup) {
-    mLoadGroup->AddRequest(this, nullptr);
-  }
-
-  // If we already have a synthesized body then we are pre-synthesized.
-  // This can happen for two reasons:
-  //  1. We have a pre-synthesized redirect in e10s mode.  In this case
-  //     we should follow the redirect.
-  //  2. We are handling a "fake" redirect for an opaque response.  Here
-  //     we should just process the synthetic body.
-  if (mBodyReader) {
-    if (ShouldRedirect()) {
-      return FollowSyntheticRedirect();
-    }
-
-    return StartPump();
-  }
-
-  // Otherwise we need to trigger a FetchEvent in a ServiceWorker.
-  nsCOMPtr<nsINetworkInterceptController> controller;
-  GetCallback(controller);
-
-  if (NS_WARN_IF(!controller)) {
-    Cancel(NS_ERROR_FAILURE);
-    DoNotifyListener();
-    return NS_ERROR_FAILURE;
-  }
-
-  nsresult rv = controller->ChannelIntercepted(this);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    Cancel(rv);
-    DoNotifyListener();
-    return rv;
-  }
+  AsyncOpenInternal();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::AsyncOpen2(nsIStreamListener* aListener)
 {
   nsCOMPtr<nsIStreamListener> listener(aListener);
   nsresult rv = nsContentSecurityManager::doContentSecurityCheck(this, listener);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    mStatus = rv;
-    DoNotifyListener();
+    Cancel(rv);
     return rv;
   }
   return AsyncOpen(listener, nullptr);
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::LogBlockedCORSRequest(const nsAString& aMessage)
 {
@@ -692,31 +739,17 @@ InterceptedHttpChannel::FinishSynthesize
   }
 
   return StartPump();
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::CancelInterception(nsresult aStatus)
 {
-  if (mCanceled) {
-    return NS_OK;
-  }
-  mCanceled = true;
-
-  MOZ_DIAGNOSTIC_ASSERT(NS_FAILED(aStatus));
-  if (NS_SUCCEEDED(mStatus)) {
-    mStatus = aStatus;
-  }
-
-  if (mPump) {
-    return mPump->Cancel(mStatus);
-  }
-
-  return AsyncAbort(mStatus);
+  return Cancel(aStatus);
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::GetResponseBody(nsIOutputStream** aResponseBody)
 {
   if (!mBodyWriter) {
     nsresult rv = NS_NewPipe(getter_AddRefs(mBodyReader),
                              getter_AddRefs(mBodyWriter),
--- a/netwerk/protocol/http/InterceptedHttpChannel.h
+++ b/netwerk/protocol/http/InterceptedHttpChannel.h
@@ -99,16 +99,19 @@ private:
   virtual void
   ReleaseListeners() override;
 
   virtual MOZ_MUST_USE nsresult
   SetupReplacementChannel(nsIURI *aURI, nsIChannel *aChannel,
                           bool aPreserveMethod,
                           uint32_t aRedirectFlags) override;
 
+  void
+  AsyncOpenInternal();
+
   bool
   ShouldRedirect() const;
 
   nsresult
   FollowSyntheticRedirect();
 
   nsresult
   RedirectForOpaqueResponse(nsIURI* aResponseURI);