From 3c0087116298d277965ba4b4cf44b8385069cc02 Mon Sep 17 00:00:00 2001 From: Jens Stutte Date: Fri, 8 Apr 2022 06:20:19 +0000 Subject: [PATCH] Bug 1696771: Always null check the ContentProcessManager singleton before use. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D143180 --- docshell/base/CanonicalBrowsingContext.cpp | 3 + dom/base/nsFrameLoader.cpp | 3 + dom/ipc/BrowserBridgeParent.cpp | 3 + dom/ipc/BrowserParent.cpp | 5 +- dom/ipc/ContentParent.cpp | 127 +++++++++++------- .../webrtc/transport/ipc/WebrtcTCPSocket.cpp | 6 +- gfx/ipc/CrossProcessPaint.cpp | 9 +- 7 files changed, 104 insertions(+), 52 deletions(-) diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index 5cd83a13645f..6048a2c0e8eb 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -184,6 +184,9 @@ ContentParent* CanonicalBrowsingContext::GetContentParent() const { } ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); + if (!cpm) { + return nullptr; + } return cpm->GetContentProcessById(ContentParentId(mProcessId)); } diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index c558a14c97c4..0ebd25360dbb 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -2750,6 +2750,9 @@ bool nsFrameLoader::TryRemoteBrowserInternal() { RefPtr contentParent; if (mChildID != 0) { ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); + if (cpm) { + return false; + } contentParent = cpm->GetContentProcessById(ContentParentId(mChildID)); } mRemoteBrowser = diff --git a/dom/ipc/BrowserBridgeParent.cpp b/dom/ipc/BrowserBridgeParent.cpp index 1039f6e24720..e856864748ce 100644 --- a/dom/ipc/BrowserBridgeParent.cpp +++ b/dom/ipc/BrowserBridgeParent.cpp @@ -83,6 +83,9 @@ nsresult BrowserBridgeParent::InitWithProcess( } ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); + if (!cpm) { + return NS_ERROR_UNEXPECTED; + } cpm->RegisterRemoteFrame(browserParent); RefPtr windowParent = diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 57cd0a5f64e4..45eb3de92a23 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -675,7 +675,10 @@ mozilla::ipc::IPCResult BrowserParent::RecvEnsureLayersConnected( void BrowserParent::ActorDestroy(ActorDestroyReason why) { Manager()->NotifyTabDestroyed(mTabId, mMarkedDestroying); - ContentProcessManager::GetSingleton()->UnregisterRemoteFrame(mTabId); + ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); + if (cpm) { + cpm->UnregisterRemoteFrame(mTabId); + } if (mRemoteLayerTreeOwner.IsInitialized()) { auto layersId = mRemoteLayerTreeOwner.GetLayersId(); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index e694f441ee36..87fbc45ef51d 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -1530,6 +1530,9 @@ already_AddRefed ContentParent::CreateBrowser( } ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); + if (NS_WARN_IF(!cpm)) { + return nullptr; + } cpm->RegisterRemoteFrame(browserParent); nsCOMPtr initialPrincipal = @@ -2090,7 +2093,9 @@ void ContentParent::ActorDestroy(ActorDestroyReason why) { mSubprocess = nullptr; ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - cpm->RemoveContentProcess(this->ChildID()); + if (cpm) { + cpm->RemoveContentProcess(this->ChildID()); + } if (mDriverCrashGuard) { mDriverCrashGuard->NotifyCrashed(); @@ -2481,6 +2486,8 @@ void ContentParent::AppendSandboxParams(std::vector& aArgs) { bool ContentParent::BeginSubprocessLaunch(ProcessPriority aPriority) { AUTO_PROFILER_LABEL("ContentParent::LaunchSubprocess", OTHER); + // XXX: This check works only late, as GetSingleton will return nullptr + // only after ClearOnShutdown happened. See bug 1632740. if (!ContentProcessManager::GetSingleton()) { NS_WARNING( "Shutdown has begun, we shouldn't spawn any more child processes"); @@ -2613,7 +2620,13 @@ bool ContentParent::LaunchSubprocessResolve(bool aIsSync, base::GetProcId(mSubprocess->GetChildProcessHandle()); Open(mSubprocess->TakeInitialPort(), procId); - ContentProcessManager::GetSingleton()->AddContentProcess(this); + ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); + if (!cpm) { + NS_WARNING("immediately shutting-down caused by our shutdown"); + ShutDownProcess(SEND_SHUTDOWN_MESSAGE); + return false; + } + cpm->AddContentProcess(this); #ifdef MOZ_CODE_COVERAGE Unused << SendShareCodeCoverageMutex( @@ -4026,7 +4039,7 @@ mozilla::ipc::IPCResult ContentParent::RecvConstructPopupBrowser( // window.open(). // We need to register remote frame with the child generated tab id. auto* cpm = ContentProcessManager::GetSingleton(); - if (!cpm->RegisterRemoteFrame(parent)) { + if (!cpm || !cpm->RegisterRemoteFrame(parent)) { return IPC_FAIL(this, "RegisterRemoteFrame Failed"); } } @@ -5156,9 +5169,12 @@ ContentParent::AllocPContentPermissionRequestParent( const IPC::Principal& aPrincipal, const IPC::Principal& aTopLevelPrincipal, const bool& aIsHandlingUserInput, const bool& aMaybeUnsafePermissionDelegate, const TabId& aTabId) { + RefPtr tp; ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - RefPtr tp = - cpm->GetTopLevelBrowserParentByProcessAndTabId(this->ChildID(), aTabId); + if (cpm) { + tp = + cpm->GetTopLevelBrowserParentByProcessAndTabId(this->ChildID(), aTabId); + } if (!tp) { return nullptr; } @@ -6812,9 +6828,11 @@ mozilla::ipc::IPCResult ContentParent::RecvWindowClose( // browsing contexts of bc. ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); - Unused << cp->SendWindowClose(context, aTrustedCaller); + if (cpm) { + ContentParent* cp = + cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); + Unused << cp->SendWindowClose(context, aTrustedCaller); + } return IPC_OK(); } @@ -6831,9 +6849,11 @@ mozilla::ipc::IPCResult ContentParent::RecvWindowFocus( CanonicalBrowsingContext* context = aContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); - Unused << cp->SendWindowFocus(context, aCallerType, aActionId); + if (cpm) { + ContentParent* cp = + cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); + Unused << cp->SendWindowFocus(context, aCallerType, aActionId); + } return IPC_OK(); } @@ -6848,9 +6868,11 @@ mozilla::ipc::IPCResult ContentParent::RecvWindowBlur( CanonicalBrowsingContext* context = aContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); - Unused << cp->SendWindowBlur(context, aCallerType); + if (cpm) { + ContentParent* cp = + cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); + Unused << cp->SendWindowBlur(context, aCallerType); + } return IPC_OK(); } @@ -6868,9 +6890,11 @@ mozilla::ipc::IPCResult ContentParent::RecvRaiseWindow( CanonicalBrowsingContext* context = aContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); - Unused << cp->SendRaiseWindow(context, aCallerType, aActionId); + if (cpm) { + ContentParent* cp = + cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); + Unused << cp->SendRaiseWindow(context, aCallerType, aActionId); + } return IPC_OK(); } @@ -6891,21 +6915,23 @@ mozilla::ipc::IPCResult ContentParent::RecvAdjustWindowFocus( processes.InsertOrUpdate(this, true); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - CanonicalBrowsingContext* context = aContext.get_canonical(); - while (context) { - BrowsingContext* parent = context->GetParent(); - if (!parent) { - break; - } + if (cpm) { + CanonicalBrowsingContext* context = aContext.get_canonical(); + while (context) { + BrowsingContext* parent = context->GetParent(); + if (!parent) { + break; + } - CanonicalBrowsingContext* canonicalParent = parent->Canonical(); - ContentParent* cp = cpm->GetContentProcessById( - ContentParentId(canonicalParent->OwnerProcessId())); - if (cp && !processes.Get(cp)) { - Unused << cp->SendAdjustWindowFocus(context, aIsVisible, aActionId); - processes.InsertOrUpdate(cp, true); + CanonicalBrowsingContext* canonicalParent = parent->Canonical(); + ContentParent* cp = cpm->GetContentProcessById( + ContentParentId(canonicalParent->OwnerProcessId())); + if (cp && !processes.Get(cp)) { + Unused << cp->SendAdjustWindowFocus(context, aIsVisible, aActionId); + processes.InsertOrUpdate(cp, true); + } + context = canonicalParent; } - context = canonicalParent; } return IPC_OK(); } @@ -6921,9 +6947,11 @@ mozilla::ipc::IPCResult ContentParent::RecvClearFocus( CanonicalBrowsingContext* context = aContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); - Unused << cp->SendClearFocus(context); + if (cpm) { + ContentParent* cp = + cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); + Unused << cp->SendClearFocus(context); + } return IPC_OK(); } @@ -7046,11 +7074,11 @@ mozilla::ipc::IPCResult ContentParent::RecvSetFocusedElement( CanonicalBrowsingContext* context = aContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); - Unused << cp->SendSetFocusedElement(context, aNeedsFocus); - + if (cpm) { + ContentParent* cp = + cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); + Unused << cp->SendSetFocusedElement(context, aNeedsFocus); + } return IPC_OK(); } @@ -7066,11 +7094,12 @@ mozilla::ipc::IPCResult ContentParent::RecvFinalizeFocusOuter( LOGFOCUS(("ContentParent::RecvFinalizeFocusOuter")); CanonicalBrowsingContext* context = aContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->EmbedderProcessId())); - if (cp) { - Unused << cp->SendFinalizeFocusOuter(context, aCanFocus, aCallerType); + if (cpm) { + ContentParent* cp = cpm->GetContentProcessById( + ContentParentId(context->EmbedderProcessId())); + if (cp) { + Unused << cp->SendFinalizeFocusOuter(context, aCanFocus, aCallerType); + } } return IPC_OK(); } @@ -7111,6 +7140,9 @@ mozilla::ipc::IPCResult ContentParent::RecvBlurToParent( aFocusedBrowsingContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); + if (!cpm) { + return IPC_OK(); + } // If aBrowsingContextToClear and aAncestorBrowsingContextToFocusHandled // didn't get handled in the process that sent this IPC message and they @@ -7158,10 +7190,11 @@ mozilla::ipc::IPCResult ContentParent::RecvMaybeExitFullscreen( CanonicalBrowsingContext* context = aContext.get_canonical(); ContentProcessManager* cpm = ContentProcessManager::GetSingleton(); - - ContentParent* cp = - cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); - Unused << cp->SendMaybeExitFullscreen(context); + if (cpm) { + ContentParent* cp = + cpm->GetContentProcessById(ContentParentId(context->OwnerProcessId())); + Unused << cp->SendMaybeExitFullscreen(context); + } return IPC_OK(); } diff --git a/dom/media/webrtc/transport/ipc/WebrtcTCPSocket.cpp b/dom/media/webrtc/transport/ipc/WebrtcTCPSocket.cpp index 26a883cece75..d33c9a1dd865 100644 --- a/dom/media/webrtc/transport/ipc/WebrtcTCPSocket.cpp +++ b/dom/media/webrtc/transport/ipc/WebrtcTCPSocket.cpp @@ -73,8 +73,10 @@ WebrtcTCPSocket::~WebrtcTCPSocket() { void WebrtcTCPSocket::SetTabId(dom::TabId aTabId) { MOZ_ASSERT(NS_IsMainThread()); dom::ContentProcessManager* cpm = dom::ContentProcessManager::GetSingleton(); - dom::ContentParentId cpId = cpm->GetTabProcessId(aTabId); - mAuthProvider = cpm->GetBrowserParentByProcessAndTabId(cpId, aTabId); + if (cpm) { + dom::ContentParentId cpId = cpm->GetTabProcessId(aTabId); + mAuthProvider = cpm->GetBrowserParentByProcessAndTabId(cpId, aTabId); + } } nsresult WebrtcTCPSocket::Write(nsTArray&& aWriteData) { diff --git a/gfx/ipc/CrossProcessPaint.cpp b/gfx/ipc/CrossProcessPaint.cpp index 608fb30af425..23eafc408f03 100644 --- a/gfx/ipc/CrossProcessPaint.cpp +++ b/gfx/ipc/CrossProcessPaint.cpp @@ -367,13 +367,18 @@ void CrossProcessPaint::LostFragment(dom::WindowGlobalParent* aWGP) { void CrossProcessPaint::QueueDependencies( const nsTHashSet& aDependencies) { + dom::ContentProcessManager* cpm = dom::ContentProcessManager::GetSingleton(); + if (!cpm) { + CPP_LOG( + "Skipping QueueDependencies with no" + " current ContentProcessManager.\n"); + return; + } for (const auto& key : aDependencies) { auto dependency = dom::TabId(key); // Get the current BrowserParent of the remote browser that was marked // as a dependency - dom::ContentProcessManager* cpm = - dom::ContentProcessManager::GetSingleton(); dom::ContentParentId cpId = cpm->GetTabProcessId(dependency); RefPtr browser = cpm->GetBrowserParentByProcessAndTabId(cpId, dependency);