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 <kozyatinskiy@chromium.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

6 files changed, 115 insertions, 161 deletions

diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js
index 038e71502b..8159d8c990 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 55755d9ac4..5455e68dd9 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 3946e91e1c..1010d649c6 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 d8c7d1c874..3a8364e4fa 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<Value>& info) {
+  Isolate* isolate = info.GetIsolate();
+  HandleScope handle_scope(isolate);
+  Local<Context> context = isolate->GetCurrentContext();
+  CHECK_LT(2, info.Length());
+  std::vector<Local<Value>> 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<Value> inspector_method = info[0];
+    CHECK(inspector_method->IsFunction());
+    Local<Value> config_value = info[2];
+    CHECK(config_value->IsObject());
+    Local<Object> config_object = config_value.As<Object>();
+    Local<String> 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<Function>()->Call(context,
+                                                   info.Holder(),
+                                                   call_args.size(),
+                                                   call_args.data()).IsEmpty());
+    }
+    CHECK(config_object->Delete(context, in_call_key).FromJust());
+  }
+
+  Local<Value> node_method = info[1];
+  CHECK(node_method->IsFunction());
+  static_cast<void>(node_method.As<Function>()->Call(context,
+                                                     info.Holder(),
+                                                     call_args.size(),
+                                                     call_args.data()));
+}
+
+void CallAndPauseOnStart(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  CHECK_GT(args.Length(), 1);
+  CHECK(args[0]->IsFunction());
+  std::vector<v8::Local<v8::Value>> 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<v8::Value> retval =
+      args[0].As<v8::Function>()->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<Context> 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<Value>& info) {
-  Isolate* isolate = info.GetIsolate();
-  Local<Context> context = isolate->GetCurrentContext();
-
-  CHECK(info.Data()->IsArray());
-  Local<v8::Array> args = info.Data().As<v8::Array>();
-  CHECK_EQ(args->Length(), 3);
-
-  std::vector<Local<Value>> 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<Value> inspector_method = args->Get(context, 0).ToLocalChecked();
-    CHECK(inspector_method->IsFunction());
-    Local<Value> config_value = args->Get(context, 2).ToLocalChecked();
-    CHECK(config_value->IsObject());
-    Local<Object> config_object = config_value.As<Object>();
-    Local<String> 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<Function>()->Call(context,
-                                                   info.Holder(),
-                                                   call_args.size(),
-                                                   call_args.data()).IsEmpty());
-    }
-    CHECK(config_object->Delete(context, in_call_key).FromJust());
-  }
-
-  Local<Value> node_method =
-      args->Get(context, 1).ToLocalChecked();
-  CHECK(node_method->IsFunction());
-  static_cast<void>(node_method.As<Function>()->Call(context,
-                                                     info.Holder(),
-                                                     call_args.size(),
-                                                     call_args.data()));
-}
-
-// static
-void Agent::InspectorWrapConsoleCall(const FunctionCallbackInfo<Value>& 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<v8::Array> 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<NodeInspectorClient>(
           new NodeInspectorClient(parent_env_, platform));
   platform_ = platform;
-  Local<Object> process = parent_env_->process_object();
-  Local<Object> inspector = Object::New(parent_env_->isolate());
-  Local<String> 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<InspectorIo>(
       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<v8::Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  CHECK_GT(args.Length(), 1);
-  CHECK(args[0]->IsFunction());
-  std::vector<v8::Local<v8::Value>> 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<v8::Value> retval =
-      args[0].As<v8::Function>()->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<Object> target, Local<Value> unused,
+                           Local<Context> 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 483ac52b55..a939f6c6ba 100644
--- a/src/inspector_agent.h
+++ b/src/inspector_agent.h
@@ -17,11 +17,13 @@ class Environment;
 }  // namespace node
 
 namespace v8 {
+class Context;
 template <typename V>
 class FunctionCallbackInfo;
 template<typename T>
 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<v8::Object> target,
+                             v8::Local<v8::Value> unused,
+                             v8::Local<v8::Context> context,
+                             void* priv);
 
  private:
-  static void CallAndPauseOnStart(const v8::FunctionCallbackInfo<v8::Value>&);
-  static void InspectorConsoleCall(
-      const v8::FunctionCallbackInfo<v8::Value>& info);
-  static void InspectorWrapConsoleCall(
-      const v8::FunctionCallbackInfo<v8::Value>& info);
-
   node::Environment* parent_env_;
   std::unique_ptr<NodeInspectorClient> inspector_;
   std::unique_ptr<InspectorIo> 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 97ba724ba8..8a99219510 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