From c839b29aa8bc49c959ddf4406cb5a7dfa4d46179 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 19 Nov 2020 17:44:30 +0000
Subject: [PATCH] Bug 1676996 - Don't try to process-switch a remote frame to
 local. r=nika

The guess in comment 3 is basically right, here's the relevant bits that
happen in that trace in order:

[content] nsFrameLoaderOwner::ChangeRemotenessCommon:
  frame becomes remote for the content process.
[parent] WindowGlobalParent::SendMakeFrameLocal (mIsDocumentLoad=true)
[content] ContentChild::SendCloneDocumentTreeInto
[parent] ContentParent::RecvCloneDocumentTreeInto
[content] WindowGlobalChild::RecvMakeFrameLocal

So basically the source frame we're cloning is mid-switch-to-local:
local already from the parent process POV, but still remote for the
child.

I think ignoring the clone in this case is fine (which will make the
print iframe just about:blank).

I've decided it to handle it in ChangeRemoteness but I could also handle
it in RecvCloneDocumentTreeInto with a check like

    if (cp->GetRemoteType() == GetRemoteType())

or such I think. Let me know if you'd prefer that.

Differential Revision: https://phabricator.services.mozilla.com/D97144
---
 docshell/base/CanonicalBrowsingContext.cpp |  2 +-
 dom/ipc/ContentParent.cpp                  | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp
index 92c4218517c9..e9d81759a0e6 100644
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -1330,7 +1330,7 @@ CanonicalBrowsingContext::ChangeRemoteness(const nsACString& aRemoteType,
       embedderWindowGlobal->GetBrowserParent();
   // Switching to local. No new process, so perform switch sync.
   if (embedderBrowser &&
-      aRemoteType.Equals(embedderBrowser->Manager()->GetRemoteType())) {
+      aRemoteType == embedderBrowser->Manager()->GetRemoteType()) {
     MOZ_DIAGNOSTIC_ASSERT(
         aPendingSwitchId,
         "We always have a PendingSwitchId, except for print-preview loads, "
diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp
index cd6d996b38dc..6a8be7f66eeb 100644
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -3664,6 +3664,19 @@ mozilla::ipc::IPCResult ContentParent::RecvCloneDocumentTreeInto(
     return IPC_OK();
   }
 
+  if (NS_WARN_IF(cp->GetRemoteType() == GetRemoteType())) {
+    // Wanted to switch to a target browsing context that's already local again.
+    // See bug 1676996 for how this can happen.
+    //
+    // Dropping the switch on the floor seems fine for this case, though we
+    // could also try to clone the local document.
+    //
+    // If the remote type matches & it's in the same group (which was confirmed
+    // by CloneIsLegal), it must be the exact same process.
+    MOZ_DIAGNOSTIC_ASSERT(cp == this);
+    return IPC_OK();
+  }
+
   target
       ->ChangeRemoteness(cp->GetRemoteType(), /* aLoadID = */ 0,
                          /* aReplaceBC = */ false, /* aSpecificGroupId = */ 0)