From 05d39d831382502ec4d4750a4083e2f4abe8f248 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Thu, 8 Jun 2023 22:40:08 +0200 Subject: [PATCH] build: remove enable_run_as_node build flag (#38413) * feat: remove enable_run_as_node flag * drop features.isRunAsNodeEnabled() * use IsEnvSet() helper in electron_main_linux.cc * cleanup [[maybe_unused]] --------- Co-authored-by: Milan Burda --- .circleci/config/base.yml | 29 +++-------------------------- BUILD.gn | 7 ------- buildflags/BUILD.gn | 1 - buildflags/buildflags.gni | 3 --- filenames.gni | 2 ++ shell/app/electron_library_main.h | 3 --- shell/app/electron_library_main.mm | 2 -- shell/app/electron_main_linux.cc | 15 ++++++++++----- shell/app/electron_main_mac.cc | 4 +--- shell/app/electron_main_win.cc | 10 ++-------- shell/common/api/features.cc | 6 ------ shell/common/crash_keys.cc | 10 ++-------- shell/common/electron_constants.cc | 2 -- shell/common/electron_constants.h | 2 -- spec/asar-spec.ts | 12 ++---------- spec/modules-spec.ts | 5 ++--- spec/node-spec.ts | 11 +++++------ typings/internal-ambient.d.ts | 1 - 18 files changed, 29 insertions(+), 96 deletions(-) diff --git a/.circleci/config/base.yml b/.circleci/config/base.yml index 275e981a2b6..480331c1d21 100644 --- a/.circleci/config/base.yml +++ b/.circleci/config/base.yml @@ -175,9 +175,6 @@ env-mac-large-release: &env-mac-large-release env-ninja-status: &env-ninja-status NINJA_STATUS: "[%r processes, %f/%t @ %o/s : %es] " -env-disable-run-as-node: &env-disable-run-as-node - GN_BUILDFLAG_ARGS: 'enable_run_as_node = false' - env-32bit-release: &env-32bit-release # Set symbol level to 1 for 32 bit releases because of https://crbug.com/648948 GN_BUILDFLAG_ARGS: 'symbol_level = 1' @@ -254,7 +251,7 @@ step-depot-tools-get: &step-depot-tools-get --- a/gclient.py +++ b/gclient.py @@ -712,7 +712,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): - + if dep_type == 'cipd': cipd_root = self.GetCipdRoot() - for package in dep_value.get('packages', []): @@ -1058,7 +1055,7 @@ commands: artifact-key: type: string build-type: - type: string + type: string build-nonproprietary-ffmpeg: type: boolean default: true @@ -1276,7 +1273,7 @@ commands: artifact-key: type: string build-type: - type: string + type: string after-build-and-save: type: steps default: [] @@ -1730,23 +1727,6 @@ jobs: artifact-key: 'linux-x64-asan' build-type: 'Linux' - linux-x64-testing-no-run-as-node: - executor: - name: linux-docker - size: xlarge - environment: - <<: *env-linux-2xlarge - <<: *env-testing-build - <<: *env-ninja-status - <<: *env-disable-run-as-node - GCLIENT_EXTRA_ARGS: '--custom-var=checkout_arm=True --custom-var=checkout_arm64=True' - steps: - - electron-build: - persist: false - checkout: true - artifact-key: 'linux-x64-no-run-as-node' - build-type: 'Linux' - linux-x64-testing-gn-check: executor: name: linux-docker @@ -2230,9 +2210,6 @@ workflows: - linux-x64-testing-asan: requires: - linux-make-src-cache - - linux-x64-testing-no-run-as-node: - requires: - - linux-make-src-cache - linux-x64-testing-gn-check: requires: - linux-make-src-cache diff --git a/BUILD.gn b/BUILD.gn index ff73f9c47eb..33049678c06 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -669,13 +669,6 @@ source_set("electron_lib") { ] } - if (enable_run_as_node) { - sources += [ - "shell/app/node_main.cc", - "shell/app/node_main.h", - ] - } - if (enable_osr) { sources += [ "shell/browser/osr/osr_host_display_client.cc", diff --git a/buildflags/BUILD.gn b/buildflags/BUILD.gn index 48a46be84a1..2b8f65791a8 100644 --- a/buildflags/BUILD.gn +++ b/buildflags/BUILD.gn @@ -9,7 +9,6 @@ buildflag_header("buildflags") { header = "buildflags.h" flags = [ - "ENABLE_RUN_AS_NODE=$enable_run_as_node", "ENABLE_OSR=$enable_osr", "ENABLE_VIEWS_API=$enable_views_api", "ENABLE_PDF_VIEWER=$enable_pdf_viewer", diff --git a/buildflags/buildflags.gni b/buildflags/buildflags.gni index f9afd8af959..7737eabaa3c 100644 --- a/buildflags/buildflags.gni +++ b/buildflags/buildflags.gni @@ -3,9 +3,6 @@ # found in the LICENSE file. declare_args() { - # Allow running Electron as a node binary. - enable_run_as_node = true - enable_osr = true enable_views_api = true diff --git a/filenames.gni b/filenames.gni index 90701d66455..10e359baa9e 100644 --- a/filenames.gni +++ b/filenames.gni @@ -237,6 +237,8 @@ filenames = { "shell/app/electron_crash_reporter_client.h", "shell/app/electron_main_delegate.cc", "shell/app/electron_main_delegate.h", + "shell/app/node_main.cc", + "shell/app/node_main.h", "shell/app/uv_task_runner.cc", "shell/app/uv_task_runner.h", "shell/browser/api/electron_api_app.cc", diff --git a/shell/app/electron_library_main.h b/shell/app/electron_library_main.h index 36fc713272e..1e09d33eb4b 100644 --- a/shell/app/electron_library_main.h +++ b/shell/app/electron_library_main.h @@ -11,12 +11,9 @@ #if BUILDFLAG(IS_MAC) extern "C" { __attribute__((visibility("default"))) int ElectronMain(int argc, char* argv[]); - -#if BUILDFLAG(ENABLE_RUN_AS_NODE) __attribute__((visibility("default"))) int ElectronInitializeICUandStartNode( int argc, char* argv[]); -#endif } #endif diff --git a/shell/app/electron_library_main.mm b/shell/app/electron_library_main.mm index dc4fa9391d0..5cba9d752a5 100644 --- a/shell/app/electron_library_main.mm +++ b/shell/app/electron_library_main.mm @@ -26,7 +26,6 @@ int ElectronMain(int argc, char* argv[]) { return content::ContentMain(std::move(params)); } -#if BUILDFLAG(ENABLE_RUN_AS_NODE) int ElectronInitializeICUandStartNode(int argc, char* argv[]) { if (!electron::fuses::IsRunAsNodeEnabled()) { CHECK(false) << "run_as_node fuse is disabled"; @@ -43,4 +42,3 @@ int ElectronInitializeICUandStartNode(int argc, char* argv[]) { base::i18n::InitializeICU(); return electron::NodeMain(argc, argv); } -#endif diff --git a/shell/app/electron_main_linux.cc b/shell/app/electron_main_linux.cc index 5c83366d4ac..37e78c40302 100644 --- a/shell/app/electron_main_linux.cc +++ b/shell/app/electron_main_linux.cc @@ -18,18 +18,23 @@ #include "shell/common/electron_command_line.h" #include "shell/common/electron_constants.h" +namespace { + +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) - char* indicator = getenv(electron::kRunAsNode); - if (electron::fuses::IsRunAsNodeEnabled() && indicator && - indicator[0] != '\0') { + 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); diff --git a/shell/app/electron_main_mac.cc b/shell/app/electron_main_mac.cc index cd45a83c5a9..045c18a1811 100644 --- a/shell/app/electron_main_mac.cc +++ b/shell/app/electron_main_mac.cc @@ -28,7 +28,7 @@ void abort_report_np(const char* fmt, ...); namespace { -[[maybe_unused]] bool IsEnvSet(const char* name) { +bool IsEnvSet(const char* name) { char* indicator = getenv(name); return indicator && indicator[0] != '\0'; } @@ -53,12 +53,10 @@ int main(int argc, char* argv[]) { partition_alloc::EarlyMallocZoneRegistration(); FixStdioStreams(); -#if BUILDFLAG(ENABLE_RUN_AS_NODE) if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet("ELECTRON_RUN_AS_NODE")) { return ElectronInitializeICUandStartNode(argc, argv); } -#endif #if defined(HELPER_EXECUTABLE) && !IS_MAS_BUILD() uint32_t exec_path_size = 0; diff --git a/shell/app/electron_main_win.cc b/shell/app/electron_main_win.cc index 8fe75892566..4a9712b2118 100644 --- a/shell/app/electron_main_win.cc +++ b/shell/app/electron_main_win.cc @@ -45,7 +45,7 @@ namespace { const char kUserDataDir[] = "user-data-dir"; const char kProcessType[] = "type"; -[[maybe_unused]] bool IsEnvSet(const char* name) { +bool IsEnvSet(const char* name) { size_t required_size; getenv_s(&required_size, nullptr, 0, name); return required_size != 0; @@ -150,12 +150,8 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) { } #endif -#if BUILDFLAG(ENABLE_RUN_AS_NODE) bool run_as_node = electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode); -#else - bool run_as_node = false; -#endif // Make sure the output is printed to console. if (run_as_node || !IsEnvSet("ELECTRON_NO_ATTACH_CONSOLE")) @@ -164,15 +160,13 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) { std::vector argv(arguments.argc); std::transform(arguments.argv, arguments.argv + arguments.argc, argv.begin(), [](auto& a) { return _strdup(base::WideToUTF8(a).c_str()); }); -#if BUILDFLAG(ENABLE_RUN_AS_NODE) - if (electron::fuses::IsRunAsNodeEnabled() && run_as_node) { + if (run_as_node) { base::AtExitManager atexit_manager; base::i18n::InitializeICU(); auto ret = electron::NodeMain(argv.size(), argv.data()); std::for_each(argv.begin(), argv.end(), free); return ret; } -#endif base::CommandLine::Init(argv.size(), argv.data()); const base::CommandLine* command_line = diff --git a/shell/common/api/features.cc b/shell/common/api/features.cc index fd4a9aeb44b..8003edb113e 100644 --- a/shell/common/api/features.cc +++ b/shell/common/api/features.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "electron/buildflags/buildflags.h" -#include "electron/fuses.h" #include "printing/buildflags/buildflags.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/node_includes.h" @@ -22,10 +21,6 @@ bool IsPDFViewerEnabled() { return BUILDFLAG(ENABLE_PDF_VIEWER); } -bool IsRunAsNodeEnabled() { - return electron::fuses::IsRunAsNodeEnabled() && BUILDFLAG(ENABLE_RUN_AS_NODE); -} - bool IsFakeLocationProviderEnabled() { return BUILDFLAG(OVERRIDE_LOCATION_PROVIDER); } @@ -58,7 +53,6 @@ void Initialize(v8::Local exports, dict.SetMethod("isBuiltinSpellCheckerEnabled", &IsBuiltinSpellCheckerEnabled); dict.SetMethod("isOffscreenRenderingEnabled", &IsOffscreenRenderingEnabled); dict.SetMethod("isPDFViewerEnabled", &IsPDFViewerEnabled); - dict.SetMethod("isRunAsNodeEnabled", &IsRunAsNodeEnabled); dict.SetMethod("isFakeLocationProviderEnabled", &IsFakeLocationProviderEnabled); dict.SetMethod("isViewApiEnabled", &IsViewApiEnabled); diff --git a/shell/common/crash_keys.cc b/shell/common/crash_keys.cc index ec56e25520e..26307a3d18e 100644 --- a/shell/common/crash_keys.cc +++ b/shell/common/crash_keys.cc @@ -97,14 +97,8 @@ void GetCrashKeys(std::map* keys) { namespace { bool IsRunningAsNode() { -#if BUILDFLAG(ENABLE_RUN_AS_NODE) - if (!electron::fuses::IsRunAsNodeEnabled()) - return false; - - return base::Environment::Create()->HasVar(electron::kRunAsNode); -#else - return false; -#endif + return electron::fuses::IsRunAsNodeEnabled() && + base::Environment::Create()->HasVar(electron::kRunAsNode); } } // namespace diff --git a/shell/common/electron_constants.cc b/shell/common/electron_constants.cc index 2bc5727c00e..a47d40a9fa1 100644 --- a/shell/common/electron_constants.cc +++ b/shell/common/electron_constants.cc @@ -13,9 +13,7 @@ const char kDeviceVendorIdKey[] = "vendorId"; const char kDeviceProductIdKey[] = "productId"; const char kDeviceSerialNumberKey[] = "serialNumber"; -#if BUILDFLAG(ENABLE_RUN_AS_NODE) const char kRunAsNode[] = "ELECTRON_RUN_AS_NODE"; -#endif #if BUILDFLAG(ENABLE_PDF_VIEWER) const char kPDFExtensionPluginName[] = "Chromium PDF Viewer"; diff --git a/shell/common/electron_constants.h b/shell/common/electron_constants.h index de9fde85f29..ed40d73b23c 100644 --- a/shell/common/electron_constants.h +++ b/shell/common/electron_constants.h @@ -20,9 +20,7 @@ extern const char kDeviceVendorIdKey[]; extern const char kDeviceProductIdKey[]; extern const char kDeviceSerialNumberKey[]; -#if BUILDFLAG(ENABLE_RUN_AS_NODE) extern const char kRunAsNode[]; -#endif #if BUILDFLAG(ENABLE_PDF_VIEWER) extern const char kPDFExtensionPluginName[]; diff --git a/spec/asar-spec.ts b/spec/asar-spec.ts index 92cf07164bf..23d96e143db 100644 --- a/spec/asar-spec.ts +++ b/spec/asar-spec.ts @@ -8,8 +8,6 @@ import { getRemoteContext, ifdescribe, ifit, itremote, useRemoteContext } from ' import * as importedFs from 'fs'; import { once } from 'events'; -const features = process._linkedBinding('electron_common_features'); - describe('asar package', () => { const fixtures = path.join(__dirname, 'fixtures'); const asarDir = path.join(fixtures, 'test.asar'); @@ -1222,7 +1220,7 @@ describe('asar package', function () { }); }); - ifdescribe(features.isRunAsNodeEnabled())('child_process.fork', function () { + describe('child_process.fork', function () { itremote('opens a normal js file', async function () { const child = require('child_process').fork(path.join(asarDir, 'a.asar', 'ping.js')); child.send('message'); @@ -1410,12 +1408,6 @@ describe('asar package', function () { /* describe('process.env.ELECTRON_NO_ASAR', function () { - before(function () { - if (!features.isRunAsNodeEnabled()) { - this.skip(); - } - }); - itremote('disables asar support in forked processes', function (done) { const forked = ChildProcess.fork(path.join(__dirname, 'fixtures', 'module', 'no-asar.js'), [], { env: { @@ -1509,7 +1501,7 @@ describe('asar package', function () { }); /* - ifit(features.isRunAsNodeEnabled())('is available in forked scripts', async function () { + it('is available in forked scripts', async function () { const child = ChildProcess.fork(path.join(fixtures, 'module', 'original-fs.js')); const message = once(child, 'message'); child.send('message'); diff --git a/spec/modules-spec.ts b/spec/modules-spec.ts index a3e0468638a..7204121f4a4 100644 --- a/spec/modules-spec.ts +++ b/spec/modules-spec.ts @@ -9,7 +9,6 @@ import { once } from 'events'; const Module = require('module'); -const features = process._linkedBinding('electron_common_features'); const nativeModulesEnabled = !process.env.ELECTRON_SKIP_NATIVE_MODULE_TESTS; describe('modules support', () => { @@ -28,7 +27,7 @@ describe('modules support', () => { ).to.be.fulfilled(); }); - ifit(features.isRunAsNodeEnabled())('can be required in node binary', async function () { + it('can be required in node binary', async function () { const child = childProcess.fork(path.join(fixtures, 'module', 'echo.js')); const [msg] = await once(child, 'message'); expect(msg).to.equal('ok'); @@ -60,7 +59,7 @@ describe('modules support', () => { await expect(w.webContents.executeJavaScript('{ require(\'@electron-ci/uv-dlopen\'); null }')).to.be.fulfilled(); }); - ifit(features.isRunAsNodeEnabled())('can be required in node binary', async function () { + it('can be required in node binary', async function () { const child = childProcess.fork(path.join(fixtures, 'module', 'uv-dlopen.js')); const [exitCode] = await once(child, 'exit'); expect(exitCode).to.equal(0); diff --git a/spec/node-spec.ts b/spec/node-spec.ts index eac32fa0d7d..1a0b9cfc864 100644 --- a/spec/node-spec.ts +++ b/spec/node-spec.ts @@ -8,7 +8,6 @@ import { webContents } from 'electron/main'; import { EventEmitter } from 'stream'; import { once } from 'events'; -const features = process._linkedBinding('electron_common_features'); const mainFixturesPath = path.resolve(__dirname, 'fixtures'); describe('node feature', () => { @@ -26,7 +25,7 @@ describe('node feature', () => { }); }); - ifdescribe(features.isRunAsNodeEnabled())('child_process in renderer', () => { + describe('child_process in renderer', () => { useRemoteContext(); describe('child_process.fork', () => { @@ -360,7 +359,7 @@ describe('node feature', () => { }); }); - ifdescribe(features.isRunAsNodeEnabled() && process.platform === 'darwin')('net.connect', () => { + ifdescribe(process.platform === 'darwin')('net.connect', () => { itremote('emit error when connect to a socket path without listeners', async (fixtures: string) => { const socketPath = require('path').join(require('os').tmpdir(), 'electron-test.sock'); const script = require('path').join(fixtures, 'module', 'create_socket.js'); @@ -660,7 +659,7 @@ describe('node feature', () => { }); }); - ifdescribe(features.isRunAsNodeEnabled())('Node.js cli flags', () => { + describe('Node.js cli flags', () => { let child: childProcess.ChildProcessWithoutNullStreams; let exitPromise: Promise; @@ -713,7 +712,7 @@ describe('node feature', () => { }); }); - ifdescribe(features.isRunAsNodeEnabled())('inspector', () => { + describe('inspector', () => { let child: childProcess.ChildProcessWithoutNullStreams; let exitPromise: Promise | null; @@ -856,7 +855,7 @@ describe('node feature', () => { expect(result.status).to.equal(0); }); - ifit(features.isRunAsNodeEnabled())('handles Promise timeouts correctly', async () => { + it('handles Promise timeouts correctly', async () => { const scriptPath = path.join(fixtures, 'module', 'node-promise-timer.js'); const child = childProcess.spawn(process.execPath, [scriptPath], { env: { ELECTRON_RUN_AS_NODE: 'true' } diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 38758a97aab..11a73317c69 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -7,7 +7,6 @@ declare namespace NodeJS { isBuiltinSpellCheckerEnabled(): boolean; isOffscreenRenderingEnabled(): boolean; isPDFViewerEnabled(): boolean; - isRunAsNodeEnabled(): boolean; isFakeLocationProviderEnabled(): boolean; isViewApiEnabled(): boolean; isPrintingEnabled(): boolean;