From 1963d6e5e3f72ec62edc3b1253b126b169f3d721 Mon Sep 17 00:00:00 2001 From: Ryan Hunt Date: Wed, 15 May 2019 12:34:14 -0500 Subject: [PATCH] Bug 1525720, part 17 - Ignore nsIRemoteTab methods after we have destroyed the browser. r=nika It's possible for front-end references to nsIRemoteTab to outlive the IPDL actor. When this happens, we should ignore methods and property accesses. The one special case is that some code expects to be able to access the TabId after the browser has been destroyed. For this we can just cache the ID. Differential Revision: https://phabricator.services.mozilla.com/D31449 --HG-- extra : rebase_source : 06791db921203a5dfc6cc386e420bfa0de113941 extra : histedit_source : 4617c237d14e01cdbfff66d391069bcdf2267c51 --- dom/ipc/BrowserHost.cpp | 82 ++++++++++++++++++++++++++++++++++++++++- dom/ipc/BrowserHost.h | 3 ++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/dom/ipc/BrowserHost.cpp b/dom/ipc/BrowserHost.cpp index dccd9f746dbb..b94120aeee8b 100644 --- a/dom/ipc/BrowserHost.cpp +++ b/dom/ipc/BrowserHost.cpp @@ -23,7 +23,8 @@ NS_IMPL_CYCLE_COLLECTION(BrowserHost, mRoot) NS_IMPL_CYCLE_COLLECTING_ADDREF(BrowserHost) NS_IMPL_CYCLE_COLLECTING_RELEASE(BrowserHost) -BrowserHost::BrowserHost(BrowserParent* aParent) : mRoot(aParent) { +BrowserHost::BrowserHost(BrowserParent* aParent) + : mId(aParent->GetTabId()), mRoot(aParent) { mRoot->SetBrowserHost(this); } @@ -77,12 +78,19 @@ void BrowserHost::UpdateDimensions(const nsIntRect& aRect, /* attribute boolean docShellIsActive; */ NS_IMETHODIMP BrowserHost::GetDocShellIsActive(bool* aDocShellIsActive) { + if (!mRoot) { + *aDocShellIsActive = false; + return NS_OK; + } *aDocShellIsActive = mRoot->GetDocShellIsActive(); return NS_OK; } NS_IMETHODIMP BrowserHost::SetDocShellIsActive(bool aDocShellIsActive) { + if (!mRoot) { + return NS_OK; + } VisitAll([&](BrowserParent* aBrowserParent) { aBrowserParent->SetDocShellIsActive(aDocShellIsActive); }); @@ -92,12 +100,19 @@ BrowserHost::SetDocShellIsActive(bool aDocShellIsActive) { /* attribute boolean renderLayers; */ NS_IMETHODIMP BrowserHost::GetRenderLayers(bool* aRenderLayers) { + if (!mRoot) { + *aRenderLayers = false; + return NS_OK; + } *aRenderLayers = mRoot->GetRenderLayers(); return NS_OK; } NS_IMETHODIMP BrowserHost::SetRenderLayers(bool aRenderLayers) { + if (!mRoot) { + return NS_OK; + } VisitAll([&](BrowserParent* aBrowserParent) { aBrowserParent->SetRenderLayers(aRenderLayers); }); @@ -107,6 +122,10 @@ BrowserHost::SetRenderLayers(bool aRenderLayers) { /* readonly attribute boolean hasLayers; */ NS_IMETHODIMP BrowserHost::GetHasLayers(bool* aHasLayers) { + if (!mRoot) { + *aHasLayers = false; + return NS_OK; + } *aHasLayers = mRoot->GetHasLayers(); return NS_OK; } @@ -114,6 +133,9 @@ BrowserHost::GetHasLayers(bool* aHasLayers) { /* void forceRepaint (); */ NS_IMETHODIMP BrowserHost::ForceRepaint(void) { + if (!mRoot) { + return NS_OK; + } VisitAll( [](BrowserParent* aBrowserParent) { aBrowserParent->ForceRepaint(); }); return NS_OK; @@ -122,6 +144,9 @@ BrowserHost::ForceRepaint(void) { /* void resolutionChanged (); */ NS_IMETHODIMP BrowserHost::NotifyResolutionChanged(void) { + if (!mRoot) { + return NS_OK; + } VisitAll([](BrowserParent* aBrowserParent) { aBrowserParent->NotifyResolutionChanged(); }); @@ -131,6 +156,9 @@ BrowserHost::NotifyResolutionChanged(void) { /* void deprioritize (); */ NS_IMETHODIMP BrowserHost::Deprioritize(void) { + if (!mRoot) { + return NS_OK; + } VisitAll( [](BrowserParent* aBrowserParent) { aBrowserParent->Deprioritize(); }); return NS_OK; @@ -139,6 +167,9 @@ BrowserHost::Deprioritize(void) { /* void preserveLayers (in boolean aPreserveLayers); */ NS_IMETHODIMP BrowserHost::PreserveLayers(bool aPreserveLayers) { + if (!mRoot) { + return NS_OK; + } VisitAll([&](BrowserParent* aBrowserParent) { aBrowserParent->PreserveLayers(aPreserveLayers); }); @@ -148,13 +179,17 @@ BrowserHost::PreserveLayers(bool aPreserveLayers) { /* readonly attribute uint64_t tabId; */ NS_IMETHODIMP BrowserHost::GetTabId(uint64_t* aTabId) { - *aTabId = mRoot->GetTabId(); + *aTabId = mId; return NS_OK; } /* readonly attribute uint64_t contentProcessId; */ NS_IMETHODIMP BrowserHost::GetContentProcessId(uint64_t* aContentProcessId) { + if (!mRoot) { + *aContentProcessId = 0; + return NS_OK; + } *aContentProcessId = GetContentParent()->ChildID(); return NS_OK; } @@ -162,6 +197,10 @@ BrowserHost::GetContentProcessId(uint64_t* aContentProcessId) { /* readonly attribute int32_t osPid; */ NS_IMETHODIMP BrowserHost::GetOsPid(int32_t* aOsPid) { + if (!mRoot) { + *aOsPid = 0; + return NS_OK; + } *aOsPid = GetContentParent()->Pid(); return NS_OK; } @@ -169,6 +208,10 @@ BrowserHost::GetOsPid(int32_t* aOsPid) { /* readonly attribute boolean hasContentOpener; */ NS_IMETHODIMP BrowserHost::GetHasContentOpener(bool* aHasContentOpener) { + if (!mRoot) { + *aHasContentOpener = false; + return NS_OK; + } *aHasContentOpener = mRoot->GetHasContentOpener(); return NS_OK; } @@ -176,6 +219,10 @@ BrowserHost::GetHasContentOpener(bool* aHasContentOpener) { /* readonly attribute boolean hasPresented; */ NS_IMETHODIMP BrowserHost::GetHasPresented(bool* aHasPresented) { + if (!mRoot) { + *aHasPresented = false; + return NS_OK; + } *aHasPresented = mRoot->GetHasPresented(); return NS_OK; } @@ -183,6 +230,10 @@ BrowserHost::GetHasPresented(bool* aHasPresented) { NS_IMETHODIMP BrowserHost::GetWindowGlobalParents( nsTArray>& aWindowGlobalParents) { + if (!mRoot) { + aWindowGlobalParents = nsTArray>(); + return NS_OK; + } VisitAll([&](BrowserParent* aBrowser) { const auto& windowGlobalParents = aBrowser->ManagedPWindowGlobalParent(); for (auto iter = windowGlobalParents.ConstIter(); !iter.Done(); @@ -198,12 +249,19 @@ BrowserHost::GetWindowGlobalParents( /* void transmitPermissionsForPrincipal (in nsIPrincipal aPrincipal); */ NS_IMETHODIMP BrowserHost::TransmitPermissionsForPrincipal(nsIPrincipal* aPrincipal) { + if (!mRoot) { + return NS_OK; + } return GetContentParent()->TransmitPermissionsForPrincipal(aPrincipal); } /* readonly attribute boolean hasBeforeUnload; */ NS_IMETHODIMP BrowserHost::GetHasBeforeUnload(bool* aHasBeforeUnload) { + if (!mRoot) { + *aHasBeforeUnload = false; + return NS_OK; + } *aHasBeforeUnload = mRoot->GetHasBeforeUnload(); return NS_OK; } @@ -211,6 +269,10 @@ BrowserHost::GetHasBeforeUnload(bool* aHasBeforeUnload) { /* readonly attribute Element ownerElement; */ NS_IMETHODIMP BrowserHost::GetOwnerElement(mozilla::dom::Element** aOwnerElement) { + if (!mRoot) { + *aOwnerElement = nullptr; + return NS_OK; + } *aOwnerElement = mRoot->GetOwnerElement(); return NS_OK; } @@ -221,6 +283,9 @@ NS_IMETHODIMP BrowserHost::StartApzAutoscroll(float aAnchorX, float aAnchorY, nsViewID aScrollId, uint32_t aPresShellId, bool* _retval) { + if (!mRoot) { + return NS_OK; + } *_retval = mRoot->StartApzAutoscroll(aAnchorX, aAnchorY, aScrollId, aPresShellId); return NS_OK; @@ -229,6 +294,9 @@ BrowserHost::StartApzAutoscroll(float aAnchorX, float aAnchorY, /* void stopApzAutoscroll (in nsViewID aScrollId, in uint32_t aPresShellId); */ NS_IMETHODIMP BrowserHost::StopApzAutoscroll(nsViewID aScrollId, uint32_t aPresShellId) { + if (!mRoot) { + return NS_OK; + } mRoot->StopApzAutoscroll(aScrollId, aPresShellId); return NS_OK; } @@ -236,6 +304,9 @@ BrowserHost::StopApzAutoscroll(nsViewID aScrollId, uint32_t aPresShellId) { /* bool saveRecording (in AString aFileName); */ NS_IMETHODIMP BrowserHost::SaveRecording(const nsAString& aFileName, bool* _retval) { + if (!mRoot) { + return NS_OK; + } nsCOMPtr file; nsresult rv = NS_NewLocalFile(aFileName, false, getter_AddRefs(file)); if (NS_FAILED(rv)) { @@ -247,6 +318,10 @@ BrowserHost::SaveRecording(const nsAString& aFileName, bool* _retval) { /* Promise getContentBlockingLog (); */ NS_IMETHODIMP BrowserHost::GetContentBlockingLog(::mozilla::dom::Promise** aPromise) { + if (!mRoot) { + *aPromise = nullptr; + return NS_OK; + } NS_ENSURE_ARG_POINTER(aPromise); *aPromise = nullptr; @@ -286,6 +361,9 @@ NS_IMETHODIMP BrowserHost::MaybeCancelContentJSExecutionFromScript( nsIRemoteTab::NavigationType aNavigationType, JS::Handle aCancelContentJSOptions, JSContext* aCx) { + if (!mRoot) { + return NS_OK; + } dom::CancelContentJSOptions cancelContentJSOptions; if (!cancelContentJSOptions.Init(aCx, aCancelContentJSOptions)) { return NS_ERROR_INVALID_ARG; diff --git a/dom/ipc/BrowserHost.h b/dom/ipc/BrowserHost.h index 25e71a78c739..0d3854fbf183 100644 --- a/dom/ipc/BrowserHost.h +++ b/dom/ipc/BrowserHost.h @@ -89,6 +89,9 @@ class BrowserHost : public RemoteBrowser, private: virtual ~BrowserHost() = default; + // The TabID for the root BrowserParent, we cache this so that we can access + // it after the remote browser has been destroyed + TabId mId; // The root BrowserParent of this remote browser RefPtr mRoot; };