Bug 1575744 - P1. Only perform process switch once we have completed all redirects. r=mayhemer

This is a stepped transition ; as SessionStore currently only knows how to deal with nsHttpChannel we have to go through the redirect only if the current channel is a nsHttpChannel.
In a followup change we will allow SessionStore to directly deal with the DocumentParentProcess.

Differential Revision: https://phabricator.services.mozilla.com/D46014

--HG--
extra : moz-landing-system : lando

netwerk/ipc/DocumentChannelParent.cpp

@@ -239,7 +239,7 @@ void DocumentChannelParent::FinishReplacementChannelSetup(bool aSucceeded) {
         newChannel->Cancel(NS_BINDING_ABORTED);
       }
     }
-    // Release all previously registered channels, they are no longer need to be
+    // Release all previously registered channels, they are no longer needed to be
     // kept in the registrar from this moment.
     registrar->DeregisterChannels(mRedirectChannelId);
 
@@ -360,43 +360,40 @@ void DocumentChannelParent::FinishReplacementChannelSetup(bool aSucceeded) {
 NS_IMETHODIMP
 DocumentChannelParent::FinishCrossProcessSwitch(
     nsIAsyncVerifyRedirectCallback* aCallback, nsresult aStatusCode) {
-  // We only manually Suspend mChannel when we initiate the redirect
-  // from OnStartRequest, which is currently only in the same-process
-  // case.
-  MOZ_ASSERT(!mSuspendedChannel);
+  // This updates ParentChannelListener to point to this parent and at
+  // the same time cancels the old channel.
+  FinishReplacementChannelSetup(NS_SUCCEEDED(aStatusCode));
 
-  if (NS_SUCCEEDED(aStatusCode)) {
-    // This updates ParentChannelListener to point to this parent and at
-    // the same time cancels the old channel.
-    FinishReplacementChannelSetup(true);
-  }
-
-  aCallback->OnRedirectVerifyCallback(aStatusCode);
   return NS_OK;
 }
 
 nsresult DocumentChannelParent::TriggerCrossProcessSwitch(
     nsIHttpChannel* aChannel, uint64_t aIdentifier) {
-  CancelChildForProcessSwitch();
-
   RefPtr<nsHttpChannel> httpChannel = do_QueryObject(aChannel);
   MOZ_DIAGNOSTIC_ASSERT(httpChannel,
                         "Must be called with nsHttpChannel object");
-  RefPtr<nsHttpChannel::ContentProcessIdPromise> p =
+  RefPtr<ContentProcessIdPromise> p =
       httpChannel->TakeRedirectContentProcessIdPromise();
+  TriggerCrossProcessSwitch(p.forget(), aIdentifier, httpChannel);
+  return NS_OK;
+}
 
+void DocumentChannelParent::TriggerCrossProcessSwitch(
+    already_AddRefed<ContentProcessIdPromise> aPromise, uint64_t aIdentifier,
+    nsHttpChannel* aChannel) {
+  CancelChildForProcessSwitch();
+  RefPtr<DocumentChannelParent> self = this;
+  RefPtr<nsHttpChannel> channel = aChannel;
+  RefPtr<ContentProcessIdPromise> p = aPromise;
   p->Then(
       GetMainThreadSerialEventTarget(), __func__,
-      [self = RefPtr<DocumentChannelParent>(this),
-       channel = nsCOMPtr<nsIChannel>(aChannel), aIdentifier](uint64_t aCpId) {
+      [self, aIdentifier, channel](uint64_t aCpId) {
         self->TriggerRedirectToRealChannel(channel, Some(aCpId), aIdentifier);
       },
-      [httpChannel](nsresult aStatusCode) {
+      [self](nsresult aStatusCode) {
         MOZ_ASSERT(NS_FAILED(aStatusCode), "Status should be error");
-        httpChannel->OnRedirectVerifyCallback(aStatusCode);
+        self->RedirectToRealChannelFinished(aStatusCode);
       });
-
-  return NS_OK;
 }
 
 void DocumentChannelParent::TriggerRedirectToRealChannel(
@@ -606,6 +603,19 @@ DocumentChannelParent::OnStartRequest(nsIRequest* aRequest) {
   if (channel) {
     Unused << channel->GetApplyConversion(&mOldApplyConversion);
     channel->SetApplyConversion(false);
+
+    // notify "http-on-may-change-process" observers which is typically
+    // SessionStore.jsm. This will determine if a new process needs to be
+    // spawned and if so channel->SwitchProcessTo() will be called which will
+    // set a ContentProcessIdPromise.
+    gHttpHandler->OnMayChangeProcess(channel);
+    RefPtr<ContentProcessIdPromise> promise =
+        channel->TakeRedirectContentProcessIdPromise();
+    if (promise) {
+      TriggerCrossProcessSwitch(
+          promise.forget(), channel->CrossProcessRedirectIdentifier(), channel);
+      return NS_OK;
+    }
   }
 
   TriggerRedirectToRealChannel(mChannel);

netwerk/ipc/DocumentChannelParent.h

@@ -7,6 +7,7 @@
 #ifndef mozilla_net_DocumentChannelParent_h
 #define mozilla_net_DocumentChannelParent_h
 
+#include "mozilla/MozPromise.h"
 #include "mozilla/Variant.h"
 #include "mozilla/net/NeckoCommon.h"
 #include "mozilla/net/NeckoParent.h"
@@ -110,6 +111,11 @@ class DocumentChannelParent : public nsIInterfaceRequestor,
 
   void FinishReplacementChannelSetup(bool aSucceeded);
 
+  typedef MozPromise<uint64_t, nsresult, false> ContentProcessIdPromise;
+  void TriggerCrossProcessSwitch(
+      already_AddRefed<ContentProcessIdPromise> aPromise, uint64_t aIdentifier,
+      nsHttpChannel* aChannel);
+
   // This defines a variant that describes all the attribute setters (and their
   // parameters) from nsIParentChannel
   //

netwerk/protocol/http/HttpChannelParent.cpp

@@ -1261,7 +1261,11 @@ static void FinishCrossProcessSwitchHelper(nsHttpChannel* aChannel,
                                            nsresult aStatus) {
   nsCOMPtr<nsICrossProcessSwitchChannel> switchListener;
   NS_QueryNotificationCallbacks(aChannel, switchListener);
-  MOZ_ASSERT(switchListener);
+  if (!switchListener) {
+    // This can happen if the channel got aborted before the switch completed
+    // and OnStopRequest got called in between which cleared the callback.
+    return;
+  }
 
   switchListener->FinishCrossProcessSwitch(aChannel, aStatus);
 }

netwerk/protocol/http/moz.build

@@ -132,6 +132,7 @@ LOCAL_INCLUDES += [
     '/extensions/auth',
     '/netwerk/base',
     '/netwerk/cookie',
+    '/netwerk/ipc',
     '/netwerk/url-classifier',
 ]
 

netwerk/protocol/http/nsHttpChannel.cpp

@@ -9,6 +9,7 @@
 
 #include <inttypes.h>
 
+#include "DocumentChannelParent.h"
 #include "mozilla/dom/nsCSPContext.h"
 #include "mozilla/ScopeExit.h"
 #include "mozilla/Sprintf.h"
@@ -2579,18 +2580,26 @@ nsresult nsHttpChannel::ContinueProcessResponse1() {
       return NS_OK;
     }
 
-    // notify "http-on-may-change-process" observers
-    gHttpHandler->OnMayChangeProcess(this);
+    nsCOMPtr<nsIParentChannel> parentChannel;
+    NS_QueryNotificationCallbacks(this, parentChannel);
 
-    if (mRedirectContentProcessIdPromise) {
-      MOZ_ASSERT(!mOnStartRequestCalled);
+    RefPtr<DocumentChannelParent> documentChannelParent =
+        do_QueryObject(parentChannel);
 
-      PushRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
-      rv = StartCrossProcessRedirect();
-      if (NS_SUCCEEDED(rv)) {
-        return NS_OK;
+    if (!documentChannelParent) {
+      // notify "http-on-may-change-process" observers
+      gHttpHandler->OnMayChangeProcess(this);
+
+      if (mRedirectContentProcessIdPromise) {
+        MOZ_ASSERT(!mOnStartRequestCalled);
+
+        PushRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
+        rv = StartCrossProcessRedirect();
+        if (NS_SUCCEEDED(rv)) {
+          return NS_OK;
+        }
+        PopRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
       }
-      PopRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
     }
   }
 
@@ -7284,8 +7293,16 @@ NS_IMETHODIMP nsHttpChannel::SwitchProcessTo(
   LOG(("nsHttpChannel::SwitchProcessTo [this=%p]", this));
   LogCallingScriptLocation(this);
 
-  // We cannot do this after OnStartRequest of the listener has been called.
-  NS_ENSURE_FALSE(mOnStartRequestCalled, NS_ERROR_NOT_AVAILABLE);
+  nsCOMPtr<nsIParentChannel> parentChannel;
+  NS_QueryNotificationCallbacks(this, parentChannel);
+  RefPtr<DocumentChannelParent> documentChannelParent =
+      do_QueryObject(parentChannel);
+  // This is a temporary change as the DocumentChannelParent currently must go
+  // through the nsHttpChannel to perform a process switch via SessionStore.
+  if (!documentChannelParent) {
+    // We cannot do this after OnStartRequest of the listener has been called.
+    NS_ENSURE_FALSE(mOnStartRequestCalled, NS_ERROR_NOT_AVAILABLE);
+  }
 
   mRedirectContentProcessIdPromise =
       DomPromiseListener::Create(aContentProcessIdPromise);
@@ -7731,15 +7748,21 @@ nsHttpChannel::OnStartRequest(nsIRequest* request) {
       return NS_OK;
     }
 
-    gHttpHandler->OnMayChangeProcess(this);
+    nsCOMPtr<nsIParentChannel> parentChannel;
+    NS_QueryNotificationCallbacks(this, parentChannel);
+    RefPtr<DocumentChannelParent> documentChannelParent =
+        do_QueryObject(parentChannel);
+    if (!documentChannelParent) {
+      gHttpHandler->OnMayChangeProcess(this);
 
-    if (mRedirectContentProcessIdPromise) {
-      PushRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
-      rv = StartCrossProcessRedirect();
-      if (NS_SUCCEEDED(rv)) {
-        return NS_OK;
+      if (mRedirectContentProcessIdPromise) {
+        PushRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
+        rv = StartCrossProcessRedirect();
+        if (NS_SUCCEEDED(rv)) {
+          return NS_OK;
+        }
+        PopRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
       }
-      PopRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
     }
   }