From 3f48ab30420981f888ff2c69fc1ea5abb37f3f2e Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Tue, 25 Apr 2017 14:55:55 -0700 Subject: [PATCH] inspector: do not add 'inspector' property 'inspector' property is not an official API and should not be published on process object, where the user may discover it. This change was extracted from https://github.com/nodejs/node/pull/12263 that will be focused on creating JS bindings. PR-URL: https://github.com/nodejs/node/pull/12656 Reviewed-By: Aleksey Kozyatinskiy Reviewed-By: James M Snell Reviewed-By: Timothy Gu --- lib/internal/bootstrap_node.js | 45 +++----- lib/module.js | 15 +-- src/env.h | 1 + src/inspector_agent.cc | 191 ++++++++++++++------------------- src/inspector_agent.h | 18 ++-- src/node.cc | 6 ++ 6 files changed, 115 insertions(+), 161 deletions(-) diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 038e71502bdb1a..8159d8c990e261 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -254,52 +254,41 @@ } function setupGlobalConsole() { - var inspectorConsole; - var wrapConsoleCall; - if (process.inspector) { - inspectorConsole = global.console; - wrapConsoleCall = process.inspector.wrapConsoleCall; - delete process.inspector.wrapConsoleCall; - if (Object.keys(process.inspector).length === 0) - delete process.inspector; - } - var console; + const originalConsole = global.console; + let console; Object.defineProperty(global, 'console', { configurable: true, enumerable: true, get: function() { if (!console) { - console = NativeModule.require('console'); - installInspectorConsoleIfNeeded(console, - inspectorConsole, - wrapConsoleCall); + console = installInspectorConsole(originalConsole); } return console; } }); } - function installInspectorConsoleIfNeeded(console, - inspectorConsole, - wrapConsoleCall) { - if (!inspectorConsole) - return; + function installInspectorConsole(globalConsole) { + const wrappedConsole = NativeModule.require('console'); + const inspector = process.binding('inspector'); const config = {}; - for (const key of Object.keys(console)) { - if (!inspectorConsole.hasOwnProperty(key)) + for (const key of Object.keys(wrappedConsole)) { + if (!globalConsole.hasOwnProperty(key)) continue; - // If node console has the same method as inspector console, + // If global console has the same method as inspector console, // then wrap these two methods into one. Native wrapper will preserve // the original stack. - console[key] = wrapConsoleCall(inspectorConsole[key], - console[key], - config); + wrappedConsole[key] = inspector.consoleCall.bind(wrappedConsole, + globalConsole[key], + wrappedConsole[key], + config); } - for (const key of Object.keys(inspectorConsole)) { - if (console.hasOwnProperty(key)) + for (const key of Object.keys(globalConsole)) { + if (wrappedConsole.hasOwnProperty(key)) continue; - console[key] = inspectorConsole[key]; + wrappedConsole[key] = globalConsole[key]; } + return wrappedConsole; } function setupProcessFatal() { diff --git a/lib/module.js b/lib/module.js index 55755d9ac42da9..5455e68dd98b22 100644 --- a/lib/module.js +++ b/lib/module.js @@ -472,19 +472,6 @@ function tryModuleLoad(module, filename) { } } -function getInspectorCallWrapper() { - var inspector = process.inspector; - if (!inspector || !inspector.callAndPauseOnStart) { - return null; - } - var wrapper = inspector.callAndPauseOnStart.bind(inspector); - delete inspector.callAndPauseOnStart; - if (Object.keys(process.inspector).length === 0) { - delete process.inspector; - } - return wrapper; -} - Module._resolveFilename = function(request, parent, isMain) { if (NativeModule.nonInternalExists(request)) { return request; @@ -563,7 +550,7 @@ Module.prototype._compile = function(content, filename) { // Set breakpoint on module start if (filename === resolvedArgv) { delete process._debugWaitConnect; - inspectorWrapper = getInspectorCallWrapper(); + inspectorWrapper = process.binding('inspector').callAndPauseOnStart; if (!inspectorWrapper) { const Debug = vm.runInDebugContext('Debug'); Debug.setBreakPoint(compiledWrapper, 0, 0); diff --git a/src/env.h b/src/env.h index 3946e91e1c3ca0..1010d649c6408a 100644 --- a/src/env.h +++ b/src/env.h @@ -72,6 +72,7 @@ namespace node { V(arrow_message_private_symbol, "node:arrowMessage") \ V(contextify_context_private_symbol, "node:contextify:context") \ V(contextify_global_private_symbol, "node:contextify:global") \ + V(inspector_delegate_private_symbol, "node:inspector:delegate") \ V(decorated_private_symbol, "node:decorated") \ V(npn_buffer_private_symbol, "node:npnBuffer") \ V(processed_private_symbol, "node:processed") \ diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index d8c7d1c874d761..3a8364e4fa54e4 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -154,8 +154,61 @@ static int RegisterDebugSignalHandler() { return 0; } #endif // _WIN32 -} // namespace +void InspectorConsoleCall(const v8::FunctionCallbackInfo& info) { + Isolate* isolate = info.GetIsolate(); + HandleScope handle_scope(isolate); + Local context = isolate->GetCurrentContext(); + CHECK_LT(2, info.Length()); + std::vector> call_args; + for (int i = 3; i < info.Length(); ++i) { + call_args.push_back(info[i]); + } + Environment* env = Environment::GetCurrent(isolate); + if (env->inspector_agent()->enabled()) { + Local inspector_method = info[0]; + CHECK(inspector_method->IsFunction()); + Local config_value = info[2]; + CHECK(config_value->IsObject()); + Local config_object = config_value.As(); + Local in_call_key = FIXED_ONE_BYTE_STRING(isolate, "in_call"); + if (!config_object->Has(context, in_call_key).FromMaybe(false)) { + CHECK(config_object->Set(context, + in_call_key, + v8::True(isolate)).FromJust()); + CHECK(!inspector_method.As()->Call(context, + info.Holder(), + call_args.size(), + call_args.data()).IsEmpty()); + } + CHECK(config_object->Delete(context, in_call_key).FromJust()); + } + + Local node_method = info[1]; + CHECK(node_method->IsFunction()); + static_cast(node_method.As()->Call(context, + info.Holder(), + call_args.size(), + call_args.data())); +} + +void CallAndPauseOnStart( + const v8::FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + CHECK_GT(args.Length(), 1); + CHECK(args[0]->IsFunction()); + std::vector> call_args; + for (int i = 2; i < args.Length(); i++) { + call_args.push_back(args[i]); + } + + env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start"); + v8::MaybeLocal retval = + args[0].As()->Call(env->context(), args[1], + call_args.size(), call_args.data()); + args.GetReturnValue().Set(retval.ToLocalChecked()); +} +} // namespace // Used in NodeInspectorClient::currentTimeMS() below. const int NANOS_PER_MSEC = 1000000; @@ -263,12 +316,6 @@ class NodeInspectorClient : public v8_inspector::V8InspectorClient { channel_->dispatchProtocolMessage(message); } - void schedulePauseOnNextStatement(const std::string& reason) { - if (channel_ != nullptr) { - channel_->schedulePauseOnNextStatement(reason); - } - } - Local ensureDefaultContextInGroup(int contextGroupId) override { return env_->context(); } @@ -302,10 +349,8 @@ class NodeInspectorClient : public v8_inspector::V8InspectorClient { script_id); } - InspectorSessionDelegate* delegate() { - if (channel_ == nullptr) - return nullptr; - return channel_->delegate(); + ChannelImpl* channel() { + return channel_.get(); } private: @@ -320,98 +365,22 @@ class NodeInspectorClient : public v8_inspector::V8InspectorClient { Agent::Agent(Environment* env) : parent_env_(env), inspector_(nullptr), platform_(nullptr), - inspector_console_(false) {} + enabled_(false) {} -// Header has unique_ptr to some incomplete types - this definition tells -// the compiler to figure out destruction here, were those types are complete +// Destructor needs to be defined here in implementation file as the header +// does not have full definition of some classes. Agent::~Agent() { } -// static -void Agent::InspectorConsoleCall(const v8::FunctionCallbackInfo& info) { - Isolate* isolate = info.GetIsolate(); - Local context = isolate->GetCurrentContext(); - - CHECK(info.Data()->IsArray()); - Local args = info.Data().As(); - CHECK_EQ(args->Length(), 3); - - std::vector> call_args(info.Length()); - for (int i = 0; i < info.Length(); ++i) { - call_args[i] = info[i]; - } - - Environment* env = Environment::GetCurrent(isolate); - if (env->inspector_agent()->inspector_console_) { - Local inspector_method = args->Get(context, 0).ToLocalChecked(); - CHECK(inspector_method->IsFunction()); - Local config_value = args->Get(context, 2).ToLocalChecked(); - CHECK(config_value->IsObject()); - Local config_object = config_value.As(); - Local in_call_key = FIXED_ONE_BYTE_STRING(isolate, "in_call"); - if (!config_object->Has(context, in_call_key).FromMaybe(false)) { - CHECK(config_object->Set(context, - in_call_key, - v8::True(isolate)).FromJust()); - CHECK(!inspector_method.As()->Call(context, - info.Holder(), - call_args.size(), - call_args.data()).IsEmpty()); - } - CHECK(config_object->Delete(context, in_call_key).FromJust()); - } - - Local node_method = - args->Get(context, 1).ToLocalChecked(); - CHECK(node_method->IsFunction()); - static_cast(node_method.As()->Call(context, - info.Holder(), - call_args.size(), - call_args.data())); -} - -// static -void Agent::InspectorWrapConsoleCall(const FunctionCallbackInfo& info) { - Environment* env = Environment::GetCurrent(info); - if (info.Length() != 3 || !info[0]->IsFunction() || - !info[1]->IsFunction() || !info[2]->IsObject()) { - return env->ThrowError("inspector.wrapConsoleCall takes exactly 3 " - "arguments: two functions and an object."); - } - - Local array = v8::Array::New(env->isolate(), info.Length()); - CHECK(array->Set(env->context(), 0, info[0]).FromJust()); - CHECK(array->Set(env->context(), 1, info[1]).FromJust()); - CHECK(array->Set(env->context(), 2, info[2]).FromJust()); - info.GetReturnValue().Set(Function::New(env->context(), - InspectorConsoleCall, - array).ToLocalChecked()); -} - bool Agent::Start(v8::Platform* platform, const char* path, const DebugOptions& options) { path_ = path == nullptr ? "" : path; debug_options_ = options; - inspector_console_ = false; inspector_ = std::unique_ptr( new NodeInspectorClient(parent_env_, platform)); platform_ = platform; - Local process = parent_env_->process_object(); - Local inspector = Object::New(parent_env_->isolate()); - Local name = - FIXED_ONE_BYTE_STRING(parent_env_->isolate(), "inspector"); - process->DefineOwnProperty(parent_env_->context(), - name, - inspector, - v8::ReadOnly).FromJust(); - parent_env_->SetMethod(inspector, "wrapConsoleCall", - InspectorWrapConsoleCall); if (options.inspector_enabled()) { - if (options.wait_for_connect()) { - parent_env_->SetMethod(inspector, "callAndPauseOnStart", - CallAndPauseOnStart); - } return StartIoThread(); } else { CHECK_EQ(0, uv_async_init(uv_default_loop(), @@ -431,7 +400,7 @@ bool Agent::StartIoThread() { CHECK_NE(inspector_, nullptr); - inspector_console_ = true; + enabled_ = true; io_ = std::unique_ptr( new InspectorIo(parent_env_, platform_, path_, debug_options_)); if (!io_->Start()) { @@ -469,7 +438,7 @@ void Agent::Stop() { } void Agent::Connect(InspectorSessionDelegate* delegate) { - inspector_console_ = true; + enabled_ = true; inspector_->connectFrontend(delegate); } @@ -481,26 +450,6 @@ bool Agent::IsStarted() { return !!inspector_; } -// static -void Agent::CallAndPauseOnStart( - const v8::FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - CHECK_GT(args.Length(), 1); - CHECK(args[0]->IsFunction()); - std::vector> call_args; - for (int i = 2; i < args.Length(); i++) { - call_args.push_back(args[i]); - } - - Agent* agent = env->inspector_agent(); - agent->inspector_->schedulePauseOnNextStatement("Break on start"); - - v8::MaybeLocal retval = - args[0].As()->Call(env->context(), args[1], - call_args.size(), call_args.data()); - args.GetReturnValue().Set(retval.ToLocalChecked()); -} - void Agent::WaitForDisconnect() { if (io_ != nullptr) { io_->WaitForDisconnect(); @@ -529,5 +478,23 @@ void Agent::RunMessageLoop() { inspector_->runMessageLoopOnPause(CONTEXT_GROUP_ID); } +void Agent::PauseOnNextJavascriptStatement(const std::string& reason) { + ChannelImpl* channel = inspector_->channel(); + if (channel != nullptr) + channel->schedulePauseOnNextStatement(reason); +} + +// static +void Agent::InitJSBindings(Local target, Local unused, + Local context, void* priv) { + Environment* env = Environment::GetCurrent(context); + Agent* agent = env->inspector_agent(); + env->SetMethod(target, "consoleCall", InspectorConsoleCall); + if (agent->debug_options_.wait_for_connect()) + env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart); +} } // namespace inspector } // namespace node + +NODE_MODULE_CONTEXT_AWARE_BUILTIN(inspector, + node::inspector::Agent::InitJSBindings); diff --git a/src/inspector_agent.h b/src/inspector_agent.h index 483ac52b55080e..a939f6c6ba0c47 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -17,11 +17,13 @@ class Environment; } // namespace node namespace v8 { +class Context; template class FunctionCallbackInfo; template class Local; class Message; +class Object; class Platform; class Value; } // namespace v8 @@ -61,19 +63,21 @@ class Agent { void Disconnect(); void Dispatch(const v8_inspector::StringView& message); void RunMessageLoop(); + bool enabled() { + return enabled_; + } + void PauseOnNextJavascriptStatement(const std::string& reason); + static void InitJSBindings(v8::Local target, + v8::Local unused, + v8::Local context, + void* priv); private: - static void CallAndPauseOnStart(const v8::FunctionCallbackInfo&); - static void InspectorConsoleCall( - const v8::FunctionCallbackInfo& info); - static void InspectorWrapConsoleCall( - const v8::FunctionCallbackInfo& info); - node::Environment* parent_env_; std::unique_ptr inspector_; std::unique_ptr io_; v8::Platform* platform_; - bool inspector_console_; + bool enabled_; std::string path_; DebugOptions debug_options_; }; diff --git a/src/node.cc b/src/node.cc index 97ba724ba8a077..8a99219510511f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4624,3 +4624,9 @@ int Start(int argc, char** argv) { } // namespace node + +#if !HAVE_INSPECTOR +static void InitEmptyBindings() {} + +NODE_MODULE_CONTEXT_AWARE_BUILTIN(inspector, InitEmptyBindings); +#endif // !HAVE_INSPECTOR