tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 01601ac352f1bde45f88bfd4f2b7d9363135b9ab
parent 76bd5afb87424685b79333cd79d4e77731e717f5
Author: Jamie Nicol <jnicol@mozilla.com>
Date:   Tue, 18 Nov 2025 18:00:35 +0000

Bug 1999645 - Avoid synchronous GPUChild::SendGetDeviceStatus after GPU process channel is connected asynchronously. r=aosmond

In the case where the GPU process channel is connected asynchronously
we unfortunately send a synchronous GPUChild::SendGetDeviceStatus()
message immediately afterwards. This is a regression from bug 1993758,
which added a call to GPUChild::EnsureGPUReady() to
GPUProcessManager::OnProcessLaunchComplete(). Ideally we'd like to
avoid using this synchronous message unless required, and instead
allow time for the asynchronous Init()/InitComplete() callback to
complete.

This patch achieves just that by delaying calling the listener's
OnProcessLaunchComplete() callback until after the post-connection
initialization is complete, rather than immediately after the channel
has connected. This means that by the time the GPUProcessManager calls
EnsureGPUReady(), initialization will have already completed.

A new "Connected" LaunchPhase is added prior to "Complete", indicating
that the channel has been connected but post-connection initialization
has not yet completed. Upon connection we start asynchronous
initialization as before, but now track its completion using a Promise
and call OnProcessLaunchComplete when that resolves.

As before, if GPUProcessManager requires the launch to complete
synchronously it calls GPUProcessHost::WaitForLaunch(). This will now
additionally ensure initialization has completed prior to returning by
sending the synchronous SendGetDeviceData message.

Differential Revision: https://phabricator.services.mozilla.com/D272308

Diffstat:
Mgfx/ipc/GPUChild.cpp | 38+++++++++++++++++++++++---------------
Mgfx/ipc/GPUChild.h | 6++++--
Mgfx/ipc/GPUParent.cpp | 5+++--
Mgfx/ipc/GPUParent.h | 3++-
Mgfx/ipc/GPUProcessHost.cpp | 57+++++++++++++++++++++++++++++++++++++++++++++++++--------
Mgfx/ipc/GPUProcessHost.h | 19++++++++++++++-----
Mgfx/ipc/PGPU.ipdl | 6+-----
7 files changed, 96 insertions(+), 38 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; -void GPUChild::Init() { +RefPtr<GPUChild::InitPromiseType> GPUChild::Init() { nsTArray<GfxVarUpdate> updates = gfxVars::FetchNonDefaultVars(); DevicePrefs devicePrefs; @@ -70,12 +70,24 @@ void GPUChild::Init() { features = gfxInfoRaw->GetAllFeatures(); } - SendInit(updates, devicePrefs, mappings, features, - GPUProcessManager::Get()->AllocateNamespace()); + 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__); + }); gfxVars::AddReceiver(this); (void)SendInitProfiler(ProfilerParent::CreateForProcess(OtherPid())); + + return promise; } void GPUChild::OnVarChanged(const nsTArray<GfxVarUpdate>& aVar) { @@ -96,14 +108,7 @@ bool GPUChild::EnsureGPUReady(bool aForceSync /* = false */) { return false; } - // 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; - } - + OnInitComplete(data); return true; } @@ -136,17 +141,20 @@ void GPUChild::DeletePairedMinidump() { } } -mozilla::ipc::IPCResult GPUChild::RecvInitComplete(const GPUDeviceData& aData) { - // We synchronously requested GPU parameters before this arrived. +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. if (mGPUReady) { - return IPC_OK(); + return; } 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,7 +33,8 @@ class GPUChild final : public mozilla::ipc::CrashReporterHelper<GPUChild>, explicit GPUChild(GPUProcessHost* aHost); - void Init(); + using InitPromiseType = MozPromise<Ok, Ok, true>; + RefPtr<InitPromiseType> Init(); bool IsGPUReady() const { return mGPUReady; } @@ -58,7 +59,6 @@ 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,6 +108,8 @@ 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,7 +275,8 @@ void GPUParent::NotifyDisableRemoteCanvas() { mozilla::ipc::IPCResult GPUParent::RecvInit( nsTArray<GfxVarUpdate>&& vars, const DevicePrefs& devicePrefs, nsTArray<LayerTreeIdMapping>&& aMappings, - nsTArray<GfxInfoFeatureStatus>&& aFeatures, uint32_t aWrNamespace) { + nsTArray<GfxInfoFeatureStatus>&& aFeatures, uint32_t aWrNamespace, + InitResolver&& aInitResolver) { gfxVars::ApplyUpdate(vars); // Inherit device preferences. @@ -396,7 +397,7 @@ mozilla::ipc::IPCResult GPUParent::RecvInit( // Send a message to the UI process that we're done. GPUDeviceData data; RecvGetDeviceStatus(&data); - (void)SendInitComplete(data); + aInitResolver(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,7 +52,8 @@ class GPUParent final : public PGPUParent { const DevicePrefs& devicePrefs, nsTArray<LayerTreeIdMapping>&& mappings, nsTArray<GfxInfoFeatureStatus>&& features, - uint32_t wrNamespace); + uint32_t wrNamespace, + InitResolver&& aInitResolver); 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 @@ -94,12 +94,22 @@ bool GPUProcessHost::WaitForLaunch() { timeoutMs = 0; } - // 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; + 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(); } void GPUProcessHost::OnChannelConnected(base::ProcessId peer_pid) { @@ -121,16 +131,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); - mGPUChild->Init(); + mGPUChild->Init()->Then( + GetCurrentSerialEventTarget(), __func__, + [this, liveToken = mLiveToken]( + const GPUChild::InitPromiseType::ResolveOrRejectValue&) { + if (*liveToken) { + this->OnAsyncInitComplete(); + } + }); #ifdef MOZ_WIDGET_ANDROID nsCOMPtr<nsIEventTarget> launcherThread(GetIPCLauncher()); @@ -147,11 +164,35 @@ void GPUProcessHost::InitAfterConnect(bool aSucceeded) { task.Wait(); #endif + } 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(); + + mLaunchPhase = LaunchPhase::Complete; if (mListener) { mListener->OnProcessLaunchComplete(this); } + + return result; } void GPUProcessHost::Shutdown(bool aUnexpectedShutdown) { diff --git a/gfx/ipc/GPUProcessHost.h b/gfx/ipc/GPUProcessHost.h @@ -60,17 +60,19 @@ 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, or - // if a connection could not be established due to an asynchronous error. + // 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. // // @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. If a launch task is pending, it will fire immediately. + // connected, and any initialization has completed. If a launch task is + // pending, it will fire immediately. // - // Returns true if the process is successfully connected; false otherwise. + // Returns true if the process is successfully initialized; false otherwise. bool WaitForLaunch(); // Inform the process that it should clean up its resources and shut down. @@ -125,7 +127,14 @@ class GPUProcessHost final : public mozilla::ipc::GeckoChildProcessHost { ~GPUProcessHost(); // 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(); @@ -147,7 +156,7 @@ class GPUProcessHost final : public mozilla::ipc::GeckoChildProcessHost { Listener* mListener; - enum class LaunchPhase { Unlaunched, Waiting, Complete }; + enum class LaunchPhase { Unlaunched, Waiting, Connected, Complete }; LaunchPhase mLaunchPhase; RefPtr<GPUChild> mGPUChild; 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); + uint32_t wrNamespace) returns (GPUDeviceData data); async InitCompositorManager(Endpoint<PCompositorManagerParent> endpoint, uint32_t aNamespace); async InitVsyncBridge(Endpoint<PVsyncBridgeParent> endpoint); @@ -138,10 +138,6 @@ 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);