From 10ad01950ecca8d006041e99e1e34a88294daf26 Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
Date: Sun, 17 Dec 2017 13:38:15 -0800
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/17720
Refs: https://github.com/nodejs/node/pull/14231#issuecomment-315924067
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/vm.md                              |  38 +++++-
 lib/vm.js                                  |  65 +++++++++--
 src/env-inl.h                              |   7 +-
 src/env.h                                  |  11 +-
 src/inspector_agent.cc                     |  34 ++++--
 src/inspector_agent.h                      |   4 +-
 src/node_contextify.cc                     |  36 +++++-
 test/sequential/test-inspector-contexts.js | 128 +++++++++++++++++----
 8 files changed, 264 insertions(+), 59 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index 609ee6b895c91a..00a311da7f9198 100644
--- 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,10 +541,12 @@ 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
@@ -521,3 +554,4 @@ associating it with the `sandbox` object is what this document refers to as
 [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
index 3c59e32805c297..4d12ea47d938c5 100644
--- 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
index a6a2e4f621b79e..b627cc159eafd9 100644
--- 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
index 05eb00e7d243b4..99491c5dbd11eb 100644
--- 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
index 95e1d642915b5c..7ba1a144711524 100644
--- 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
index a2d61d0c8db28e..1d632ea8f88ac5 100644
--- 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
index 1f1609e794d145..d50c6023dc8349 100644
--- 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();
diff --git a/test/sequential/test-inspector-contexts.js b/test/sequential/test-inspector-contexts.js
index c7db962f2af006..6ad02edeb1a59c 100644
--- a/test/sequential/test-inspector-contexts.js
+++ b/test/sequential/test-inspector-contexts.js
@@ -6,7 +6,7 @@ const common = require('../common');
 common.skipIfInspectorDisabled();
 
 const { strictEqual } = require('assert');
-const { runInNewContext } = require('vm');
+const { createContext, runInNewContext } = require('vm');
 const { Session } = require('inspector');
 
 const session = new Session();
@@ -18,13 +18,13 @@ function notificationPromise(method) {
 
 async function testContextCreatedAndDestroyed() {
   console.log('Testing context created/destroyed notifications');
-  const mainContextPromise =
-      notificationPromise('Runtime.executionContextCreated');
-
-  session.post('Runtime.enable');
-  let contextCreated = await mainContextPromise;
   {
-    const { name } = contextCreated.params.context;
+    const mainContextPromise =
+        notificationPromise('Runtime.executionContextCreated');
+
+    session.post('Runtime.enable');
+    const contextCreated = await mainContextPromise;
+    const { name, origin, auxData } = contextCreated.params.context;
     if (common.isSunOS || common.isWindows) {
       // uv_get_process_title() is unimplemented on Solaris-likes, it returns
       // an empy string.  On the Windows CI buildbots it returns "Administrator:
@@ -34,29 +34,111 @@ async function testContextCreatedAndDestroyed() {
     } else {
       strictEqual(`${process.argv0}[${process.pid}]`, name);
     }
+    strictEqual(origin, '',
+                JSON.stringify(contextCreated));
+    strictEqual(auxData.isDefault, true,
+                JSON.stringify(contextCreated));
+  }
+
+  {
+    const vmContextCreatedPromise =
+        notificationPromise('Runtime.executionContextCreated');
+
+    let contextDestroyed = null;
+    session.once('Runtime.executionContextDestroyed',
+                 (notification) => contextDestroyed = notification);
+
+    runInNewContext('1 + 1');
+
+    const contextCreated = await vmContextCreatedPromise;
+    const { id, name, origin, auxData } = contextCreated.params.context;
+    strictEqual(name, 'VM Context 1',
+                JSON.stringify(contextCreated));
+    strictEqual(origin, '',
+                JSON.stringify(contextCreated));
+    strictEqual(auxData.isDefault, false,
+                JSON.stringify(contextCreated));
+
+    // GC is unpredictable...
+    while (!contextDestroyed)
+      global.gc();
+
+    strictEqual(contextDestroyed.params.executionContextId, id,
+                JSON.stringify(contextDestroyed));
+  }
+
+  {
+    const vmContextCreatedPromise =
+        notificationPromise('Runtime.executionContextCreated');
+
+    let contextDestroyed = null;
+    session.once('Runtime.executionContextDestroyed',
+                 (notification) => contextDestroyed = notification);
+
+    runInNewContext('1 + 1', {}, {
+      contextName: 'Custom context',
+      contextOrigin: 'https://origin.example'
+    });
+
+    const contextCreated = await vmContextCreatedPromise;
+    const { name, origin, auxData } = contextCreated.params.context;
+    strictEqual(name, 'Custom context',
+                JSON.stringify(contextCreated));
+    strictEqual(origin, 'https://origin.example',
+                JSON.stringify(contextCreated));
+    strictEqual(auxData.isDefault, false,
+                JSON.stringify(contextCreated));
+
+    // GC is unpredictable...
+    while (!contextDestroyed)
+      global.gc();
   }
 
-  const secondContextCreatedPromise =
-      notificationPromise('Runtime.executionContextCreated');
+  {
+    const vmContextCreatedPromise =
+        notificationPromise('Runtime.executionContextCreated');
+
+    let contextDestroyed = null;
+    session.once('Runtime.executionContextDestroyed',
+                 (notification) => contextDestroyed = notification);
+
+    createContext({}, { origin: 'https://nodejs.org' });
+
+    const contextCreated = await vmContextCreatedPromise;
+    const { name, origin, auxData } = contextCreated.params.context;
+    strictEqual(name, 'VM Context 2',
+                JSON.stringify(contextCreated));
+    strictEqual(origin, 'https://nodejs.org',
+                JSON.stringify(contextCreated));
+    strictEqual(auxData.isDefault, false,
+                JSON.stringify(contextCreated));
+
+    // GC is unpredictable...
+    while (!contextDestroyed)
+      global.gc();
+  }
 
-  let contextDestroyed = null;
-  session.once('Runtime.executionContextDestroyed',
-               (notification) => contextDestroyed = notification);
+  {
+    const vmContextCreatedPromise =
+        notificationPromise('Runtime.executionContextCreated');
 
-  runInNewContext('1 + 1', {});
+    let contextDestroyed = null;
+    session.once('Runtime.executionContextDestroyed',
+                 (notification) => contextDestroyed = notification);
 
-  contextCreated = await secondContextCreatedPromise;
-  strictEqual('VM Context 1',
-              contextCreated.params.context.name,
-              JSON.stringify(contextCreated));
+    createContext({}, { name: 'Custom context 2' });
 
-  // GC is unpredictable...
-  while (!contextDestroyed)
-    global.gc();
+    const contextCreated = await vmContextCreatedPromise;
+    const { name, auxData } = contextCreated.params.context;
+    strictEqual(name, 'Custom context 2',
+                JSON.stringify(contextCreated));
+    strictEqual(auxData.isDefault, false,
+                JSON.stringify(contextCreated));
 
-  strictEqual(contextCreated.params.context.id,
-              contextDestroyed.params.executionContextId,
-              JSON.stringify(contextDestroyed));
+    // GC is unpredictable...
+    while (!contextDestroyed)
+      global.gc();
+  }
 }
 
 async function testBreakpointHit() {