From af6e2fb257d81d6ea216f773f1fb671bf2fb7849 Mon Sep 17 00:00:00 2001 From: Robo Date: Thu, 17 Oct 2024 00:03:00 +0900 Subject: [PATCH] fix: trace-startup crashing child process on macOS (#44257) --- shell/app/electron_library_main.mm | 7 +++++++ shell/app/electron_main_delegate.cc | 6 ------ shell/app/electron_main_delegate.h | 12 +++++------ spec/chromium-spec.ts | 32 ++++++++++++++++++++++++++++- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/shell/app/electron_library_main.mm b/shell/app/electron_library_main.mm index 6f43e9c846..114b8e22e8 100644 --- a/shell/app/electron_library_main.mm +++ b/shell/app/electron_library_main.mm @@ -23,6 +23,13 @@ int ElectronMain(int argc, char* argv[]) { params.argc = argc; params.argv = const_cast(argv); electron::ElectronCommandLine::Init(argc, argv); + + // Ensure that Bundle Id is set before ContentMain. + // Refs https://chromium-review.googlesource.com/c/chromium/src/+/5581006 + delegate.OverrideChildProcessPath(); + delegate.OverrideFrameworkBundlePath(); + delegate.SetUpBundleOverrides(); + return content::ContentMain(std::move(params)); } diff --git a/shell/app/electron_main_delegate.cc b/shell/app/electron_main_delegate.cc index 4a73b80414..c093db6801 100644 --- a/shell/app/electron_main_delegate.cc +++ b/shell/app/electron_main_delegate.cc @@ -271,12 +271,6 @@ std::optional ElectronMainDelegate::BasicStartupComplete() { kNonWildcardDomainNonPortSchemes, kNonWildcardDomainNonPortSchemesSize); #endif -#if BUILDFLAG(IS_MAC) - OverrideChildProcessPath(); - OverrideFrameworkBundlePath(); - SetUpBundleOverrides(); -#endif - #if BUILDFLAG(IS_WIN) // Ignore invalid parameter errors. _set_invalid_parameter_handler(InvalidParameterHandler); diff --git a/shell/app/electron_main_delegate.h b/shell/app/electron_main_delegate.h index 1776cfe5ed..ed484077f3 100644 --- a/shell/app/electron_main_delegate.h +++ b/shell/app/electron_main_delegate.h @@ -34,6 +34,12 @@ class ElectronMainDelegate : public content::ContentMainDelegate { ElectronMainDelegate(const ElectronMainDelegate&) = delete; ElectronMainDelegate& operator=(const ElectronMainDelegate&) = delete; +#if BUILDFLAG(IS_MAC) + void OverrideChildProcessPath(); + void OverrideFrameworkBundlePath(); + void SetUpBundleOverrides(); +#endif + protected: // content::ContentMainDelegate: std::string_view GetBrowserV8SnapshotFilename() override; @@ -57,12 +63,6 @@ class ElectronMainDelegate : public content::ContentMainDelegate { #endif private: -#if BUILDFLAG(IS_MAC) - void OverrideChildProcessPath(); - void OverrideFrameworkBundlePath(); - void SetUpBundleOverrides(); -#endif - std::unique_ptr browser_client_; std::unique_ptr content_client_; std::unique_ptr gpu_client_; diff --git a/spec/chromium-spec.ts b/spec/chromium-spec.ts index 5f49ff5559..ce7f6e5023 100644 --- a/spec/chromium-spec.ts +++ b/spec/chromium-spec.ts @@ -15,7 +15,7 @@ import * as path from 'node:path'; import { setTimeout } from 'node:timers/promises'; import * as url from 'node:url'; -import { ifit, ifdescribe, defer, itremote, listen } from './lib/spec-helpers'; +import { ifit, ifdescribe, defer, itremote, listen, startRemoteControlApp } from './lib/spec-helpers'; import { closeAllWindows } from './lib/window-helpers'; import { PipeTransport } from './pipe-transport'; @@ -556,6 +556,36 @@ describe('command line switches', () => { }); }); }); + + describe('--trace-startup switch', () => { + const outputFilePath = path.join(app.getPath('temp'), 'trace.json'); + afterEach(() => { + if (fs.existsSync(outputFilePath)) { + fs.unlinkSync(outputFilePath); + } + }); + + it('creates startup trace', async () => { + const rc = await startRemoteControlApp(['--trace-startup=*', `--trace-startup-file=${outputFilePath}`, '--trace-startup-duration=1', '--enable-logging']); + const stderrComplete = new Promise(resolve => { + let stderr = ''; + rc.process.stderr!.on('data', (chunk) => { + stderr += chunk.toString('utf8'); + }); + rc.process.on('close', () => { resolve(stderr); }); + }); + rc.remotely(() => { + global.setTimeout(() => { + require('electron').app.quit(); + }, 5000); + }); + const stderr = await stderrComplete; + expect(stderr).to.match(/Completed startup tracing to/); + expect(fs.existsSync(outputFilePath)).to.be.true('output exists'); + expect(fs.statSync(outputFilePath).size).to.be.above(0, + `the trace output file is empty, check "${outputFilePath}"`); + }); + }); }); describe('chromium features', () => {