Bug 1871625 - Move AppShutdownConfirmed check in nsFrameLoader to a more appropriate place, r=smaug

This check was introduced in bug 1811195, but was added outside of the
normal opening flow, meaning that failures after this check would not be
handled properly, leading to potential assertion failures or unhandled
content process creation failures.

This patch adds a clarifying comment to ensure that we don't add checks
to the wrong place in the future, and moves the check into
`ContentParent::CreateBrowser` such that it is handled in a similar way
to other content process creation failures.

In addition, extra logic was added to ensure `mInitialized` gets set on
TryRemoteBrowserInternal failure, as new checks have been added before
the `mInitialized` check, and this seems likely to happen more in the
future.

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

dom/base/nsFrameLoader.cpp

@@ -2800,17 +2800,19 @@ bool nsFrameLoader::TryRemoteBrowserInternal() {
 }
 
 bool nsFrameLoader::TryRemoteBrowser() {
-  // Creating remote browsers may result in creating new processes, but during
+  // NOTE: Do not add new checks to this function, it exists to ensure we always
-  // parent shutdown that would add just noise, so better bail out.
+  // MaybeNotifyCrashed for any errors within TryRemoteBrowserInternal. If new
-  if (AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownConfirmed)) {
+  // checks are added, they should be added into that function, not here.
-    return false;
-  }
 
   // Try to create the internal remote browser.
   if (TryRemoteBrowserInternal()) {
     return true;
   }
 
+  // We shouldn't TryRemoteBrowser again, even if a check failed before we
+  // initialize mInitialized within TryRemoteBrowserInternal.
+  mInitialized = true;
+
   // Check if we should report a browser-crashed error because the browser
   // failed to start.
   if (XRE_IsParentProcess() && mOwnerContent && mOwnerContent->IsXULElement()) {

dom/ipc/ContentParent.cpp

@@ -1444,6 +1444,14 @@ already_AddRefed<RemoteBrowser> ContentParent::CreateBrowser(
       "BrowsingContext must not have BrowserParent, or have previous "
       "BrowserParent cleared");
 
+  // Don't bother creating new content browsers after entering shutdown. This
+  // could lead to starting a new content process, which may significantly delay
+  // shutdown, and the content is unlikely to be displayed.
+  if (AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownConfirmed)) {
+    NS_WARNING("Ignoring remote browser creation request during shutdown");
+    return nullptr;
+  }
+
   nsAutoCString remoteType(aRemoteType);
   if (remoteType.IsEmpty()) {
     remoteType = DEFAULT_REMOTE_TYPE;