From 6e6f5efad95e6899694484ea8da1e5dcc67b3992 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Sun, 16 Jan 2022 23:46:33 -0800 Subject: [PATCH] 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 | 27 +++- filenames.gni | 5 - patches/chromium/.patches | 1 - .../build_disable_partitionalloc_on_mac.patch | 22 --- shell/app/electron_main.h | 10 -- shell/app/electron_main_linux.cc | 52 ++++++ shell/app/electron_main_mac.cc | 69 ++++++++ ...{electron_main.cc => electron_main_win.cc} | 151 ++---------------- shell/app/uv_stdio_fix.cc | 32 ++++ shell/app/uv_stdio_fix.h | 10 ++ 10 files changed, 197 insertions(+), 182 deletions(-) delete mode 100644 patches/chromium/build_disable_partitionalloc_on_mac.patch delete mode 100644 shell/app/electron_main.h create mode 100644 shell/app/electron_main_linux.cc create mode 100644 shell/app/electron_main_mac.cc rename shell/app/{electron_main.cc => electron_main_win.cc} (69%) create mode 100644 shell/app/uv_stdio_fix.cc create mode 100644 shell/app/uv_stdio_fix.h diff --git a/BUILD.gn b/BUILD.gn index 16e38288ff538..3aa40b4855f8e 100644 --- a/BUILD.gn +++ b/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", diff --git a/filenames.gni b/filenames.gni index 060276156c2c7..b7d61a9858cf5 100644 --- a/filenames.gni +++ b/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", diff --git a/patches/chromium/.patches b/patches/chromium/.patches index df308c3326bcb..30954526811f5 100644 --- a/patches/chromium/.patches +++ b/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 diff --git a/patches/chromium/build_disable_partitionalloc_on_mac.patch b/patches/chromium/build_disable_partitionalloc_on_mac.patch deleted file mode 100644 index f69913ae5734a..0000000000000 --- a/patches/chromium/build_disable_partitionalloc_on_mac.patch +++ /dev/null @@ -1,22 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: clavin -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 diff --git a/shell/app/electron_main.h b/shell/app/electron_main.h deleted file mode 100644 index aa75d6faadf54..0000000000000 --- a/shell/app/electron_main.h +++ /dev/null @@ -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_ diff --git a/shell/app/electron_main_linux.cc b/shell/app/electron_main_linux.cc new file mode 100644 index 0000000000000..7f886cd77bc90 --- /dev/null +++ b/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 +#include + +#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(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)); +} diff --git a/shell/app/electron_main_mac.cc b/shell/app/electron_main_mac.cc new file mode 100644 index 0000000000000..164dd9704698b --- /dev/null +++ b/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 +#include + +#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 +#include + +#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(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); +} diff --git a/shell/app/electron_main.cc b/shell/app/electron_main_win.cc similarity index 69% rename from shell/app/electron_main.cc rename to shell/app/electron_main_win.cc index b1657a6837310..0d73818a91082 100644 --- a/shell/app/electron_main.cc +++ b/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 must be included first + +#include // ensures that ATL statics like `_AtlWinModule` are initialized (it's an issue in static debug build) +#include +#include +#include #include #include @@ -11,101 +16,42 @@ #include #include -#if defined(OS_POSIX) -#include -#include "base/ignore_result.h" -#endif - -#if defined(OS_WIN) -#include // windows.h must be included first - -#include // ensures that ATL statics like `_AtlWinModule` are initialized (it's an issue in static debug build) -#include -#include -#include - +#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 -#include -#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 -#include -#include -#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(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(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) diff --git a/shell/app/uv_stdio_fix.cc b/shell/app/uv_stdio_fix.cc new file mode 100644 index 0000000000000..c4a7ee98e2469 --- /dev/null +++ b/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 +#include +#include +#include + +// Copied from //base/ignore_result.h to avoid taking a dependency on //base on +// macOS. +template +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)); +} diff --git a/shell/app/uv_stdio_fix.h b/shell/app/uv_stdio_fix.h new file mode 100644 index 0000000000000..6a8bf8d3d05ad --- /dev/null +++ b/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_