commit 5331201d178bcbb04c1cd64b2d12f4345c0be0b2
parent b128b27984c9e088464a33b3fc5791ce48528a86
Author: Narcis Beleuzu <nbeleuzu@mozilla.com>
Date: Mon, 17 Nov 2025 12:16:39 +0200
Revert "Bug 1960922 - Obtain reference to CompositorSurfaceManager asynchronously. r=aosmond" for causing Bug 1993818
This reverts commit 3ef34f13f0ae4db66f96da5f2d24a4a36a8f66ef.
This reverts commit 4d93b3cafc3ada6411954fdf218fffeb5ace9663.
This reverts commit 7ee10d8ccfdef59729eac166d6f04f291e8c77f8.
Diffstat:
9 files changed, 79 insertions(+), 151 deletions(-)
diff --git a/gfx/ipc/GPUChild.cpp b/gfx/ipc/GPUChild.cpp
@@ -46,7 +46,7 @@ GPUChild::GPUChild(GPUProcessHost* aHost) : mHost(aHost), mGPUReady(false) {}
GPUChild::~GPUChild() = default;
-RefPtr<GPUChild::InitPromiseType> GPUChild::Init() {
+void GPUChild::Init() {
nsTArray<GfxVarUpdate> updates = gfxVars::FetchNonDefaultVars();
DevicePrefs devicePrefs;
@@ -70,24 +70,12 @@ RefPtr<GPUChild::InitPromiseType> GPUChild::Init() {
features = gfxInfoRaw->GetAllFeatures();
}
- RefPtr<InitPromiseType> promise =
- SendInit(updates, devicePrefs, mappings, features,
- GPUProcessManager::Get()->AllocateNamespace())
- ->Then(
- GetCurrentSerialEventTarget(), __func__,
- [self = RefPtr{this}](GPUDeviceData&& aData) {
- self->OnInitComplete(aData);
- return InitPromiseType::CreateAndResolve(Ok{}, __func__);
- },
- [](ipc::ResponseRejectReason) {
- return InitPromiseType::CreateAndReject(Ok{}, __func__);
- });
+ SendInit(updates, devicePrefs, mappings, features,
+ GPUProcessManager::Get()->AllocateNamespace());
gfxVars::AddReceiver(this);
(void)SendInitProfiler(ProfilerParent::CreateForProcess(OtherPid()));
-
- return promise;
}
void GPUChild::OnVarChanged(const nsTArray<GfxVarUpdate>& aVar) {
@@ -108,7 +96,14 @@ bool GPUChild::EnsureGPUReady(bool aForceSync /* = false */) {
return false;
}
- OnInitComplete(data);
+ // Only import and collect telemetry for the initial GPU process launch.
+ if (!mGPUReady) {
+ gfxPlatform::GetPlatform()->ImportGPUDeviceData(data);
+ glean::gpu_process::launch_time.AccumulateRawDuration(
+ TimeStamp::Now() - mHost->GetLaunchTime());
+ mGPUReady = true;
+ }
+
return true;
}
@@ -141,20 +136,17 @@ void GPUChild::DeletePairedMinidump() {
}
}
-void GPUChild::OnInitComplete(const GPUDeviceData& aData) {
- // This function may be called multiple times, for example if we synchronously
- // requested GPU parameters before the asynchronous SendInit completed, or if
- // EnsureGPUReady is called after launch to force a synchronization after a
- // configuration change. We only want to import device data and collect
- // telemetry for the initial GPU process launch.
+mozilla::ipc::IPCResult GPUChild::RecvInitComplete(const GPUDeviceData& aData) {
+ // We synchronously requested GPU parameters before this arrived.
if (mGPUReady) {
- return;
+ return IPC_OK();
}
gfxPlatform::GetPlatform()->ImportGPUDeviceData(aData);
glean::gpu_process::launch_time.AccumulateRawDuration(TimeStamp::Now() -
mHost->GetLaunchTime());
mGPUReady = true;
+ return IPC_OK();
}
mozilla::ipc::IPCResult GPUChild::RecvDeclareStable() {
diff --git a/gfx/ipc/GPUChild.h b/gfx/ipc/GPUChild.h
@@ -33,8 +33,7 @@ class GPUChild final : public mozilla::ipc::CrashReporterHelper<GPUChild>,
explicit GPUChild(GPUProcessHost* aHost);
- using InitPromiseType = MozPromise<Ok, Ok, true>;
- RefPtr<InitPromiseType> Init();
+ void Init();
bool IsGPUReady() const { return mGPUReady; }
@@ -59,6 +58,7 @@ class GPUChild final : public mozilla::ipc::CrashReporterHelper<GPUChild>,
void OnVarChanged(const nsTArray<GfxVarUpdate>& aVar) override;
// PGPUChild overrides.
+ mozilla::ipc::IPCResult RecvInitComplete(const GPUDeviceData& aData);
mozilla::ipc::IPCResult RecvDeclareStable();
mozilla::ipc::IPCResult RecvReportCheckerboard(const uint32_t& aSeverity,
const nsCString& aLog);
@@ -108,8 +108,6 @@ class GPUChild final : public mozilla::ipc::CrashReporterHelper<GPUChild>,
private:
virtual ~GPUChild();
- void OnInitComplete(const GPUDeviceData& aData);
-
GPUProcessHost* mHost;
UniquePtr<MemoryReportRequestHost> mMemoryReportRequest;
bool mGPUReady;
diff --git a/gfx/ipc/GPUParent.cpp b/gfx/ipc/GPUParent.cpp
@@ -275,8 +275,7 @@ void GPUParent::NotifyDisableRemoteCanvas() {
mozilla::ipc::IPCResult GPUParent::RecvInit(
nsTArray<GfxVarUpdate>&& vars, const DevicePrefs& devicePrefs,
nsTArray<LayerTreeIdMapping>&& aMappings,
- nsTArray<GfxInfoFeatureStatus>&& aFeatures, uint32_t aWrNamespace,
- InitResolver&& aInitResolver) {
+ nsTArray<GfxInfoFeatureStatus>&& aFeatures, uint32_t aWrNamespace) {
gfxVars::ApplyUpdate(vars);
// Inherit device preferences.
@@ -397,7 +396,7 @@ mozilla::ipc::IPCResult GPUParent::RecvInit(
// Send a message to the UI process that we're done.
GPUDeviceData data;
RecvGetDeviceStatus(&data);
- aInitResolver(data);
+ (void)SendInitComplete(data);
// Dispatch a task to background thread to determine the media codec supported
// result, and propagate it back to the chrome process on the main thread.
diff --git a/gfx/ipc/GPUParent.h b/gfx/ipc/GPUParent.h
@@ -52,8 +52,7 @@ class GPUParent final : public PGPUParent {
const DevicePrefs& devicePrefs,
nsTArray<LayerTreeIdMapping>&& mappings,
nsTArray<GfxInfoFeatureStatus>&& features,
- uint32_t wrNamespace,
- InitResolver&& aInitResolver);
+ uint32_t wrNamespace);
mozilla::ipc::IPCResult RecvInitCompositorManager(
Endpoint<PCompositorManagerParent>&& aEndpoint, uint32_t aNamespace);
mozilla::ipc::IPCResult RecvInitVsyncBridge(
diff --git a/gfx/ipc/GPUProcessHost.cpp b/gfx/ipc/GPUProcessHost.cpp
@@ -34,11 +34,11 @@ using namespace ipc;
GPUProcessHost::GPUProcessHost(Listener* aListener)
: GeckoChildProcessHost(GeckoProcessType_GPU),
mListener(aListener),
+ mTaskFactory(this),
mLaunchPhase(LaunchPhase::Unlaunched),
mProcessToken(0),
mShutdownRequested(false),
- mChannelClosed(false),
- mLiveToken(new media::Refcountable<bool>(true)) {
+ mChannelClosed(false) {
MOZ_COUNT_CTOR(GPUProcessHost);
#if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
@@ -79,7 +79,6 @@ bool GPUProcessHost::Launch(geckoargs::ChildProcessArgs aExtraOpts) {
}
bool GPUProcessHost::WaitForLaunch() {
- MOZ_ASSERT(mLaunchPhase != LaunchPhase::Unlaunched);
if (mLaunchPhase == LaunchPhase::Complete) {
return !!mGPUChild;
}
@@ -95,22 +94,12 @@ bool GPUProcessHost::WaitForLaunch() {
timeoutMs = 0;
}
- if (mLaunchPhase == LaunchPhase::Waiting) {
- // Our caller expects the connection to be finished by the time we return,
- // so we immediately set up the IPDL actor and fire callbacks. The IO thread
- // will still dispatch a notification to the main thread - we'll just ignore
- // it.
- bool result = GeckoChildProcessHost::WaitUntilConnected(timeoutMs);
- InitAfterConnect(result);
- if (!result) {
- return false;
- }
- }
- MOZ_ASSERT(mLaunchPhase == LaunchPhase::Connected);
- // Our caller expects post-connection initialization tasks, such as ensuring
- // the GPUChild is initialized, to be finished by the time we return, so
- // finish these tasks synchronously now.
- return CompleteInitSynchronously();
+ // Our caller expects the connection to be finished after we return, so we
+ // immediately set up the IPDL actor and fire callbacks. The IO thread will
+ // still dispatch a notification to the main thread - we'll just ignore it.
+ bool result = GeckoChildProcessHost::WaitUntilConnected(timeoutMs);
+ InitAfterConnect(result);
+ return result;
}
void GPUProcessHost::OnChannelConnected(base::ProcessId peer_pid) {
@@ -118,12 +107,27 @@ void GPUProcessHost::OnChannelConnected(base::ProcessId peer_pid) {
GeckoChildProcessHost::OnChannelConnected(peer_pid);
- NS_DispatchToMainThread(NS_NewRunnableFunction(
- "GPUProcessHost::OnChannelConnected", [this, liveToken = mLiveToken]() {
- if (*mLiveToken && mLaunchPhase == LaunchPhase::Waiting) {
- InitAfterConnect(true);
- }
- }));
+ // Post a task to the main thread. Take the lock because mTaskFactory is not
+ // thread-safe.
+ RefPtr<Runnable> runnable;
+ {
+ MonitorAutoLock lock(mMonitor);
+ runnable =
+ mTaskFactory.NewRunnableMethod(&GPUProcessHost::OnChannelConnectedTask);
+ }
+ NS_DispatchToMainThread(runnable);
+}
+
+void GPUProcessHost::OnChannelConnectedTask() {
+ if (mLaunchPhase == LaunchPhase::Waiting) {
+ InitAfterConnect(true);
+ }
+}
+
+void GPUProcessHost::OnChannelErrorTask() {
+ if (mLaunchPhase == LaunchPhase::Waiting) {
+ InitAfterConnect(false);
+ }
}
static uint64_t sProcessTokenCounter = 0;
@@ -132,78 +136,23 @@ void GPUProcessHost::InitAfterConnect(bool aSucceeded) {
MOZ_ASSERT(mLaunchPhase == LaunchPhase::Waiting);
MOZ_ASSERT(!mGPUChild);
+ mLaunchPhase = LaunchPhase::Complete;
mPrefSerializer = nullptr;
if (aSucceeded) {
- mLaunchPhase = LaunchPhase::Connected;
mProcessToken = ++sProcessTokenCounter;
mGPUChild = MakeRefPtr<GPUChild>(this);
DebugOnly<bool> rv = TakeInitialEndpoint().Bind(mGPUChild.get());
MOZ_ASSERT(rv);
- nsTArray<RefPtr<GPUChild::InitPromiseType>> initPromises;
- initPromises.AppendElement(mGPUChild->Init());
+ mGPUChild->Init();
#ifdef MOZ_WIDGET_ANDROID
- nsCOMPtr<nsISerialEventTarget> launcherThread(GetIPCLauncher());
+ nsCOMPtr<nsIEventTarget> launcherThread(GetIPCLauncher());
MOZ_ASSERT(launcherThread);
- RefPtr<GPUChild::InitPromiseType> csmPromise =
- InvokeAsync(
- launcherThread, __func__,
- [] {
- java::CompositorSurfaceManager::LocalRef csm =
- java::GeckoProcessManager::GetCompositorSurfaceManager();
- return MozPromise<java::CompositorSurfaceManager::GlobalRef, Ok,
- true>::CreateAndResolve(csm, __func__);
- })
- ->Map(GetCurrentSerialEventTarget(), __func__,
- [this, liveToken = mLiveToken](
- java::CompositorSurfaceManager::GlobalRef&& aCsm) {
- if (*liveToken) {
- mCompositorSurfaceManager = aCsm;
- }
- return Ok{};
- });
- initPromises.AppendElement(csmPromise);
-#endif
-
- GPUChild::InitPromiseType::All(GetCurrentSerialEventTarget(), initPromises)
- ->Then(GetCurrentSerialEventTarget(), __func__,
- [this, liveToken = mLiveToken]() {
- if (*liveToken) {
- this->OnAsyncInitComplete();
- }
- });
- } else {
- mLaunchPhase = LaunchPhase::Complete;
- if (mListener) {
- mListener->OnProcessLaunchComplete(this);
- }
- }
-}
-
-void GPUProcessHost::OnAsyncInitComplete() {
- MOZ_ASSERT(NS_IsMainThread());
- if (mLaunchPhase == LaunchPhase::Connected) {
- mLaunchPhase = LaunchPhase::Complete;
- if (mListener) {
- mListener->OnProcessLaunchComplete(this);
- }
- }
-}
-
-bool GPUProcessHost::CompleteInitSynchronously() {
- MOZ_ASSERT(mLaunchPhase == LaunchPhase::Connected);
-
- const bool result = mGPUChild->EnsureGPUReady();
-
-#ifdef MOZ_WIDGET_ANDROID
- if (!mCompositorSurfaceManager) {
layers::SynchronousTask task(
"GeckoProcessManager::GetCompositorSurfaceManager");
- nsCOMPtr<nsIEventTarget> launcherThread(GetIPCLauncher());
- MOZ_ASSERT(launcherThread);
launcherThread->Dispatch(NS_NewRunnableFunction(
"GeckoProcessManager::GetCompositorSurfaceManager", [&]() {
layers::AutoCompleteTask complete(&task);
@@ -212,15 +161,12 @@ bool GPUProcessHost::CompleteInitSynchronously() {
}));
task.Wait();
- }
#endif
+ }
- mLaunchPhase = LaunchPhase::Complete;
if (mListener) {
mListener->OnProcessLaunchComplete(this);
}
-
- return result;
}
void GPUProcessHost::Shutdown(bool aUnexpectedShutdown) {
@@ -306,12 +252,14 @@ void GPUProcessHost::KillProcess(bool aGenerateMinidump) {
void GPUProcessHost::CrashProcess() { mGPUChild->SendCrashProcess(); }
void GPUProcessHost::DestroyProcess() {
- MOZ_ASSERT(NS_IsMainThread());
-
- // Any pending tasks will be cancelled from now on.
- *mLiveToken = false;
+ // Cancel all tasks. We don't want anything triggering after our caller
+ // expects this to go away.
+ {
+ MonitorAutoLock lock(mMonitor);
+ mTaskFactory.RevokeAll();
+ }
- NS_DispatchToMainThread(
+ GetCurrentSerialEventTarget()->Dispatch(
NS_NewRunnableFunction("DestroyProcessRunnable", [this] { Destroy(); }));
}
diff --git a/gfx/ipc/GPUProcessHost.h b/gfx/ipc/GPUProcessHost.h
@@ -11,7 +11,7 @@
#include "mozilla/gfx/Types.h"
#include "mozilla/ipc/GeckoChildProcessHost.h"
#include "mozilla/ipc/ProtocolUtils.h"
-#include "mozilla/media/MediaUtils.h"
+#include "mozilla/ipc/TaskFactory.h"
#ifdef MOZ_WIDGET_ANDROID
# include "mozilla/java/CompositorSurfaceManagerWrappers.h"
@@ -60,19 +60,17 @@ class GPUProcessHost final : public mozilla::ipc::GeckoChildProcessHost {
// Launch the subprocess asynchronously. On failure, false is returned.
// Otherwise, true is returned, and the OnProcessLaunchComplete listener
- // callback will be invoked either when a connection has been established and
- // process initialization is complete, or if a connection could not be
- // established due to an asynchronous error.
+ // callback will be invoked either when a connection has been established, or
+ // if a connection could not be established due to an asynchronous error.
//
// @param aExtraOpts (geckoargs::ChildProcessArgs)
// Extra options to pass to the subprocess.
bool Launch(geckoargs::ChildProcessArgs aExtraOpts);
// If the process is being launched, block until it has launched and
- // connected, and any initialization has completed. If a launch task is
- // pending, it will fire immediately.
+ // connected. If a launch task is pending, it will fire immediately.
//
- // Returns true if the process is successfully initialized; false otherwise.
+ // Returns true if the process is successfully connected; false otherwise.
bool WaitForLaunch();
// Inform the process that it should clean up its resources and shut down.
@@ -126,15 +124,12 @@ class GPUProcessHost final : public mozilla::ipc::GeckoChildProcessHost {
private:
~GPUProcessHost();
+ // Called on the main thread.
+ void OnChannelConnectedTask();
+ void OnChannelErrorTask();
+
// Called on the main thread after a connection has been established.
- // Creates the PGPU endpoints and begins asynchronous initialization.
void InitAfterConnect(bool aSucceeded);
- // Called on the main thread after post-connection initialization tasks have
- // completed asynchronously.
- void OnAsyncInitComplete();
- // Synchronously completes any outstanding post-connection initialization
- // tasks which have not yet completed asynchronously.
- bool CompleteInitSynchronously();
// Called on the main thread when the mGPUChild actor is shutting down.
void OnChannelClosed();
@@ -155,8 +150,9 @@ class GPUProcessHost final : public mozilla::ipc::GeckoChildProcessHost {
DISALLOW_COPY_AND_ASSIGN(GPUProcessHost);
Listener* mListener;
+ mozilla::ipc::TaskFactory<GPUProcessHost> mTaskFactory;
- enum class LaunchPhase { Unlaunched, Waiting, Connected, Complete };
+ enum class LaunchPhase { Unlaunched, Waiting, Complete };
LaunchPhase mLaunchPhase;
RefPtr<GPUChild> mGPUChild;
@@ -169,14 +165,6 @@ class GPUProcessHost final : public mozilla::ipc::GeckoChildProcessHost {
TimeStamp mLaunchTime;
- // Set to true on construction and to false just prior deletion.
- // The GPUProcessHost isn't refcounted; so we can capture this by value in
- // lambdas along with a strong reference to mLiveToken and check if that value
- // is true before accessing "this".
- // While a reference to mLiveToken can be taken on any thread; its value can
- // only be read on the main thread.
- const RefPtr<media::Refcountable<bool>> mLiveToken;
-
#ifdef MOZ_WIDGET_ANDROID
// Binder interface used to send compositor surfaces to GPU process. There is
// one instance per GPU process which gets initialized after launch, then
diff --git a/gfx/ipc/PGPU.ipdl b/gfx/ipc/PGPU.ipdl
@@ -66,7 +66,7 @@ parent:
DevicePrefs devicePrefs,
LayerTreeIdMapping[] mapping,
GfxInfoFeatureStatus[] features,
- uint32_t wrNamespace) returns (GPUDeviceData data);
+ uint32_t wrNamespace);
async InitCompositorManager(Endpoint<PCompositorManagerParent> endpoint, uint32_t aNamespace);
async InitVsyncBridge(Endpoint<PVsyncBridgeParent> endpoint);
@@ -138,6 +138,10 @@ parent:
async CrashProcess();
child:
+ // Sent when the GPU process has initialized devices. This occurs once, after
+ // Init().
+ async InitComplete(GPUDeviceData data);
+
// Sent when APZ detects checkerboarding and apz checkerboard reporting is enabled.
async ReportCheckerboard(uint32_t severity, nsCString log);
diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp
@@ -994,7 +994,7 @@ IPCLaunchThreadObserver::Observe(nsISupports* aSubject, const char* aTopic,
return rv;
}
-nsCOMPtr<nsISerialEventTarget> GetIPCLauncher() {
+nsCOMPtr<nsIEventTarget> GetIPCLauncher() {
StaticMutexAutoLock lock(gIPCLaunchThreadMutex);
if (!gIPCLaunchThread) {
nsCOMPtr<nsIThread> thread;
@@ -1011,7 +1011,7 @@ nsCOMPtr<nsISerialEventTarget> GetIPCLauncher() {
}
}
- nsCOMPtr<nsISerialEventTarget> thread = gIPCLaunchThread.get();
+ nsCOMPtr<nsIEventTarget> thread = gIPCLaunchThread.get();
MOZ_DIAGNOSTIC_ASSERT(thread);
return thread;
}
diff --git a/ipc/glue/GeckoChildProcessHost.h b/ipc/glue/GeckoChildProcessHost.h
@@ -315,7 +315,7 @@ class GeckoChildProcessHost : public SupportsWeakPtr,
static StaticMutex sMutex;
};
-nsCOMPtr<nsISerialEventTarget> GetIPCLauncher();
+nsCOMPtr<nsIEventTarget> GetIPCLauncher();
} /* namespace ipc */
} /* namespace mozilla */
