Bug 1637869 - P6. Propagate proper error code when cancelling a channel. r=mattwoodrow
authorJean-Yves Avenard <jyavenard@mozilla.com>
Thu, 28 May 2020 10:22:31 +0000
changeset 596500 64c0bc294d5048ce1a9e8ac02cdd5c59fa1a0271
parent 596499 f3f319432899985db2d6ff2b34c37b8a460f072f
child 596501 3e61fd04eac7893a5a98a874ec9bb20fe218d310
push id13186
push userffxbld-merge
push dateMon, 01 Jun 2020 09:52:46 +0000
treeherdermozilla-beta@3e7c70a1e4a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1637869
milestone78.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 1637869 - P6. Propagate proper error code when cancelling a channel. r=mattwoodrow Differential Revision: https://phabricator.services.mozilla.com/D77201
netwerk/ipc/DocumentLoadListener.cpp
netwerk/ipc/DocumentLoadListener.h
netwerk/ipc/ParentProcessDocumentChannel.cpp
--- a/netwerk/ipc/DocumentLoadListener.cpp
+++ b/netwerk/ipc/DocumentLoadListener.cpp
@@ -749,71 +749,66 @@ void DocumentLoadListener::DisconnectChi
   mStreamFilterRequests.Clear();
 }
 
 void DocumentLoadListener::RedirectToRealChannelFinished(nsresult aRv) {
   LOG(
       ("DocumentLoadListener RedirectToRealChannelFinished [this=%p, "
        "aRv=%" PRIx32 " ]",
        this, static_cast<uint32_t>(aRv)));
-  if (NS_FAILED(aRv)) {
-    FinishReplacementChannelSetup(false);
-    return;
-  }
-
-  if (!mRedirectChannelId) {
-    FinishReplacementChannelSetup(true);
+  if (NS_FAILED(aRv) || !mRedirectChannelId) {
+    FinishReplacementChannelSetup(aRv);
     return;
   }
 
   // Wait for background channel ready on target channel
   nsCOMPtr<nsIRedirectChannelRegistrar> redirectReg =
       RedirectChannelRegistrar::GetOrCreate();
   MOZ_ASSERT(redirectReg);
 
   nsCOMPtr<nsIParentChannel> redirectParentChannel;
   redirectReg->GetParentChannel(mRedirectChannelId,
                                 getter_AddRefs(redirectParentChannel));
   if (!redirectParentChannel) {
-    FinishReplacementChannelSetup(false);
+    FinishReplacementChannelSetup(NS_ERROR_FAILURE);
     return;
   }
 
   nsCOMPtr<nsIParentRedirectingChannel> redirectingParent =
       do_QueryInterface(redirectParentChannel);
   if (!redirectingParent) {
     // Continue verification procedure if redirecting to non-Http protocol
-    FinishReplacementChannelSetup(true);
+    FinishReplacementChannelSetup(NS_OK);
     return;
   }
 
   // Ask redirected channel if verification can proceed.
   // ReadyToVerify will be invoked when redirected channel is ready.
   redirectingParent->ContinueVerification(this);
 }
 
 NS_IMETHODIMP
 DocumentLoadListener::ReadyToVerify(nsresult aResultCode) {
-  FinishReplacementChannelSetup(NS_SUCCEEDED(aResultCode));
+  FinishReplacementChannelSetup(aResultCode);
   return NS_OK;
 }
 
-void DocumentLoadListener::FinishReplacementChannelSetup(bool aSucceeded) {
+void DocumentLoadListener::FinishReplacementChannelSetup(nsresult aResult) {
   LOG(
       ("DocumentLoadListener FinishReplacementChannelSetup [this=%p, "
-       "aSucceeded=%d]",
-       this, aSucceeded));
+       "aResult=%x]",
+       this, int(aResult)));
 
   if (mDoingProcessSwitch) {
     DisconnectChildListeners(NS_BINDING_ABORTED, NS_BINDING_ABORTED);
   }
 
   if (!mRedirectChannelId) {
-    if (!aSucceeded) {
-      mChannel->Cancel(NS_BINDING_ABORTED);
+    if (NS_FAILED(aResult)) {
+      mChannel->Cancel(aResult);
       mChannel->Resume();
       return;
     }
     ApplyPendingFunctions(mChannel);
     // ResumeSuspendedChannel will be called later as RedirectToRealChannel
     // continues, so we can return early.
     return;
   }
@@ -828,32 +823,32 @@ void DocumentLoadListener::FinishReplace
   if (NS_FAILED(rv) || !redirectChannel) {
     // Redirect might get canceled before we got AsyncOnChannelRedirect
     nsCOMPtr<nsIChannel> newChannel;
     rv = registrar->GetRegisteredChannel(mRedirectChannelId,
                                          getter_AddRefs(newChannel));
     MOZ_ASSERT(newChannel, "Already registered channel not found");
 
     if (NS_SUCCEEDED(rv)) {
-      newChannel->Cancel(NS_BINDING_ABORTED);
+      newChannel->Cancel(NS_ERROR_FAILURE);
     }
     if (!redirectChannel) {
-      aSucceeded = false;
+      aResult = NS_ERROR_FAILURE;
     }
   }
 
   // Release all previously registered channels, they are no longer needed to
   // be kept in the registrar from this moment.
   registrar->DeregisterChannels(mRedirectChannelId);
   mRedirectChannelId = 0;
-  if (!aSucceeded) {
+  if (NS_FAILED(aResult)) {
     if (redirectChannel) {
       redirectChannel->Delete();
     }
-    mChannel->Cancel(NS_BINDING_ABORTED);
+    mChannel->Cancel(aResult);
     mChannel->Resume();
     if (auto* ctx = GetBrowsingContext()) {
       ctx->EndDocumentLoad(this);
     }
     return;
   }
 
   MOZ_ASSERT(
--- a/netwerk/ipc/DocumentLoadListener.h
+++ b/netwerk/ipc/DocumentLoadListener.h
@@ -235,17 +235,17 @@ class DocumentLoadListener : public nsII
   // channel is complete. May wait for the new parent channel to
   // finish, and then calls into FinishReplacementChannelSetup.
   void RedirectToRealChannelFinished(nsresult aRv);
 
   // Completes the replacement of the new channel.
   // This redirects the ParentChannelListener to forward any future
   // messages to the new channel, manually forwards any being held
   // by us, and resumes the underlying source channel.
-  void FinishReplacementChannelSetup(bool aSucceeded);
+  void FinishReplacementChannelSetup(nsresult aResult);
 
   // Called from `OnStartRequest` to make the decision about whether or not to
   // change process. This method will return `nullptr` if the current target
   // process is appropriate.
   bool MaybeTriggerProcessSwitch();
 
   // A helper for TriggerRedirectToRealChannel that abstracts over
   // the same-process and cross-process switch cases and returns
--- a/netwerk/ipc/ParentProcessDocumentChannel.cpp
+++ b/netwerk/ipc/ParentProcessDocumentChannel.cpp
@@ -52,17 +52,17 @@ ParentProcessDocumentChannel::RedirectTo
     MOZ_ALWAYS_SUCCEEDS(NS_GetFinalChannelURI(channel, getter_AddRefs(uri)));
     if (!nsDocShell::CanLoadInParentProcess(uri)) {
       nsAutoCString msg;
       uri->GetSpec(msg);
       msg.Insert(
           "Attempt to load a non-authorised load in the parent process: ", 0);
       NS_ASSERTION(false, msg.get());
       return PDocumentChannelParent::RedirectToRealChannelPromise::
-          CreateAndResolve(NS_BINDING_ABORTED, __func__);
+          CreateAndResolve(NS_ERROR_CONTENT_BLOCKED, __func__);
     }
   }
   mStreamFilterEndpoints = std::move(aStreamFilterEndpoints);
 
   RefPtr<PDocumentChannelParent::RedirectToRealChannelPromise> p =
       mPromise.Ensure(__func__);
 
   nsresult rv =
@@ -81,17 +81,17 @@ ParentProcessDocumentChannel::OnRedirect
        "aResult=%d]",
        this, int(aResult)));
 
   MOZ_ASSERT(mCanceled || mDocumentLoadListener);
 
   if (NS_FAILED(aResult)) {
     Cancel(aResult);
   } else if (mCanceled && NS_SUCCEEDED(aResult)) {
-    aResult = NS_BINDING_ABORTED;
+    aResult = NS_ERROR_ABORT;
   } else {
     const nsCOMPtr<nsIChannel> channel = mDocumentLoadListener->GetChannel();
     mLoadGroup->AddRequest(channel, nullptr);
     // Adding the channel to the loadgroup could have triggered a status
     // change with an observer being called destroying the docShell, resulting
     // in the PPDC to be canceled.
     if (!mCanceled) {
       mLoadGroup->RemoveRequest(this, nullptr, NS_BINDING_REDIRECTED);