Bug 1725572 - Avoid process-switching to a dead process, r=kmag

After some investigation, I was able to find a theoretical codepath
which could lead to the "missing initial frame browsing context" error:

1. Two iframes are created for the same origin, and begin process
   switching.
2. The first iframe finishes process switching, but for some reason
   (e.g. being in shutdown) the call to `LaunchSubprocessResolve`
   errors.
3. The second callback is called and also calls LaunchSubprocessResolve,
   which this time returns `true` due to it previously having been
   called.
4. The BrowserParent is created in the new content process despite
   `InitInternal()` never having been finished, and therefore the
   ContentParent never becoming subscribed to the BrowsingContextGroup.

To fix this, I made 2 changes:

1. Abort from process switching if the target process which we're going
   to be creating a BrowserParent in `IsDead()`, and
2. Track the return value from `LaunchSubprocessResolve`, so we return
   `false` if it is called a second time after a failed content process
   launch.

I'm not confident that this is the cause of the crashes, as I was unable
to reproduce the issue.

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

docshell/base/CanonicalBrowsingContext.cpp

@@ -1424,6 +1424,11 @@ nsresult CanonicalBrowsingContext::PendingRemotenessChange::FinishTopContent() {
                         "We shouldn't be trying to change the remoteness of "
                         "non-remote iframes");
 
+  // Abort if our ContentParent died while process switching.
+  if (mContentParent && NS_WARN_IF(mContentParent->IsDead())) {
+    return NS_ERROR_FAILURE;
+  }
+
   // 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.
@@ -1547,6 +1552,13 @@ nsresult CanonicalBrowsingContext::PendingRemotenessChange::FinishSubframe() {
     return NS_ERROR_FAILURE;
   }
 
+  // If we're creating a new remote browser, and the host process is already
+  // dead, abort the process switch.
+  if (mContentParent != embedderBrowser->Manager() &&
+      NS_WARN_IF(mContentParent->IsDead())) {
+    return NS_ERROR_FAILURE;
+  }
+
   RefPtr<BrowserParent> oldBrowser = target->GetBrowserParent();
   target->SetCurrentBrowserParent(nullptr);
 

dom/ipc/BrowserBridgeParent.cpp

@@ -34,6 +34,8 @@ nsresult BrowserBridgeParent::InitWithProcess(
     const WindowGlobalInit& aWindowInit, uint32_t aChromeFlags, TabId aTabId) {
   MOZ_ASSERT(!CanSend(),
              "This should be called before the object is connected to IPC");
+  MOZ_DIAGNOSTIC_ASSERT(!aContentParent->IsLaunching());
+  MOZ_DIAGNOSTIC_ASSERT(!aContentParent->IsDead());
 
   RefPtr<CanonicalBrowsingContext> browsingContext =
       CanonicalBrowsingContext::Get(aWindowInit.context().mBrowsingContextId);

dom/ipc/ContentParent.cpp

@@ -2591,7 +2591,7 @@ bool ContentParent::LaunchSubprocessResolve(bool aIsSync,
     MOZ_ASSERT(sCreatedFirstContentProcess);
     MOZ_ASSERT(!mPrefSerializer);
     MOZ_ASSERT(mLifecycleState != LifecycleState::LAUNCHING);
-    return true;
+    return mLaunchResolvedOk;
   }
   mLaunchResolved = true;
 
@@ -2675,6 +2675,8 @@ bool ContentParent::LaunchSubprocessResolve(bool aIsSync,
             ((mLaunchYieldTS - mLaunchTS) + (TimeStamp::Now() - launchResumeTS))
                 .ToMilliseconds()));
   }
+
+  mLaunchResolvedOk = true;
   return true;
 }
 
@@ -2747,6 +2749,7 @@ ContentParent::ContentParent(const nsACString& aRemoteType, int32_t aJSPluginID)
       mCreatedPairedMinidumps(false),
       mShutdownPending(false),
       mLaunchResolved(false),
+      mLaunchResolvedOk(false),
       mIsRemoteInputEventQueueEnabled(false),
       mIsInputPriorityEventEnabled(false),
       mIsInPool(false),
@@ -7122,7 +7125,7 @@ void ContentParent::AddBrowsingContextGroup(BrowsingContextGroup* aGroup) {
 void ContentParent::RemoveBrowsingContextGroup(BrowsingContextGroup* aGroup) {
   MOZ_DIAGNOSTIC_ASSERT(aGroup);
   // Remove the group from our list. This is called from the
-  // BrowisngContextGroup when unsubscribing, so we don't need to do it here.
+  // BrowsingContextGroup when unsubscribing, so we don't need to do it here.
   if (mGroups.EnsureRemoved(aGroup) && CanSend()) {
     // If we're removing the entry for the first time, tell the content process
     // to clean up the group.

dom/ipc/ContentParent.h

@@ -1550,7 +1550,11 @@ class ContentParent final
   uint8_t mCalledKillHard : 1;
   uint8_t mCreatedPairedMinidumps : 1;
   uint8_t mShutdownPending : 1;
+
+  // Whether or not `LaunchSubprocessResolve` has been called, and whether or
+  // not it returned `true` when called.
   uint8_t mLaunchResolved : 1;
+  uint8_t mLaunchResolvedOk : 1;
 
   // True if the input event queue on the main thread of the content process is
   // enabled.