Skip to content

Commit

Permalink
Update against upstream patches
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Jul 15, 2020
1 parent bec5073 commit b55147b
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 31 deletions.
2 changes: 1 addition & 1 deletion patches/node/.patches
Expand Up @@ -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

This file was deleted.

113 changes: 113 additions & 0 deletions 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 <shelley.vohr@gmail.com>
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<Object> 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<Object> types = Object::New(isolate);
NODE_DEFINE_CONSTANT(types, kNoOp);
3 changes: 2 additions & 1 deletion shell/browser/electron_browser_main_parts.cc
Expand Up @@ -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<NodeEnvironment>(env);

// Enable support for v8 inspector
Expand Down
7 changes: 6 additions & 1 deletion shell/common/node_bindings.cc
Expand Up @@ -352,7 +352,8 @@ void NodeBindings::Initialize() {

node::Environment* NodeBindings::CreateEnvironment(
v8::Handle<v8::Context> context,
node::MultiIsolatePlatform* platform) {
node::MultiIsolatePlatform* platform,
bool should_not_register_esm_loader) {
#if defined(OS_WIN)
auto& atom_args = ElectronCommandLine::argv();
std::vector<std::string> args(atom_args.size());
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion shell/common/node_bindings.h
Expand Up @@ -39,7 +39,8 @@ class NodeBindings {

// Create the environment and load node.js.
node::Environment* CreateEnvironment(v8::Handle<v8::Context> context,
node::MultiIsolatePlatform* platform);
node::MultiIsolatePlatform* platform,
bool should_not_register_esm_loader);

// Load node.js in the environment.
void LoadEnvironment(node::Environment* env);
Expand Down
4 changes: 2 additions & 2 deletions shell/renderer/electron_renderer_client.cc
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions shell/renderer/web_worker_observer.cc
Expand Up @@ -51,8 +51,8 @@ void WebWorkerObserver::ContextCreated(v8::Local<v8::Context> 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());
Expand Down

0 comments on commit b55147b

Please sign in to comment.