Bug 1553952 - Fire oop-browser-crashed when mBrowserParent construction fails, r=mconley

This is an attempt to reduce the negative impact of bug 1553644 by replacing a
remote browser which fails to create an `mBrowserParent` actor with a tab
crashed display rather than a failed `nsFrameLoader`. This is done by firing the
`oop-browser-crashed` event on the owner `<browser>` element when the attempt
fails, even if no `BrowserParent` was ever created.

This does not fix the root cause of bug 1553644, but may make the browser better
at recovering.

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

--HG--
extra : moz-landing-system : lando

dom/base/nsFrameLoader.cpp

@@ -83,6 +83,7 @@
 #include "mozilla/Unused.h"
 #include "mozilla/dom/ChromeMessageSender.h"
 #include "mozilla/dom/Element.h"
+#include "mozilla/dom/FrameCrashedEvent.h"
 #include "mozilla/dom/FrameLoaderBinding.h"
 #include "mozilla/dom/MozFrameLoaderOwnerBinding.h"
 #include "mozilla/dom/SessionStoreListener.h"
@@ -186,7 +187,8 @@ nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext,
       mLoadingOriginalSrc(false),
       mRemoteBrowserShown(false),
       mIsRemoteFrame(false),
-      mObservingOwnerContent(false) {
+      mObservingOwnerContent(false),
+      mTabProcessCrashFired(false) {
   mIsRemoteFrame = ShouldUseRemoteProcess();
   MOZ_ASSERT(!mIsRemoteFrame || !mBrowsingContext->HasOpener(),
              "Cannot pass aOpener for a remote frame!");
@@ -210,7 +212,8 @@ nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext,
       mLoadingOriginalSrc(false),
       mRemoteBrowserShown(false),
       mIsRemoteFrame(false),
-      mObservingOwnerContent(false) {
+      mObservingOwnerContent(false),
+      mTabProcessCrashFired(false) {
   if (aOptions.mRemoteType.WasPassed() &&
       (!aOptions.mRemoteType.Value().IsVoid())) {
     mIsRemoteFrame = true;
@@ -2565,7 +2568,7 @@ bool nsFrameLoader::EnsureRemoteBrowser() {
   return mRemoteBrowser || TryRemoteBrowser();
 }
 
-bool nsFrameLoader::TryRemoteBrowser() {
+bool nsFrameLoader::TryRemoteBrowserInternal() {
   NS_ASSERTION(!mRemoteBrowser,
                "TryRemoteBrowser called with a remote browser already?");
 
@@ -2753,6 +2756,21 @@ bool nsFrameLoader::TryRemoteBrowser() {
   return true;
 }
 
+bool nsFrameLoader::TryRemoteBrowser() {
+  // Try to create the internal remote browser.
+  if (TryRemoteBrowserInternal()) {
+    return true;
+  }
+
+  // Check if we should report a browser-crashed error because the browser
+  // failed to start.
+  if (XRE_IsParentProcess() && mOwnerContent && mOwnerContent->IsXULElement()) {
+    MaybeNotifyCrashed(nullptr);
+  }
+
+  return false;
+}
+
 bool nsFrameLoader::IsRemoteFrame() {
   if (mIsRemoteFrame) {
     MOZ_ASSERT(!GetDocShell(), "Found a remote frame with a DocShell");
@@ -3504,3 +3522,51 @@ void nsFrameLoader::SkipBrowsingContextDetach() {
   MOZ_ASSERT(docshell);
   docshell->SkipBrowsingContextDetach();
 }
+
+void nsFrameLoader::MaybeNotifyCrashed(mozilla::ipc::MessageChannel* aChannel) {
+  if (mTabProcessCrashFired) {
+    return;
+  }
+  mTabProcessCrashFired = true;
+
+  // Fire the crashed observer notification.
+  nsCOMPtr<nsIObserverService> os = services::GetObserverService();
+  if (!os) {
+    return;
+  }
+  os->NotifyObservers(ToSupports(this), "oop-frameloader-crashed", nullptr);
+
+  // Check our owner element still references us. If it's moved, on, events
+  // don't need to be fired.
+  RefPtr<nsFrameLoaderOwner> owner = do_QueryObject(mOwnerContent);
+  if (!owner) {
+    return;
+  }
+
+  RefPtr<nsFrameLoader> currentFrameLoader = owner->GetFrameLoader();
+  if (currentFrameLoader != this) {
+    return;
+  }
+
+  // Fire the actual crashed event.
+  nsString eventName;
+  if (aChannel && !aChannel->DoBuildIDsMatch()) {
+    eventName = NS_LITERAL_STRING("oop-browser-buildid-mismatch");
+  } else {
+    eventName = NS_LITERAL_STRING("oop-browser-crashed");
+  }
+
+  FrameCrashedEventInit init;
+  init.mBubbles = true;
+  init.mCancelable = true;
+  if (mBrowsingContext) {
+    init.mBrowsingContextId = mBrowsingContext->Id();
+    init.mIsTopFrame = !mBrowsingContext->GetParent();
+  }
+
+  RefPtr<FrameCrashedEvent> event = FrameCrashedEvent::Constructor(
+      mOwnerContent->OwnerDoc(), eventName, init);
+  event->SetTrusted(true);
+  EventDispatcher::DispatchDOMEvent(mOwnerContent, nullptr, event, nullptr,
+                                    nullptr);
+}

dom/base/nsFrameLoader.h

@@ -71,6 +71,10 @@ class StructuredCloneData;
 
 }  // namespace dom
 
+namespace ipc {
+class MessageChannel;
+}  // namespace ipc
+
 namespace layout {
 class RenderFrame;
 }  // namespace layout
@@ -390,6 +394,8 @@ class nsFrameLoader final : public nsStubMutationObserver,
 
   void SkipBrowsingContextDetach();
 
+  void MaybeNotifyCrashed(mozilla::ipc::MessageChannel* aChannel);
+
  private:
   nsFrameLoader(mozilla::dom::Element* aOwner,
                 mozilla::dom::BrowsingContext* aBrowsingContext,
@@ -452,6 +458,7 @@ class nsFrameLoader final : public nsStubMutationObserver,
 
   // Return true if remote browser created; nothing else to do
   bool TryRemoteBrowser();
+  bool TryRemoteBrowserInternal();
 
   // Tell the remote browser that it's now "virtually visible"
   bool ShowRemoteFrame(const mozilla::ScreenIntSize& size,
@@ -526,6 +533,10 @@ class nsFrameLoader final : public nsStubMutationObserver,
   bool mRemoteBrowserShown : 1;
   bool mIsRemoteFrame : 1;
   bool mObservingOwnerContent : 1;
+
+  // When an out-of-process nsFrameLoader crashes, an event is fired on the
+  // frame. To ensure this is only fired once, this bit is checked.
+  bool mTabProcessCrashFired : 1;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsFrameLoader, NS_FRAMELOADER_IID)

dom/ipc/BrowserParent.cpp

@@ -19,7 +19,6 @@
 #include "mozilla/dom/DataTransfer.h"
 #include "mozilla/dom/DataTransferItemList.h"
 #include "mozilla/dom/Event.h"
-#include "mozilla/dom/FrameCrashedEvent.h"
 #include "mozilla/dom/indexedDB/ActorsParent.h"
 #include "mozilla/dom/IPCBlobUtils.h"
 #include "mozilla/dom/PaymentRequestParent.h"
@@ -137,9 +136,9 @@ using namespace mozilla::widget;
 using namespace mozilla::jsipc;
 using namespace mozilla::gfx;
 
-using mozilla::Unused;
 using mozilla::LazyLogModule;
 using mozilla::StaticAutoPtr;
+using mozilla::Unused;
 
 LazyLogModule gBrowserFocusLog("BrowserFocus");
 
@@ -673,7 +672,6 @@ void BrowserParent::ActorDestroy(ActorDestroyReason why) {
   RefPtr<nsFrameLoader> frameLoader = GetFrameLoader(true);
   nsCOMPtr<nsIObserverService> os = services::GetObserverService();
   if (frameLoader) {
-    nsCOMPtr<Element> frameElement(mFrameElement);
     ReceiveMessage(CHILD_PROCESS_SHUTDOWN_MESSAGE, false, nullptr, nullptr,
                    nullptr);
 
@@ -684,38 +682,9 @@ void BrowserParent::ActorDestroy(ActorDestroyReason why) {
       frameLoader->DestroyComplete();
     }
 
-    if (why == AbnormalShutdown && os) {
+    // If this was a crash, tell our nsFrameLoader to fire crash events.
-      os->NotifyObservers(ToSupports(frameLoader), "oop-frameloader-crashed",
+    if (why == AbnormalShutdown) {
-                          nullptr);
+      frameLoader->MaybeNotifyCrashed(GetIPCChannel());
-      RefPtr<nsFrameLoaderOwner> owner = do_QueryObject(frameElement);
-      if (owner) {
-        RefPtr<nsFrameLoader> currentFrameLoader = owner->GetFrameLoader();
-        // It's possible that the frameloader owner has already moved on
-        // and created a new frameloader. If so, we don't fire the event,
-        // since the frameloader owner has clearly moved on.
-        if (currentFrameLoader == frameLoader) {
-          nsString eventName;
-          MessageChannel* channel = GetIPCChannel();
-          if (channel && !channel->DoBuildIDsMatch()) {
-            eventName = NS_LITERAL_STRING("oop-browser-buildid-mismatch");
-          } else {
-            eventName = NS_LITERAL_STRING("oop-browser-crashed");
-          }
-
-          dom::FrameCrashedEventInit init;
-          init.mBubbles = true;
-          init.mCancelable = true;
-          init.mBrowsingContextId = mBrowsingContext->Id();
-          init.mIsTopFrame = !mBrowsingContext->GetParent();
-
-          RefPtr<dom::FrameCrashedEvent> event =
-              dom::FrameCrashedEvent::Constructor(frameElement->OwnerDoc(),
-                                                  eventName, init);
-          event->SetTrusted(true);
-          EventDispatcher::DispatchDOMEvent(frameElement, nullptr, event,
-                                            nullptr, nullptr);
-        }
-      }
     }
   }