vm: allow modifying context name in inspector

The `auxData` field is not exposed to JavaScript, as DevTools uses it for its `isDefault` parameter, which is implemented faithfully, contributing to the nice indentation in the context selection panel. Without the indentation, when `Target` domain gets implemented (along with a single Inspector for cluster) in #16627, subprocesses and VM contexts will be mixed up, causing confusion. PR-URL: #17720 Refs: #14231 (comment) Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 8, 2018 · 10ad019 · 10ad019
1 parent b6f0bf1
commit 10ad019
Show file tree

Hide file tree

Showing 8 changed files with 264 additions and 59 deletions.
diff --git a/doc/api/vm.md b/doc/api/vm.md
@@ -175,6 +175,15 @@ added: v0.3.1
   * `timeout` {number} Specifies the number of milliseconds to execute `code`
     before terminating execution. If execution is terminated, an [`Error`][]
     will be thrown.
+  * `contextName` {string} Human-readable name of the newly created context.
+    **Default:** `'VM Context i'`, where `i` is an ascending numerical index of
+    the created context.
+  * `contextOrigin` {string} [Origin][origin] corresponding to the newly
+    created context for display purposes. The origin should be formatted like a
+    URL, but with only the scheme, host, and port (if necessary), like the
+    value of the [`url.origin`][] property of a [`URL`][] object. Most notably,
+    this string should omit the trailing slash, as that denotes a path.
+    **Default:** `''`.
 
 First contextifies the given `sandbox`, runs the compiled code contained by
 the `vm.Script` object within the created sandbox, and returns the result.
@@ -242,12 +251,22 @@ console.log(globalVar);
 // 1000
 ```
 
-## vm.createContext([sandbox])
+## vm.createContext([sandbox[, options]])
 <!-- YAML
 added: v0.3.1
 -->
 
 * `sandbox` {Object}
+* `options` {Object}
+  * `name` {string} Human-readable name of the newly created context.
+    **Default:** `'VM Context i'`, where `i` is an ascending numerical index of
+    the created context.
+  * `origin` {string} [Origin][origin] corresponding to the newly created
+    context for display purposes. The origin should be formatted like a URL,
+    but with only the scheme, host, and port (if necessary), like the value of
+    the [`url.origin`][] property of a [`URL`][] object. Most notably, this
+    string should omit the trailing slash, as that denotes a path.
+    **Default:** `''`.
 
 If given a `sandbox` object, the `vm.createContext()` method will [prepare
 that sandbox][contextified] so that it can be used in calls to
@@ -282,6 +301,9 @@ web browser, the method can be used to create a single sandbox representing a
 window's global object, then run all `<script>` tags together within the context
 of that sandbox.
 
+The provided `name` and `origin` of the context are made visible through the
+Inspector API.
+
 ## vm.isContext(sandbox)
 <!-- YAML
 added: v0.11.7
@@ -385,6 +407,15 @@ added: v0.3.1
   * `timeout` {number} Specifies the number of milliseconds to execute `code`
     before terminating execution. If execution is terminated, an [`Error`][]
     will be thrown.
+  * `contextName` {string} Human-readable name of the newly created context.
+    **Default:** `'VM Context i'`, where `i` is an ascending numerical index of
+    the created context.
+  * `contextOrigin` {string} [Origin][origin] corresponding to the newly
+    created context for display purposes. The origin should be formatted like a
+    URL, but with only the scheme, host, and port (if necessary), like the
+    value of the [`url.origin`][] property of a [`URL`][] object. Most notably,
+    this string should omit the trailing slash, as that denotes a path.
+    **Default:** `''`.
 
 The `vm.runInNewContext()` first contextifies the given `sandbox` object (or
 creates a new `sandbox` if passed as `undefined`), compiles the `code`, runs it
@@ -510,14 +541,17 @@ associating it with the `sandbox` object is what this document refers to as
 "contextifying" the `sandbox`.
 
 [`Error`]: errors.html#errors_class_error
+[`URL`]: url.html#url_class_url
 [`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval
 [`script.runInContext()`]: #vm_script_runincontext_contextifiedsandbox_options
 [`script.runInThisContext()`]: #vm_script_runinthiscontext_options
-[`vm.createContext()`]: #vm_vm_createcontext_sandbox
+[`url.origin`]: https://nodejs.org/api/url.html#url_url_origin
+[`vm.createContext()`]: #vm_vm_createcontext_sandbox_options
 [`vm.runInContext()`]: #vm_vm_runincontext_code_contextifiedsandbox_options
 [`vm.runInThisContext()`]: #vm_vm_runinthiscontext_code_options
 [V8 Embedder's Guide]: https://github.com/v8/v8/wiki/Embedder's%20Guide#contexts
 [command line option]: cli.html
 [contextified]: #vm_what_does_it_mean_to_contextify_an_object
 [global object]: https://es5.github.io/#x15.1
 [indirect `eval()` call]: https://es5.github.io/#x10.4.2
+[origin]: https://developer.mozilla.org/en-US/docs/Glossary/Origin
diff --git a/lib/vm.js b/lib/vm.js
@@ -30,6 +30,8 @@ const {
   runInDebugContext: runInDebugContext_
 } = process.binding('contextify');
 
+const errors = require('internal/errors');
+
 // The binding provides a few useful primitives:
 // - Script(code, { filename = "evalmachine.anonymous",
 //                  displayErrors = true } = {})
@@ -74,18 +76,61 @@ Script.prototype.runInContext = function(contextifiedSandbox, options) {
 };
 
 Script.prototype.runInNewContext = function(sandbox, options) {
-  var context = createContext(sandbox);
+  const context = createContext(sandbox, getContextOptions(options));
   return this.runInContext(context, options);
 };
 
-function createContext(sandbox) {
+function getContextOptions(options) {
+  const contextOptions = options ? {
+    name: options.contextName,
+    origin: options.contextOrigin
+  } : {};
+  if (contextOptions.name !== undefined &&
+      typeof contextOptions.name !== 'string') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.contextName',
+                               'string', contextOptions.name);
+  }
+  if (contextOptions.origin !== undefined &&
+      typeof contextOptions.origin !== 'string') {
+    throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.contextOrigin',
+                               'string', contextOptions.origin);
+  }
+  return contextOptions;
+}
+
+let defaultContextNameIndex = 1;
+function createContext(sandbox, options) {
   if (sandbox === undefined) {
     sandbox = {};
   } else if (isContext(sandbox)) {
     return sandbox;
   }
 
-  makeContext(sandbox);
+  if (options !== undefined) {
+    if (typeof options !== 'object' || options === null) {
+      throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options',
+                                 'object', options);
+    }
+    options = {
+      name: options.name,
+      origin: options.origin
+    };
+    if (options.name === undefined) {
+      options.name = `VM Context ${defaultContextNameIndex++}`;
+    } else if (typeof options.name !== 'string') {
+      throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.name',
+                                 'string', options.name);
+    }
+    if (options.origin !== undefined && typeof options.origin !== 'string') {
+      throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.origin',
+                                 'string', options.origin);
+    }
+  } else {
+    options = {
+      name: `VM Context ${defaultContextNameIndex++}`
+    };
+  }
+  makeContext(sandbox, options);
   return sandbox;
 }
 
@@ -140,17 +185,13 @@ function runInContext(code, contextifiedSandbox, options) {
 }
 
 function runInNewContext(code, sandbox, options) {
-  sandbox = createContext(sandbox);
   if (typeof options === 'string') {
-    options = {
-      filename: options,
-      [kParsingContext]: sandbox
-    };
-  } else {
-    options = Object.assign({}, options, {
-      [kParsingContext]: sandbox
-    });
+    options = { filename: options };
   }
+  sandbox = createContext(sandbox, getContextOptions(options));
+  options = Object.assign({}, options, {
+    [kParsingContext]: sandbox
+  });
   return createScript(code, options).runInNewContext(sandbox, options);
 }
 

diff --git a/src/env-inl.h b/src/env-inl.h
@@ -217,10 +217,11 @@ inline void Environment::TickInfo::set_index(uint32_t value) {
   fields_[kIndex] = value;
 }
 
-inline void Environment::AssignToContext(v8::Local<v8::Context> context) {
+inline void Environment::AssignToContext(v8::Local<v8::Context> context,
+                                         const ContextInfo& info) {
   context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
 #if HAVE_INSPECTOR
-  inspector_agent()->ContextCreated(context);
+  inspector_agent()->ContextCreated(context, info);
 #endif  // HAVE_INSPECTOR
 }
 
@@ -285,7 +286,7 @@ inline Environment::Environment(IsolateData* isolate_data,
 
   set_module_load_list_array(v8::Array::New(isolate()));
 
-  AssignToContext(context);
+  AssignToContext(context, ContextInfo(""));
 
   destroy_async_id_list_.reserve(512);
   performance_state_ = Calloc<performance::performance_state>(1);

diff --git a/src/env.h b/src/env.h
@@ -359,6 +359,13 @@ class IsolateData {
   DISALLOW_COPY_AND_ASSIGN(IsolateData);
 };
 
+struct ContextInfo {
+  explicit ContextInfo(const std::string& name) : name(name) {}
+  const std::string name;
+  std::string origin;
+  bool is_default = false;
+};
+
 class Environment {
  public:
   class AsyncHooks {
@@ -505,9 +512,11 @@ class Environment {
              int exec_argc,
              const char* const* exec_argv,
              bool start_profiler_idle_notifier);
-  void AssignToContext(v8::Local<v8::Context> context);
   void CleanupHandles();
 
+  inline void AssignToContext(v8::Local<v8::Context> context,
+                              const ContextInfo& info);
+
   void StartProfilerIdleNotifier();
   void StopProfilerIdleNotifier();
 

diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -31,6 +31,7 @@ using v8::Isolate;
 using v8::Local;
 using v8::Object;
 using v8::Persistent;
+using v8::String;
 using v8::Value;
 
 using v8_inspector::StringBuffer;
@@ -304,7 +305,9 @@ class NodeInspectorClient : public V8InspectorClient {
         running_nested_loop_(false) {
     client_ = V8Inspector::create(env->isolate(), this);
     // TODO(bnoordhuis) Make name configurable from src/node.cc.
-    contextCreated(env->context(), GetHumanReadableProcessName());
+    ContextInfo info(GetHumanReadableProcessName());
+    info.is_default = true;
+    contextCreated(env->context(), info);
   }
 
   void runMessageLoopOnPause(int context_group_id) override {
@@ -334,11 +337,23 @@ class NodeInspectorClient : public V8InspectorClient {
     }
   }
 
-  void contextCreated(Local<Context> context, const std::string& name) {
-    std::unique_ptr<StringBuffer> name_buffer = Utf8ToStringView(name);
-    v8_inspector::V8ContextInfo info(context, CONTEXT_GROUP_ID,
-                                     name_buffer->string());
-    client_->contextCreated(info);
+  void contextCreated(Local<Context> context, const ContextInfo& info) {
+    auto name_buffer = Utf8ToStringView(info.name);
+    auto origin_buffer = Utf8ToStringView(info.origin);
+    std::unique_ptr<StringBuffer> aux_data_buffer;
+
+    v8_inspector::V8ContextInfo v8info(
+        context, CONTEXT_GROUP_ID, name_buffer->string());
+    v8info.origin = origin_buffer->string();
+
+    if (info.is_default) {
+      aux_data_buffer = Utf8ToStringView("{\"isDefault\":true}");
+    } else {
+      aux_data_buffer = Utf8ToStringView("{\"isDefault\":false}");
+    }
+    v8info.auxData = aux_data_buffer->string();
+
+    client_->contextCreated(v8info);
   }
 
   void contextDestroyed(Local<Context> context) {
@@ -464,7 +479,6 @@ Agent::Agent(Environment* env) : parent_env_(env),
                                  client_(nullptr),
                                  platform_(nullptr),
                                  enabled_(false),
-                                 next_context_number_(1),
                                  pending_enable_async_hook_(false),
                                  pending_disable_async_hook_(false) {}
 
@@ -676,12 +690,10 @@ void Agent::RequestIoThreadStart() {
   uv_async_send(&start_io_thread_async);
 }
 
-void Agent::ContextCreated(Local<Context> context) {
+void Agent::ContextCreated(Local<Context> context, const ContextInfo& info) {
   if (client_ == nullptr)  // This happens for a main context
     return;
-  std::ostringstream name;
-  name << "VM Context " << next_context_number_++;
-  client_->contextCreated(context, name.str());
+  client_->contextCreated(context, info);
 }
 
 bool Agent::IsWaitingForConnect() {

diff --git a/src/inspector_agent.h b/src/inspector_agent.h
@@ -20,6 +20,7 @@ class StringView;
 namespace node {
 // Forward declaration to break recursive dependency chain with src/env.h.
 class Environment;
+struct ContextInfo;
 
 namespace inspector {
 
@@ -89,7 +90,7 @@ class Agent {
   void RequestIoThreadStart();
 
   DebugOptions& options() { return debug_options_; }
-  void ContextCreated(v8::Local<v8::Context> context);
+  void ContextCreated(v8::Local<v8::Context> context, const ContextInfo& info);
 
   void EnableAsyncHook();
   void DisableAsyncHook();
@@ -105,7 +106,6 @@ class Agent {
   bool enabled_;
   std::string path_;
   DebugOptions debug_options_;
-  int next_context_number_;
 
   bool pending_enable_async_hook_;
   bool pending_disable_async_hook_;

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -102,8 +102,11 @@ class ContextifyContext {
   Persistent<Context> context_;
 
  public:
-  ContextifyContext(Environment* env, Local<Object> sandbox_obj) : env_(env) {
-    Local<Context> v8_context = CreateV8Context(env, sandbox_obj);
+  ContextifyContext(Environment* env,
+                    Local<Object> sandbox_obj,
+                    Local<Object> options_obj)
+      : env_(env) {
+    Local<Context> v8_context = CreateV8Context(env, sandbox_obj, options_obj);
     context_.Reset(env->isolate(), v8_context);
 
     // Allocation failure or maximum call stack size reached
@@ -156,7 +159,9 @@ class ContextifyContext {
   }
 
 
-  Local<Context> CreateV8Context(Environment* env, Local<Object> sandbox_obj) {
+  Local<Context> CreateV8Context(Environment* env,
+                                 Local<Object> sandbox_obj,
+                                 Local<Object> options_obj) {
     EscapableHandleScope scope(env->isolate());
     Local<FunctionTemplate> function_template =
         FunctionTemplate::New(env->isolate());
@@ -206,7 +211,25 @@ class ContextifyContext {
                             env->contextify_global_private_symbol(),
                             ctx->Global());
 
-    env->AssignToContext(ctx);
+    Local<Value> name =
+        options_obj->Get(env->context(), env->name_string())
+            .ToLocalChecked();
+    CHECK(name->IsString());
+    Utf8Value name_val(env->isolate(), name);
+
+    ContextInfo info(*name_val);
+
+    Local<Value> origin =
+        options_obj->Get(env->context(),
+                         FIXED_ONE_BYTE_STRING(env->isolate(), "origin"))
+            .ToLocalChecked();
+    if (!origin->IsUndefined()) {
+      CHECK(origin->IsString());
+      Utf8Value origin_val(env->isolate(), origin);
+      info.origin = *origin_val;
+    }
+
+    env->AssignToContext(ctx, info);
 
     return scope.Escape(ctx);
   }
@@ -268,8 +291,11 @@ class ContextifyContext {
             env->context(),
             env->contextify_context_private_symbol()).FromJust());
 
+    Local<Object> options = args[1].As<Object>();
+    CHECK(options->IsObject());
+
     TryCatch try_catch(env->isolate());
-    ContextifyContext* context = new ContextifyContext(env, sandbox);
+    ContextifyContext* context = new ContextifyContext(env, sandbox, options);
 
     if (try_catch.HasCaught()) {
       try_catch.ReThrow();