From aa211c6c506af041ee2d3f6ed6716e951423291a Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 15 Jul 2019 18:58:39 -0700 Subject: [PATCH] chore: update to Node.js v12.4.0 (#18924) * chore: update to node 12.4.0 * chore: fix js2c compilation and usage * update branch reference * chore: roll node * refactor: use the new node::options_parser::Parse method * fix: make node create our context for us so that everything is initialized correctly * fix: let node do it's thing to the all contexts We need to let node know about all the contexts that Chromium creates for the renderer processes so that it does not crash when trying to access primordials. Similar to node::NewContext but with an existing context * chore: roll node * chore: roll node * chore: roll node * chore: roll node * fix: ensure that _noBrowserGlobals is set before the node bootstrapper runs Co-authored-by: Jeremy Apthorp --- BUILD.gn | 2 +- DEPS | 2 +- shell/browser/javascript_environment.cc | 2 +- shell/browser/node_debugger.cc | 6 +++--- shell/common/api/atom_api_asar.cc | 4 ++-- shell/common/node_bindings.cc | 17 ++++++++++++++--- shell/renderer/atom_renderer_client.cc | 13 ++++++++----- .../renderer/atom_sandboxed_renderer_client.cc | 8 ++++---- shell/renderer/web_worker_observer.cc | 6 ++++-- tools/js2c.py | 4 ++-- 10 files changed, 40 insertions(+), 24 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 2f3b312df4..c834fb1f5d 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -176,7 +176,7 @@ action("atom_js2c") { "$target_gen_dir/js2c/asar_init.js", ] - inputs = sources + inputs = sources + [ "//third_party/electron_node/tools/js2c.py" ] outputs = [ "$root_gen_dir/atom_natives.cc", ] diff --git a/DEPS b/DEPS index 71f227d77e..b9589c4812 100644 --- a/DEPS +++ b/DEPS @@ -12,7 +12,7 @@ vars = { 'chromium_version': '5a48e127c8cb8ae827f4fead0b527079194b9899', 'node_version': - '780436005ffc7f317abfba48b236428858284e99', + 'cbb519597cf085e7d759080c6a6b8f1c4dab85bc', 'nan_version': '2ee313aaca52e2b478965ac50eb5082520380d1b', diff --git a/shell/browser/javascript_environment.cc b/shell/browser/javascript_environment.cc index 5b1306e727..5a5fce8741 100644 --- a/shell/browser/javascript_environment.cc +++ b/shell/browser/javascript_environment.cc @@ -30,7 +30,7 @@ JavascriptEnvironment::JavascriptEnvironment(uv_loop_t* event_loop) isolate_scope_(isolate_), locker_(isolate_), handle_scope_(isolate_), - context_(isolate_, v8::Context::New(isolate_)), + context_(isolate_, node::NewContext(isolate_)), context_scope_(v8::Local::New(isolate_, context_)) {} JavascriptEnvironment::~JavascriptEnvironment() = default; diff --git a/shell/browser/node_debugger.cc b/shell/browser/node_debugger.cc index d8669c30d2..188076c6a2 100644 --- a/shell/browser/node_debugger.cc +++ b/shell/browser/node_debugger.cc @@ -37,13 +37,13 @@ void NodeDebugger::Start() { } node::DebugOptions options; - node::options_parser::DebugOptionsParser options_parser; std::vector exec_args; std::vector v8_args; std::vector errors; - options_parser.Parse(&args, &exec_args, &v8_args, &options, - node::options_parser::kDisallowedInEnvironment, &errors); + node::options_parser::Parse(&args, &exec_args, &v8_args, &options, + node::options_parser::kDisallowedInEnvironment, + &errors); if (!errors.empty()) { // TODO(jeremy): what's the appropriate behaviour here? diff --git a/shell/common/api/atom_api_asar.cc b/shell/common/api/atom_api_asar.cc index 2de787ce50..84589d8c08 100644 --- a/shell/common/api/atom_api_asar.cc +++ b/shell/common/api/atom_api_asar.cc @@ -14,7 +14,7 @@ #include "shell/common/native_mate_converters/callback.h" #include "shell/common/native_mate_converters/file_path_converter.h" #include "shell/common/node_includes.h" -#include "third_party/electron_node/src/node_native_module.h" +#include "third_party/electron_node/src/node_native_module_env.h" namespace { @@ -122,7 +122,7 @@ void InitAsarSupport(v8::Isolate* isolate, v8::Local require) { std::vector> asar_init_params = { node::FIXED_ONE_BYTE_STRING(isolate, "require")}; std::vector> asar_init_args = {require}; - node::per_process::native_module_loader.CompileAndCall( + node::native_module::NativeModuleEnv::CompileAndCall( isolate->GetCurrentContext(), "electron/js2c/asar_init", &asar_init_params, &asar_init_args, nullptr); } diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index a78c346316..9c3f13b019 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -311,6 +311,14 @@ node::Environment* NodeBindings::CreateEnvironment( process_type = "worker"; break; } + + mate::Dictionary global(context->GetIsolate(), context->Global()); + // Do not set DOM globals for renderer process. + // We must set this before the node bootstrapper which is run inside + // CreateEnvironment + if (browser_env_ != BrowserEnvironment::BROWSER) + global.Set("_noBrowserGlobals", true); + base::FilePath resources_path = GetResourcesPath(); std::string init_script = "electron/js2c/" + process_type + "_init"; @@ -320,6 +328,12 @@ node::Environment* NodeBindings::CreateEnvironment( node::Environment* env = node::CreateEnvironment( node::CreateIsolateData(context->GetIsolate(), uv_loop_, platform), context, args.size(), c_argv.get(), 0, nullptr); + DCHECK(env); + + // Clean up the global _noBrowserGlobals that we unironically injected into + // the global scope + if (browser_env_ != BrowserEnvironment::BROWSER) + global.Delete("_noBrowserGlobals"); if (browser_env_ == BrowserEnvironment::BROWSER) { // SetAutorunMicrotasks is no longer called in node::CreateEnvironment @@ -334,9 +348,6 @@ node::Environment* NodeBindings::CreateEnvironment( mate::Dictionary process(context->GetIsolate(), env->process_object()); process.SetReadOnly("type", process_type); process.Set("resourcesPath", resources_path); - // Do not set DOM globals for renderer process. - if (browser_env_ != BrowserEnvironment::BROWSER) - process.Set("_noBrowserGlobals", true); // The path to helper app. base::FilePath helper_exec_path; base::PathService::Get(content::CHILD_PROCESS_EXE, &helper_exec_path); diff --git a/shell/renderer/atom_renderer_client.cc b/shell/renderer/atom_renderer_client.cc index f8b0c0c02e..a25ce6c68e 100644 --- a/shell/renderer/atom_renderer_client.cc +++ b/shell/renderer/atom_renderer_client.cc @@ -20,7 +20,7 @@ #include "shell/renderer/web_worker_observer.h" #include "third_party/blink/public/web/web_document.h" #include "third_party/blink/public/web/web_local_frame.h" -#include "third_party/electron_node/src/node_native_module.h" +#include "third_party/electron_node/src/node_native_module_env.h" namespace electron { @@ -67,9 +67,9 @@ void AtomRendererClient::RunScriptsAtDocumentEnd( } void AtomRendererClient::DidCreateScriptContext( - v8::Handle context, + v8::Handle renderer_context, content::RenderFrame* render_frame) { - RendererClientBase::DidCreateScriptContext(context, render_frame); + RendererClientBase::DidCreateScriptContext(renderer_context, render_frame); // TODO(zcbenz): Do not create Node environment if node integration is not // enabled. @@ -103,6 +103,9 @@ void AtomRendererClient::DidCreateScriptContext( node::tracing::TraceEventHelper::SetAgent(node::CreateAgent()); // Setup node environment for each window. + v8::Local context = + node::MaybeInitializeContext(renderer_context); + DCHECK(!context.IsEmpty()); node::Environment* env = node_bindings_->CreateEnvironment(context); auto* command_line = base::CommandLine::ForCurrentProcess(); // If we have disabled the site instance overrides we should prevent loading @@ -209,7 +212,7 @@ void AtomRendererClient::SetupMainWorldOverrides( env->process_object(), GetContext(render_frame->GetWebFrame(), isolate)->Global()}; - node::per_process::native_module_loader.CompileAndCall( + node::native_module::NativeModuleEnv::CompileAndCall( context, "electron/js2c/isolated_bundle", &isolated_bundle_params, &isolated_bundle_args, nullptr); } @@ -234,7 +237,7 @@ void AtomRendererClient::SetupExtensionWorldOverrides( GetContext(render_frame->GetWebFrame(), isolate)->Global(), v8::Integer::New(isolate, world_id)}; - node::per_process::native_module_loader.CompileAndCall( + node::native_module::NativeModuleEnv::CompileAndCall( context, "electron/js2c/content_script_bundle", &isolated_bundle_params, &isolated_bundle_args, nullptr); } diff --git a/shell/renderer/atom_sandboxed_renderer_client.cc b/shell/renderer/atom_sandboxed_renderer_client.cc index 304eb3db73..0711125789 100644 --- a/shell/renderer/atom_sandboxed_renderer_client.cc +++ b/shell/renderer/atom_sandboxed_renderer_client.cc @@ -23,7 +23,7 @@ #include "third_party/blink/public/web/blink.h" #include "third_party/blink/public/web/web_document.h" #include "third_party/electron_node/src/node_binding.h" -#include "third_party/electron_node/src/node_native_module.h" +#include "third_party/electron_node/src/node_native_module_env.h" namespace electron { @@ -226,7 +226,7 @@ void AtomSandboxedRendererClient::DidCreateScriptContext( std::vector> sandbox_preload_bundle_args = {binding}; - node::per_process::native_module_loader.CompileAndCall( + node::native_module::NativeModuleEnv::CompileAndCall( isolate->GetCurrentContext(), "electron/js2c/sandbox_bundle", &sandbox_preload_bundle_params, &sandbox_preload_bundle_args, nullptr); @@ -254,7 +254,7 @@ void AtomSandboxedRendererClient::SetupMainWorldOverrides( process.GetHandle(), GetContext(render_frame->GetWebFrame(), isolate)->Global()}; - node::per_process::native_module_loader.CompileAndCall( + node::native_module::NativeModuleEnv::CompileAndCall( context, "electron/js2c/isolated_bundle", &isolated_bundle_params, &isolated_bundle_args, nullptr); } @@ -278,7 +278,7 @@ void AtomSandboxedRendererClient::SetupExtensionWorldOverrides( GetContext(render_frame->GetWebFrame(), isolate)->Global(), v8::Integer::New(isolate, world_id)}; - node::per_process::native_module_loader.CompileAndCall( + node::native_module::NativeModuleEnv::CompileAndCall( context, "electron/js2c/content_script_bundle", &isolated_bundle_params, &isolated_bundle_args, nullptr); } diff --git a/shell/renderer/web_worker_observer.cc b/shell/renderer/web_worker_observer.cc index 7cf3d0d8ae..f05d5f035d 100644 --- a/shell/renderer/web_worker_observer.cc +++ b/shell/renderer/web_worker_observer.cc @@ -41,13 +41,15 @@ WebWorkerObserver::~WebWorkerObserver() { asar::ClearArchives(); } -void WebWorkerObserver::ContextCreated(v8::Local context) { - v8::Context::Scope context_scope(context); +void WebWorkerObserver::ContextCreated(v8::Local worker_context) { + v8::Context::Scope context_scope(worker_context); // Start the embed thread. node_bindings_->PrepareMessageLoop(); // Setup node environment for each window. + v8::Local context = node::MaybeInitializeContext(worker_context); + DCHECK(!context.IsEmpty()); node::Environment* env = node_bindings_->CreateEnvironment(context); // Add Electron extended APIs. diff --git a/tools/js2c.py b/tools/js2c.py index dac5b3cb8c..0f7178baf0 100755 --- a/tools/js2c.py +++ b/tools/js2c.py @@ -30,9 +30,9 @@ def main(): js2c = os.path.join(node_path, 'tools', 'js2c.py') subprocess.check_call( - [sys.executable, js2c, natives] + + [sys.executable, js2c] + js_source_files + - ['-t', TEMPLATE]) + ['--only-js', '--target', natives]) if __name__ == '__main__':