Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: remove enable_run_as_node build flag #38413

Merged
merged 4 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 3 additions & 26 deletions .circleci/config/base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,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'
Expand Down Expand Up @@ -256,7 +253,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', []):
Expand Down Expand Up @@ -1042,7 +1039,7 @@ commands:
artifact-key:
type: string
build-type:
type: string
type: string
build-nonproprietary-ffmpeg:
type: boolean
default: true
Expand Down Expand Up @@ -1286,7 +1283,7 @@ commands:
artifact-key:
type: string
build-type:
type: string
type: string
after-build-and-save:
type: steps
default: []
Expand Down Expand Up @@ -1721,23 +1718,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
Expand Down Expand Up @@ -2222,9 +2202,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
Expand Down
7 changes: 0 additions & 7 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -672,13 +672,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",
Expand Down
1 change: 0 additions & 1 deletion buildflags/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 0 additions & 3 deletions buildflags/buildflags.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions filenames.gni
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,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",
Expand Down
3 changes: 0 additions & 3 deletions shell/app/electron_library_main.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 0 additions & 2 deletions shell/app/electron_library_main.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -43,4 +42,3 @@ int ElectronInitializeICUandStartNode(int argc, char* argv[]) {
base::i18n::InitializeICU();
return electron::NodeMain(argc, argv);
}
#endif
15 changes: 10 additions & 5 deletions shell/app/electron_main_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 1 addition & 3 deletions shell/app/electron_main_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
Expand All @@ -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;
Expand Down
10 changes: 2 additions & 8 deletions shell/app/electron_main_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"))
Expand All @@ -164,15 +160,13 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
std::vector<char*> 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 =
Expand Down
6 changes: 0 additions & 6 deletions shell/common/api/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -22,10 +21,6 @@ bool IsPDFViewerEnabled() {
return BUILDFLAG(ENABLE_PDF_VIEWER);
}

bool IsRunAsNodeEnabled() {
return electron::fuses::IsRunAsNodeEnabled() && BUILDFLAG(ENABLE_RUN_AS_NODE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this the feature api is still needed so that tests can be excluded based on the fuse value ?

Copy link
Contributor Author

@miniak miniak Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't have infra to test different fuse values unfortunately right now, it would make sense to add that as none of the other fuses are being tested

}

bool IsFakeLocationProviderEnabled() {
return BUILDFLAG(OVERRIDE_LOCATION_PROVIDER);
}
Expand Down Expand Up @@ -62,7 +57,6 @@ void Initialize(v8::Local<v8::Object> 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);
Expand Down
10 changes: 2 additions & 8 deletions shell/common/crash_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,8 @@ void GetCrashKeys(std::map<std::string, std::string>* 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

Expand Down
2 changes: 0 additions & 2 deletions shell/common/electron_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,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";
Expand Down
2 changes: 0 additions & 2 deletions shell/common/electron_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,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[];
Expand Down
12 changes: 2 additions & 10 deletions spec/asar-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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');
Expand Down
5 changes: 2 additions & 3 deletions spec/modules-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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');
Expand Down Expand Up @@ -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);
Expand Down