From af842b5083511e3b5d7b3150088d4eb6b420d699 Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Fri, 9 Oct 2020 17:36:22 +0000 Subject: [PATCH] Bug 1670134, remove message manager reference from LightweightThemeChild.jsm, by getting chromeOuterWindowId from BrowserChild instead, r=mconley Differential Revision: https://phabricator.services.mozilla.com/D93043 --- browser/actors/LightweightThemeChild.jsm | 13 +++++++++---- dom/base/ContentFrameMessageManager.h | 1 - .../InProcessBrowserChildMessageManager.cpp | 19 ------------------- .../InProcessBrowserChildMessageManager.h | 1 - .../chrome/test_chromeOuterWindowID.xhtml | 16 ++++++++++------ dom/chrome-webidl/MessageManager.webidl | 7 ------- dom/interfaces/base/nsIBrowserChild.idl | 5 +++++ dom/ipc/BrowserChild.cpp | 13 ++++++------- dom/ipc/BrowserChild.h | 1 - 9 files changed, 30 insertions(+), 46 deletions(-) diff --git a/browser/actors/LightweightThemeChild.jsm b/browser/actors/LightweightThemeChild.jsm index b98c8eb7955b..72b524299dcc 100644 --- a/browser/actors/LightweightThemeChild.jsm +++ b/browser/actors/LightweightThemeChild.jsm @@ -25,12 +25,17 @@ class LightweightThemeChild extends JSWindowActorChild { } _getChromeOuterWindowID() { - if (this.docShell.messageManager) { - return this.docShell.messageManager.chromeOuterWindowID; - } + try { + // Getting the browserChild throws an exception when it is null. + let browserChild = this.docShell.browserChild; + if (browserChild) { + return browserChild.chromeOuterWindowID; + } + } catch (ex) {} + // We don't have a message manager, so presumable we're running in a sidebar // in the parent process. - return this.contentWindow.top.docShell.outerWindowID; + return this.contentWindow.top.docShell?.outerWindowID; } /** diff --git a/dom/base/ContentFrameMessageManager.h b/dom/base/ContentFrameMessageManager.h index 2591e78cb064..89e8cb3dc937 100644 --- a/dom/base/ContentFrameMessageManager.h +++ b/dom/base/ContentFrameMessageManager.h @@ -40,7 +40,6 @@ class ContentFrameMessageManager : public DOMEventTargetHelper, virtual Nullable GetContent(ErrorResult& aError) = 0; virtual already_AddRefed GetDocShell(ErrorResult& aError) = 0; virtual already_AddRefed GetTabEventTarget() = 0; - virtual uint64_t ChromeOuterWindowID() = 0; nsFrameMessageManager* GetMessageManager() { return mMessageManager; } void DisconnectMessageManager() { diff --git a/dom/base/InProcessBrowserChildMessageManager.cpp b/dom/base/InProcessBrowserChildMessageManager.cpp index aa2e1b022451..0dc6cf859b38 100644 --- a/dom/base/InProcessBrowserChildMessageManager.cpp +++ b/dom/base/InProcessBrowserChildMessageManager.cpp @@ -186,25 +186,6 @@ InProcessBrowserChildMessageManager::GetTabEventTarget() { return target.forget(); } -uint64_t InProcessBrowserChildMessageManager::ChromeOuterWindowID() { - if (!mDocShell) { - return 0; - } - - nsCOMPtr root; - nsresult rv = mDocShell->GetInProcessRootTreeItem(getter_AddRefs(root)); - if (NS_WARN_IF(NS_FAILED(rv))) { - return 0; - } - - nsPIDOMWindowOuter* topWin = root->GetWindow(); - if (!topWin) { - return 0; - } - - return topWin->WindowID(); -} - void InProcessBrowserChildMessageManager::FireUnloadEvent() { // We're called from Document::MaybeInitializeFinalizeFrameLoaders, so it // should be safe to run script. diff --git a/dom/base/InProcessBrowserChildMessageManager.h b/dom/base/InProcessBrowserChildMessageManager.h index 9f586385ffa3..bedfa48eaeb5 100644 --- a/dom/base/InProcessBrowserChildMessageManager.h +++ b/dom/base/InProcessBrowserChildMessageManager.h @@ -62,7 +62,6 @@ class InProcessBrowserChildMessageManager final return do_AddRef(mDocShell); } virtual already_AddRefed GetTabEventTarget() override; - virtual uint64_t ChromeOuterWindowID() override; NS_FORWARD_SAFE_NSIMESSAGESENDER(mMessageManager) diff --git a/dom/base/test/chrome/test_chromeOuterWindowID.xhtml b/dom/base/test/chrome/test_chromeOuterWindowID.xhtml index cca49158174e..f7b2489b30be 100644 --- a/dom/base/test/chrome/test_chromeOuterWindowID.xhtml +++ b/dom/base/test/chrome/test_chromeOuterWindowID.xhtml @@ -41,13 +41,17 @@ windows. "Both browsers should belong to the same document."); let winID = getOuterWindowID(browser1.ownerGlobal); - let browser1ID = await ContentTask.spawn(browser1, null, () => { - return chromeOuterWindowID; - }); + let getChildRootOuterId = browser => { + try { + return docShell.browserChild?.chromeOuterWindowID; + } catch(ex) { } - let browser2ID = await ContentTask.spawn(browser2, null, () => { - return chromeOuterWindowID; - }); + // Not a remote tab + return content.top.windowRoot.ownerGlobal.docShell.outerWindowID; + }; + + let browser1ID = await SpecialPowers.spawn(browser1, [], getChildRootOuterId); + let browser2ID = await SpecialPowers.spawn(browser2, [], getChildRootOuterId); is(browser1ID, winID, "Browser 1 frame script environment should have the correct chromeOuterWindowID"); diff --git a/dom/chrome-webidl/MessageManager.webidl b/dom/chrome-webidl/MessageManager.webidl index a91b88e2d4bc..92db4fefc8df 100644 --- a/dom/chrome-webidl/MessageManager.webidl +++ b/dom/chrome-webidl/MessageManager.webidl @@ -473,13 +473,6 @@ interface ContentFrameMessageManager : EventTarget */ readonly attribute nsIEventTarget? tabEventTarget; - /** - * Returns the outerWindowID of the browser window hosting the frame. - * If, for some reason, the frameloader can't be resolved to a browser - * window, this will return 0. - */ - readonly attribute long long chromeOuterWindowID; - }; ContentFrameMessageManager includes MessageManagerGlobal; ContentFrameMessageManager includes SyncMessageSenderMixin; diff --git a/dom/interfaces/base/nsIBrowserChild.idl b/dom/interfaces/base/nsIBrowserChild.idl index e145bc43e53c..7120425a3329 100644 --- a/dom/interfaces/base/nsIBrowserChild.idl +++ b/dom/interfaces/base/nsIBrowserChild.idl @@ -50,4 +50,9 @@ interface nsIBrowserChild : nsISupports * nsIWebNavigation navigation finished in the child. */ void notifyNavigationFinished(); + + /** + * Id of the chrome window the tab is within. + */ + readonly attribute uint64_t chromeOuterWindowID; }; diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 12d7c1c2562e..27d2fd7125dc 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -2993,6 +2993,12 @@ void BrowserChild::SetTabId(const TabId& aTabId) { NestedBrowserChildMap()[mUniqueId] = this; } +NS_IMETHODIMP +BrowserChild::GetChromeOuterWindowID(uint64_t* aId) { + *aId = ChromeOuterWindowID(); + return NS_OK; +} + bool BrowserChild::DoSendBlockingMessage( const nsAString& aMessage, StructuredCloneData& aData, nsTArray* aRetVal) { @@ -4024,13 +4030,6 @@ BrowserChildMessageManager::GetTabEventTarget() { return target.forget(); } -uint64_t BrowserChildMessageManager::ChromeOuterWindowID() { - if (!mBrowserChild) { - return 0; - } - return mBrowserChild->ChromeOuterWindowID(); -} - nsresult BrowserChildMessageManager::Dispatch( TaskCategory aCategory, already_AddRefed&& aRunnable) { return DispatcherTrait::Dispatch(aCategory, std::move(aRunnable)); diff --git a/dom/ipc/BrowserChild.h b/dom/ipc/BrowserChild.h index 6dcdd765b486..5cb1926cd6ec 100644 --- a/dom/ipc/BrowserChild.h +++ b/dom/ipc/BrowserChild.h @@ -106,7 +106,6 @@ class BrowserChildMessageManager : public ContentFrameMessageManager, virtual already_AddRefed GetDocShell( ErrorResult& aError) override; virtual already_AddRefed GetTabEventTarget() override; - virtual uint64_t ChromeOuterWindowID() override; NS_FORWARD_SAFE_NSIMESSAGESENDER(mMessageManager)