fix: dangling raw_ptr in ElectronBrowserMainParts dtor (#39593)

* fix: dangling raw_ptr in ElectronBrowserMainParts dtor

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* fixup! fix: dangling raw_ptr in ElectronBrowserMainParts dtor

Browser::WhenReady() holds a reference to JsEnv isolate so must come after

Co-authored-by: Charles Kerr <charles@charleskerr.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>

docs/api/app.md

@@ -412,18 +412,7 @@ Returns:
 
 * `event` Event
 * `webContents` [WebContents](web-contents.md)
-* `details` Object
-  * `reason` string - The reason the render process is gone.  Possible values:
-    * `clean-exit` - Process exited with an exit code of zero
-    * `abnormal-exit` - Process exited with a non-zero exit code
-    * `killed` - Process was sent a SIGTERM or otherwise killed externally
-    * `crashed` - Process crashed
-    * `oom` - Process ran out of memory
-    * `launch-failed` - Process never successfully launched
-    * `integrity-failure` - Windows code integrity checks failed
-  * `exitCode` Integer - The exit code of the process, unless `reason` is
-    `launch-failed`, in which case `exitCode` will be a platform-specific
-    launch failure error code.
+* `details` [RenderProcessGoneDetails](structures/render-process-gone-details.md)
 
 Emitted when the renderer process unexpectedly disappears.  This is normally
 because it was crashed or killed.

docs/api/browser-window.md

@@ -820,10 +820,14 @@ win.setBounds({ width: 100 })
 console.log(win.getBounds())
 ```
 
+**Note:** On macOS, the y-coordinate value cannot be smaller than the [Tray](tray.md) height. The tray height has changed over time and depends on the operating system, but is between 20-40px. Passing a value lower than the tray height will result in a window that is flush to the tray.
+
 #### `win.getBounds()`
 
 Returns [`Rectangle`](structures/rectangle.md) - The `bounds` of the window as `Object`.
 
+**Note:** On macOS, the y-coordinate value returned will be at minimum the [Tray](tray.md) height. For example, calling `win.setBounds({ x: 25, y: 20, width: 800, height: 600 })` with a tray height of 38 means that `win.getBounds()` will return `{ x: 25, y: 38, width: 800, height: 600 }`.
+
 #### `win.getBackgroundColor()`
 
 Returns `string` - Gets the background color of the window in Hex (`#RRGGBB`) format.

docs/api/structures/render-process-gone-details.md

@@ -0,0 +1,13 @@
+# RenderProcessGoneDetails Object
+
+* `reason` string - The reason the render process is gone.  Possible values:
+  * `clean-exit` - Process exited with an exit code of zero
+  * `abnormal-exit` - Process exited with a non-zero exit code
+  * `killed` - Process was sent a SIGTERM or otherwise killed externally
+  * `crashed` - Process crashed
+  * `oom` - Process ran out of memory
+  * `launch-failed` - Process never successfully launched
+  * `integrity-failure` - Windows code integrity checks failed
+* `exitCode` Integer - The exit code of the process, unless `reason` is
+  `launch-failed`, in which case `exitCode` will be a platform-specific
+  launch failure error code.

docs/api/web-contents.md

@@ -479,18 +479,7 @@ checking `reason === 'killed'` when you switch to that event.
 Returns:
 
 * `event` Event
-* `details` Object
-  * `reason` string - The reason the render process is gone.  Possible values:
-    * `clean-exit` - Process exited with an exit code of zero
-    * `abnormal-exit` - Process exited with a non-zero exit code
-    * `killed` - Process was sent a SIGTERM or otherwise killed externally
-    * `crashed` - Process crashed
-    * `oom` - Process ran out of memory
-    * `launch-failed` - Process never successfully launched
-    * `integrity-failure` - Windows code integrity checks failed
-  * `exitCode` Integer - The exit code of the process, unless `reason` is
-    `launch-failed`, in which case `exitCode` will be a platform-specific
-    launch failure error code.
+* `details` [RenderProcessGoneDetails](structures/render-process-gone-details.md)
 
 Emitted when the renderer process unexpectedly disappears.  This is normally
 because it was crashed or killed.

docs/api/webview-tag.md

@@ -983,9 +983,22 @@ ipcRenderer.on('ping', () => {
 })
 ```
 
-### Event: 'crashed'
+### Event: 'crashed' _Deprecated_
 
-Fired when the renderer process is crashed.
+Fired when the renderer process crashes or is killed.
+
+**Deprecated:** This event is superceded by the `render-process-gone` event
+which contains more information about why the render process disappeared. It
+isn't always because it crashed.
+
+### Event: 'render-process-gone'
+
+Returns:
+
+* `details` [RenderProcessGoneDetails](structures/render-process-gone-details.md)
+
+Fired when the renderer process unexpectedly disappears. This is normally
+because it was crashed or killed.
 
 ### Event: 'plugin-crashed'
 

electron_paks.gni

@@ -61,6 +61,7 @@ template("electron_extra_paks") {
       "$root_gen_dir/content/browser/tracing/tracing_resources.pak",
       "$root_gen_dir/content/browser/webrtc/resources/webrtc_internals_resources.pak",
       "$root_gen_dir/content/content_resources.pak",
+      "$root_gen_dir/content/gpu_resources.pak",
       "$root_gen_dir/mojo/public/js/mojo_bindings_resources.pak",
       "$root_gen_dir/net/net_resources.pak",
       "$root_gen_dir/third_party/blink/public/resources/blink_resources.pak",
@@ -74,6 +75,7 @@ template("electron_extra_paks") {
       "//chrome/common:resources",
       "//components/resources",
       "//content:content_resources",
+      "//content/browser/resources/gpu:resources",
       "//content/browser/resources/media:resources",
       "//content/browser/tracing:resources",
       "//content/browser/webrtc/resources",

filenames.auto.gni

@@ -115,6 +115,7 @@ auto_filenames = {
     "docs/api/structures/protocol-response.md",
     "docs/api/structures/rectangle.md",
     "docs/api/structures/referrer.md",
+    "docs/api/structures/render-process-gone-details.md",
     "docs/api/structures/resolved-endpoint.md",
     "docs/api/structures/resolved-host.md",
     "docs/api/structures/scrubber-item.md",

shell/app/node_main.cc

@@ -54,13 +54,14 @@ namespace {
 // See https://nodejs.org/api/cli.html#cli_options
 void ExitIfContainsDisallowedFlags(const std::vector<std::string>& argv) {
   // Options that are unilaterally disallowed.
-  static constexpr auto disallowed = base::MakeFixedFlatSet<base::StringPiece>({
-      "--enable-fips",
-      "--force-fips",
-      "--openssl-config",
-      "--use-bundled-ca",
-      "--use-openssl-ca",
-  });
+  static constexpr auto disallowed =
+      base::MakeFixedFlatSetSorted<base::StringPiece>({
+          "--enable-fips",
+          "--force-fips",
+          "--openssl-config",
+          "--use-bundled-ca",
+          "--use-openssl-ca",
+      });
 
   for (const auto& arg : argv) {
     const auto key = base::StringPiece(arg).substr(0, arg.find('='));

shell/browser/electron_browser_main_parts.cc

@@ -204,11 +204,11 @@ ElectronBrowserMainParts* ElectronBrowserMainParts::self_ = nullptr;
 
 ElectronBrowserMainParts::ElectronBrowserMainParts()
     : fake_browser_process_(std::make_unique<BrowserProcessImpl>()),
-      browser_(std::make_unique<Browser>()),
-      node_bindings_(
-          NodeBindings::Create(NodeBindings::BrowserEnvironment::kBrowser)),
-      electron_bindings_(
-          std::make_unique<ElectronBindings>(node_bindings_->uv_loop())) {
+      node_bindings_{
+          NodeBindings::Create(NodeBindings::BrowserEnvironment::kBrowser)},
+      electron_bindings_{
+          std::make_unique<ElectronBindings>(node_bindings_->uv_loop())},
+      browser_{std::make_unique<Browser>()} {
   DCHECK(!self_) << "Cannot have two ElectronBrowserMainParts";
   self_ = this;
 }

shell/browser/electron_browser_main_parts.h

@@ -157,11 +157,20 @@ class ElectronBrowserMainParts : public content::BrowserMainParts {
   // Before then, we just exit() without any intermediate steps.
   absl::optional<int> exit_code_;
 
-  std::unique_ptr<JavascriptEnvironment> js_env_;
-  std::unique_ptr<Browser> browser_;
   std::unique_ptr<NodeBindings> node_bindings_;
+
+  // depends-on: node_bindings_
   std::unique_ptr<ElectronBindings> electron_bindings_;
+
+  // depends-on: node_bindings_
+  std::unique_ptr<JavascriptEnvironment> js_env_;
+
+  // depends-on: js_env_'s isolate
   std::unique_ptr<NodeEnvironment> node_env_;
+
+  // depends-on: js_env_'s isolate
+  std::unique_ptr<Browser> browser_;
+
   std::unique_ptr<IconManager> icon_manager_;
   std::unique_ptr<base::FieldTrialList> field_trial_list_;
 

shell/browser/javascript_environment.cc

@@ -104,9 +104,10 @@ gin::IsolateHolder CreateIsolateHolder(v8::Isolate* isolate) {
 
 JavascriptEnvironment::JavascriptEnvironment(uv_loop_t* event_loop,
                                              bool setup_wasm_streaming)
-    : isolate_(Initialize(event_loop, setup_wasm_streaming)),
-      isolate_holder_(CreateIsolateHolder(isolate_)),
-      locker_(isolate_) {
+    : isolate_holder_{CreateIsolateHolder(
+          Initialize(event_loop, setup_wasm_streaming))},
+      isolate_{isolate_holder_.isolate()},
+      locker_{isolate_} {
   isolate_->Enter();
 
   v8::HandleScope scope(isolate_);

shell/browser/javascript_environment.h

@@ -43,8 +43,12 @@ class JavascriptEnvironment {
   v8::Isolate* Initialize(uv_loop_t* event_loop, bool setup_wasm_streaming);
   std::unique_ptr<node::MultiIsolatePlatform> platform_;
 
-  raw_ptr<v8::Isolate> isolate_;
   gin::IsolateHolder isolate_holder_;
+
+  // owned-by: isolate_holder_
+  const raw_ptr<v8::Isolate> isolate_;
+
+  // depends-on: isolate_
   v8::Locker locker_;
 
   std::unique_ptr<MicrotasksRunner> microtasks_runner_;

shell/browser/ui/views/client_frame_view_linux.cc

@@ -150,7 +150,13 @@ void ClientFrameViewLinux::Init(NativeWindowViews* window,
 }
 
 gfx::Insets ClientFrameViewLinux::GetBorderDecorationInsets() const {
-  return frame_provider_->GetFrameThicknessDip();
+  const auto insets = frame_provider_->GetFrameThicknessDip();
+  // We shouldn't draw frame decorations for the tiled edges.
+  // See https://wayland.app/protocols/xdg-shell#xdg_toplevel:enum:state
+  return gfx::Insets::TLBR(tiled_edges().top ? 0 : insets.top(),
+                           tiled_edges().left ? 0 : insets.left(),
+                           tiled_edges().bottom ? 0 : insets.bottom(),
+                           tiled_edges().right ? 0 : insets.right());
 }
 
 gfx::Insets ClientFrameViewLinux::GetInputInsets() const {

shell/browser/usb/electron_usb_delegate.cc

@@ -45,7 +45,7 @@ electron::UsbChooserContext* GetChooserContext(
 // These extensions can claim the smart card USB class and automatically gain
 // permissions for devices that have an interface with this class.
 constexpr auto kSmartCardPrivilegedExtensionIds =
-    base::MakeFixedFlatSet<base::StringPiece>({
+    base::MakeFixedFlatSetSorted<base::StringPiece>({
         // Smart Card Connector Extension and its Beta version, see
         // crbug.com/1233881.
         "khpfeaanjngmcnplbdlpegiifgpfgdco",

shell/common/node_bindings.cc

@@ -232,7 +232,7 @@ void ErrorMessageListener(v8::Local<v8::Message> message,
 // If node CLI inspect support is disabled, allow no debug options.
 bool IsAllowedOption(base::StringPiece option) {
   static constexpr auto debug_options =
-      base::MakeFixedFlatSet<base::StringPiece>({
+      base::MakeFixedFlatSetSorted<base::StringPiece>({
           "--debug",
           "--debug-brk",
           "--debug-port",
@@ -244,13 +244,14 @@ bool IsAllowedOption(base::StringPiece option) {
       });
 
   // This should be aligned with what's possible to set via the process object.
-  static constexpr auto options = base::MakeFixedFlatSet<base::StringPiece>({
-      "--trace-warnings",
-      "--trace-deprecation",
-      "--throw-deprecation",
-      "--no-deprecation",
-      "--dns-result-order",
-  });
+  static constexpr auto options =
+      base::MakeFixedFlatSetSorted<base::StringPiece>({
+          "--dns-result-order",
+          "--no-deprecation",
+          "--throw-deprecation",
+          "--trace-deprecation",
+          "--trace-warnings",
+      });
 
   if (debug_options.contains(option))
     return electron::fuses::IsNodeCliInspectEnabled();
@@ -262,14 +263,21 @@ bool IsAllowedOption(base::StringPiece option) {
 // See https://nodejs.org/api/cli.html#cli_node_options_options
 void SetNodeOptions(base::Environment* env) {
   // Options that are unilaterally disallowed
-  static constexpr auto disallowed = base::MakeFixedFlatSet<base::StringPiece>(
-      {"--enable-fips", "--force-fips", "--openssl-config", "--use-bundled-ca",
-       "--use-openssl-ca", "--experimental-policy"});
+  static constexpr auto disallowed =
+      base::MakeFixedFlatSetSorted<base::StringPiece>({
+          "--enable-fips",
+          "--experimental-policy",
+          "--force-fips",
+          "--openssl-config",
+          "--use-bundled-ca",
+          "--use-openssl-ca",
+      });
 
-  static constexpr auto pkg_opts = base::MakeFixedFlatSet<base::StringPiece>({
-      "--http-parser",
-      "--max-http-header-size",
-  });
+  static constexpr auto pkg_opts =
+      base::MakeFixedFlatSetSorted<base::StringPiece>({
+          "--http-parser",
+          "--max-http-header-size",
+      });
 
   if (env->HasVar("NODE_OPTIONS")) {
     if (electron::fuses::IsNodeOptionsEnabled()) {

spec/chromium-spec.ts

@@ -2050,10 +2050,32 @@ describe('chromium features', () => {
     });
   });
 
+  describe('chrome://accessibility', () => {
+    it('loads the page successfully', async () => {
+      const w = new BrowserWindow({ show: false });
+      await w.loadURL('chrome://accessibility');
+      const pageExists = await w.webContents.executeJavaScript(
+        "window.hasOwnProperty('chrome') && window.chrome.hasOwnProperty('send')"
+      );
+      expect(pageExists).to.be.true();
+    });
+  });
+
+  describe('chrome://gpu', () => {
+    it('loads the page successfully', async () => {
+      const w = new BrowserWindow({ show: false });
+      await w.loadURL('chrome://gpu');
+      const pageExists = await w.webContents.executeJavaScript(
+        "window.hasOwnProperty('chrome') && window.chrome.hasOwnProperty('send')"
+      );
+      expect(pageExists).to.be.true();
+    });
+  });
+
   describe('chrome://media-internals', () => {
     it('loads the page successfully', async () => {
       const w = new BrowserWindow({ show: false });
-      w.loadURL('chrome://media-internals');
+      await w.loadURL('chrome://media-internals');
       const pageExists = await w.webContents.executeJavaScript(
         "window.hasOwnProperty('chrome') && window.chrome.hasOwnProperty('send')"
       );
@@ -2064,7 +2086,7 @@ describe('chromium features', () => {
   describe('chrome://webrtc-internals', () => {
     it('loads the page successfully', async () => {
       const w = new BrowserWindow({ show: false });
-      w.loadURL('chrome://webrtc-internals');
+      await w.loadURL('chrome://webrtc-internals');
       const pageExists = await w.webContents.executeJavaScript(
         "window.hasOwnProperty('chrome') && window.chrome.hasOwnProperty('send')"
       );