console: refactor inspector console extension installation

lib/inspector.js

@@ -106,6 +106,6 @@ module.exports = {
   url: url,
   // This is dynamically added during bootstrap,
   // where the console from the VM is still available
-  console: require('internal/console/inspector').consoleFromVM,
+  console: require('internal/util/inspector').consoleFromVM,
   Session
 };

lib/internal/bootstrap/node.js

@@ -679,13 +679,17 @@ function setupGlobalConsole() {
     value: consoleFromNode,
     writable: true
   });
-  // TODO(joyeecheung): can we skip this if inspector is not active?
+
   if (config.hasInspector) {
-    const inspector =
-      NativeModule.require('internal/console/inspector');
-    inspector.addInspectorApis(consoleFromNode, consoleFromVM);
+    const inspector = NativeModule.require('internal/util/inspector');
     // This will be exposed by `require('inspector').console` later.
     inspector.consoleFromVM = consoleFromVM;
+    // TODO(joyeecheung): postpone this until the first time inspector
+    // is activated.
+    inspector.wrapConsole(consoleFromNode, consoleFromVM);
+    const { setConsoleExtensionInstaller } = internalBinding('inspector');
+    // Setup inspector command line API.
+    setConsoleExtensionInstaller(inspector.installConsoleExtensions);
   }
 }
 

lib/internal/util/inspector.js

@@ -1,11 +1,9 @@
 'use strict';
 
-const { hasInspector } = internalBinding('config');
-const inspector = hasInspector ? require('inspector') : undefined;
-
 let session;
-
 function sendInspectorCommand(cb, onError) {
+  const { hasInspector } = internalBinding('config');
+  const inspector = hasInspector ? require('inspector') : undefined;
   if (!hasInspector) return onError();
   if (session === undefined) session = new inspector.Session();
   try {
@@ -20,6 +18,49 @@ function sendInspectorCommand(cb, onError) {
   }
 }
 
+// Create a special require function for the inspector command line API
+function installConsoleExtensions(commandLineApi) {
+  if (commandLineApi.require) { return; }
+  const { tryGetCwd } = require('internal/process/execution');
+  const CJSModule = require('internal/modules/cjs/loader');
+  const { makeRequireFunction } = require('internal/modules/cjs/helpers');
+  const consoleAPIModule = new CJSModule('<inspector console>');
+  const cwd = tryGetCwd();
+  consoleAPIModule.paths =
+      CJSModule._nodeModulePaths(cwd).concat(CJSModule.globalPaths);
+  commandLineApi.require = makeRequireFunction(consoleAPIModule);
+}
+
+// Wrap a console implemented by Node.js with features from the VM inspector
+function wrapConsole(consoleFromNode, consoleFromVM) {
+  const config = {};
+  const { consoleCall } = internalBinding('inspector');
+  for (const key of Object.keys(consoleFromVM)) {
+    // If global console has the same method as inspector console,
+    // then wrap these two methods into one. Native wrapper will preserve
+    // the original stack.
+    if (consoleFromNode.hasOwnProperty(key)) {
+      consoleFromNode[key] = consoleCall.bind(consoleFromNode,
+                                              consoleFromVM[key],
+                                              consoleFromNode[key],
+                                              config);
+    } else {
+      // Add additional console APIs from the inspector
+      consoleFromNode[key] = consoleFromVM[key];
+    }
+  }
+}
+
+// Stores the console from VM, should be set during bootstrap.
+let consoleFromVM;
 module.exports = {
-  sendInspectorCommand
+  installConsoleExtensions,
+  sendInspectorCommand,
+  wrapConsole,
+  get consoleFromVM() {
+    return consoleFromVM;
+  },
+  set consoleFromVM(val) {
+    consoleFromVM = val;
+  }
 };

node.gyp

@@ -97,7 +97,6 @@
       'lib/internal/cluster/worker.js',
       'lib/internal/console/constructor.js',
       'lib/internal/console/global.js',
-      'lib/internal/console/inspector.js',
       'lib/internal/coverage-gen/with_profiler.js',
       'lib/internal/coverage-gen/with_instrumentation.js',
       'lib/internal/crypto/certificate.js',

src/env.cc

@@ -345,13 +345,6 @@ void Environment::Start(const std::vector<std::string>& args,
   static uv_once_t init_once = UV_ONCE_INIT;
   uv_once(&init_once, InitThreadLocalOnce);
   uv_key_set(&thread_local_env, this);
-
-#if HAVE_INSPECTOR
-  // This needs to be set before we start the inspector
-  Local<Object> obj = Object::New(isolate());
-  CHECK(obj->SetPrototype(context(), Null(isolate())).FromJust());
-  set_inspector_console_api_object(obj);
-#endif  // HAVE_INSPECTOR
 }
 
 void Environment::RegisterHandleCleanups() {

src/env.h

@@ -356,7 +356,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
   V(http2settings_constructor_template, v8::ObjectTemplate)                    \
   V(http2stream_constructor_template, v8::ObjectTemplate)                      \
   V(immediate_callback_function, v8::Function)                                 \
-  V(inspector_console_api_object, v8::Object)                                  \
+  V(inspector_console_extension_installer, v8::Function)                       \
   V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate)                     \
   V(message_port, v8::Object)                                                  \
   V(message_port_constructor_template, v8::FunctionTemplate)                   \

src/inspector_agent.cc

@@ -32,7 +32,6 @@ namespace {
 
 using node::FatalError;
 
-using v8::Array;
 using v8::Context;
 using v8::Function;
 using v8::HandleScope;
@@ -493,16 +492,11 @@ class NodeInspectorClient : public V8InspectorClient {
 
   void installAdditionalCommandLineAPI(Local<Context> context,
                                        Local<Object> target) override {
-    Local<Object> console_api = env_->inspector_console_api_object();
-    CHECK(!console_api.IsEmpty());
-
-    Local<Array> properties =
-        console_api->GetOwnPropertyNames(context).ToLocalChecked();
-    for (uint32_t i = 0; i < properties->Length(); ++i) {
-      Local<Value> key = properties->Get(context, i).ToLocalChecked();
-      target->Set(context,
-                  key,
-                  console_api->Get(context, key).ToLocalChecked()).FromJust();
+    Local<Function> installer = env_->inspector_console_extension_installer();
+    if (!installer.IsEmpty()) {
+      Local<Value> argv[] = {target};
+      // If there is an exception, proceed in JS land
+      USE(installer->Call(context, target, arraysize(argv), argv));
     }
   }
 

src/inspector_js_api.cc

@@ -123,16 +123,13 @@ static bool InspectorEnabled(Environment* env) {
   return agent->IsActive();
 }
 
-void AddCommandLineAPI(const FunctionCallbackInfo<Value>& info) {
+void SetConsoleExtensionInstaller(const FunctionCallbackInfo<Value>& info) {
   auto env = Environment::GetCurrent(info);
-  Local<Context> context = env->context();
 
-  // inspector.addCommandLineAPI takes 2 arguments: a string and a value.
-  CHECK_EQ(info.Length(), 2);
-  CHECK(info[0]->IsString());
+  CHECK_EQ(info.Length(), 1);
+  CHECK(info[0]->IsFunction());
 
-  Local<Object> console_api = env->inspector_console_api_object();
-  console_api->Set(context, info[0], info[1]).FromJust();
+  env->set_inspector_console_extension_installer(info[0].As<Function>());
 }
 
 void CallAndPauseOnStart(const FunctionCallbackInfo<v8::Value>& args) {
@@ -280,7 +277,8 @@ void Initialize(Local<Object> target, Local<Value> unused,
 
   Agent* agent = env->inspector_agent();
   env->SetMethod(target, "consoleCall", InspectorConsoleCall);
-  env->SetMethod(target, "addCommandLineAPI", AddCommandLineAPI);
+  env->SetMethod(
+      target, "setConsoleExtensionInstaller", SetConsoleExtensionInstaller);
   if (agent->WillWaitForConnect())
     env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
   env->SetMethod(target, "open", Open);