diff --git a/patches/node/.patches b/patches/node/.patches index a6c2c52cef660..504ae1f97e972 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -41,4 +41,4 @@ test_account_for_non-node_basename.patch lib_src_switch_buffer_kmaxlength_to_size_t.patch update_tests_after_increasing_typed_array_size.patch darwin_work_around_clock_jumping_back_in_time.patch -fix_do_not_register_the_esm_loader_in_renderer_processes.patch +src_allow_embedders_to_disable_esm_loader.patch diff --git a/patches/node/fix_do_not_register_the_esm_loader_in_renderer_processes.patch b/patches/node/fix_do_not_register_the_esm_loader_in_renderer_processes.patch deleted file mode 100644 index 5a6441548e86a..0000000000000 --- a/patches/node/fix_do_not_register_the_esm_loader_in_renderer_processes.patch +++ /dev/null @@ -1,23 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Samuel Attard -Date: Thu, 25 Jun 2020 09:29:04 -0700 -Subject: fix: do not register the ESM loader in renderer processes - -Only one ESM loader can be registered per isolate, in renderer processes this should be blink. This patches node so that it won't register it's handler (overriding blinks) in non-browser processes. - -This can be attempted to be upstreamed as a new option --disable-esm-loader or something similar. - -diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js -index b4a0f71af5853f427a10449b52509052fbe3facd..ba5b0b6e51bcddbc5b9dd9c31231585d61b131a0 100644 ---- a/lib/internal/bootstrap/pre_execution.js -+++ b/lib/internal/bootstrap/pre_execution.js -@@ -411,6 +411,9 @@ function initializeESMLoader() { - // Create this WeakMap in js-land because V8 has no C++ API for WeakMap. - internalBinding('module_wrap').callbackMap = new SafeWeakMap(); - -+ // Do not hook the ESM loader in renderer processes as it overrides blinks loader -+ if (typeof process.type === 'string' && process.type !== 'browser') return; -+ - const { - setImportModuleDynamicallyCallback, - setInitializeImportMetaObjectCallback diff --git a/patches/node/src_allow_embedders_to_disable_esm_loader.patch b/patches/node/src_allow_embedders_to_disable_esm_loader.patch new file mode 100644 index 0000000000000..c31a241c5d5ff --- /dev/null +++ b/patches/node/src_allow_embedders_to_disable_esm_loader.patch @@ -0,0 +1,113 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shelley Vohr +Date: Tue, 14 Jul 2020 16:57:36 -0700 +Subject: src: allow embedders to disable esm loader + +Blink has its own independent ESM loader, which is set up in the +renderer process of Electron. When ESM was unilaterally unflagged, +this meant that there would be two competing ESM loaders in the same process, +both of which made assumptions about code being run. + +Thus, it makes more sense to allow embedders to specify that we don't +want to register the ESM loader when creating an Environment via flag. + +This has already been upstreamed and merged upstream. + +diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js +index b4a0f71af5853f427a10449b52509052fbe3facd..6f1c56af89626ae47a31995ee3d7c6fa96c71b86 100644 +--- a/lib/internal/bootstrap/pre_execution.js ++++ b/lib/internal/bootstrap/pre_execution.js +@@ -6,7 +6,10 @@ const { + SafeWeakMap, + } = primordials; + +-const { getOptionValue } = require('internal/options'); ++const { ++ getOptionValue, ++ shouldNotRegisterESMLoader ++} = require('internal/options'); + const { Buffer } = require('buffer'); + const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes; + const assert = require('internal/assert'); +@@ -411,6 +414,8 @@ function initializeESMLoader() { + // Create this WeakMap in js-land because V8 has no C++ API for WeakMap. + internalBinding('module_wrap').callbackMap = new SafeWeakMap(); + ++ if (shouldNotRegisterESMLoader) return; ++ + const { + setImportModuleDynamicallyCallback, + setInitializeImportMetaObjectCallback +diff --git a/lib/internal/options.js b/lib/internal/options.js +index 03586f9dae6d7690acd7e5e22b6ccbd28dd0752b..10c6aa2d9a0978abcde10477276be6f7a0404219 100644 +--- a/lib/internal/options.js ++++ b/lib/internal/options.js +@@ -1,6 +1,6 @@ + 'use strict'; + +-const { getOptions } = internalBinding('options'); ++const { getOptions, shouldNotRegisterESMLoader } = internalBinding('options'); + const { options, aliases } = getOptions(); + + let warnOnAllowUnauthorized = true; +@@ -32,4 +32,5 @@ module.exports = { + aliases, + getOptionValue, + getAllowUnauthorized, ++ shouldNotRegisterESMLoader + }; +diff --git a/src/env-inl.h b/src/env-inl.h +index 222555831aa1bf0b7b29b4b46e235c98a5dd4ac5..eaebe683b3308ca0e5475b761eb5b32aa85609ff 100644 +--- a/src/env-inl.h ++++ b/src/env-inl.h +@@ -802,6 +802,14 @@ inline bool Environment::is_main_thread() const { + return flags_ & kIsMainThread; + } + ++inline bool Environment::should_not_register_esm_loader() const { ++ return flags_ & kNoRegisterESMLoader; ++} ++ ++inline bool Environment::set_should_not_register_esm_loader() const { ++ flags_ |= kNoRegisterESMLoader; ++} ++ + inline bool Environment::owns_process_state() const { + return flags_ & kOwnsProcessState; + } +diff --git a/src/env.h b/src/env.h +index c1966a9f55126bdd65d8c9d529d934977bb4ad65..7a5ff132a75763d1cf256d34e4dcee105cf44007 100644 +--- a/src/env.h ++++ b/src/env.h +@@ -866,6 +866,7 @@ class Environment : public MemoryRetainer { + kIsMainThread = 1 << 0, + kOwnsProcessState = 1 << 1, + kOwnsInspector = 1 << 2, ++ kNoRegisterESMLoader = 1 << 3 + }; + + static inline Environment* GetCurrent(v8::Isolate* isolate); +@@ -1066,6 +1067,7 @@ class Environment : public MemoryRetainer { + static constexpr uint64_t kNoThreadId = -1; + + inline bool is_main_thread() const; ++ inline bool should_not_register_esm_loader() const; + inline bool owns_process_state() const; + inline bool owns_inspector() const; + inline uint64_t thread_id() const; +diff --git a/src/node_options.cc b/src/node_options.cc +index 8b3a161c2d2f83d6c061fb05f854e06f6f0294ab..34d16e6ac36182f2edc335c26c134bf056864c85 100644 +--- a/src/node_options.cc ++++ b/src/node_options.cc +@@ -978,6 +978,11 @@ void Initialize(Local target, + ->Set( + context, FIXED_ONE_BYTE_STRING(isolate, "envSettings"), env_settings) + .Check(); ++ target ++ ->Set(context, ++ FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"), ++ Boolean::New(isolate, env->should_not_register_esm_loader())) ++ .Check(); + + Local types = Object::New(isolate); + NODE_DEFINE_CONSTANT(types, kNoOp); diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index c1ddc4a622723..7ae8f0a72b40d 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -318,7 +318,8 @@ void ElectronBrowserMainParts::PostEarlyInitialization() { node_bindings_->Initialize(); // Create the global environment. node::Environment* env = node_bindings_->CreateEnvironment( - js_env_->context(), js_env_->platform()); + js_env_->context(), js_env_->platform(), + false /* should_not_register_esm_loader */); node_env_ = std::make_unique(env); // Enable support for v8 inspector diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 3e31ed9fb90ae..471c754232a62 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -352,7 +352,8 @@ void NodeBindings::Initialize() { node::Environment* NodeBindings::CreateEnvironment( v8::Handle context, - node::MultiIsolatePlatform* platform) { + node::MultiIsolatePlatform* platform, + bool should_not_register_esm_loader) { #if defined(OS_WIN) auto& atom_args = ElectronCommandLine::argv(); std::vector args(atom_args.size()); @@ -395,6 +396,10 @@ node::Environment* NodeBindings::CreateEnvironment( isolate_data_, context, args.size(), c_argv.get(), 0, nullptr); DCHECK(env); + if (should_not_register_esm_loader) { + env->set_should_not_register_esm_loader(); + } + // Clean up the global _noBrowserGlobals that we unironically injected into // the global scope if (browser_env_ != BrowserEnvironment::BROWSER) { diff --git a/shell/common/node_bindings.h b/shell/common/node_bindings.h index bf737a51ddffb..5f4c5422ac634 100644 --- a/shell/common/node_bindings.h +++ b/shell/common/node_bindings.h @@ -39,7 +39,8 @@ class NodeBindings { // Create the environment and load node.js. node::Environment* CreateEnvironment(v8::Handle context, - node::MultiIsolatePlatform* platform); + node::MultiIsolatePlatform* platform, + bool should_not_register_esm_loader); // Load node.js in the environment. void LoadEnvironment(node::Environment* env); diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index 3901eb23bad36..4cd3ba1c40794 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -124,8 +124,8 @@ void ElectronRendererClient::DidCreateScriptContext( bool initialized = node::InitializeContext(renderer_context); CHECK(initialized); - node::Environment* env = - node_bindings_->CreateEnvironment(renderer_context, nullptr); + node::Environment* env = node_bindings_->CreateEnvironment( + renderer_context, nullptr, true /*bool should_not_register_esm_loader */); // If we have disabled the site instance overrides we should prevent loading // any non-context aware native module diff --git a/shell/renderer/web_worker_observer.cc b/shell/renderer/web_worker_observer.cc index a52d3305802f2..b0f455a931485 100644 --- a/shell/renderer/web_worker_observer.cc +++ b/shell/renderer/web_worker_observer.cc @@ -51,8 +51,8 @@ void WebWorkerObserver::ContextCreated(v8::Local worker_context) { // Setup node environment for each window. bool initialized = node::InitializeContext(worker_context); CHECK(initialized); - node::Environment* env = - node_bindings_->CreateEnvironment(worker_context, nullptr); + node::Environment* env = node_bindings_->CreateEnvironment( + worker_context, nullptr, true /* should_not_register_esm_loader */); // Add Electron extended APIs. electron_bindings_->BindTo(env->isolate(), env->process_object());