From e24cefae3f610fd8237856941ca9d83a2389405e Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Fri, 24 May 2019 22:14:44 +0000 Subject: [PATCH] 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 `` 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 | 72 ++++++++++++++++++++++++++++++++++++-- dom/base/nsFrameLoader.h | 11 ++++++ dom/ipc/BrowserParent.cpp | 39 +++------------------ 3 files changed, 84 insertions(+), 38 deletions(-) diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 62c143b3c6ee..86423cebaab8 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/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 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 owner = do_QueryObject(mOwnerContent); + if (!owner) { + return; + } + + RefPtr 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 event = FrameCrashedEvent::Constructor( + mOwnerContent->OwnerDoc(), eventName, init); + event->SetTrusted(true); + EventDispatcher::DispatchDOMEvent(mOwnerContent, nullptr, event, nullptr, + nullptr); +} diff --git a/dom/base/nsFrameLoader.h b/dom/base/nsFrameLoader.h index 972cc20224ef..cb6c59315cb0 100644 --- a/dom/base/nsFrameLoader.h +++ b/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) diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 43cc2c471ce5..e2cd3ba8aff9 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/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 frameLoader = GetFrameLoader(true); nsCOMPtr os = services::GetObserverService(); if (frameLoader) { - nsCOMPtr 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) { - os->NotifyObservers(ToSupports(frameLoader), "oop-frameloader-crashed", - nullptr); - RefPtr owner = do_QueryObject(frameElement); - if (owner) { - RefPtr 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 event = - dom::FrameCrashedEvent::Constructor(frameElement->OwnerDoc(), - eventName, init); - event->SetTrusted(true); - EventDispatcher::DispatchDOMEvent(frameElement, nullptr, event, - nullptr, nullptr); - } - } + // If this was a crash, tell our nsFrameLoader to fire crash events. + if (why == AbnormalShutdown) { + frameLoader->MaybeNotifyCrashed(GetIPCChannel()); } }