From 9e0032b97f220247e42ff2842138d64f53ef2124 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Thu, 7 May 2020 22:18:56 +0000 Subject: [PATCH] Bug 1633820 - Part 5: Get rid of UnsafeIPCTabContext, r=kmag This variant was only used for service workers' openWindow method, which has been changed to no longer behave in this way, meaning that the type can be removed. The follow-up simplification of removing 'ContentChild::ProvideWindowCommon', and moving the logic directly into 'BrowserChild' is not done in this bug, and will be done in a follow-up instead. Differential Revision: https://phabricator.services.mozilla.com/D72935 --- dom/base/nsContentUtils.cpp | 7 -- dom/base/nsContentUtils.h | 5 -- dom/ipc/ContentChild.cpp | 65 ++++--------------- dom/ipc/ContentChild.h | 4 -- dom/ipc/ContentParent.cpp | 8 +-- dom/ipc/PTabContext.ipdlh | 8 --- dom/ipc/TabContext.cpp | 12 ---- .../windowwatcher/nsWindowWatcher.cpp | 4 -- 8 files changed, 17 insertions(+), 96 deletions(-) diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index a123403668f6..bad0233eea6c 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -234,7 +234,6 @@ #include "nsViewManager.h" #include "nsViewportInfo.h" #include "nsWidgetsCID.h" -#include "nsIWindowProvider.h" #include "nsWrapperCacheInlines.h" #include "nsXULPopupManager.h" #include "xpcprivate.h" // nsXPConnect @@ -5354,12 +5353,6 @@ void nsContentUtils::RemoveScriptBlocker() { sBlockedScriptRunners->RemoveElementsAt(originalFirstBlocker, blockersCount); } -/* static */ -nsIWindowProvider* nsContentUtils::GetWindowProviderForContentProcess() { - MOZ_ASSERT(XRE_IsContentProcess()); - return ContentChild::GetSingleton(); -} - /* static */ already_AddRefed nsContentUtils::GetMostRecentNonPBWindow() { diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 093cf9125748..56b46fad7545 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -109,7 +109,6 @@ class nsWrapperCache; class nsAttrValue; class nsITransferable; class nsPIWindowRoot; -class nsIWindowProvider; class nsIReferrerInfo; struct JSRuntime; @@ -2097,10 +2096,6 @@ class nsContentUtils { return sScriptBlockerCount == 0; } - // XXXcatalinb: workaround for weird include error when trying to reference - // ipdl types in WindowWatcher. - static nsIWindowProvider* GetWindowProviderForContentProcess(); - // Returns the browser window with the most recent time stamp that is // not in private browsing mode. static already_AddRefed GetMostRecentNonPBWindow(); diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 5ccb14270503..c2cdabec68c5 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -611,7 +611,6 @@ ContentChild::~ContentChild() { NS_INTERFACE_MAP_BEGIN(ContentChild) NS_INTERFACE_MAP_ENTRY(nsIContentChild) - NS_INTERFACE_MAP_ENTRY(nsIWindowProvider) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContentChild) NS_INTERFACE_MAP_END @@ -780,20 +779,6 @@ void ContentChild::SetProcessName(const nsAString& aName) { #endif } -NS_IMETHODIMP -ContentChild::ProvideWindow(nsIOpenWindowInfo* aOpenWindowInfo, - uint32_t aChromeFlags, bool aCalledFromJS, - bool aWidthSpecified, nsIURI* aURI, - const nsAString& aName, const nsACString& aFeatures, - bool aForceNoOpener, bool aForceNoReferrer, - nsDocShellLoadState* aLoadState, bool* aWindowIsNew, - BrowsingContext** aReturn) { - return ProvideWindowCommon(nullptr, aOpenWindowInfo, aChromeFlags, - aCalledFromJS, aWidthSpecified, aURI, aName, - aFeatures, aForceNoOpener, aForceNoReferrer, - aLoadState, aWindowIsNew, aReturn); -} - static nsresult GetCreateWindowParams(nsIOpenWindowInfo* aOpenWindowInfo, nsDocShellLoadState* aLoadState, bool aForceNoReferrer, float* aFullZoom, @@ -864,17 +849,16 @@ nsresult ContentChild::ProvideWindowCommon( nsIURI* aURI, const nsAString& aName, const nsACString& aFeatures, bool aForceNoOpener, bool aForceNoReferrer, nsDocShellLoadState* aLoadState, bool* aWindowIsNew, BrowsingContext** aReturn) { + MOZ_DIAGNOSTIC_ASSERT(aTabOpener, "We must have a tab opener"); + *aReturn = nullptr; - UniquePtr ipcContext; nsAutoCString features(aFeatures); nsAutoString name(aName); nsresult rv; RefPtr parent = aOpenWindowInfo->GetParent(); - MOZ_ASSERT(!parent || aTabOpener, - "If parent is non-null, we should have an aTabOpener"); // Cache the boolean preference for allowing noopener windows to open in a // separate process. @@ -896,7 +880,7 @@ nsresult ContentChild::ProvideWindowCommon( // load in the current process. bool loadInDifferentProcess = aForceNoOpener && sNoopenerNewProcess && !useRemoteSubframes; - if (aTabOpener && !loadInDifferentProcess && aURI) { + if (!loadInDifferentProcess && aURI) { // Only special-case cross-process loads if Fission is disabled. With // Fission enabled, the initial in-process load will automatically be // retargeted to the correct process. @@ -942,25 +926,13 @@ nsresult ContentChild::ProvideWindowCommon( return NS_ERROR_ABORT; } - if (aTabOpener) { - PopupIPCTabContext context; - context.openerChild() = aTabOpener; - ipcContext = MakeUnique(context); - } else { - // It's possible to not have a BrowserChild opener in the case - // of ServiceWorker::OpenWindow. - UnsafeIPCTabContext unsafeTabContext; - ipcContext = MakeUnique(unsafeTabContext); - } - - MOZ_ASSERT(ipcContext); TabId tabId(nsContentUtils::GenerateTabId()); // We need to assign a TabGroup to the PBrowser actor before we send it to the // parent. Otherwise, the parent could send messages to us before we have a // proper TabGroup for that actor. RefPtr openerBC; - if (aTabOpener && !aForceNoOpener) { + if (!aForceNoOpener) { openerBC = parent; } @@ -980,17 +952,9 @@ nsresult ContentChild::ProvideWindowCommon( // Awkwardly manually construct the new TabContext in order to ensure our // OriginAttributes perfectly matches it. MutableTabContext newTabContext; - if (aTabOpener) { - newTabContext.SetTabContext( - aTabOpener->ChromeOuterWindowID(), aTabOpener->ShowFocusRings(), - aTabOpener->PresentationURL(), aTabOpener->MaxTouchPoints()); - } else { - newTabContext.SetTabContext( - /* chromeOuterWindowID */ 0, - /* showFocusRings */ UIStateChangeType_NoChange, - /* presentationURL */ EmptyString(), - /* maxTouchPoints */ 0); - } + newTabContext.SetTabContext( + aTabOpener->ChromeOuterWindowID(), aTabOpener->ShowFocusRings(), + aTabOpener->PresentationURL(), aTabOpener->MaxTouchPoints()); // The initial about:blank document we generate within the nsDocShell will // almost certainly be replaced at some point. Unfortunately, getting the @@ -1029,8 +993,10 @@ nsresult ContentChild::ProvideWindowCommon( } // Tell the parent process to set up its PBrowserParent. + PopupIPCTabContext ipcContext; + ipcContext.openerChild() = aTabOpener; if (NS_WARN_IF(!SendConstructPopupBrowser( - std::move(parentEp), std::move(windowParentEp), tabId, *ipcContext, + std::move(parentEp), std::move(windowParentEp), tabId, ipcContext, windowInit, aChromeFlags))) { return NS_ERROR_ABORT; } @@ -1087,13 +1053,10 @@ nsresult ContentChild::ProvideWindowCommon( return; } - ParentShowInfo showInfo(EmptyString(), false, true, false, 0, 0, 0); - if (aTabOpener) { - showInfo = ParentShowInfo( - EmptyString(), false, true, false, aTabOpener->WebWidget()->GetDPI(), - aTabOpener->WebWidget()->RoundsWidgetCoordinatesTo(), - aTabOpener->WebWidget()->GetDefaultScale().scale); - } + ParentShowInfo showInfo( + EmptyString(), false, true, false, aTabOpener->WebWidget()->GetDPI(), + aTabOpener->WebWidget()->RoundsWidgetCoordinatesTo(), + aTabOpener->WebWidget()->GetDefaultScale().scale); newChild->SetMaxTouchPoints(maxTouchPoints); newChild->SetHasSiblings(hasSiblings); diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index 82711aa8ddd8..79c6172e1879 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -27,8 +27,6 @@ #include "nsTArrayForwardDeclare.h" #include "nsRefPtrHashtable.h" -#include "nsIWindowProvider.h" - #if defined(XP_MACOSX) && defined(MOZ_SANDBOX) # include "nsIFile.h" #endif @@ -77,7 +75,6 @@ enum class MediaControlKeysEvent : uint32_t; class ContentChild final : public PContentChild, public nsIContentChild, - public nsIWindowProvider, public mozilla::ipc::IShmemAllocator, public mozilla::ipc::ChildToParentStreamActorManager, public ProcessActor { @@ -89,7 +86,6 @@ class ContentChild final : public PContentChild, public: NS_DECL_NSICONTENTCHILD - NS_DECL_NSIWINDOWPROVIDER ContentChild(); virtual ~ContentChild(); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 98b3dd216b48..af49814d20ce 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -3160,8 +3160,7 @@ bool ContentParent::CanOpenBrowser(const IPCTabContext& aContext) { // the app it's trying to open.) // On e10s we also allow UnsafeTabContext to allow service workers to open // windows. This is enforced in MaybeInvalidTabContext. - if (aContext.type() != IPCTabContext::TPopupIPCTabContext && - aContext.type() != IPCTabContext::TUnsafeIPCTabContext) { + if (aContext.type() != IPCTabContext::TPopupIPCTabContext) { ASSERT_UNLESS_FUZZING( "Unexpected IPCTabContext type. Aborting AllocPBrowserParent."); return false; @@ -3260,10 +3259,9 @@ mozilla::ipc::IPCResult ContentParent::RecvConstructPopupBrowser( // XXX: Why are we checking these requirements? It seems we should register // the created frame unconditionally? - if (openerTabId > 0 || - aContext.type() == IPCTabContext::TUnsafeIPCTabContext) { + if (openerTabId > 0) { // The creation of PBrowser was triggered from content process through - // either window.open() or service worker's openWindow(). + // window.open(). // We need to register remote frame with the child generated tab id. auto* cpm = ContentProcessManager::GetSingleton(); if (!cpm->RegisterRemoteFrame(parent)) { diff --git a/dom/ipc/PTabContext.ipdlh b/dom/ipc/PTabContext.ipdlh index 8681a342875d..b83f87d26bf9 100644 --- a/dom/ipc/PTabContext.ipdlh +++ b/dom/ipc/PTabContext.ipdlh @@ -44,13 +44,6 @@ struct JSPluginFrameIPCTabContext uint32_t jsPluginId; }; -// XXXcatalinb: This is only used by ServiceWorkerClients::OpenWindow. -// Because service workers don't have an associated BrowserChild -// we can't satisfy the security constraints on b2g. As such, the parent -// process will accept this tab context only on desktop. -struct UnsafeIPCTabContext -{ }; - // IPCTabContext is an analog to mozilla::dom::TabContext. Both specify an // iframe/PBrowser's own and containing app-ids and tell you whether the // iframe/PBrowser is a browser frame. But only IPCTabContext is allowed to @@ -63,7 +56,6 @@ union IPCTabContext PopupIPCTabContext; FrameIPCTabContext; JSPluginFrameIPCTabContext; - UnsafeIPCTabContext; }; } diff --git a/dom/ipc/TabContext.cpp b/dom/ipc/TabContext.cpp index 0a529e235486..7d6e36fbc98b 100644 --- a/dom/ipc/TabContext.cpp +++ b/dom/ipc/TabContext.cpp @@ -121,18 +121,6 @@ MaybeInvalidTabContext::MaybeInvalidTabContext(const IPCTabContext& aParams) maxTouchPoints = ipcContext.maxTouchPoints(); break; } - case IPCTabContext::TUnsafeIPCTabContext: { - // XXXcatalinb: This used *only* by ServiceWorkerClients::OpenWindow. - // It is meant as a temporary solution until service workers can - // provide a BrowserChild equivalent. Don't allow this on b2g since - // it might be used to escalate privileges. - if (!StaticPrefs::dom_serviceWorkers_enabled()) { - mInvalidReason = "ServiceWorkers should be enabled."; - return; - } - - break; - } default: { MOZ_CRASH(); } diff --git a/toolkit/components/windowwatcher/nsWindowWatcher.cpp b/toolkit/components/windowwatcher/nsWindowWatcher.cpp index cadec60062e4..04e7fd261a71 100644 --- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp +++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp @@ -831,10 +831,6 @@ nsresult nsWindowWatcher::OpenWindowInternal( nsCOMPtr provider; if (parentTreeOwner) { provider = do_GetInterface(parentTreeOwner); - } else if (XRE_IsContentProcess()) { - // we're in a content process but we don't have a tabchild we can - // use. - provider = nsContentUtils::GetWindowProviderForContentProcess(); } if (provider) {