Bug 1699721 - Part 2: Track BrowserParent lifecycles during process switches, r=kmag

This patch contains a large number of changes around the process switching
mechanism in order to avoid issues which are caused by a mismatched
understanding of the state of the process switch between processes in the
presence of nested event loops.

This includes:
 1. The "InFlightProcessId" value is no longer recorded. All remaining uses
    were removed in part 1, and the new mechanism tracks this information in
    a better way.
 2. The current BrowserParent instance is now tracked on
    CanonicalBrowsingContext, meaning that logic which needs to work with this
    information can now access it without depending on the current
    WindowGlobalParent instance.
 3. When doing a process switch, the previous host process for the
    BrowsingContext is tracked until the process switch is completed, allowing
    for future attempts to switch into that process to be delayed until the
    previous unload event has finished running.
 4. The process switch logic was refactored to simplify some of the
    error-handling logic, and share more code between different cases.

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

docshell/base/BrowsingContext.cpp

@@ -439,6 +439,9 @@ already_AddRefed<BrowsingContext> BrowsingContext::CreateDetached(
 
 already_AddRefed<BrowsingContext> BrowsingContext::CreateIndependent(
     Type aType) {
+  MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess(),
+                        "BCs created in the content process must be related to "
+                        "some BrowserChild");
   RefPtr<BrowsingContext> bc(
       CreateDetached(nullptr, nullptr, nullptr, u""_ns, aType));
   bc->mWindowless = bc->IsContent();
@@ -695,6 +698,7 @@ void BrowsingContext::Embed() {
 
 void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) {
   MOZ_DIAGNOSTIC_ASSERT(!mEverAttached);
+  MOZ_DIAGNOSTIC_ASSERT_IF(aFromIPC, aOriginProcess || XRE_IsContentProcess());
   mEverAttached = true;
 
   if (MOZ_LOG_TEST(GetLog(), LogLevel::Debug)) {
@@ -745,6 +749,18 @@ void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) {
     ContentChild::GetSingleton()->SendCreateBrowsingContext(
         mGroup->Id(), GetIPCInitializer());
   } else if (XRE_IsParentProcess()) {
+    // If this window was created as a subframe by a content process, it must be
+    // being hosted within the same BrowserParent as its mParentWindow.
+    // Toplevel BrowsingContexts created by content have their BrowserParent
+    // configured during `RecvConstructPopupBrowser`.
+    if (mParentWindow && aOriginProcess) {
+      MOZ_DIAGNOSTIC_ASSERT(
+          mParentWindow->Canonical()->GetContentParent() == aOriginProcess,
+          "Creator process isn't the same as our embedder?");
+      Canonical()->SetCurrentBrowserParent(
+          mParentWindow->Canonical()->GetBrowserParent());
+    }
+
     mGroup->EachOtherParent(aOriginProcess, [&](ContentParent* aParent) {
       MOZ_DIAGNOSTIC_ASSERT(IsContent(),
                             "chrome BCG cannot be synced to content process");

docshell/base/CanonicalBrowsingContext.cpp

@@ -234,18 +234,6 @@ void CanonicalBrowsingContext::UpdateSecurityState() {
   }
 }
 
-void CanonicalBrowsingContext::SetInFlightProcessId(uint64_t aProcessId) {
-  MOZ_ASSERT(aProcessId);
-  mInFlightProcessId = aProcessId;
-}
-
-void CanonicalBrowsingContext::ClearInFlightProcessId(uint64_t aProcessId) {
-  MOZ_ASSERT(aProcessId);
-  if (mInFlightProcessId == aProcessId) {
-    mInFlightProcessId = 0;
-  }
-}
-
 void CanonicalBrowsingContext::GetWindowGlobals(
     nsTArray<RefPtr<WindowGlobalParent>>& aWindows) {
   aWindows.SetCapacity(GetWindowContexts().Length());
@@ -1107,6 +1095,25 @@ void CanonicalBrowsingContext::Stop(uint32_t aStopFlags) {
   }
 }
 
+void CanonicalBrowsingContext::PendingRemotenessChange::ProcessLaunched() {
+  if (!mPromise) {
+    return;
+  }
+
+  if (mContentParent) {
+    // If our new content process is still unloading from a previous process
+    // switch, wait for that unload to complete before continuing.
+    auto found = mTarget->FindUnloadingHost(mContentParent->ChildID());
+    if (found != mTarget->mUnloadingHosts.end()) {
+      found->mCallbacks.AppendElement(
+          [self = RefPtr{this}]() { self->ProcessReady(); });
+      return;
+    }
+  }
+
+  ProcessReady();
+}
+
 void CanonicalBrowsingContext::PendingRemotenessChange::ProcessReady() {
   if (!mPromise) {
     return;
@@ -1129,162 +1136,185 @@ void CanonicalBrowsingContext::PendingRemotenessChange::Finish() {
     return;
   }
 
-  RefPtr<CanonicalBrowsingContext> target(mTarget);
+  // If this BrowsingContext is embedded within the parent process, perform the
-  if (target->IsDiscarded()) {
+  // process switch directly.
-    Cancel(NS_ERROR_FAILURE);
+  nsresult rv = mTarget->IsTopContent() ? FinishTopContent() : FinishSubframe();
-    return;
+  if (NS_FAILED(rv)) {
+    NS_WARNING("Error finishing PendingRemotenessChange!");
+    Cancel(rv);
+  } else {
+    Clear();
   }
+}
+
+// Logic for finishing a toplevel process change embedded within the parent
+// process. Due to frontend integration the logic differs substantially from
+// subframe process switches, and is handled separately.
+nsresult CanonicalBrowsingContext::PendingRemotenessChange::FinishTopContent() {
+  MOZ_DIAGNOSTIC_ASSERT(mTarget->IsTop(),
+                        "We shouldn't be trying to change the remoteness of "
+                        "non-remote iframes");
 
   // While process switching, we need to check if any of our ancestors are
   // discarded or no longer current, in which case the process switch needs to
   // be aborted.
-  if (!target->AncestorsAreCurrent()) {
+  RefPtr<CanonicalBrowsingContext> target(mTarget);
-    NS_WARNING("Ancestor context is no longer current");
+  if (target->IsDiscarded() || !target->AncestorsAreCurrent()) {
-    Cancel(NS_ERROR_FAILURE);
+    return NS_ERROR_FAILURE;
-    return;
   }
 
-  // If this BrowsingContext is embedded within the parent process, perform the
+  Element* browserElement = target->GetEmbedderElement();
-  // process switch directly.
+  if (!browserElement) {
-  if (Element* browserElement = target->GetEmbedderElement()) {
+    return NS_ERROR_FAILURE;
-    MOZ_DIAGNOSTIC_ASSERT(target->IsTop(),
+  }
-                          "We shouldn't be trying to change the remoteness of "
-                          "non-remote iframes");
 
-    nsCOMPtr<nsIBrowser> browser = browserElement->AsBrowser();
+  nsCOMPtr<nsIBrowser> browser = browserElement->AsBrowser();
-    if (!browser) {
+  if (!browser) {
-      Cancel(NS_ERROR_FAILURE);
+    return NS_ERROR_FAILURE;
-      return;
+  }
+
+  RefPtr<nsFrameLoaderOwner> frameLoaderOwner = do_QueryObject(browserElement);
+  MOZ_RELEASE_ASSERT(frameLoaderOwner,
+                     "embedder browser must be nsFrameLoaderOwner");
+
+  // Tell frontend code that this browser element is about to change process.
+  nsresult rv = browser->BeforeChangeRemoteness();
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  // Some frontend code checks the value of the `remote` attribute on the
+  // browser to determine if it is remote, so update the value.
+  browserElement->SetAttr(kNameSpaceID_None, nsGkAtoms::remote,
+                          mContentParent ? u"true"_ns : u"false"_ns,
+                          /* notify */ true);
+
+  // The process has been created, hand off to nsFrameLoaderOwner to finish
+  // the process switch.
+  ErrorResult error;
+  frameLoaderOwner->ChangeRemotenessToProcess(mContentParent, mOptions,
+                                              mSpecificGroup, error);
+  if (error.Failed()) {
+    return error.StealNSResult();
+  }
+
+  // Tell frontend the load is done.
+  bool loadResumed = false;
+  rv = browser->FinishChangeRemoteness(mPendingSwitchId, &loadResumed);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  // We did it! The process switch is complete.
+  RefPtr<nsFrameLoader> frameLoader = frameLoaderOwner->GetFrameLoader();
+  RefPtr<BrowserParent> newBrowser = frameLoader->GetBrowserParent();
+  if (!newBrowser) {
+    if (mContentParent) {
+      // Failed to create the BrowserParent somehow! Abort the process switch
+      // attempt.
+      return NS_ERROR_UNEXPECTED;
     }
 
-    RefPtr<nsFrameLoaderOwner> frameLoaderOwner =
+    if (!loadResumed) {
-        do_QueryObject(browserElement);
+      RefPtr<nsDocShell> newDocShell = frameLoader->GetDocShell(error);
-    MOZ_RELEASE_ASSERT(frameLoaderOwner,
+      if (error.Failed()) {
-                       "embedder browser must be nsFrameLoaderOwner");
+        return error.StealNSResult();
-
-    // Tell frontend code that this browser element is about to change process.
-    nsresult rv = browser->BeforeChangeRemoteness();
-    if (NS_FAILED(rv)) {
-      Cancel(rv);
-      return;
-    }
-
-    // Some frontend code checks the value of the `remote` attribute on the
-    // browser to determine if it is remote, so update the value.
-    browserElement->SetAttr(kNameSpaceID_None, nsGkAtoms::remote,
-                            mContentParent ? u"true"_ns : u"false"_ns,
-                            /* notify */ true);
-
-    // The process has been created, hand off to nsFrameLoaderOwner to finish
-    // the process switch.
-    ErrorResult error;
-    frameLoaderOwner->ChangeRemotenessToProcess(mContentParent, mOptions,
-                                                mSpecificGroup, error);
-    if (error.Failed()) {
-      Cancel(error.StealNSResult());
-      return;
-    }
-
-    // Tell frontend the load is done.
-    bool loadResumed = false;
-    rv = browser->FinishChangeRemoteness(mPendingSwitchId, &loadResumed);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      Cancel(rv);
-      return;
-    }
-
-    // We did it! The process switch is complete.
-    RefPtr<nsFrameLoader> frameLoader = frameLoaderOwner->GetFrameLoader();
-    RefPtr<BrowserParent> newBrowser = frameLoader->GetBrowserParent();
-    if (!newBrowser) {
-      if (mContentParent) {
-        // Failed to create the BrowserParent somehow! Abort the process switch
-        // attempt.
-        Cancel(NS_ERROR_UNEXPECTED);
-        return;
       }
 
-      if (!loadResumed) {
+      rv = newDocShell->ResumeRedirectedLoad(mPendingSwitchId,
-        RefPtr<nsDocShell> newDocShell = frameLoader->GetDocShell(error);
+                                             /* aHistoryIndex */ -1);
-        if (error.Failed()) {
+      if (NS_FAILED(rv)) {
-          Cancel(error.StealNSResult());
+        return rv;
-          return;
-        }
-
-        rv = newDocShell->ResumeRedirectedLoad(mPendingSwitchId,
-                                               /* aHistoryIndex */ -1);
-        if (NS_FAILED(rv)) {
-          Cancel(error.StealNSResult());
-          return;
-        }
       }
-    } else if (!loadResumed) {
-      newBrowser->ResumeLoad(mPendingSwitchId);
     }
+  } else if (!loadResumed) {
+    newBrowser->ResumeLoad(mPendingSwitchId);
+  }
 
-    mPromise->Resolve(newBrowser, __func__);
+  mPromise->Resolve(newBrowser, __func__);
-    Clear();
+  return NS_OK;
-    return;
+}
+
+nsresult CanonicalBrowsingContext::PendingRemotenessChange::FinishSubframe() {
+  MOZ_DIAGNOSTIC_ASSERT(!mOptions.mReplaceBrowsingContext,
+                        "Cannot replace BC for subframe");
+  MOZ_DIAGNOSTIC_ASSERT(!mTarget->IsTop());
+
+  // While process switching, we need to check if any of our ancestors are
+  // discarded or no longer current, in which case the process switch needs to
+  // be aborted.
+  RefPtr<CanonicalBrowsingContext> target(mTarget);
+  if (target->IsDiscarded() || !target->AncestorsAreCurrent()) {
+    return NS_ERROR_FAILURE;
   }
 
   if (NS_WARN_IF(!mContentParent)) {
-    Cancel(NS_ERROR_FAILURE);
+    return NS_ERROR_FAILURE;
-    return;
   }
 
-  RefPtr<WindowGlobalParent> embedderWindow = target->GetEmbedderWindowGlobal();
+  RefPtr<WindowGlobalParent> embedderWindow = target->GetParentWindowContext();
   if (NS_WARN_IF(!embedderWindow) || NS_WARN_IF(!embedderWindow->CanSend())) {
-    Cancel(NS_ERROR_FAILURE);
+    return NS_ERROR_FAILURE;
-    return;
   }
 
   RefPtr<BrowserParent> embedderBrowser = embedderWindow->GetBrowserParent();
   if (NS_WARN_IF(!embedderBrowser)) {
-    Cancel(NS_ERROR_FAILURE);
+    return NS_ERROR_FAILURE;
-    return;
   }
 
-  // Pull load flags from our embedder browser.
+  RefPtr<BrowserParent> oldBrowser = target->GetBrowserParent();
-  nsCOMPtr<nsILoadContext> loadContext = embedderBrowser->GetLoadContext();
+  target->SetCurrentBrowserParent(nullptr);
-  MOZ_DIAGNOSTIC_ASSERT(
-      loadContext->UseRemoteTabs() && loadContext->UseRemoteSubframes(),
-      "Not supported without fission");
 
-  // NOTE: These are the only flags we actually care about
+  // If we were in a remote frame, trigger unloading of the remote window. The
-  uint32_t chromeFlags = nsIWebBrowserChrome::CHROME_REMOTE_WINDOW |
+  // previous BrowserParent is registered in `mUnloadingHosts` and will only be
-                         nsIWebBrowserChrome::CHROME_FISSION_WINDOW;
+  // cleared when the BrowserParent is fully destroyed.
-  if (loadContext->UsePrivateBrowsing()) {
+  bool wasRemote = oldBrowser && oldBrowser->GetBrowsingContext() == target;
-    chromeFlags |= nsIWebBrowserChrome::CHROME_PRIVATE_WINDOW;
-  }
-
-  RefPtr<WindowGlobalParent> oldWindow = target->GetCurrentWindowGlobal();
-  RefPtr<BrowserParent> oldBrowser =
-      oldWindow ? oldWindow->GetBrowserParent() : nullptr;
-  bool wasRemote = oldWindow && oldWindow->IsProcessRoot();
-
-  // Update which process is considered the current owner
-  uint64_t inFlightProcessId = target->OwnerProcessId();
-  target->SetInFlightProcessId(inFlightProcessId);
-  target->SetOwnerProcessId(mContentParent->ChildID());
-
-  auto resetInFlightId = [target, inFlightProcessId] {
-    target->ClearInFlightProcessId(inFlightProcessId);
-  };
-
-  // If we were in a remote frame, trigger unloading of the remote window. When
-  // the original remote window acknowledges, we can clear the in-flight ID.
   if (wasRemote) {
-    MOZ_DIAGNOSTIC_ASSERT(oldBrowser);
     MOZ_DIAGNOSTIC_ASSERT(oldBrowser != embedderBrowser);
     MOZ_DIAGNOSTIC_ASSERT(oldBrowser->GetBrowserBridgeParent());
 
-    auto callback = [resetInFlightId](auto) { resetInFlightId(); };
+    // `oldBrowser` will clear the `UnloadingHost` status once the actor has
-    oldBrowser->SendWillChangeProcess(callback, callback);
+    // been destroyed.
-    oldBrowser->Destroy();
+    if (oldBrowser->CanSend()) {
+      target->StartUnloadingHost(oldBrowser->Manager()->ChildID());
+      Unused << oldBrowser->SendWillChangeProcess();
+      oldBrowser->Destroy();
+    }
+  }
+
+  // Update which process is considered the current owner
+  target->SetOwnerProcessId(mContentParent->ChildID());
+
+  // If we're switching from remote to local, we don't need to create a
+  // BrowserBridge, and can instead perform the switch directly.
+  if (mContentParent == embedderBrowser->Manager()) {
+    MOZ_DIAGNOSTIC_ASSERT(
+        mPendingSwitchId,
+        "We always have a PendingSwitchId, except for print-preview loads, "
+        "which will never perform a process-switch to being in-process with "
+        "their embedder");
+    MOZ_DIAGNOSTIC_ASSERT(wasRemote,
+                          "Attempt to process-switch from local to local?");
+
+    target->SetCurrentBrowserParent(embedderBrowser);
+    Unused << embedderWindow->SendMakeFrameLocal(target, mPendingSwitchId);
+    mPromise->Resolve(embedderBrowser, __func__);
+    return NS_OK;
+  }
+
+  // The BrowsingContext will be remote, either as an already-remote frame
+  // changing processes, or as a local frame becoming remote. Construct a new
+  // BrowserBridgeParent to host the remote content.
+  target->SetCurrentBrowserParent(nullptr);
+
+  MOZ_DIAGNOSTIC_ASSERT(target->UseRemoteTabs() && target->UseRemoteSubframes(),
+                        "Not supported without fission");
+  uint32_t chromeFlags = nsIWebBrowserChrome::CHROME_REMOTE_WINDOW |
+                         nsIWebBrowserChrome::CHROME_FISSION_WINDOW;
+  if (target->UsePrivateBrowsing()) {
+    chromeFlags |= nsIWebBrowserChrome::CHROME_PRIVATE_WINDOW;
   }
 
-  MOZ_ASSERT(!mOptions.mReplaceBrowsingContext,
-             "Cannot replace BC for subframe");
   nsCOMPtr<nsIPrincipal> initialPrincipal =
       NullPrincipal::CreateWithInheritedAttributes(
           target->OriginAttributesRef(),
@@ -1298,26 +1328,36 @@ void CanonicalBrowsingContext::PendingRemotenessChange::Finish() {
   nsresult rv = bridge->InitWithProcess(embedderBrowser, mContentParent,
                                         windowInit, chromeFlags, tabId);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    Cancel(rv);
+    // If we've already destroyed our previous document, make a best-effort
-    return;
+    // attempt to recover from this failure and show the crashed tab UI. We only
+    // do this in the previously-remote case, as previously in-process frames
+    // will have their navigation cancelled, and will remain visible.
+    if (wasRemote) {
+      target->ShowSubframeCrashedUI(oldBrowser->GetBrowserBridgeParent());
+    }
+    return rv;
   }
 
   // Tell the embedder process a remoteness change is in-process. When this is
   // acknowledged, reset the in-flight ID if it used to be an in-process load.
   RefPtr<BrowserParent> newBrowser = bridge->GetBrowserParent();
   {
-    auto callback = [wasRemote, resetInFlightId](auto) {
+    // If we weren't remote, mark our embedder window browser as unloading until
-      if (!wasRemote) {
+    // our embedder process has acked our MakeFrameRemote message.
-        resetInFlightId();
+    Maybe<uint64_t> clearChildID;
+    if (!wasRemote) {
+      clearChildID = Some(embedderBrowser->Manager()->ChildID());
+      target->StartUnloadingHost(*clearChildID);
+    }
+    auto callback = [target, clearChildID](auto&&) {
+      if (clearChildID) {
+        target->ClearUnloadingHost(*clearChildID);
       }
     };
 
     ManagedEndpoint<PBrowserBridgeChild> endpoint =
         embedderBrowser->OpenPBrowserBridgeEndpoint(bridge);
-    if (NS_WARN_IF(!endpoint.IsValid())) {
+    MOZ_DIAGNOSTIC_ASSERT(endpoint.IsValid());
-      Cancel(NS_ERROR_UNEXPECTED);
-      return;
-    }
     embedderWindow->SendMakeFrameRemote(target, std::move(endpoint), tabId,
                                         newBrowser->GetLayersId(), callback,
                                         callback);
@@ -1330,7 +1370,7 @@ void CanonicalBrowsingContext::PendingRemotenessChange::Finish() {
 
   // We did it! The process switch is complete.
   mPromise->Resolve(newBrowser, __func__);
-  Clear();
+  return NS_OK;
 }
 
 void CanonicalBrowsingContext::PendingRemotenessChange::Cancel(nsresult aRv) {
@@ -1383,10 +1423,25 @@ CanonicalBrowsingContext::PendingRemotenessChange::~PendingRemotenessChange() {
 }
 
 BrowserParent* CanonicalBrowsingContext::GetBrowserParent() const {
-  if (auto* wg = GetCurrentWindowGlobal()) {
+  return mCurrentBrowserParent;
-    return wg->GetBrowserParent();
+}
-  }
+
-  return nullptr;
+void CanonicalBrowsingContext::SetCurrentBrowserParent(
+    BrowserParent* aBrowserParent) {
+  MOZ_DIAGNOSTIC_ASSERT(!mCurrentBrowserParent || !aBrowserParent,
+                        "BrowsingContext already has a current BrowserParent!");
+  MOZ_DIAGNOSTIC_ASSERT_IF(aBrowserParent, aBrowserParent->CanSend());
+  MOZ_DIAGNOSTIC_ASSERT_IF(aBrowserParent,
+                           aBrowserParent->Manager()->ChildID() == mProcessId);
+
+  // BrowserParent must either be directly for this BrowsingContext, or the
+  // manager out our embedder WindowGlobal.
+  MOZ_DIAGNOSTIC_ASSERT_IF(
+      aBrowserParent && aBrowserParent->GetBrowsingContext() != this,
+      GetParentWindowContext() &&
+          GetParentWindowContext()->Manager() == aBrowserParent);
+
+  mCurrentBrowserParent = aBrowserParent;
 }
 
 RefPtr<CanonicalBrowsingContext::RemotenessPromise>
@@ -1434,40 +1489,6 @@ CanonicalBrowsingContext::ChangeRemoteness(
     MOZ_DIAGNOSTIC_ASSERT(!mPendingRemotenessChange, "Should have cleared");
   }
 
-  RefPtr<BrowserParent> embedderBrowser =
-      embedderWindowGlobal->GetBrowserParent();
-  // Switching to local. No new process, so perform switch sync.
-  if (embedderBrowser &&
-      aOptions.mRemoteType == embedderBrowser->Manager()->GetRemoteType()) {
-    MOZ_DIAGNOSTIC_ASSERT(
-        aPendingSwitchId,
-        "We always have a PendingSwitchId, except for print-preview loads, "
-        "which will never perform a process-switch to being in-process with "
-        "their embedder");
-    if (GetCurrentWindowGlobal()) {
-      MOZ_DIAGNOSTIC_ASSERT(GetCurrentWindowGlobal()->IsProcessRoot());
-      RefPtr<BrowserParent> oldBrowser =
-          GetCurrentWindowGlobal()->GetBrowserParent();
-
-      uint64_t targetProcessId = OwnerProcessId();
-      SetInFlightProcessId(targetProcessId);
-      auto callback = [target = RefPtr{this}, targetProcessId](auto) {
-        target->ClearInFlightProcessId(targetProcessId);
-      };
-      oldBrowser->SendWillChangeProcess(callback, callback);
-      oldBrowser->Destroy();
-    }
-
-    // If the embedder process is remote, tell that remote process to become
-    // the owner.
-    MOZ_DIAGNOSTIC_ASSERT(!aOptions.mReplaceBrowsingContext);
-    MOZ_DIAGNOSTIC_ASSERT(!aOptions.mRemoteType.IsEmpty());
-    SetOwnerProcessId(embedderBrowser->Manager()->ChildID());
-    Unused << embedderWindowGlobal->SendMakeFrameLocal(this, aPendingSwitchId);
-    return RemotenessPromise::CreateAndResolve(embedderBrowser, __func__);
-  }
-
-  // Switching to remote. Wait for new process to launch before switch.
   auto promise = MakeRefPtr<RemotenessPromise::Private>(__func__);
   RefPtr<PendingRemotenessChange> change =
       new PendingRemotenessChange(this, promise, aPendingSwitchId, aOptions);
@@ -1499,40 +1520,64 @@ CanonicalBrowsingContext::ChangeRemoteness(
     change->mPrepareToChangePromise = GenericPromise::FromDomPromise(blocker);
   }
 
-  if (aOptions.mRemoteType.IsEmpty()) {
+  // Switching a subframe to be local within it's embedding process.
-    change->ProcessReady();
+  RefPtr<BrowserParent> embedderBrowser =
-  } else {
+      embedderWindowGlobal->GetBrowserParent();
-    // Try to predict which BrowsingContextGroup will be used for the final load
+  if (embedderBrowser &&
-    // in this BrowsingContext. This has to be accurate if switching into an
+      aOptions.mRemoteType == embedderBrowser->Manager()->GetRemoteType()) {
-    // existing group, as it will control what pool of processes will be used
+    MOZ_DIAGNOSTIC_ASSERT(
-    // for process selection.
+        aPendingSwitchId,
-    //
+        "We always have a PendingSwitchId, except for print-preview loads, "
-    // It's _technically_ OK to provide a group here if we're actually going to
+        "which will never perform a process-switch to being in-process with "
-    // switch into a brand new group, though it's sub-optimal, as it can
+        "their embedder");
-    // restrict the set of processes we're using.
+    MOZ_DIAGNOSTIC_ASSERT(!aOptions.mReplaceBrowsingContext);
-    BrowsingContextGroup* finalGroup = aOptions.mReplaceBrowsingContext
+    MOZ_DIAGNOSTIC_ASSERT(!aOptions.mRemoteType.IsEmpty());
-                                           ? change->mSpecificGroup.get()
+    MOZ_DIAGNOSTIC_ASSERT(!change->mPrepareToChangePromise);
-                                           : Group();
+    MOZ_DIAGNOSTIC_ASSERT(!change->mSpecificGroup);
 
-    change->mContentParent = ContentParent::GetNewOrUsedLaunchingBrowserProcess(
+    // Switching to local, so we don't need to create a new process, and will
-        /* aRemoteType = */ aOptions.mRemoteType,
+    // instead use our embedder process.
-        /* aGroup = */ finalGroup,
+    change->mContentParent = embedderBrowser->Manager();
-        /* aPriority = */ hal::PROCESS_PRIORITY_FOREGROUND,
-        /* aPreferUsed = */ false);
-    if (!change->mContentParent) {
-      change->Cancel(NS_ERROR_FAILURE);
-      return promise.forget();
-    }
-
-    // Add a KeepAlive used by this ContentParent, which will be cleared when
-    // the change is complete. This should prevent the process dying before
-    // we're ready to use it.
     change->mContentParent->AddKeepAlive();
-    change->mContentParent->WaitForLaunchAsync()->Then(
+    change->ProcessLaunched();
-        GetMainThreadSerialEventTarget(), __func__,
+    return promise.forget();
-        [change](ContentParent*) { change->ProcessReady(); },
-        [change](LaunchError) { change->Cancel(NS_ERROR_FAILURE); });
   }
+
+  // Switching to the parent process.
+  if (aOptions.mRemoteType.IsEmpty()) {
+    change->ProcessLaunched();
+    return promise.forget();
+  }
+
+  // Try to predict which BrowsingContextGroup will be used for the final load
+  // in this BrowsingContext. This has to be accurate if switching into an
+  // existing group, as it will control what pool of processes will be used
+  // for process selection.
+  //
+  // It's _technically_ OK to provide a group here if we're actually going to
+  // switch into a brand new group, though it's sub-optimal, as it can
+  // restrict the set of processes we're using.
+  BrowsingContextGroup* finalGroup =
+      aOptions.mReplaceBrowsingContext ? change->mSpecificGroup.get() : Group();
+
+  change->mContentParent = ContentParent::GetNewOrUsedLaunchingBrowserProcess(
+      /* aRemoteType = */ aOptions.mRemoteType,
+      /* aGroup = */ finalGroup,
+      /* aPriority = */ hal::PROCESS_PRIORITY_FOREGROUND,
+      /* aPreferUsed = */ false);
+  if (!change->mContentParent) {
+    change->Cancel(NS_ERROR_FAILURE);
+    return promise.forget();
+  }
+
+  // Add a KeepAlive used by this ContentParent, which will be cleared when
+  // the change is complete. This should prevent the process dying before
+  // we're ready to use it.
+  change->mContentParent->AddKeepAlive();
+  change->mContentParent->WaitForLaunchAsync()->Then(
+      GetMainThreadSerialEventTarget(), __func__,
+      [change](ContentParent*) { change->ProcessLaunched(); },
+      [change](LaunchError) { change->Cancel(NS_ERROR_FAILURE); });
   return promise.forget();
 }
 
@@ -1817,8 +1862,73 @@ void CanonicalBrowsingContext::SetCrossGroupOpenerId(uint64_t aOpenerId) {
   mCrossGroupOpenerId = aOpenerId;
 }
 
+auto CanonicalBrowsingContext::FindUnloadingHost(uint64_t aChildID)
+    -> nsTArray<UnloadingHost>::iterator {
+  return std::find_if(
+      mUnloadingHosts.begin(), mUnloadingHosts.end(),
+      [&](const auto& host) { return host.mChildID == aChildID; });
+}
+
+void CanonicalBrowsingContext::ClearUnloadingHost(uint64_t aChildID) {
+  // Notify any callbacks which were waiting for the host to finish unloading
+  // that it has.
+  auto found = FindUnloadingHost(aChildID);
+  if (found != mUnloadingHosts.end()) {
+    auto callbacks = std::move(found->mCallbacks);
+    mUnloadingHosts.RemoveElementAt(found);
+    for (const auto& callback : callbacks) {
+      callback();
+    }
+  }
+}
+
+void CanonicalBrowsingContext::StartUnloadingHost(uint64_t aChildID) {
+  MOZ_DIAGNOSTIC_ASSERT(FindUnloadingHost(aChildID) == mUnloadingHosts.end());
+  mUnloadingHosts.AppendElement(UnloadingHost{aChildID, {}});
+}
+
+void CanonicalBrowsingContext::BrowserParentDestroyed(
+    BrowserParent* aBrowserParent, bool aAbnormalShutdown) {
+  ClearUnloadingHost(aBrowserParent->Manager()->ChildID());
+
+  // Handling specific to when the current BrowserParent has been destroyed.
+  if (mCurrentBrowserParent == aBrowserParent) {
+    mCurrentBrowserParent = nullptr;
+
+    // If this BrowserParent is for a subframe, attempt to recover from a
+    // subframe crash by rendering the subframe crashed page in the embedding
+    // content.
+    if (aAbnormalShutdown) {
+      ShowSubframeCrashedUI(aBrowserParent->GetBrowserBridgeParent());
+    }
+  }
+}
+
+void CanonicalBrowsingContext::ShowSubframeCrashedUI(
+    BrowserBridgeParent* aBridge) {
+  if (!aBridge || IsDiscarded() || !aBridge->CanSend()) {
+    return;
+  }
+
+  MOZ_DIAGNOSTIC_ASSERT(!aBridge->GetBrowsingContext() ||
+                        aBridge->GetBrowsingContext() == this);
+
+  // There is no longer a current inner window within this
+  // BrowsingContext, update the `CurrentInnerWindowId` field to reflect
+  // this.
+  MOZ_ALWAYS_SUCCEEDS(SetCurrentInnerWindowId(0));
+
+  // The owning process will now be the embedder to render the subframe
+  // crashed page, switch ownership back over.
+  SetOwnerProcessId(aBridge->Manager()->Manager()->ChildID());
+  SetCurrentBrowserParent(aBridge->Manager());
+
+  Unused << aBridge->SendSubFrameCrashed();
+}
+
 NS_IMPL_CYCLE_COLLECTION_INHERITED(CanonicalBrowsingContext, BrowsingContext,
-                                   mSessionHistory, mContainerFeaturePolicy)
+                                   mSessionHistory, mContainerFeaturePolicy,
+                                   mCurrentBrowserParent)
 
 NS_IMPL_ADDREF_INHERITED(CanonicalBrowsingContext, BrowsingContext)
 NS_IMPL_RELEASE_INHERITED(CanonicalBrowsingContext, BrowsingContext)

docshell/base/CanonicalBrowsingContext.h

@@ -39,6 +39,7 @@ class DocumentLoadListener;
 namespace dom {
 
 class BrowserParent;
+class BrowserBridgeParent;
 class FeaturePolicy;
 struct LoadURIOptions;
 class MediaController;
@@ -84,10 +85,6 @@ class CanonicalBrowsingContext final : public BrowsingContext {
 
   void SetOwnerProcessId(uint64_t aProcessId);
 
-  void SetInFlightProcessId(uint64_t aProcessId);
-  void ClearInFlightProcessId(uint64_t aProcessId);
-  uint64_t GetInFlightProcessId() const { return mInFlightProcessId; }
-
   // The ID of the BrowsingContext which caused this BrowsingContext to be
   // opened, or `0` if this is unknown.
   // Only set for toplevel content BrowsingContexts, and may be from a different
@@ -215,6 +212,7 @@ class CanonicalBrowsingContext final : public BrowsingContext {
   void SetCurrentRemoteURI(nsIURI* aCurrentRemoteURI);
 
   BrowserParent* GetBrowserParent() const;
+  void SetCurrentBrowserParent(BrowserParent* aBrowserParent);
 
   // Internal method to change which process a BrowsingContext is being loaded
   // in. The returned promise will resolve when the process switch is completed.
@@ -287,6 +285,14 @@ class CanonicalBrowsingContext final : public BrowsingContext {
   void SetRestoreData(SessionStoreRestoreData* aData);
   void RequestRestoreTabContent(WindowGlobalParent* aWindow);
 
+  // Called when a BrowserParent for this BrowsingContext has been fully
+  // destroyed (i.e. `ActorDestroy` was called).
+  void BrowserParentDestroyed(BrowserParent* aBrowserParent,
+                              bool aAbnormalShutdown);
+
+  void StartUnloadingHost(uint64_t aChildID);
+  void ClearUnloadingHost(uint64_t aChildID);
+
  protected:
   // Called when the browsing context is being discarded.
   void CanonicalDiscard();
@@ -319,10 +325,14 @@ class CanonicalBrowsingContext final : public BrowsingContext {
     friend class CanonicalBrowsingContext;
 
     ~PendingRemotenessChange();
+    void ProcessLaunched();
     void ProcessReady();
     void Finish();
     void Clear();
 
+    nsresult FinishTopContent();
+    nsresult FinishSubframe();
+
     RefPtr<CanonicalBrowsingContext> mTarget;
     RefPtr<RemotenessPromise::Private> mPromise;
     RefPtr<GenericPromise> mPrepareToChangePromise;
@@ -350,6 +360,16 @@ class CanonicalBrowsingContext final : public BrowsingContext {
       const nsID& aChangeID,
       const CallerWillNotifyHistoryIndexAndLengthChanges& aProofOfCaller);
 
+  struct UnloadingHost {
+    uint64_t mChildID;
+    nsTArray<std::function<void()>> mCallbacks;
+  };
+  nsTArray<UnloadingHost>::iterator FindUnloadingHost(uint64_t aChildID);
+
+  // Called when we want to show the subframe crashed UI as our previous browser
+  // has become unloaded for one reason or another.
+  void ShowSubframeCrashedUI(BrowserBridgeParent* aBridge);
+
   // XXX(farre): Store a ContentParent pointer here rather than mProcessId?
   // Indicates which process owns the docshell.
   uint64_t mProcessId;
@@ -357,10 +377,6 @@ class CanonicalBrowsingContext final : public BrowsingContext {
   // Indicates which process owns the embedder element.
   uint64_t mEmbedderProcessId;
 
-  // The ID of the former owner process during an ownership change, which may
-  // have in-flight messages that assume it is still the owner.
-  uint64_t mInFlightProcessId = 0;
-
   uint64_t mCrossGroupOpenerId = 0;
 
   // This function will make the top window context reset its
@@ -369,6 +385,10 @@ class CanonicalBrowsingContext final : public BrowsingContext {
   // entries are added or replaced.
   void ResetSHEntryHasUserInteractionCache();
 
+  RefPtr<BrowserParent> mCurrentBrowserParent;
+
+  nsTArray<UnloadingHost> mUnloadingHosts;
+
   // The current URI loaded in this BrowsingContext. This value is only set for
   // BrowsingContexts loaded in content processes.
   nsCOMPtr<nsIURI> mCurrentRemoteURI;

dom/base/nsFrameLoader.cpp

@@ -3745,34 +3745,16 @@ void nsFrameLoader::SetWillChangeProcess() {
   mWillChangeProcess = true;
 
   if (IsRemoteFrame()) {
-    // OOP Browser - Go directly over Browser Parent
     if (auto* browserParent = GetBrowserParent()) {
-      // We're going to be synchronously changing the owner of the
+      if (auto* bc = CanonicalBrowsingContext::Cast(mPendingBrowsingContext);
-      // BrowsingContext in the parent process while the current owner may still
+          bc && bc->EverAttached()) {
-      // have in-flight requests which only the owner is allowed to make. Those
+        bc->StartUnloadingHost(browserParent->Manager()->ChildID());
-      // requests will typically trigger assertions if they come from a child
+        bc->SetCurrentBrowserParent(nullptr);
-      // other than the owner.
+      }
-      //
+      // OOP Browser - Go directly over Browser Parent
-      // To work around this, we record the previous owner at the start of the
+      Unused << browserParent->SendWillChangeProcess();
-      // process switch, and clear it when we've received a reply from the
+    } else if (auto* browserBridgeChild = GetBrowserBridgeChild()) {
-      // child, treating ownership mismatches as warnings in the interim.
+      // OOP IFrame - Through Browser Bridge Parent, set on browser child
-      //
-      // In the future, this sort of issue will probably need to be handled
-      // using ownership epochs, which should be more both flexible and
-      // resilient. For the moment, though, the surrounding process switch code
-      // is enough in flux that we're better off with a workable interim
-      // solution.
-      MOZ_DIAGNOSTIC_ASSERT(mPendingBrowsingContext == GetBrowsingContext());
-      RefPtr<CanonicalBrowsingContext> bc(mPendingBrowsingContext->Canonical());
-      uint64_t targetProcessId = browserParent->Manager()->ChildID();
-      bc->SetInFlightProcessId(targetProcessId);
-      auto callback = [bc, targetProcessId](auto) {
-        bc->ClearInFlightProcessId(targetProcessId);
-      };
-      browserParent->SendWillChangeProcess(callback, callback);
-    }
-    // OOP IFrame - Through Browser Bridge Parent, set on browser child
-    else if (auto* browserBridgeChild = GetBrowserBridgeChild()) {
       Unused << browserBridgeChild->SendWillChangeProcess();
     }
     return;

dom/ipc/BrowserBridgeParent.cpp

@@ -41,6 +41,10 @@ nsresult BrowserBridgeParent::InitWithProcess(
     return NS_ERROR_UNEXPECTED;
   }
 
+  MOZ_DIAGNOSTIC_ASSERT(
+      !browsingContext->GetBrowserParent(),
+      "BrowsingContext must have had previous BrowserParent cleared");
+
   // Unfortunately, due to the current racy destruction of BrowsingContext
   // instances when Fission is enabled, while `browsingContext` may not be
   // discarded, an ancestor might be.
@@ -108,6 +112,8 @@ nsresult BrowserBridgeParent::InitWithProcess(
   mBrowserParent->SetOwnerElement(aParentBrowser->GetOwnerElement());
   mBrowserParent->InitRendering();
 
+  GetBrowsingContext()->SetCurrentBrowserParent(mBrowserParent);
+
   windowParent->Init();
   return NS_OK;
 }

dom/ipc/BrowserChild.cpp

@@ -958,12 +958,10 @@ BrowserChild::~BrowserChild() {
   mozilla::DropJSObjects(this);
 }
 
-mozilla::ipc::IPCResult BrowserChild::RecvWillChangeProcess(
+mozilla::ipc::IPCResult BrowserChild::RecvWillChangeProcess() {
-    WillChangeProcessResolver&& aResolve) {
   if (mWebBrowser) {
     mWebBrowser->SetWillChangeProcess();
   }
-  aResolve(true);
   return IPC_OK();
 }
 

dom/ipc/BrowserChild.h

@@ -558,8 +558,7 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
   mozilla::ipc::IPCResult RecvUpdateNativeWindowHandle(
       const uintptr_t& aNewHandle);
 
-  mozilla::ipc::IPCResult RecvWillChangeProcess(
+  mozilla::ipc::IPCResult RecvWillChangeProcess();
-      WillChangeProcessResolver&& aResolve);
   /**
    * Native widget remoting protocol for use with windowed plugins with e10s.
    */

dom/ipc/BrowserParent.cpp

@@ -738,25 +738,12 @@ void BrowserParent::ActorDestroy(ActorDestroyReason why) {
     if (why == AbnormalShutdown) {
       frameLoader->MaybeNotifyCrashed(mBrowsingContext, Manager()->ChildID(),
                                       GetIPCChannel());
-
-      auto* bridge = GetBrowserBridgeParent();
-      if (bridge && bridge->CanSend() && !mBrowsingContext->IsDiscarded()) {
-        MOZ_ASSERT(!mBrowsingContext->IsTop());
-
-        // Set the owner process of the root context belonging to a crashed
-        // process to the embedding process, since we'll be showing the crashed
-        // page in that process.
-        mBrowsingContext->SetOwnerProcessId(
-            bridge->Manager()->Manager()->ChildID());
-        MOZ_ALWAYS_SUCCEEDS(mBrowsingContext->SetCurrentInnerWindowId(0));
-
-        // Tell the browser bridge to show the subframe crashed page.
-        Unused << bridge->SendSubFrameCrashed();
-      }
     }
   }
 
   mFrameLoader = nullptr;
+
+  mBrowsingContext->BrowserParentDestroyed(this, why == AbnormalShutdown);
 }
 
 mozilla::ipc::IPCResult BrowserParent::RecvMoveFocus(

dom/ipc/ContentParent.cpp

@@ -241,6 +241,7 @@
 #include "private/pprio.h"
 #include "xpcpublic.h"
 #include "nsOpenWindowInfo.h"
+#include "nsFrameLoaderOwner.h"
 
 #ifdef MOZ_WEBRTC
 #  include "jsapi/WebrtcGlobalParent.h"
@@ -1478,6 +1479,11 @@ already_AddRefed<RemoteBrowser> ContentParent::CreateBrowser(
     ContentParent* aOpenerContentParent) {
   AUTO_PROFILER_LABEL("ContentParent::CreateBrowser", OTHER);
 
+  MOZ_DIAGNOSTIC_ASSERT(
+      !aBrowsingContext->Canonical()->GetBrowserParent(),
+      "BrowsingContext must not have BrowserParent, or have previous "
+      "BrowserParent cleared");
+
   if (!sCanLaunchSubprocesses) {
     return nullptr;
   }
@@ -1603,6 +1609,10 @@ already_AddRefed<RemoteBrowser> ContentParent::CreateBrowser(
     return nullptr;
   }
 
+  // Ensure that we're marked as the current BrowserParent on our
+  // CanonicalBrowsingContext.
+  aBrowsingContext->Canonical()->SetCurrentBrowserParent(browserParent);
+
   windowParent->Init();
 
   RefPtr<BrowserHost> browserHost = new BrowserHost(browserParent);
@@ -3893,6 +3903,10 @@ mozilla::ipc::IPCResult ContentParent::RecvConstructPopupBrowser(
     return IPC_FAIL(this, "Cannot create without valid initial principal");
   }
 
+  if (browsingContext->GetBrowserParent()) {
+    return IPC_FAIL(this, "BrowsingContext already has a BrowserParent");
+  }
+
   uint32_t chromeFlags = aChromeFlags;
   TabId openerTabId(0);
   ContentParentId openerCpId(0);
@@ -3961,6 +3975,8 @@ mozilla::ipc::IPCResult ContentParent::RecvConstructPopupBrowser(
     return IPC_FAIL(this, "BindPWindowGlobalEndpoint failed");
   }
 
+  browsingContext->SetCurrentBrowserParent(parent);
+
   initialWindow->Init();
 
   // When enabling input event prioritization, input events may preempt other

dom/ipc/PBrowser.ipdl

@@ -1015,7 +1015,7 @@ child:
      */
     async SetWidgetNativeData(WindowsHandle aHandle);
 
-    async WillChangeProcess() returns (bool success);
+    async WillChangeProcess();
 
 parent:
     /**

dom/ipc/WindowGlobalChild.cpp

@@ -390,13 +390,14 @@ mozilla::ipc::IPCResult WindowGlobalChild::RecvMakeFrameRemote(
   MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,
           ("RecvMakeFrameRemote ID=%" PRIx64, aFrameContext.ContextId()));
 
-  // Immediately resolve the promise, acknowledging the request.
-  aResolve(true);
-
   if (!aLayersId.IsValid()) {
     return IPC_FAIL(this, "Received an invalid LayersId");
   }
 
+  // Resolve the promise when this function exits, as we'll have fully unloaded
+  // at that point.
+  auto scopeExit = MakeScopeExit([&] { aResolve(true); });
+
   // Get a BrowsingContext if we're not null or discarded. We don't want to
   // early-return before we connect the BrowserBridgeChild, as otherwise we'll
   // never break the channel in the parent.
@@ -415,20 +416,20 @@ mozilla::ipc::IPCResult WindowGlobalChild::RecvMakeFrameRemote(
     return IPC_OK();
   }
 
+  auto deleteBridge =
+      MakeScopeExit([&] { BrowserBridgeChild::Send__delete__(bridge); });
+
   // Immediately tear down the actor if we don't have a valid FrameContext.
   if (NS_WARN_IF(aFrameContext.IsNullOrDiscarded())) {
-    BrowserBridgeChild::Send__delete__(bridge);
     return IPC_OK();
   }
 
   RefPtr<Element> embedderElt = frameContext->GetEmbedderElement();
   if (NS_WARN_IF(!embedderElt)) {
-    BrowserBridgeChild::Send__delete__(bridge);
     return IPC_OK();
   }
 
   if (NS_WARN_IF(embedderElt->GetOwnerGlobal() != GetWindowGlobal())) {
-    BrowserBridgeChild::Send__delete__(bridge);
     return IPC_OK();
   }
 
@@ -439,10 +440,12 @@ mozilla::ipc::IPCResult WindowGlobalChild::RecvMakeFrameRemote(
   IgnoredErrorResult rv;
   flo->ChangeRemotenessWithBridge(bridge, rv);
   if (NS_WARN_IF(rv.Failed())) {
-    BrowserBridgeChild::Send__delete__(bridge);
     return IPC_OK();
   }
 
+  // Everything succeeded, so don't delete the bridge.
+  deleteBridge.release();
+
   return IPC_OK();
 }