Bug 1525720, part 5 - Redirect nsIHttpChannel using content process ID instead of nsIRemoteTab. r=valentin
☠☠ backed out by f287bb6c1894 ☠ ☠
authorRyan Hunt <rhunt@eqrion.net>
Wed, 15 May 2019 12:33:42 -0500
changeset 474958 757b4f595cc4b18ae35483d23edff4896d15d4b1
parent 474957 b255e0a84e12657a62a2cdfd4a4c2ebb893b2a0d
child 474959 07678a2fa9e7cb164b2d3b07b8c61653253b7540
push id113182
push userrhunt@eqrion.net
push dateWed, 22 May 2019 20:02:12 +0000
treeherdermozilla-inbound@9b79caa460a0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1525720
milestone69.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 1525720, part 5 - Redirect nsIHttpChannel using content process ID instead of nsIRemoteTab. r=valentin This code currently works for remote subframes assuming that nsIRemoteTab is implemented by BrowserParent, but will break when nsIRemoteTab is only for a top-level BrowserParent. What this code really wants is a content process ID to retarget the channel to. This commit switches the interfaces to pass this around instead of nsIRemoteTab. Differential Revision: https://phabricator.services.mozilla.com/D31435
browser/components/sessionstore/SessionStore.jsm
dom/chrome-webidl/WindowGlobalActors.webidl
dom/interfaces/base/nsIRemoteTab.idl
dom/ipc/BrowserParent.cpp
dom/ipc/WindowGlobalParent.cpp
netwerk/protocol/http/HttpChannelParentListener.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
netwerk/test/browser/browser_cross_process_redirect.js
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -2326,17 +2326,17 @@ var SessionStoreInternal = {
     if (aBrowser.remoteType != aRemoteType ||
         !aBrowser.frameLoader || !aBrowser.frameLoader.remoteTab) {
       throw Cr.NS_ERROR_FAILURE;
     }
 
     // Tell our caller to redirect the load into this newly created process.
     let remoteTab = aBrowser.frameLoader.remoteTab;
     debug(`[process-switch]: new tabID: ${remoteTab.tabId}`);
-    return remoteTab;
+    return remoteTab.contentProcessId;
   },
 
   /**
    * Perform a destructive process switch into a distinct process.
    * This method is asynchronous, as it requires multiple calls into content
    * processes.
    */
   async _doProcessSwitch(aBrowsingContext, aRemoteType, aChannel, aSwitchId, aReplaceBrowsingContext) {
--- a/dom/chrome-webidl/WindowGlobalActors.webidl
+++ b/dom/chrome-webidl/WindowGlobalActors.webidl
@@ -30,17 +30,17 @@ interface WindowGlobalParent {
   readonly attribute URI? documentURI;
 
   static WindowGlobalParent? getByInnerWindowId(unsigned long long innerWindowId);
 
   [Throws]
   JSWindowActorParent getActor(DOMString name);
 
   [Throws]
-  Promise<RemoteTab> changeFrameRemoteness(
+  Promise<unsigned long long> changeFrameRemoteness(
     BrowsingContext? bc, DOMString remoteType,
     unsigned long long pendingSwitchId);
 };
 
 [Exposed=Window, ChromeOnly]
 interface WindowGlobalChild {
   readonly attribute boolean isClosed;
   readonly attribute boolean isInProcess;
--- a/dom/interfaces/base/nsIRemoteTab.idl
+++ b/dom/interfaces/base/nsIRemoteTab.idl
@@ -52,16 +52,18 @@ interface nsIRemoteTab : nsISupports
    * potentially unhelpful memory usage. Calling preserveLayers
    * will cause the layers to be preserved even for inactive
    * docshells.
    */
   void preserveLayers(in boolean aPreserveLayers);
 
   readonly attribute uint64_t tabId;
 
+  readonly attribute uint64_t contentProcessId;
+
   /**
    * The OS level process Id of the related child process.
    */
   readonly attribute int32_t osPid;
 
   readonly attribute boolean hasContentOpener;
   /**
    * True if we've previously received layers for this tab when switching to
--- a/dom/ipc/BrowserParent.cpp
+++ b/dom/ipc/BrowserParent.cpp
@@ -3184,16 +3184,22 @@ void BrowserParent::SuppressDisplayport(
 
 NS_IMETHODIMP
 BrowserParent::GetTabId(uint64_t* aId) {
   *aId = GetTabId();
   return NS_OK;
 }
 
 NS_IMETHODIMP
+BrowserParent::GetContentProcessId(uint64_t* aId) {
+  *aId = Manager()->GetChildID();
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 BrowserParent::GetOsPid(int32_t* aId) {
   *aId = Manager()->Pid();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BrowserParent::GetHasContentOpener(bool* aResult) {
   *aResult = mHasContentOpener;
--- a/dom/ipc/WindowGlobalParent.cpp
+++ b/dom/ipc/WindowGlobalParent.cpp
@@ -272,22 +272,23 @@ already_AddRefed<Promise> WindowGlobalPa
         RefPtr<BrowserBridgeParent> bridge =
             static_cast<BrowserBridgeParent*>(Get<1>(aResult));
         if (NS_FAILED(rv)) {
           promise->MaybeReject(rv);
           return;
         }
 
         // If we got a BrowserBridgeParent, the frame is out-of-process, so pull
-        // our target BrowserParent off of it. Otherwise, it's an in-process
+        // our target content process off of it. Otherwise, it's an in-process
         // frame, so we can directly use ours.
         if (bridge) {
-          promise->MaybeResolve(bridge->GetBrowserParent());
+          promise->MaybeResolve(
+              bridge->GetBrowserParent()->Manager()->ChildID());
         } else {
-          promise->MaybeResolve(browserParent);
+          promise->MaybeResolve(browserParent->Manager()->ChildID());
         }
       };
 
   auto reject = [=](ResponseRejectReason aReason) {
     promise->MaybeReject(NS_ERROR_FAILURE);
   };
 
   SendChangeFrameRemoteness(aBc, PromiseFlatString(aRemoteType),
--- a/netwerk/protocol/http/HttpChannelParentListener.cpp
+++ b/netwerk/protocol/http/HttpChannelParentListener.cpp
@@ -3,16 +3,18 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // HttpLog.h should generally be included first
 #include "HttpLog.h"
 
 #include "HttpChannelParentListener.h"
+#include "mozilla/dom/ContentParent.h"
+#include "mozilla/dom/ContentProcessManager.h"
 #include "mozilla/dom/ServiceWorkerInterceptController.h"
 #include "mozilla/dom/ServiceWorkerUtils.h"
 #include "mozilla/net/HttpChannelParent.h"
 #include "mozilla/net/RedirectChannelRegistrar.h"
 #include "mozilla/Unused.h"
 #include "nsIAuthPrompt.h"
 #include "nsIAuthPrompt2.h"
 #include "nsIHttpHeaderVisitor.h"
@@ -153,23 +155,24 @@ HttpChannelParentListener::GetInterface(
 nsresult HttpChannelParentListener::TriggerCrossProcessRedirect(
     nsIChannel* aChannel, nsILoadInfo* aLoadInfo, uint64_t aIdentifier) {
   RefPtr<HttpChannelParent> channelParent = do_QueryObject(mNextListener);
   MOZ_ASSERT(channelParent);
   channelParent->CancelChildCrossProcessRedirect();
 
   nsCOMPtr<nsIChannel> channel = aChannel;
   RefPtr<nsHttpChannel> httpChannel = do_QueryObject(channel);
-  RefPtr<nsHttpChannel::TabPromise> p = httpChannel->TakeRedirectTabPromise();
+  RefPtr<nsHttpChannel::ContentProcessIdPromise> p =
+      httpChannel->TakeRedirectContentProcessIdPromise();
   nsCOMPtr<nsILoadInfo> loadInfo = aLoadInfo;
 
   RefPtr<HttpChannelParentListener> self = this;
   p->Then(
       GetMainThreadSerialEventTarget(), __func__,
-      [=](nsCOMPtr<nsIRemoteTab> tp) {
+      [=](uint64_t cpId) {
         nsresult rv;
 
         // Register the new channel and obtain id for it
         nsCOMPtr<nsIRedirectChannelRegistrar> registrar =
             RedirectChannelRegistrar::GetOrCreate();
         MOZ_ASSERT(registrar);
         rv = registrar->RegisterChannel(channel, &self->mRedirectChannelId);
         NS_ENSURE_SUCCESS(rv, rv);
@@ -194,18 +197,23 @@ nsresult HttpChannelParentListener::Trig
 
         uint32_t redirectMode = nsIHttpChannelInternal::REDIRECT_MODE_FOLLOW;
         nsCOMPtr<nsIHttpChannelInternal> internalChannel =
             do_QueryInterface(channel);
         if (internalChannel) {
           MOZ_ALWAYS_SUCCEEDS(internalChannel->GetRedirectMode(&redirectMode));
         }
 
-        dom::BrowserParent* browserParent = dom::BrowserParent::GetFrom(tp);
-        auto result = browserParent->Manager()->SendCrossProcessRedirect(
+        dom::ContentParent* cp =
+            dom::ContentProcessManager::GetSingleton()->GetContentProcessById(
+                ContentParentId{cpId});
+        if (!cp) {
+          return NS_ERROR_UNEXPECTED;
+        }
+        auto result = cp->SendCrossProcessRedirect(
             self->mRedirectChannelId, uri, newLoadFlags, loadInfoArgs,
             channelId, originalURI, aIdentifier, redirectMode);
 
         MOZ_ASSERT(result, "SendCrossProcessRedirect failed");
 
         return result ? NS_OK : NS_ERROR_UNEXPECTED;
       },
       [httpChannel](nsresult aStatus) {
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -123,16 +123,17 @@
 #include "mozilla/dom/ServiceWorkerUtils.h"
 #include "mozilla/net/AsyncUrlChannelClassifier.h"
 #include "mozilla/net/NeckoChannelParams.h"
 #include "mozilla/net/UrlClassifierFeatureFactory.h"
 #include "nsIWebNavigation.h"
 #include "HttpTrafficAnalyzer.h"
 #include "mozilla/dom/CanonicalBrowsingContext.h"
 #include "mozilla/dom/WindowGlobalParent.h"
+#include "js/Conversions.h"
 
 #ifdef MOZ_TASK_TRACER
 #  include "GeckoTaskTracer.h"
 #endif
 
 #ifdef MOZ_GECKO_PROFILER
 #  include "ProfilerMarkerPayload.h"
 #endif
@@ -2564,17 +2565,17 @@ nsresult nsHttpChannel::ContinueProcessR
     return NS_OK;
   }
 
   rv = NS_OK;
   if (!mCanceled) {
     // notify "http-on-may-change-process" observers
     gHttpHandler->OnMayChangeProcess(this);
 
-    if (mRedirectTabPromise) {
+    if (mRedirectContentProcessIdPromise) {
       MOZ_ASSERT(!mOnStartRequestCalled);
 
       PushRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
       rv = StartCrossProcessRedirect();
       if (NS_SUCCEEDED(rv)) {
         return NS_OK;
       }
       PopRedirectAsyncFunc(&nsHttpChannel::ContinueProcessResponse2);
@@ -7239,67 +7240,66 @@ nsHttpChannel::GetRequestMethod(nsACStri
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIRequestObserver
 //-----------------------------------------------------------------------------
 
 // This class is used to convert from a DOM promise to a MozPromise.
 class DomPromiseListener final : dom::PromiseNativeHandler {
   NS_DECL_ISUPPORTS
 
-  static RefPtr<nsHttpChannel::TabPromise> Create(dom::Promise* aDOMPromise) {
+  static RefPtr<nsHttpChannel::ContentProcessIdPromise> Create(
+      dom::Promise* aDOMPromise) {
     MOZ_ASSERT(aDOMPromise);
     RefPtr<DomPromiseListener> handler = new DomPromiseListener();
-    RefPtr<nsHttpChannel::TabPromise> promise =
+    RefPtr<nsHttpChannel::ContentProcessIdPromise> promise =
         handler->mPromiseHolder.Ensure(__func__);
     aDOMPromise->AppendNativeHandler(handler);
     return promise;
   }
 
   virtual void ResolvedCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue) override {
-    nsCOMPtr<nsIRemoteTab> browserParent;
-    JS::Rooted<JSObject*> obj(aCx, &aValue.toObject());
-    nsresult rv =
-        UnwrapArg<nsIRemoteTab>(aCx, obj, getter_AddRefs(browserParent));
-    if (NS_FAILED(rv)) {
-      mPromiseHolder.Reject(rv, __func__);
+    uint64_t cpId;
+    if (!JS::ToUint64(aCx, aValue, &cpId)) {
+      mPromiseHolder.Reject(NS_ERROR_FAILURE, __func__);
       return;
     }
-    mPromiseHolder.Resolve(browserParent, __func__);
+    mPromiseHolder.Resolve(cpId, __func__);
   }
 
   virtual void RejectedCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue) override {
     if (!aValue.isInt32()) {
       mPromiseHolder.Reject(NS_ERROR_DOM_NOT_NUMBER_ERR, __func__);
       return;
     }
     mPromiseHolder.Reject((nsresult)aValue.toInt32(), __func__);
   }
 
  private:
   DomPromiseListener() = default;
   ~DomPromiseListener() = default;
-  MozPromiseHolder<nsHttpChannel::TabPromise> mPromiseHolder;
+  MozPromiseHolder<nsHttpChannel::ContentProcessIdPromise> mPromiseHolder;
 };
 
 NS_IMPL_ISUPPORTS0(DomPromiseListener)
 
-NS_IMETHODIMP nsHttpChannel::SwitchProcessTo(dom::Promise* aTabPromise,
-                                             uint64_t aIdentifier) {
+NS_IMETHODIMP nsHttpChannel::SwitchProcessTo(
+    dom::Promise* aContentProcessIdPromise, uint64_t aIdentifier) {
   MOZ_ASSERT(NS_IsMainThread());
-  NS_ENSURE_ARG(aTabPromise);
+  NS_ENSURE_ARG(aContentProcessIdPromise);
 
   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);
 
-  mRedirectTabPromise = DomPromiseListener::Create(aTabPromise);
+  mRedirectContentProcessIdPromise =
+      DomPromiseListener::Create(aContentProcessIdPromise);
   mCrossProcessRedirectIdentifier = aIdentifier;
   return NS_OK;
 }
 
 nsresult nsHttpChannel::StartCrossProcessRedirect() {
   nsresult rv;
 
   LOG(("nsHttpChannel::StartCrossProcessRedirect [this=%p]", this));
@@ -7640,17 +7640,17 @@ nsHttpChannel::OnStartRequest(nsIRequest
 
   // before we check for redirects, check if the load should be shifted into a
   // new process.
   rv = NS_OK;
   if (!mCanceled) {
     // notify "http-on-may-change-process" observers
     gHttpHandler->OnMayChangeProcess(this);
 
-    if (mRedirectTabPromise) {
+    if (mRedirectContentProcessIdPromise) {
       PushRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
       rv = StartCrossProcessRedirect();
       if (NS_SUCCEEDED(rv)) {
         return NS_OK;
       }
       PopRedirectAsyncFunc(&nsHttpChannel::ContinueOnStartRequest1);
     }
   }
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -281,19 +281,20 @@ class nsHttpChannel final : public HttpB
 
  public:
   void SetConnectionInfo(nsHttpConnectionInfo*);  // clones the argument
   void SetTransactionObserver(TransactionObserver* arg) {
     mTransactionObserver = arg;
   }
   TransactionObserver* GetTransactionObserver() { return mTransactionObserver; }
 
-  typedef MozPromise<nsCOMPtr<nsIRemoteTab>, nsresult, false> TabPromise;
-  already_AddRefed<TabPromise> TakeRedirectTabPromise() {
-    return mRedirectTabPromise.forget();
+  typedef MozPromise<uint64_t, nsresult, false> ContentProcessIdPromise;
+  already_AddRefed<ContentProcessIdPromise>
+  TakeRedirectContentProcessIdPromise() {
+    return mRedirectContentProcessIdPromise.forget();
   }
   uint64_t CrossProcessRedirectIdentifier() {
     return mCrossProcessRedirectIdentifier;
   }
 
   CacheDisposition mCacheDisposition;
 
  protected:
@@ -571,17 +572,17 @@ class nsHttpChannel final : public HttpB
   // auth specific data
   nsCOMPtr<nsIHttpChannelAuthProvider> mAuthProvider;
   nsCOMPtr<nsIURI> mRedirectURI;
   nsCOMPtr<nsIChannel> mRedirectChannel;
   nsCOMPtr<nsIChannel> mPreflightChannel;
 
   // The associated childChannel is getting relocated to another process.
   // This promise will be resolved when that process is set up.
-  RefPtr<TabPromise> mRedirectTabPromise;
+  RefPtr<ContentProcessIdPromise> mRedirectContentProcessIdPromise;
   // This identifier is passed to the childChannel in order to identify it.
   uint64_t mCrossProcessRedirectIdentifier = 0;
 
   // nsChannelClassifier checks this channel's URI against
   // the URI classifier service.
   // nsChannelClassifier will be invoked twice in InitLocalBlockList() and
   // BeginConnectActual(), so save the nsChannelClassifier here to keep the
   // state of whether tracking protection is enabled or not.
--- a/netwerk/test/browser/browser_cross_process_redirect.js
+++ b/netwerk/test/browser/browser_cross_process_redirect.js
@@ -65,17 +65,17 @@ ProcessChooser.prototype = {
       if (self.rejectPromise) {
         info("rejecting");
         reject(Cr.NS_ERROR_NOT_AVAILABLE);
         return;
       }
       // Can asyncly create a tab, or can resolve with a tab that was
       // previously created.
       info("resolving");
-      resolve(self.remoteTab);
+      resolve(self.remoteTab.contentProcessId);
     });
 
     info("calling switchprocessto");
     aChannel.switchProcessTo(tabPromise, identifier);
   },
 
   observe(aSubject, aTopic, aData) {
     switch (aTopic) {