Bug 1559460 - Support subframe process switches into embedder process, r=mccr8
authorNika Layzell <nika@thelayzells.com>
Fri, 21 Jun 2019 16:34:30 +0000
changeset 539462 0c0ea8d6d4a3423f78a44cad25de6fa2d036fe9d
parent 539461 07ced2dfb3b476391cbe6eb3065deba556dc3da8
child 539463 e1c559e95913f348c9b23bf8545a6dc98629793e
push id11522
push userffxbld-merge
push dateMon, 01 Jul 2019 09:00:55 +0000
treeherdermozilla-beta@53ea74d2bd09 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1559460
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 1559460 - Support subframe process switches into embedder process, r=mccr8 This change comes in two parts. First, the code in WindowGlobalChild was changed to detect the in-process case, and instruct the nsFrameLoader to become a non-remote nsFrameLoader, and second the logic in WindowGlobalParent was updated to ensure that the OwnerProcessID is updated after the change. Differential Revision: https://phabricator.services.mozilla.com/D35060
docshell/base/CanonicalBrowsingContext.cpp
docshell/base/CanonicalBrowsingContext.h
dom/ipc/WindowGlobalChild.cpp
dom/ipc/WindowGlobalParent.cpp
dom/tests/browser/browser_windowProxy_transplant.js
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -77,16 +77,25 @@ void CanonicalBrowsingContext::GetCurren
   if (!cp) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
 
   aRemoteType.Assign(cp->GetRemoteType());
 }
 
+void CanonicalBrowsingContext::SetOwnerProcessId(uint64_t aProcessId) {
+  MOZ_LOG(GetLog(), LogLevel::Debug,
+          ("SetOwnerProcessId for 0x%08" PRIx64 " (0x%08" PRIx64
+           " -> 0x%08" PRIx64 ")",
+           Id(), mProcessId, aProcessId));
+
+  mProcessId = aProcessId;
+}
+
 void CanonicalBrowsingContext::GetWindowGlobals(
     nsTArray<RefPtr<WindowGlobalParent>>& aWindows) {
   aWindows.SetCapacity(mWindowGlobals.Count());
   for (auto iter = mWindowGlobals.Iter(); !iter.Done(); iter.Next()) {
     aWindows.AppendElement(iter.Get()->GetKey());
   }
 }
 
@@ -124,16 +133,28 @@ void CanonicalBrowsingContext::SetEmbedd
                        "Embedder has incorrect browsing context");
   }
 
   mEmbedderWindowGlobal = aGlobal;
 }
 
 bool CanonicalBrowsingContext::ValidateTransaction(
     const Transaction& aTransaction, ContentParent* aProcess) {
+  if (MOZ_LOG_TEST(GetLog(), LogLevel::Debug)) {
+#define MOZ_BC_FIELD(name, ...)                                               \
+  if (aTransaction.m##name.isSome()) {                                        \
+    MOZ_LOG(GetLog(), LogLevel::Debug,                                        \
+            ("Validate Transaction 0x%08" PRIx64 " set " #name                \
+             " (from: 0x%08" PRIx64 " owner: 0x%08" PRIx64 ")",               \
+             Id(), aProcess ? static_cast<uint64_t>(aProcess->ChildID()) : 0, \
+             mProcessId));                                                    \
+  }
+#include "mozilla/dom/BrowsingContextFieldList.h"
+  }
+
   // Check that the correct process is performing sets for transactions with
   // non-racy fields.
   if (aTransaction.HasNonRacyField()) {
     if (NS_WARN_IF(aProcess && mProcessId != aProcess->ChildID())) {
       return false;
     }
   }
 
--- a/docshell/base/CanonicalBrowsingContext.h
+++ b/docshell/base/CanonicalBrowsingContext.h
@@ -33,17 +33,17 @@ class CanonicalBrowsingContext final : p
   bool IsOwnedByProcess(uint64_t aProcessId) const {
     return mProcessId == aProcessId;
   }
   uint64_t OwnerProcessId() const { return mProcessId; }
   ContentParent* GetContentParent() const;
 
   void GetCurrentRemoteType(nsAString& aRemoteType, ErrorResult& aRv) const;
 
-  void SetOwnerProcessId(uint64_t aProcessId) { mProcessId = aProcessId; }
+  void SetOwnerProcessId(uint64_t aProcessId);
 
   void GetWindowGlobals(nsTArray<RefPtr<WindowGlobalParent>>& aWindows);
 
   // Called by WindowGlobalParent to register and unregister window globals.
   void RegisterWindowGlobal(WindowGlobalParent* aGlobal);
   void UnregisterWindowGlobal(WindowGlobalParent* aGlobal);
 
   // The current active WindowGlobal.
--- a/dom/ipc/WindowGlobalChild.cpp
+++ b/dom/ipc/WindowGlobalChild.cpp
@@ -198,19 +198,23 @@ static nsresult ChangeFrameRemoteness(Wi
 
   RefPtr<nsFrameLoaderOwner> flo = do_QueryObject(embedderElt);
   MOZ_ASSERT(flo, "Embedder must be a nsFrameLoaderOwner!");
 
   MOZ_ASSERT(nsContentUtils::IsSafeToRunScript());
 
   // Actually perform the remoteness swap.
   RemotenessOptions options;
-  options.mRemoteType.Construct(aRemoteType);
   options.mPendingSwitchID.Construct(aPendingSwitchId);
 
+  // Only set mRemoteType if it doesn't match the current process' remote type.
+  if (!ContentChild::GetSingleton()->GetRemoteType().Equals(aRemoteType)) {
+    options.mRemoteType.Construct(aRemoteType);
+  }
+
   ErrorResult error;
   flo->ChangeRemoteness(options, error);
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
 
   // Make sure we successfully created either an in-process nsDocShell or a
   // cross-process BrowserBridgeChild. If we didn't, produce an error.
--- a/dom/ipc/WindowGlobalParent.cpp
+++ b/dom/ipc/WindowGlobalParent.cpp
@@ -273,48 +273,67 @@ IPCResult WindowGlobalParent::RecvDidEmb
   MOZ_ASSERT(aContext);
   aContext->Canonical()->SetEmbedderWindowGlobal(this);
   return IPC_OK();
 }
 
 already_AddRefed<Promise> WindowGlobalParent::ChangeFrameRemoteness(
     dom::BrowsingContext* aBc, const nsAString& aRemoteType,
     uint64_t aPendingSwitchId, ErrorResult& aRv) {
-  RefPtr<BrowserParent> browserParent = GetBrowserParent();
-  if (NS_WARN_IF(!browserParent)) {
+  RefPtr<BrowserParent> embedderBrowserParent = GetBrowserParent();
+  if (NS_WARN_IF(!embedderBrowserParent)) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   nsIGlobalObject* global = xpc::NativeGlobal(xpc::PrivilegedJunkScope());
   RefPtr<Promise> promise = Promise::Create(global, aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
+  RefPtr<CanonicalBrowsingContext> browsingContext =
+      CanonicalBrowsingContext::Cast(aBc);
+
   // When the reply comes back from content, either resolve or reject.
   auto resolve =
       [=](mozilla::Tuple<nsresult, PBrowserBridgeParent*>&& aResult) {
         nsresult rv = Get<0>(aResult);
         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 content process off of it. Otherwise, it's an in-process
-        // frame, so we can directly use ours.
+        // If we got a `BrowserBridgeParent`, the frame is out-of-process, so we
+        // can get the target off of it. Otherwise, it's an in-process frame, so
+        // we can use the embedder `BrowserParent`.
+        RefPtr<BrowserParent> browserParent;
         if (bridge) {
-          promise->MaybeResolve(
-              bridge->GetBrowserParent()->Manager()->ChildID());
+          browserParent = bridge->GetBrowserParent();
         } else {
-          promise->MaybeResolve(browserParent->Manager()->ChildID());
+          browserParent = embedderBrowserParent;
+        }
+        MOZ_ASSERT(browserParent);
+
+        if (!browserParent || !browserParent->CanSend()) {
+          promise->MaybeReject(NS_ERROR_FAILURE);
+          return;
         }
+
+        // Update our BrowsingContext to its new owner, if it hasn't been
+        // updated yet. This can happen when switching from a out-of-process to
+        // in-process frame. For remote frames, the BrowserBridgeParent::Init
+        // method should've already set up the OwnerProcessId.
+        uint64_t childId = browserParent->Manager()->ChildID();
+        MOZ_ASSERT_IF(bridge, browsingContext->IsOwnedByProcess(childId));
+        browsingContext->SetOwnerProcessId(childId);
+
+        promise->MaybeResolve(childId);
       };
 
   auto reject = [=](ResponseRejectReason aReason) {
     promise->MaybeReject(NS_ERROR_FAILURE);
   };
 
   SendChangeFrameRemoteness(aBc, PromiseFlatString(aRemoteType),
                             aPendingSwitchId, resolve, reject);
--- a/dom/tests/browser/browser_windowProxy_transplant.js
+++ b/dom/tests/browser/browser_windowProxy_transplant.js
@@ -79,40 +79,30 @@ add_task(async function() {
       content.win2 = iframe.contentWindow;
       content.bc2 = iframe.browsingContext;
 
       is(content.bc1, content.bc2, "same to cross-origin navigation BrowsingContext match");
       todo(content.win1 == content.win2, "same to cross-origin navigation WindowProxy match");
 
       ok(!Cu.isDeadWrapper(content.win1), "win1 shouldn't be a dead wrapper after navigation");
 
-      // This load still doesn't work for some reason, so we skip it for now.
-      /*
       askLoad(URL3);
       await waitLoad();
 
       content.win3 = iframe.contentWindow;
       content.bc3 = iframe.browsingContext;
 
       is(content.bc2, content.bc3, "cross to cross-origin navigation BrowsingContext match");
       ok(content.win2 == content.win3, "cross to cross-origin navigation WindowProxy match");
-      */
 
-      // It would also be useful and important to handle navigating back into an
-      // in-process URL, but that actually doesn't work yet... so........
-      /*
       askLoad(URL1);
       await waitLoad();
-      let win4 = iframe.contentWindow;
-      let bc4 = iframe.browsingContext;
 
-      is(bc3, bc4, "cross to same-origin navigation BrowsingContext match");
-      try {
-        is(win3, win4, "cross to same-origin navigation WindowProxy match");
-      } catch(e) {
-        ok(false, "cross to same-origin navigation WindowProxy exception");
-      }
-      */
+      content.win4 = iframe.contentWindow;
+      content.bc4 = iframe.browsingContext;
+
+      is(content.bc3, content.bc4, "cross to same-origin navigation BrowsingContext match");
+      todo(content.win3 == content.win4, "cross to same-origin navigation WindowProxty match");
     });
   } finally {
     await BrowserTestUtils.closeWindow(win);
   }
 });