Browse Source

feat: upgrade to Node 12 (#17838) (#18044)

* fix: add boringssl backport to support node upgrade

* fix: Update node_includes.h, add DCHECK macros

* fix: Update node Debug Options parser usage

* fix: Fix asar setup

* fix: using v8Util in isolated context

* fix: make "process" available in preload scripts

* fix: use proper options parser and remove setting of _breakFirstLine

_breakFirstLine was being set on the process, but that has changed in node 12 and so is no longer needed. Node will handle it properly when --inspect-brk is provided

* chore: update node dep sha

* fix: process.binding => _linkedBinding in sandboxed isolated preload

* fix: make original-fs work with streams

* build: override node module version

* fix: use _linkedBinding in content_script/init.js

* chore: update node ref in DEPS

* build: node_module_version should be 73
Samuel Attard 6 years ago
parent
commit
275e277721

+ 1 - 1
DEPS

@@ -12,7 +12,7 @@ vars = {
   'chromium_version':
     '76.0.3774.1',
   'node_version':
-    '2dc0f8811b2b295c08d797b8a11b030234c98502',
+    '696d8fb66d6f65fc82869d390e0d2078970b1eb4',
 
   'boto_version': 'f7574aa6cc2c819430c1f05e9a1a1a666ef8169b',
   'pyyaml_version': '3.12',

+ 3 - 10
atom/browser/node_debugger.cc

@@ -37,13 +37,13 @@ void NodeDebugger::Start() {
   }
 
   node::DebugOptions options;
+  node::options_parser::DebugOptionsParser options_parser;
   std::vector<std::string> exec_args;
   std::vector<std::string> v8_args;
   std::vector<std::string> errors;
 
-  node::options_parser::DebugOptionsParser::instance.Parse(
-      &args, &exec_args, &v8_args, &options,
-      node::options_parser::kDisallowedInEnvironment, &errors);
+  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?
@@ -51,13 +51,6 @@ void NodeDebugger::Start() {
                << base::JoinString(errors, " ");
   }
 
-  // Set process._debugWaitConnect if --inspect-brk was specified to stop
-  // the debugger on the first line
-  if (options.wait_for_connect()) {
-    mate::Dictionary process(env_->isolate(), env_->process_object());
-    process.Set("_breakFirstLine", true);
-  }
-
   const char* path = "";
   if (inspector->Start(path, options,
                        std::make_shared<node::HostPort>(options.host_port),

+ 2 - 7
atom/common/api/atom_api_asar.cc

@@ -117,16 +117,11 @@ class Archive : public mate::Wrappable<Archive> {
   DISALLOW_COPY_AND_ASSIGN(Archive);
 };
 
-void InitAsarSupport(v8::Isolate* isolate,
-                     v8::Local<v8::Value> source,
-                     v8::Local<v8::Value> require) {
+void InitAsarSupport(v8::Isolate* isolate, v8::Local<v8::Value> require) {
   // Evaluate asar_init.js.
   std::vector<v8::Local<v8::String>> asar_init_params = {
-      node::FIXED_ONE_BYTE_STRING(isolate, "source"),
       node::FIXED_ONE_BYTE_STRING(isolate, "require")};
-
-  std::vector<v8::Local<v8::Value>> asar_init_args = {source, require};
-
+  std::vector<v8::Local<v8::Value>> asar_init_args = {require};
   node::per_process::native_module_loader.CompileAndCall(
       isolate->GetCurrentContext(), "electron/js2c/asar_init",
       &asar_init_params, &asar_init_args, nullptr);

+ 21 - 0
atom/common/node_includes.h

@@ -25,6 +25,13 @@
 #pragma push_macro("CHECK_LE")
 #pragma push_macro("CHECK_LT")
 #pragma push_macro("CHECK_NE")
+#pragma push_macro("DCHECK")
+#pragma push_macro("DCHECK_EQ")
+#pragma push_macro("DCHECK_GE")
+#pragma push_macro("DCHECK_GT")
+#pragma push_macro("DCHECK_LE")
+#pragma push_macro("DCHECK_LT")
+#pragma push_macro("DCHECK_NE")
 #pragma push_macro("DISALLOW_COPY_AND_ASSIGN")
 #pragma push_macro("LIKELY")
 #pragma push_macro("NO_RETURN")
@@ -38,6 +45,13 @@
 #undef CHECK_LE
 #undef CHECK_LT
 #undef CHECK_NE
+#undef DCHECK
+#undef DCHECK_EQ
+#undef DCHECK_GE
+#undef DCHECK_GT
+#undef DCHECK_LE
+#undef DCHECK_LT
+#undef DCHECK_NE
 #undef DISALLOW_COPY_AND_ASSIGN
 #undef LIKELY
 #undef NO_RETURN
@@ -67,6 +81,13 @@
 #pragma pop_macro("CHECK_LE")
 #pragma pop_macro("CHECK_LT")
 #pragma pop_macro("CHECK_NE")
+#pragma pop_macro("DCHECK")
+#pragma pop_macro("DCHECK_EQ")
+#pragma pop_macro("DCHECK_GE")
+#pragma pop_macro("DCHECK_GT")
+#pragma pop_macro("DCHECK_LE")
+#pragma pop_macro("DCHECK_LT")
+#pragma pop_macro("DCHECK_NE")
 #pragma pop_macro("DISALLOW_COPY_AND_ASSIGN")
 #pragma pop_macro("LIKELY")
 #pragma pop_macro("NO_RETURN")

+ 3 - 0
atom/renderer/api/atom_api_web_frame.cc

@@ -372,6 +372,9 @@ v8::Local<v8::Promise> ExecuteJavaScriptInIsolatedWorld(
       blink::WebLocalFrame::kSynchronous;
   args->GetNext(&scriptExecutionType);
 
+  // Debugging tip: if you see a crash stack trace beginning from this call,
+  // then it is very likely that some exception happened when executing the
+  // "content_script/init.js" script.
   GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
       world_id, &sources.front(), sources.size(), has_user_gesture,
       scriptExecutionType, new ScriptExecutionCallback(std::move(promise)));

+ 2 - 2
atom/renderer/atom_sandboxed_renderer_client.cc

@@ -245,7 +245,7 @@ void AtomSandboxedRendererClient::SetupMainWorldOverrides(
   auto* isolate = context->GetIsolate();
 
   mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate);
-  process.SetMethod("binding", GetBinding);
+  process.SetMethod("_linkedBinding", GetBinding);
 
   std::vector<v8::Local<v8::String>> isolated_bundle_params = {
       node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"),
@@ -267,7 +267,7 @@ void AtomSandboxedRendererClient::SetupExtensionWorldOverrides(
   auto* isolate = context->GetIsolate();
 
   mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate);
-  process.SetMethod("binding", GetBinding);
+  process.SetMethod("_linkedBinding", GetBinding);
 
   std::vector<v8::Local<v8::String>> isolated_bundle_params = {
       node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"),

+ 3 - 0
build/args/all.gn

@@ -2,6 +2,9 @@ is_electron_build = true
 use_jumbo_build = true
 root_extra_deps = [ "//electron" ]
 
+# Registry of NMVs --> https://github.com/nodejs/node/blob/master/doc/abi_version_registry.json
+node_module_version = 73
+
 v8_promise_internal_field_count = 1
 v8_typed_array_max_size_in_heap = 0
 v8_embedder_string = "-electron.0"

+ 1 - 1
lib/common/asar.js

@@ -1,7 +1,7 @@
 'use strict';
 
 (function () {
-  const asar = process.binding('atom_common_asar')
+  const asar = process._linkedBinding('atom_common_asar')
   const assert = require('assert')
   const { Buffer } = require('buffer')
   const childProcess = require('child_process')

+ 1 - 9
lib/common/asar_init.js

@@ -1,14 +1,6 @@
 'use strict'
 
-/* global source, require */
-
-// Expose fs module without asar support.
-// NB: Node's 'fs' and 'internal/fs/streams' have a lazy-loaded circular
-// dependency. So to expose the unmodified Node 'fs' functionality here,
-// we have to copy both 'fs' *and* 'internal/fs/streams' and modify the
-// copies to depend on each other instead of on our asarified 'fs' code.
-source['original-fs'].replace("require('internal/fs/streams')", "require('original-fs/streams')")
-source['original-fs/streams'].replace("require('fs')", "require('original-fs')")
+/* global require */
 
 // Monkey-patch the fs module.
 require('electron/js2c/asar').wrapFsWithAsar(require('fs'))

+ 1 - 1
lib/content_script/init.js

@@ -4,7 +4,7 @@
 
 const { EventEmitter } = require('events')
 
-process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess.binding, 'renderer')
+process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess._linkedBinding, 'renderer')
 
 const v8Util = process.electronBinding('v8_util')
 

+ 1 - 1
lib/isolated_renderer/init.js

@@ -2,7 +2,7 @@
 
 /* global nodeProcess, isolatedWorld */
 
-process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess.binding, 'renderer')
+process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess._linkedBinding, 'renderer')
 
 const v8Util = process.electronBinding('v8_util')
 

+ 18 - 0
lib/renderer/init.ts

@@ -4,6 +4,24 @@ import * as path from 'path'
 
 const Module = require('module')
 
+// Make sure globals like "process" and "global" are always available in preload
+// scripts even after they are deleted in "loaded" script.
+//
+// Note 1: We rely on a Node patch to actually pass "process" and "global" and
+// other arguments to the wrapper.
+//
+// Note 2: Node introduced a new code path to use native code to wrap module
+// code, which does not work with this hack. However by modifying the
+// "Module.wrapper" we can force Node to use the old code path to wrap module
+// code with JavaScript.
+Module.wrapper = [
+  '(function (exports, require, module, __filename, __dirname, process, global, Buffer) { ' +
+  // By running the code in a new closure, it would be possible for the module
+  // code to override "process" and "Buffer" with local variables.
+  'return function (exports, require, module, __filename, __dirname) { ',
+  '\n}.call(this, exports, require, module, __filename, __dirname); });'
+]
+
 // We modified the original process.argv to let node.js load the
 // init.js, we need to restore it here.
 process.argv.splice(1, 1)

+ 4 - 0
spec/asar-spec.js

@@ -1231,6 +1231,10 @@ describe('asar package', function () {
       })
       child.send('message')
     })
+
+    it('can be used with streams', () => {
+      originalFs.createReadStream(path.join(fixtures, 'asar', 'a.asar'))
+    })
   })
 
   describe('graceful-fs module', function () {