commit 57fca9d512cb15cb3b3af4f746e8ff517524d4d2
parent 19e5b824619d58f0acf1a0d8b6a357e97d696909
Author: Serban Stanca <sstanca@mozilla.com>
Date: Fri, 10 Oct 2025 21:43:34 +0300
Revert "Bug 1993742 - Consolidate GPUProcessManager::EnsureGPUReady calls when setting up IPDL actors. r=gfx-reviewers,lsalzman" for causing GPU process crashes.
This reverts commit 3ae6567b846f7290fe66f78af21209899f070ca4.
Diffstat:
7 files changed, 101 insertions(+), 125 deletions(-)
diff --git a/gfx/ipc/CompositorSession.h b/gfx/ipc/CompositorSession.h
@@ -73,8 +73,8 @@ class CompositorSession {
// Set the UiCompositorControllerChild after Session creation so the Session
// constructor doesn't get mucked up for other platforms.
void SetUiCompositorControllerChild(
- RefPtr<UiCompositorControllerChild>&& aUiController) {
- mUiCompositorControllerChild = std::move(aUiController);
+ RefPtr<UiCompositorControllerChild> aUiController) {
+ mUiCompositorControllerChild = aUiController;
}
RefPtr<UiCompositorControllerChild> GetUiCompositorControllerChild() {
diff --git a/gfx/ipc/GPUChild.h b/gfx/ipc/GPUChild.h
@@ -35,8 +35,6 @@ class GPUChild final : public mozilla::ipc::CrashReporterHelper<GPUChild>,
void Init();
- bool IsGPUReady() const { return mGPUReady && !mWaitForVarUpdate; }
-
bool EnsureGPUReady();
void MarkWaitForVarUpdate() { mWaitForVarUpdate = true; }
diff --git a/gfx/ipc/GPUProcessManager.cpp b/gfx/ipc/GPUProcessManager.cpp
@@ -363,22 +363,6 @@ bool GPUProcessManager::MaybeDisableGPUProcess(const char* aMessage,
return true;
}
-bool GPUProcessManager::IsGPUReady() const {
- // If we have disabled the GPU process, then we know we are always ready.
- if (!gfxConfig::IsEnabled(Feature::GPU_PROCESS)) {
- MOZ_ASSERT(!mProcess);
- MOZ_ASSERT(!mGPUChild);
- return true;
- }
-
- // If we have a GPUChild, then we know the process has finished launching.
- if (mGPUChild) {
- return mGPUChild->IsGPUReady();
- }
-
- return false;
-}
-
nsresult GPUProcessManager::EnsureGPUReady(
bool aRetryAfterFallback /* = true */) {
MOZ_ASSERT(NS_IsMainThread());
@@ -397,8 +381,7 @@ nsresult GPUProcessManager::EnsureGPUReady(
}
if (!LaunchGPUProcess()) {
- MOZ_DIAGNOSTIC_ASSERT(!gfxConfig::IsEnabled(Feature::GPU_PROCESS));
- return NS_OK;
+ return NS_ERROR_FAILURE;
}
}
@@ -413,13 +396,6 @@ nsresult GPUProcessManager::EnsureGPUReady(
// or it will have disabled the GPU process.
MOZ_ASSERT(!mProcess);
MOZ_ASSERT(!mGPUChild);
-
- // If aRetryAfterFallback is true, we will relaunch the process
- // immediately in this loop (if still enabled). Otherwise we return to
- // the caller to allow them to reconfigure first.
- if (!aRetryAfterFallback) {
- return NS_ERROR_ABORT;
- }
continue;
}
}
@@ -427,6 +403,7 @@ nsresult GPUProcessManager::EnsureGPUReady(
// If we don't have a connected process by this stage, we must have
// explicitly disabled the GPU process.
if (!mGPUChild) {
+ MOZ_DIAGNOSTIC_ASSERT(!gfxConfig::IsEnabled(Feature::GPU_PROCESS));
break;
}
@@ -447,12 +424,10 @@ nsresult GPUProcessManager::EnsureGPUReady(
// in this loop (if still enabled). Otherwise we return to the caller to
// allow them to reconfigure first.
if (!aRetryAfterFallback) {
- return NS_ERROR_ABORT;
+ return NS_ERROR_NOT_AVAILABLE;
}
}
- MOZ_DIAGNOSTIC_ASSERT(!gfxConfig::IsEnabled(Feature::GPU_PROCESS));
-
// This is the first time we are trying to use the in-process compositor.
if (mTotalProcessAttempts == 0) {
if (NS_WARN_IF(inShutdown)) {
@@ -460,36 +435,34 @@ nsresult GPUProcessManager::EnsureGPUReady(
}
ResetProcessStable();
}
- return NS_OK;
+ return NS_ERROR_FAILURE;
}
bool GPUProcessManager::EnsureProtocolsReady() {
- nsresult rv = EnsureGPUReady();
- if (NS_WARN_IF(NS_FAILED(rv))) {
- return false;
- }
-
return EnsureCompositorManagerChild() && EnsureImageBridgeChild() &&
EnsureVRManager();
}
bool GPUProcessManager::EnsureCompositorManagerChild() {
- MOZ_ASSERT(IsGPUReady());
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return false;
+ }
if (CompositorManagerChild::IsInitialized(mProcessToken)) {
return true;
}
- if (!mGPUChild) {
+ if (NS_FAILED(rv)) {
CompositorManagerChild::InitSameProcess(AllocateNamespace(), mProcessToken);
return true;
}
ipc::Endpoint<PCompositorManagerParent> parentPipe;
ipc::Endpoint<PCompositorManagerChild> childPipe;
- nsresult rv = PCompositorManager::CreateEndpoints(
- mGPUChild->OtherEndpointProcInfo(), ipc::EndpointProcInfo::Current(),
- &parentPipe, &childPipe);
+ rv = PCompositorManager::CreateEndpoints(mGPUChild->OtherEndpointProcInfo(),
+ ipc::EndpointProcInfo::Current(),
+ &parentPipe, &childPipe);
if (NS_FAILED(rv)) {
DisableGPUProcess("Failed to create PCompositorManager endpoints");
return true;
@@ -503,22 +476,25 @@ bool GPUProcessManager::EnsureCompositorManagerChild() {
}
bool GPUProcessManager::EnsureImageBridgeChild() {
- MOZ_ASSERT(IsGPUReady());
-
if (ImageBridgeChild::GetSingleton()) {
return true;
}
- if (!mGPUChild) {
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return false;
+ }
+
+ if (NS_FAILED(rv)) {
ImageBridgeChild::InitSameProcess(AllocateNamespace());
return true;
}
ipc::Endpoint<PImageBridgeParent> parentPipe;
ipc::Endpoint<PImageBridgeChild> childPipe;
- nsresult rv = PImageBridge::CreateEndpoints(
- mGPUChild->OtherEndpointProcInfo(), ipc::EndpointProcInfo::Current(),
- &parentPipe, &childPipe);
+ rv = PImageBridge::CreateEndpoints(mGPUChild->OtherEndpointProcInfo(),
+ ipc::EndpointProcInfo::Current(),
+ &parentPipe, &childPipe);
if (NS_FAILED(rv)) {
DisableGPUProcess("Failed to create PImageBridge endpoints");
return true;
@@ -531,22 +507,25 @@ bool GPUProcessManager::EnsureImageBridgeChild() {
}
bool GPUProcessManager::EnsureVRManager() {
- MOZ_ASSERT(IsGPUReady());
-
if (VRManagerChild::IsCreated()) {
return true;
}
- if (!mGPUChild) {
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return false;
+ }
+
+ if (NS_FAILED(rv)) {
VRManagerChild::InitSameProcess();
return true;
}
ipc::Endpoint<PVRManagerParent> parentPipe;
ipc::Endpoint<PVRManagerChild> childPipe;
- nsresult rv = PVRManager::CreateEndpoints(mGPUChild->OtherEndpointProcInfo(),
- ipc::EndpointProcInfo::Current(),
- &parentPipe, &childPipe);
+ rv = PVRManager::CreateEndpoints(mGPUChild->OtherEndpointProcInfo(),
+ ipc::EndpointProcInfo::Current(),
+ &parentPipe, &childPipe);
if (NS_FAILED(rv)) {
DisableGPUProcess("Failed to create PVRManager endpoints");
return true;
@@ -558,35 +537,39 @@ bool GPUProcessManager::EnsureVRManager() {
}
#if defined(MOZ_WIDGET_ANDROID)
-RefPtr<UiCompositorControllerChild>
+already_AddRefed<UiCompositorControllerChild>
GPUProcessManager::CreateUiCompositorController(nsBaseWidget* aWidget,
const LayersId aId) {
- MOZ_ASSERT(IsGPUReady());
+ RefPtr<UiCompositorControllerChild> result;
- if (!mGPUChild) {
- return UiCompositorControllerChild::CreateForSameProcess(aId, aWidget);
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return nullptr;
}
- ipc::Endpoint<PUiCompositorControllerParent> parentPipe;
- ipc::Endpoint<PUiCompositorControllerChild> childPipe;
- nsresult rv = PUiCompositorController::CreateEndpoints(
- mGPUChild->OtherEndpointProcInfo(), ipc::EndpointProcInfo::Current(),
- &parentPipe, &childPipe);
if (NS_FAILED(rv)) {
- DisableGPUProcess("Failed to create PUiCompositorController endpoints");
- return nullptr;
- }
+ result = UiCompositorControllerChild::CreateForSameProcess(aId, aWidget);
+ } else {
+ ipc::Endpoint<PUiCompositorControllerParent> parentPipe;
+ ipc::Endpoint<PUiCompositorControllerChild> childPipe;
+ rv = PUiCompositorController::CreateEndpoints(
+ mGPUChild->OtherEndpointProcInfo(), ipc::EndpointProcInfo::Current(),
+ &parentPipe, &childPipe);
+ if (NS_FAILED(rv)) {
+ DisableGPUProcess("Failed to create PUiCompositorController endpoints");
+ return nullptr;
+ }
- mGPUChild->SendInitUiCompositorController(aId, std::move(parentPipe));
- RefPtr<UiCompositorControllerChild> result =
- UiCompositorControllerChild::CreateForGPUProcess(
- mProcessToken, std::move(childPipe), aWidget);
+ mGPUChild->SendInitUiCompositorController(aId, std::move(parentPipe));
+ result = UiCompositorControllerChild::CreateForGPUProcess(
+ mProcessToken, std::move(childPipe), aWidget);
- if (result) {
- result->SetCompositorSurfaceManager(
- mProcess->GetCompositorSurfaceManager());
+ if (result) {
+ result->SetCompositorSurfaceManager(
+ mProcess->GetCompositorSurfaceManager());
+ }
}
- return result;
+ return result.forget();
}
#endif // defined(MOZ_WIDGET_ANDROID)
@@ -1233,19 +1216,18 @@ already_AddRefed<CompositorSession> GPUProcessManager::CreateTopLevelCompositor(
RefPtr<CompositorSession> session;
nsresult rv = EnsureGPUReady(/* aRetryAfterFallback */ false);
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ *aRetryOut = false;
+ return nullptr;
+ }
// If we used fallback, then retry creating the compositor sessions because
// our configuration may have changed.
- if (rv == NS_ERROR_ABORT) {
+ if (rv == NS_ERROR_NOT_AVAILABLE) {
*aRetryOut = true;
return nullptr;
}
- if (NS_WARN_IF(NS_FAILED(rv))) {
- *aRetryOut = false;
- return nullptr;
- }
-
if (!EnsureProtocolsReady()) {
*aRetryOut = false;
return nullptr;
@@ -1276,10 +1258,10 @@ already_AddRefed<CompositorSession> GPUProcessManager::CreateTopLevelCompositor(
#if defined(MOZ_WIDGET_ANDROID)
if (session) {
// Nothing to do if controller gets a nullptr
- auto controller =
+ RefPtr<UiCompositorControllerChild> controller =
CreateUiCompositorController(aWidget, session->RootLayerTreeId());
MOZ_ASSERT(controller);
- session->SetUiCompositorControllerChild(std::move(controller));
+ session->SetUiCompositorControllerChild(controller);
}
#endif // defined(MOZ_WIDGET_ANDROID)
@@ -1365,11 +1347,6 @@ bool GPUProcessManager::CreateContentBridges(
ipc::Endpoint<PVRManagerChild>* aOutVRBridge,
ipc::Endpoint<PRemoteMediaManagerChild>* aOutVideoManager,
dom::ContentParentId aChildId, nsTArray<uint32_t>* aNamespaces) {
- nsresult rv = EnsureGPUReady();
- if (NS_WARN_IF(NS_FAILED(rv))) {
- return false;
- }
-
const uint32_t cmNamespace = AllocateNamespace();
if (!CreateContentCompositorManager(aOtherProcess, aChildId, cmNamespace,
aOutCompositor) ||
@@ -1391,17 +1368,20 @@ bool GPUProcessManager::CreateContentBridges(
bool GPUProcessManager::CreateContentCompositorManager(
ipc::EndpointProcInfo aOtherProcess, dom::ContentParentId aChildId,
uint32_t aNamespace, ipc::Endpoint<PCompositorManagerChild>* aOutEndpoint) {
- MOZ_ASSERT(IsGPUReady());
-
ipc::Endpoint<PCompositorManagerParent> parentPipe;
ipc::Endpoint<PCompositorManagerChild> childPipe;
- ipc::EndpointProcInfo parentInfo = mGPUChild
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return false;
+ }
+
+ ipc::EndpointProcInfo parentInfo = NS_SUCCEEDED(rv)
? mGPUChild->OtherEndpointProcInfo()
: ipc::EndpointProcInfo::Current();
- nsresult rv = PCompositorManager::CreateEndpoints(parentInfo, aOtherProcess,
- &parentPipe, &childPipe);
+ rv = PCompositorManager::CreateEndpoints(parentInfo, aOtherProcess,
+ &parentPipe, &childPipe);
if (NS_FAILED(rv)) {
gfxCriticalNote << "Could not create content compositor manager: "
<< hexa(int(rv));
@@ -1424,20 +1404,23 @@ bool GPUProcessManager::CreateContentCompositorManager(
bool GPUProcessManager::CreateContentImageBridge(
ipc::EndpointProcInfo aOtherProcess, dom::ContentParentId aChildId,
ipc::Endpoint<PImageBridgeChild>* aOutEndpoint) {
- MOZ_ASSERT(IsGPUReady());
-
if (!EnsureImageBridgeChild()) {
return false;
}
- ipc::EndpointProcInfo parentInfo = mGPUChild
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return false;
+ }
+
+ ipc::EndpointProcInfo parentInfo = NS_SUCCEEDED(rv)
? mGPUChild->OtherEndpointProcInfo()
: ipc::EndpointProcInfo::Current();
ipc::Endpoint<PImageBridgeParent> parentPipe;
ipc::Endpoint<PImageBridgeChild> childPipe;
- nsresult rv = PImageBridge::CreateEndpoints(parentInfo, aOtherProcess,
- &parentPipe, &childPipe);
+ rv = PImageBridge::CreateEndpoints(parentInfo, aOtherProcess, &parentPipe,
+ &childPipe);
if (NS_FAILED(rv)) {
gfxCriticalNote << "Could not create content compositor bridge: "
<< hexa(int(rv));
@@ -1470,20 +1453,23 @@ ipc::EndpointProcInfo GPUProcessManager::GPUEndpointProcInfo() {
bool GPUProcessManager::CreateContentVRManager(
ipc::EndpointProcInfo aOtherProcess, dom::ContentParentId aChildId,
ipc::Endpoint<PVRManagerChild>* aOutEndpoint) {
- MOZ_ASSERT(IsGPUReady());
-
if (NS_WARN_IF(!EnsureVRManager())) {
return false;
}
- ipc::EndpointProcInfo parentInfo = mGPUChild
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return false;
+ }
+
+ ipc::EndpointProcInfo parentInfo = NS_SUCCEEDED(rv)
? mGPUChild->OtherEndpointProcInfo()
: ipc::EndpointProcInfo::Current();
ipc::Endpoint<PVRManagerParent> parentPipe;
ipc::Endpoint<PVRManagerChild> childPipe;
- nsresult rv = PVRManager::CreateEndpoints(parentInfo, aOtherProcess,
- &parentPipe, &childPipe);
+ rv = PVRManager::CreateEndpoints(parentInfo, aOtherProcess, &parentPipe,
+ &childPipe);
if (NS_FAILED(rv)) {
gfxCriticalNote << "Could not create content compositor bridge: "
<< hexa(int(rv));
@@ -1505,9 +1491,12 @@ bool GPUProcessManager::CreateContentVRManager(
void GPUProcessManager::CreateContentRemoteMediaManager(
ipc::EndpointProcInfo aOtherProcess, dom::ContentParentId aChildId,
ipc::Endpoint<PRemoteMediaManagerChild>* aOutEndpoint) {
- MOZ_ASSERT(IsGPUReady());
+ nsresult rv = EnsureGPUReady();
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
+ return;
+ }
- if (!mGPUChild || !StaticPrefs::media_gpu_process_decoder() ||
+ if (NS_FAILED(rv) || !StaticPrefs::media_gpu_process_decoder() ||
!mDecodeVideoOnGpuProcess) {
return;
}
@@ -1515,9 +1504,9 @@ void GPUProcessManager::CreateContentRemoteMediaManager(
ipc::Endpoint<PRemoteMediaManagerParent> parentPipe;
ipc::Endpoint<PRemoteMediaManagerChild> childPipe;
- nsresult rv = PRemoteMediaManager::CreateEndpoints(
- mGPUChild->OtherEndpointProcInfo(), aOtherProcess, &parentPipe,
- &childPipe);
+ rv = PRemoteMediaManager::CreateEndpoints(mGPUChild->OtherEndpointProcInfo(),
+ aOtherProcess, &parentPipe,
+ &childPipe);
if (NS_FAILED(rv)) {
gfxCriticalNote << "Could not create content video decoder: "
<< hexa(int(rv));
@@ -1533,7 +1522,7 @@ void GPUProcessManager::InitVideoBridge(
ipc::Endpoint<PVideoBridgeParent>&& aVideoBridge,
layers::VideoBridgeSource aSource) {
nsresult rv = EnsureGPUReady();
- if (NS_WARN_IF(NS_FAILED(rv))) {
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
return;
}
@@ -1545,7 +1534,7 @@ void GPUProcessManager::InitVideoBridge(
void GPUProcessManager::MapLayerTreeId(LayersId aLayersId,
base::ProcessId aOwningId) {
nsresult rv = EnsureGPUReady();
- if (NS_WARN_IF(NS_FAILED(rv))) {
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
return;
}
diff --git a/gfx/ipc/GPUProcessManager.h b/gfx/ipc/GPUProcessManager.h
@@ -97,15 +97,6 @@ class GPUProcessManager final : public GPUProcessHost::Listener {
// If the GPU process is enabled but has not yet been launched then this will
// launch the process. If that is not desired then check that return value of
// Process() is non-null before calling.
- //
- // Returns:
- // - NS_OK if compositing is ready, in either the GPU process or the parent
- // process, even if in shutdown.
- // - NS_ERROR_ILLEGAL_DURING_SHUTDOWN if compositing is not ready, and we are
- // in shutdown.
- // - NS_ERROR_ABORT if compositing is not ready, we failed to make it ready
- // under the previous configuration, and that the configuration may have
- // changed. This is only returned when aRetryAfterFallback is false.
nsresult EnsureGPUReady(bool aRetryAfterFallback = true);
already_AddRefed<CompositorSession> CreateTopLevelCompositor(
@@ -240,8 +231,6 @@ class GPUProcessManager final : public GPUProcessHost::Listener {
void OnPreferenceChange(const char16_t* aData);
void ScreenInformationChanged();
- bool IsGPUReady() const;
-
bool CreateContentCompositorManager(
mozilla::ipc::EndpointProcInfo aOtherProcess,
dom::ContentParentId aChildId, uint32_t aNamespace,
@@ -330,7 +319,7 @@ class GPUProcessManager final : public GPUProcessHost::Listener {
#endif
#if defined(MOZ_WIDGET_ANDROID)
- RefPtr<UiCompositorControllerChild> CreateUiCompositorController(
+ already_AddRefed<UiCompositorControllerChild> CreateUiCompositorController(
nsBaseWidget* aWidget, const LayersId aId);
#endif // defined(MOZ_WIDGET_ANDROID)
diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp
@@ -4157,7 +4157,7 @@ void gfxPlatform::BuildContentDeviceData(
// Make sure our settings are synchronized from the GPU process.
DebugOnly<nsresult> rv = GPUProcessManager::Get()->EnsureGPUReady();
- MOZ_ASSERT(NS_SUCCEEDED(rv));
+ MOZ_ASSERT(rv != NS_ERROR_ILLEGAL_DURING_SHUTDOWN);
aOut->prefs().hwCompositing() = gfxConfig::GetValue(Feature::HW_COMPOSITING);
aOut->prefs().oglCompositing() =
diff --git a/widget/GfxInfoBase.cpp b/widget/GfxInfoBase.cpp
@@ -2009,7 +2009,7 @@ GfxInfoBase::ControlGPUProcessForXPCShell(bool aEnable, bool* _retval) {
gfxConfig::UserForceEnable(gfx::Feature::GPU_PROCESS, "xpcshell-test");
}
DebugOnly<nsresult> rv = gpm->EnsureGPUReady();
- MOZ_ASSERT(NS_SUCCEEDED(rv));
+ MOZ_ASSERT(rv != NS_ERROR_ILLEGAL_DURING_SHUTDOWN);
} else {
gfxConfig::UserDisable(gfx::Feature::GPU_PROCESS, "xpcshell-test");
gpm->KillProcess();
diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp
@@ -1464,7 +1464,7 @@ already_AddRefed<WebRenderLayerManager> nsBaseWidget::CreateCompositorSession(
// If it failed to connect to GPU process, GPU process usage is disabled in
// EnsureGPUReady(). It could update gfxVars and gfxConfigs.
nsresult rv = gpu->EnsureGPUReady();
- if (NS_WARN_IF(NS_FAILED(rv))) {
+ if (NS_WARN_IF(rv == NS_ERROR_ILLEGAL_DURING_SHUTDOWN)) {
return nullptr;
}
