Browse Source

fix: trace-startup crashing child process on macOS (#44276)

* fix: trace-startup crashing child process on macOS

Co-authored-by: deepak1556 <[email protected]>

* chore: disable test on linux arm

* chore: also disable on linux arm64

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: deepak1556 <[email protected]>
trop[bot] 6 months ago
parent
commit
8b3f2c5d7c

+ 7 - 0
shell/app/electron_library_main.mm

@@ -23,6 +23,13 @@ int ElectronMain(int argc, char* argv[]) {
   params.argc = argc;
   params.argv = const_cast<const char**>(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));
 }
 

+ 0 - 6
shell/app/electron_main_delegate.cc

@@ -270,12 +270,6 @@ std::optional<int> 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);

+ 6 - 6
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<content::ContentBrowserClient> browser_client_;
   std::unique_ptr<content::ContentClient> content_client_;
   std::unique_ptr<content::ContentGpuClient> gpu_client_;

+ 33 - 1
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,38 @@ 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);
+      }
+    });
+
+    // Disable the test on linux arm and arm64 to avoid startup crash
+    // https://github.com/electron/electron/issues/44293#issuecomment-2420077154
+    ifit(process.platform !== 'linux' || (process.arch !== 'arm' && process.arch !== 'arm64'))('creates startup trace', async () => {
+      const rc = await startRemoteControlApp(['--trace-startup=*', `--trace-startup-file=${outputFilePath}`, '--trace-startup-duration=1', '--enable-logging']);
+      const stderrComplete = new Promise<string>(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', () => {