diff --git a/patches/node/.patches b/patches/node/.patches index b57a0b421473..a7e1565d983a 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -7,7 +7,6 @@ expose_get_builtin_module_function.patch fix_build_and_expose_inspector_agent.patch fix_expose_internalcallbackscope.patch build_add_gn_build_files.patch -fix_export_debugoptions.patch fix_add_default_values_for_enable_lto_and_build_v8_with_gn_in.patch feat_add_new_built_with_electron_variable_to_config_gypi.patch feat_add_flags_for_low-level_hooks_and_exceptions.patch @@ -18,7 +17,6 @@ fixme_use_redefined_version_of_internalmodulestat.patch fixme_remove_async_id_assertion_check.patch fixme_comment_trace_event_macro.patch fix_key_gen_apis_are_not_available_in_boringssl.patch -fix_do_not_define_debugoptions_s_constructors_in_header.patch build_modify_js2c_py_to_allow_injection_of_original-fs_and_custom_embedder_js.patch refactor_allow_embedder_overriding_of_internal_fs_calls.patch chore_prevent_warn_non_context-aware_native_modules_being_loaded.patch diff --git a/patches/node/fix_do_not_define_debugoptions_s_constructors_in_header.patch b/patches/node/fix_do_not_define_debugoptions_s_constructors_in_header.patch deleted file mode 100644 index 7365c891f305..000000000000 --- a/patches/node/fix_do_not_define_debugoptions_s_constructors_in_header.patch +++ /dev/null @@ -1,54 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Cheng Zhao -Date: Fri, 29 Mar 2019 17:17:30 +0900 -Subject: fix: do not define DebugOptions's constructors in header - -The e3ca89ef49 changed NODE_EXTERN to always expose symbols for -DebugOptions, and since the constructors of DebugOptions are defined in -header, the compiler would end up generating no constructor -implementations for DebugOptions. - -Which means we would encounter crash when constructors of DebugOptions -is called. - -By putting the definitions of constructors into the implementation file, -we can avoid this problem. - -diff --git a/src/node_options.cc b/src/node_options.cc -index 2dd90e9a8cf9381c8608b73ead247bcf86b72ab9..051e7b084b6770215bd009b0e37feabbb7df7389 100644 ---- a/src/node_options.cc -+++ b/src/node_options.cc -@@ -26,6 +26,12 @@ Mutex cli_options_mutex; - std::shared_ptr cli_options{new PerProcessOptions()}; - } // namespace per_process - -+DebugOptions::DebugOptions() = default; -+DebugOptions::DebugOptions(const DebugOptions&) = default; -+DebugOptions::DebugOptions(DebugOptions&&) = default; -+DebugOptions& DebugOptions::operator=(const DebugOptions&) = default; -+DebugOptions& DebugOptions::operator=(DebugOptions&&) = default; -+ - void DebugOptions::CheckOptions(std::vector* errors) { - #if !NODE_USE_V8_PLATFORM && !HAVE_INSPECTOR - if (inspector_enabled) { -diff --git a/src/node_options.h b/src/node_options.h -index 40c19ea6ff4d98a1a1da59bca76087209445af81..4ce5551284bb5b1b4194905a9fe619f852933405 100644 ---- a/src/node_options.h -+++ b/src/node_options.h -@@ -61,11 +61,11 @@ struct InspectPublishUid { - // per-Isolate, rather than per-Environment. - class NODE_EXTERN DebugOptions : public Options { - public: -- DebugOptions() = default; -- DebugOptions(const DebugOptions&) = default; -- DebugOptions& operator=(const DebugOptions&) = default; -- DebugOptions(DebugOptions&&) = default; -- DebugOptions& operator=(DebugOptions&&) = default; -+ DebugOptions(); -+ DebugOptions(const DebugOptions&); -+ DebugOptions& operator=(const DebugOptions&); -+ DebugOptions(DebugOptions&&); -+ DebugOptions& operator=(DebugOptions&&); - - // --inspect - bool inspector_enabled = false; diff --git a/patches/node/fix_export_debugoptions.patch b/patches/node/fix_export_debugoptions.patch deleted file mode 100644 index 8fc168d6cfa9..000000000000 --- a/patches/node/fix_export_debugoptions.patch +++ /dev/null @@ -1,57 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Nitish Sakhawalkar -Date: Thu, 11 Apr 2019 11:50:49 -0700 -Subject: fix: export DebugOptions - -This exports DebugOptions so we can parse args like `--inspect-brk`. - -diff --git a/src/node_options.cc b/src/node_options.cc -index 35cac18a916c2ce104a3e7b6760d0280b33c2892..2dd90e9a8cf9381c8608b73ead247bcf86b72ab9 100644 ---- a/src/node_options.cc -+++ b/src/node_options.cc -@@ -210,11 +210,6 @@ void EnvironmentOptions::CheckOptions(std::vector* errors) { - - namespace options_parser { - --class DebugOptionsParser : public OptionsParser { -- public: -- DebugOptionsParser(); --}; -- - class EnvironmentOptionsParser : public OptionsParser { - public: - EnvironmentOptionsParser(); -diff --git a/src/node_options.h b/src/node_options.h -index 8937bfd9011e4795d22e232886e18183d698b8d4..40c19ea6ff4d98a1a1da59bca76087209445af81 100644 ---- a/src/node_options.h -+++ b/src/node_options.h -@@ -59,7 +59,7 @@ struct InspectPublishUid { - // to keep them separate since they are a group of options applying to a very - // specific part of Node. It might also make more sense for them to be - // per-Isolate, rather than per-Environment. --class DebugOptions : public Options { -+class NODE_EXTERN DebugOptions : public Options { - public: - DebugOptions() = default; - DebugOptions(const DebugOptions&) = default; -@@ -244,7 +244,7 @@ class PerProcessOptions : public Options { - - namespace options_parser { - --HostPort SplitHostPort(const std::string& arg, -+HostPort NODE_EXTERN SplitHostPort(const std::string& arg, - std::vector* errors); - void GetOptions(const v8::FunctionCallbackInfo& args); - -@@ -437,6 +437,11 @@ class OptionsParser { - friend void GetOptions(const v8::FunctionCallbackInfo& args); - }; - -+class NODE_EXTERN DebugOptionsParser : public OptionsParser { -+ public: -+ DebugOptionsParser(); -+}; -+ - using StringVector = std::vector; - template - void Parse( diff --git a/shell/browser/node_debugger.cc b/shell/browser/node_debugger.cc index 9e3027311803..d1884da750d5 100644 --- a/shell/browser/node_debugger.cc +++ b/shell/browser/node_debugger.cc @@ -27,34 +27,13 @@ void NodeDebugger::Start() { if (inspector == nullptr) return; - std::vector args; - for (auto& arg : base::CommandLine::ForCurrentProcess()->argv()) { -#if defined(OS_WIN) - args.push_back(base::UTF16ToUTF8(arg)); -#else - args.push_back(arg); -#endif - } - - node::DebugOptions options; - std::vector exec_args; - std::vector v8_args; - std::vector errors; - - // TODO(codebytere): remove this parsing and use ProcessGlobalArgs - node::options_parser::Parse(&args, &exec_args, &v8_args, &options, - node::kDisallowedInEnvironment, &errors); - - if (!errors.empty()) { - // TODO(jeremy): what's the appropriate behaviour here? - LOG(ERROR) << "Error parsing node options: " - << base::JoinString(errors, " "); - } - - const char* path = ""; - if (inspector->Start(path, options, - std::make_shared(options.host_port), - true /* is_main */)) + // DebugOptions will already have been set by ProcessGlobalArgs, + // so just pull the ones from there to pass to the InspectorAgent + const auto debug_options = env_->options()->debug_options(); + if (inspector->Start( + "" /* path */, debug_options, + std::make_shared(debug_options.host_port), + true /* is_main */)) DCHECK(env_->inspector_agent()->IsListening()); } diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index e722dec2eb09..487a79077729 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -164,7 +164,9 @@ void SetNodeCliFlags() { const auto& option = arg; #endif const auto stripped = base::StringPiece(option).substr(0, option.find('=')); - if (allowed.count(stripped) != 0) + + // Only allow in no-op (--) option or DebugOptions + if (allowed.count(stripped) != 0 || stripped == "--") args.push_back(option); } @@ -172,13 +174,13 @@ void SetNodeCliFlags() { const int exit_code = ProcessGlobalArgs(&args, nullptr, &errors, node::kDisallowedInEnvironment); + const std::string err_str = "Error parsing Node.js cli flags "; if (exit_code != 0) { - if (!errors.empty()) - LOG(INFO) << base::JoinString(errors, " "); - else - LOG(INFO) << "Error parsing Node.js cli flags"; + LOG(ERROR) << err_str; + } else if (!errors.empty()) { + LOG(ERROR) << err_str << base::JoinString(errors, " "); } -} +} // namespace // Initialize NODE_OPTIONS to pass to Node.js // See https://nodejs.org/api/cli.html#cli_node_options_options diff --git a/shell/common/node_includes.h b/shell/common/node_includes.h index a8d68dfacdcb..4789a4f93d08 100644 --- a/shell/common/node_includes.h +++ b/shell/common/node_includes.h @@ -64,6 +64,7 @@ #include "node.h" #include "node_buffer.h" #include "node_internals.h" +#include "node_options-inl.h" #include "node_options.h" #include "node_platform.h"