fix: re-enable PartitionAlloc on macOS (#32442)

* fix: re-enable PartitionAlloc on macOS

* no need to copy ignore_result on linux

* factor out FixStdioStreams

* include buildflags.h in electron_main_linux

* #include electron/fuses

* more missing includes

BUILD.gn

@@ -900,8 +900,12 @@ if (is_mac) {
         deps += [ "//sandbox/mac:seatbelt" ]
       }
       defines = [ "HELPER_EXECUTABLE" ]
-      sources = filenames.app_sources
-      sources += [ "shell/common/electron_constants.cc" ]
+      sources = [
+        "shell/app/electron_main_mac.cc",
+        "shell/app/uv_stdio_fix.cc",
+        "shell/app/uv_stdio_fix.h",
+        "shell/common/electron_constants.cc",
+      ]
       include_dirs = [ "." ]
       info_plist = "shell/renderer/resources/mac/Info.plist"
       extra_substitutions =
@@ -1040,15 +1044,18 @@ if (is_mac) {
 
   mac_app_bundle("electron_app") {
     output_name = electron_product_name
-    sources = filenames.app_sources
-    sources += [ "shell/common/electron_constants.cc" ]
+    sources = [
+      "shell/app/electron_main_mac.cc",
+      "shell/app/uv_stdio_fix.cc",
+      "shell/app/uv_stdio_fix.h",
+      "shell/common/electron_constants.cc",
+    ]
     include_dirs = [ "." ]
     deps = [
       ":electron_app_framework_bundle_data",
       ":electron_app_plist",
       ":electron_app_resources",
       ":electron_fuses",
-      "//base",
       "//electron/buildflags",
     ]
     if (is_mas_build) {
@@ -1150,7 +1157,15 @@ if (is_mac) {
 
   executable("electron_app") {
     output_name = electron_project_name
-    sources = filenames.app_sources
+    if (is_win) {
+      sources = [ "shell/app/electron_main_win.cc" ]
+    } else if (is_linux) {
+      sources = [
+        "shell/app/electron_main_linux.cc",
+        "shell/app/uv_stdio_fix.cc",
+        "shell/app/uv_stdio_fix.h",
+      ]
+    }
     include_dirs = [ "." ]
     deps = [
       ":default_app_asar",

filenames.gni

@@ -734,11 +734,6 @@ filenames = {
     "shell/renderer/extensions/electron_extensions_renderer_client.h",
   ]
 
-  app_sources = [
-    "shell/app/electron_main.cc",
-    "shell/app/electron_main.h",
-  ]
-
   framework_sources = [
     "shell/app/electron_library_main.h",
     "shell/app/electron_library_main.mm",

patches/chromium/.patches

@@ -109,7 +109,6 @@ mas_gate_private_enterprise_APIs.patch
 load_v8_snapshot_in_browser_process.patch
 fix_patch_out_permissions_checks_in_exclusive_access.patch
 fix_aspect_ratio_with_max_size.patch
-build_disable_partitionalloc_on_mac.patch
 revert_stop_using_nsrunloop_in_renderer_process.patch
 fix_dont_delete_SerialPortManager_on_main_thread.patch
 feat_add_data_transfer_to_requestsingleinstancelock.patch

patches/chromium/build_disable_partitionalloc_on_mac.patch

@@ -1,22 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: clavin <cwatford@slack-corp.com>
-Date: Tue, 21 Dec 2021 10:17:37 -0700
-Subject: build: disable PartitionAlloc on mac
-
-PartitionAlloc on mac requires some restructuring in Electron as well as considerations about the mas build. In the mean time, disabling it should be fine.
-
-This patch can be removed once the mac app runs safely with PartitionAlloc on (i.e. removing dependency to //base in the main app) & the situation with mas is figured out.
-
-diff --git a/base/allocator/allocator.gni b/base/allocator/allocator.gni
-index 7d579d6e2297c047415eeeb641c70871b5c3f80d..e6edb52dd15af0f369160696fb2a34b0f80680a4 100644
---- a/base/allocator/allocator.gni
-+++ b/base/allocator/allocator.gni
-@@ -18,7 +18,7 @@ _is_using_sanitizers = is_asan || is_hwasan || is_lsan || is_tsan || is_msan
- # - Windows: debug CRT is not compatible, see below.
- _disable_partition_alloc = is_component_build || (is_win && is_debug)
- _is_partition_alloc_platform =
--    is_android || is_win || is_mac || is_linux || is_chromeos
-+    is_android || is_win || is_linux || is_chromeos
- 
- # The debug CRT on Windows has some debug features that are incompatible with
- # the shim. NaCl in particular does seem to link some binaries statically

shell/app/electron_main.h

@@ -1,10 +0,0 @@
-// Copyright (c) 2013 GitHub, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#ifndef ELECTRON_SHELL_APP_ELECTRON_MAIN_H_
-#define ELECTRON_SHELL_APP_ELECTRON_MAIN_H_
-
-#include "content/public/app/content_main.h"
-
-#endif  // ELECTRON_SHELL_APP_ELECTRON_MAIN_H_

shell/app/electron_main_linux.cc

@@ -0,0 +1,52 @@
+// Copyright (c) 2022 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include <cstdlib>
+#include <utility>
+
+#include "base/at_exit.h"
+#include "base/base_switches.h"
+#include "base/command_line.h"
+#include "base/i18n/icu_util.h"
+#include "content/public/app/content_main.h"
+#include "electron/buildflags/buildflags.h"
+#include "electron/fuses.h"
+#include "shell/app/electron_main_delegate.h"  // NOLINT
+#include "shell/app/node_main.h"
+#include "shell/app/uv_stdio_fix.h"
+#include "shell/common/electron_command_line.h"
+#include "shell/common/electron_constants.h"
+
+namespace {
+
+ALLOW_UNUSED_TYPE bool IsEnvSet(const char* name) {
+  char* indicator = getenv(name);
+  return indicator && indicator[0] != '\0';
+}
+
+}  // namespace
+
+int main(int argc, char* argv[]) {
+  FixStdioStreams();
+
+#if BUILDFLAG(ENABLE_RUN_AS_NODE)
+  if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode)) {
+    base::i18n::InitializeICU();
+    base::AtExitManager atexit_manager;
+    return electron::NodeMain(argc, argv);
+  }
+#endif
+
+  electron::ElectronMainDelegate delegate;
+  content::ContentMainParams params(&delegate);
+  electron::ElectronCommandLine::Init(argc, argv);
+  params.argc = argc;
+  params.argv = const_cast<const char**>(argv);
+  base::CommandLine::Init(params.argc, params.argv);
+  // TODO(https://crbug.com/1176772): Remove when Chrome Linux is fully migrated
+  // to Crashpad.
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      ::switches::kEnableCrashpad);
+  return content::ContentMain(std::move(params));
+}

shell/app/electron_main_mac.cc

@@ -0,0 +1,69 @@
+// Copyright (c) 2022 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include <cstdlib>
+#include <memory>
+
+#include "electron/buildflags/buildflags.h"
+#include "electron/fuses.h"
+#include "shell/app/electron_library_main.h"
+#include "shell/app/uv_stdio_fix.h"
+#include "shell/common/electron_constants.h"
+
+#if defined(HELPER_EXECUTABLE) && !defined(MAS_BUILD)
+#include <mach-o/dyld.h>
+#include <cstdio>
+
+#include "sandbox/mac/seatbelt_exec.h"  // nogncheck
+#endif
+
+namespace {
+
+ALLOW_UNUSED_TYPE bool IsEnvSet(const char* name) {
+  char* indicator = getenv(name);
+  return indicator && indicator[0] != '\0';
+}
+
+}  // namespace
+
+int main(int argc, char* argv[]) {
+  FixStdioStreams();
+
+#if BUILDFLAG(ENABLE_RUN_AS_NODE)
+  if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode)) {
+    return ElectronInitializeICUandStartNode(argc, argv);
+  }
+#endif
+
+#if defined(HELPER_EXECUTABLE) && !defined(MAS_BUILD)
+  uint32_t exec_path_size = 0;
+  int rv = _NSGetExecutablePath(NULL, &exec_path_size);
+  if (rv != -1) {
+    fprintf(stderr, "_NSGetExecutablePath: get length failed\n");
+    abort();
+  }
+
+  auto exec_path = std::make_unique<char[]>(exec_path_size);
+  rv = _NSGetExecutablePath(exec_path.get(), &exec_path_size);
+  if (rv != 0) {
+    fprintf(stderr, "_NSGetExecutablePath: get path failed\n");
+    abort();
+  }
+  sandbox::SeatbeltExecServer::CreateFromArgumentsResult seatbelt =
+      sandbox::SeatbeltExecServer::CreateFromArguments(exec_path.get(), argc,
+                                                       argv);
+  if (seatbelt.sandbox_required) {
+    if (!seatbelt.server) {
+      fprintf(stderr, "Failed to create seatbelt sandbox server.\n");
+      abort();
+    }
+    if (!seatbelt.server->InitializeSandbox()) {
+      fprintf(stderr, "Failed to initialize sandbox.\n");
+      abort();
+    }
+  }
+#endif  // defined(HELPER_EXECUTABLE) && !defined(MAS_BUILD)
+
+  return ElectronMain(argc, argv);
+}

shell/app/electron_main_win.cc

@@ -1,8 +1,13 @@
-// Copyright (c) 2013 GitHub, Inc.
+// Copyright (c) 2022 Slack Technologies, Inc.
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#include "shell/app/electron_main.h"
+#include <windows.h>  // windows.h must be included first
+
+#include <atlbase.h>  // ensures that ATL statics like `_AtlWinModule` are initialized (it's an issue in static debug build)
+#include <shellapi.h>
+#include <shellscalingapi.h>
+#include <tchar.h>
 
 #include <algorithm>
 #include <cstdlib>
@@ -11,101 +16,42 @@
 #include <utility>
 #include <vector>
 
-#if defined(OS_POSIX)
-#include <sys/stat.h>
-#include "base/ignore_result.h"
-#endif
-
-#if defined(OS_WIN)
-#include <windows.h>  // windows.h must be included first
-
-#include <atlbase.h>  // ensures that ATL statics like `_AtlWinModule` are initialized (it's an issue in static debug build)
-#include <shellapi.h>
-#include <shellscalingapi.h>
-#include <tchar.h>
-
+#include "base/at_exit.h"
 #include "base/environment.h"
+#include "base/i18n/icu_util.h"
 #include "base/process/launch.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/windows_version.h"
 #include "components/browser_watcher/exit_code_watcher_win.h"
 #include "components/crash/core/app/crash_switches.h"
 #include "components/crash/core/app/run_as_crashpad_handler_win.h"
+#include "content/public/app/content_main.h"
 #include "content/public/app/sandbox_helper_win.h"
+#include "electron/buildflags/buildflags.h"
+#include "electron/fuses.h"
 #include "sandbox/win/src/sandbox_types.h"
 #include "shell/app/command_line_args.h"
 #include "shell/app/electron_main_delegate.h"
-#include "third_party/crashpad/crashpad/util/win/initial_client_data.h"
-
-#elif defined(OS_LINUX)  // defined(OS_WIN)
-#include <unistd.h>
-#include <cstdio>
-#include "base/base_switches.h"
-#include "base/command_line.h"
-#include "content/public/app/content_main.h"
-#include "shell/app/electron_main_delegate.h"  // NOLINT
-#else                                          // defined(OS_LINUX)
-#include <mach-o/dyld.h>
-#include <unistd.h>
-#include <cstdio>
-#include "shell/app/electron_library_main.h"
-#endif  // defined(OS_MAC)
-
-#include "base/at_exit.h"
-#include "base/i18n/icu_util.h"
-#include "electron/buildflags/buildflags.h"
-#include "electron/fuses.h"
 #include "shell/app/node_main.h"
 #include "shell/common/electron_command_line.h"
 #include "shell/common/electron_constants.h"
-
-#if defined(HELPER_EXECUTABLE) && !defined(MAS_BUILD)
-#include "sandbox/mac/seatbelt_exec.h"  // nogncheck
-#endif
+#include "third_party/crashpad/crashpad/util/win/initial_client_data.h"
 
 namespace {
 
-#if defined(OS_WIN)
 // Redefined here so we don't have to introduce a dependency on //content
 // from //electron:electron_app
 const char kUserDataDir[] = "user-data-dir";
 const char kProcessType[] = "type";
-#endif
 
 ALLOW_UNUSED_TYPE bool IsEnvSet(const char* name) {
-#if defined(OS_WIN)
   size_t required_size;
   getenv_s(&required_size, nullptr, 0, name);
   return required_size != 0;
-#else
-  char* indicator = getenv(name);
-  return indicator && indicator[0] != '\0';
-#endif
 }
 
-#if defined(OS_POSIX)
-void FixStdioStreams() {
-  // libuv may mark stdin/stdout/stderr as close-on-exec, which interferes
-  // with chromium's subprocess spawning. As a workaround, we detect if these
-  // streams are closed on startup, and reopen them as /dev/null if necessary.
-  // Otherwise, an unrelated file descriptor will be assigned as stdout/stderr
-  // which may cause various errors when attempting to write to them.
-  //
-  // For details see https://github.com/libuv/libuv/issues/2062
-  struct stat st;
-  if (fstat(STDIN_FILENO, &st) < 0 && errno == EBADF)
-    ignore_result(freopen("/dev/null", "r", stdin));
-  if (fstat(STDOUT_FILENO, &st) < 0 && errno == EBADF)
-    ignore_result(freopen("/dev/null", "w", stdout));
-  if (fstat(STDERR_FILENO, &st) < 0 && errno == EBADF)
-    ignore_result(freopen("/dev/null", "w", stderr));
-}
-#endif
-
 }  // namespace
 
-#if defined(OS_WIN)
-
 namespace crash_reporter {
 extern const char kCrashpadProcess[];
 }
@@ -291,74 +237,3 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
   electron::ElectronCommandLine::Init(arguments.argc, arguments.argv);
   return content::ContentMain(std::move(params));
 }
-
-#elif defined(OS_LINUX)  // defined(OS_WIN)
-
-int main(int argc, char* argv[]) {
-  FixStdioStreams();
-
-#if BUILDFLAG(ENABLE_RUN_AS_NODE)
-  if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode)) {
-    base::i18n::InitializeICU();
-    base::AtExitManager atexit_manager;
-    return electron::NodeMain(argc, argv);
-  }
-#endif
-
-  electron::ElectronMainDelegate delegate;
-  content::ContentMainParams params(&delegate);
-  electron::ElectronCommandLine::Init(argc, argv);
-  params.argc = argc;
-  params.argv = const_cast<const char**>(argv);
-  base::CommandLine::Init(params.argc, params.argv);
-  // TODO(https://crbug.com/1176772): Remove when Chrome Linux is fully migrated
-  // to Crashpad.
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      ::switches::kEnableCrashpad);
-  return content::ContentMain(std::move(params));
-}
-
-#else  // defined(OS_LINUX)
-
-int main(int argc, char* argv[]) {
-  FixStdioStreams();
-
-#if BUILDFLAG(ENABLE_RUN_AS_NODE)
-  if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode)) {
-    return ElectronInitializeICUandStartNode(argc, argv);
-  }
-#endif
-
-#if defined(HELPER_EXECUTABLE) && !defined(MAS_BUILD)
-  uint32_t exec_path_size = 0;
-  int rv = _NSGetExecutablePath(NULL, &exec_path_size);
-  if (rv != -1) {
-    fprintf(stderr, "_NSGetExecutablePath: get length failed\n");
-    abort();
-  }
-
-  auto exec_path = std::make_unique<char[]>(exec_path_size);
-  rv = _NSGetExecutablePath(exec_path.get(), &exec_path_size);
-  if (rv != 0) {
-    fprintf(stderr, "_NSGetExecutablePath: get path failed\n");
-    abort();
-  }
-  sandbox::SeatbeltExecServer::CreateFromArgumentsResult seatbelt =
-      sandbox::SeatbeltExecServer::CreateFromArguments(exec_path.get(), argc,
-                                                       argv);
-  if (seatbelt.sandbox_required) {
-    if (!seatbelt.server) {
-      fprintf(stderr, "Failed to create seatbelt sandbox server.\n");
-      abort();
-    }
-    if (!seatbelt.server->InitializeSandbox()) {
-      fprintf(stderr, "Failed to initialize sandbox.\n");
-      abort();
-    }
-  }
-#endif  // defined(HELPER_EXECUTABLE) && !defined(MAS_BUILD)
-
-  return ElectronMain(argc, argv);
-}
-
-#endif  // defined(OS_MAC)

shell/app/uv_stdio_fix.cc

@@ -0,0 +1,32 @@
+// Copyright (c) 2022 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/app/uv_stdio_fix.h"
+
+#include <errno.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <cstdio>
+
+// Copied from //base/ignore_result.h to avoid taking a dependency on //base on
+// macOS.
+template <typename T>
+inline void ignore_result(const T&) {}
+
+void FixStdioStreams() {
+  // libuv may mark stdin/stdout/stderr as close-on-exec, which interferes
+  // with chromium's subprocess spawning. As a workaround, we detect if these
+  // streams are closed on startup, and reopen them as /dev/null if necessary.
+  // Otherwise, an unrelated file descriptor will be assigned as stdout/stderr
+  // which may cause various errors when attempting to write to them.
+  //
+  // For details see https://github.com/libuv/libuv/issues/2062
+  struct stat st;
+  if (fstat(STDIN_FILENO, &st) < 0 && errno == EBADF)
+    ignore_result(freopen("/dev/null", "r", stdin));
+  if (fstat(STDOUT_FILENO, &st) < 0 && errno == EBADF)
+    ignore_result(freopen("/dev/null", "w", stdout));
+  if (fstat(STDERR_FILENO, &st) < 0 && errno == EBADF)
+    ignore_result(freopen("/dev/null", "w", stderr));
+}

shell/app/uv_stdio_fix.h

@@ -0,0 +1,10 @@
+// Copyright (c) 2022 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ELECTRON_SHELL_APP_UV_STDIO_FIX_H_
+#define ELECTRON_SHELL_APP_UV_STDIO_FIX_H_
+
+void FixStdioStreams();
+
+#endif  // ELECTRON_SHELL_APP_UV_STDIO_FIX_H_