From 9e708d64e223c80d17ba88da8d6a08f187a9aca6 Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
Date: Sun, 17 Dec 2017 13:38:15 -0800
Subject: [PATCH 1/4] vm: allow modifying context name in inspector

---
 doc/api/vm.md                              |  22 +++-
 lib/vm.js                                  |  75 ++++++++++--
 src/env-inl.h                              |   7 +-
 src/env.h                                  |  13 ++-
 src/inspector_agent.cc                     |  45 ++++++--
 src/inspector_agent.h                      |   4 +-
 src/node_contextify.cc                     |  35 +++++-
 test/sequential/test-inspector-contexts.js | 128 +++++++++++++++++----
 8 files changed, 270 insertions(+), 59 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index 5b47d3872c3207..f39be475f61daf 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -175,6 +175,11 @@ 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. **Default:** the empty string.
 
 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 +247,18 @@ 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. **Default:** the empty string.
 
 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 +293,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
@@ -355,6 +369,11 @@ 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. **Default:** the empty string.
 
 The `vm.runInNewContext()` first contextifies the given `sandbox` object (or
 creates a new `sandbox` if passed as `undefined`), compiles the `code`, runs it
@@ -490,3 +509,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 82f140923a8bce..017f94e2a84591 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -29,6 +29,8 @@ const {
   isContext,
 } = process.binding('contextify');
 
+const errors = require('internal/errors');
+
 // The binding provides a few useful primitives:
 // - Script(code, { filename = "evalmachine.anonymous",
 //                  displayErrors = true } = {})
@@ -73,18 +75,57 @@ Script.prototype.runInContext = function(contextifiedSandbox, options) {
 };
 
 Script.prototype.runInNewContext = function(sandbox, options) {
-  var context = createContext(sandbox);
+  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);
+  }
+  const context = createContext(sandbox, contextOptions);
   return this.runInContext(context, options);
 };
 
-function createContext(sandbox) {
+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;
 }
 
@@ -126,17 +167,27 @@ 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 };
+  }
+  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);
   }
+  sandbox = createContext(sandbox, contextOptions);
+  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 976ebae7dd4b9f..7ae5081947ea64 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -227,10 +227,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
 }
 
@@ -295,7 +296,7 @@ inline Environment::Environment(IsolateData* isolate_data,
 
   set_module_load_list_array(v8::Array::New(isolate()));
 
-  AssignToContext(context);
+  AssignToContext(context, ContextInfo(v8::String::Empty(isolate())));
 
   destroy_async_id_list_.reserve(512);
   performance_state_ = Calloc<performance::performance_state>(1);
diff --git a/src/env.h b/src/env.h
index 13517f75d9fa73..fd6c029b6360bc 100644
--- a/src/env.h
+++ b/src/env.h
@@ -360,6 +360,15 @@ class IsolateData {
   DISALLOW_COPY_AND_ASSIGN(IsolateData);
 };
 
+struct ContextInfo {
+  explicit ContextInfo(v8::Local<v8::String> name) : name(name) {
+    CHECK(!name.IsEmpty());
+  }
+  v8::Local<v8::String> name;
+  v8::Local<v8::String> origin;
+  bool is_default = false;
+};
+
 class Environment {
  public:
   class AsyncHooks {
@@ -507,9 +516,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..1a7042deff86b4 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -29,8 +29,10 @@ using v8::Function;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
+using v8::NewStringType;
 using v8::Object;
 using v8::Persistent;
+using v8::String;
 using v8::Value;
 
 using v8_inspector::StringBuffer;
@@ -53,8 +55,9 @@ class StartIoTask : public v8::Task {
   Agent* agent;
 };
 
+template <typename T>
 std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
-                                               Local<Value> value) {
+                                               Local<T> value) {
   TwoByteValue buffer(isolate, value);
   return StringBuffer::create(StringView(*buffer, buffer.length()));
 }
@@ -304,7 +307,12 @@ 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(
+        String::NewFromUtf8(env->isolate(),
+                            GetHumanReadableProcessName().c_str(),
+                            NewStringType::kNormal).ToLocalChecked());
+    info.is_default = true;
+    contextCreated(env->context(), info);
   }
 
   void runMessageLoopOnPause(int context_group_id) override {
@@ -334,11 +342,27 @@ 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) {
+    std::unique_ptr<StringBuffer> name_buffer =
+        ToProtocolString(env_->isolate(), info.name);
+    std::unique_ptr<StringBuffer> origin_buffer;
+    std::unique_ptr<StringBuffer> aux_data_buffer;
+
+    v8_inspector::V8ContextInfo v8info(
+        context, CONTEXT_GROUP_ID, name_buffer->string());
+    if (!info.origin.IsEmpty()) {
+      origin_buffer = ToProtocolString(env_->isolate(), info.origin);
+      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 +488,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 +699,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 1444d778015fe9..1d181a8395a6db 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -100,8 +100,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
@@ -154,7 +157,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());
@@ -204,7 +209,24 @@ class ContextifyContext {
                             env->contextify_global_private_symbol(),
                             ctx->Global());
 
-    env->AssignToContext(ctx);
+    Local<Value> name =
+        options_obj->Get(env->context(),
+                         FIXED_ONE_BYTE_STRING(env->isolate(), "name"))
+            .ToLocalChecked();
+    CHECK(name->IsString());
+
+    ContextInfo info(name.As<String>());
+
+    Local<Value> origin =
+        options_obj->Get(env->context(),
+                         FIXED_ONE_BYTE_STRING(env->isolate(), "origin"))
+            .ToLocalChecked();
+    if (!origin->IsUndefined()) {
+      CHECK(origin->IsString());
+      info.origin = origin.As<String>();
+    }
+
+    env->AssignToContext(ctx, info);
 
     return scope.Escape(ctx);
   }
@@ -235,8 +257,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() {

From 574b6f7273253b69caca45e6b714fc647ff6250a Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
Date: Sun, 17 Dec 2017 16:10:35 -0800
Subject: [PATCH 2/4] doc fixup

---
 doc/api/vm.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index f39be475f61daf..6ef084f8902ef0 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -179,7 +179,7 @@ added: v0.3.1
     **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. **Default:** the empty string.
+    created context for display purposes. **Default:** `''`.
 
 First contextifies the given `sandbox`, runs the compiled code contained by
 the `vm.Script` object within the created sandbox, and returns the result.
@@ -258,7 +258,7 @@ added: v0.3.1
     **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. **Default:** the empty string.
+    context for display purposes. **Default:** `''`.
 
 If given a `sandbox` object, the `vm.createContext()` method will [prepare
 that sandbox][contextified] so that it can be used in calls to
@@ -373,7 +373,7 @@ added: v0.3.1
     **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. **Default:** the empty string.
+    created context for display purposes. **Default:** `''`.
 
 The `vm.runInNewContext()` first contextifies the given `sandbox` object (or
 creates a new `sandbox` if passed as `undefined`), compiles the `code`, runs it

From e6a939de9396911541a4f1e16f0bd94a82b94e4c Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
Date: Tue, 19 Dec 2017 19:28:47 -0800
Subject: [PATCH 3/4] more comments

---
 doc/api/vm.md          | 16 +++++++++++++---
 lib/vm.js              | 26 ++++++++------------------
 src/env-inl.h          |  2 +-
 src/env.h              |  8 +++-----
 src/inspector_agent.cc | 19 +++++--------------
 src/node_contextify.cc |  9 +++++----
 6 files changed, 35 insertions(+), 45 deletions(-)

diff --git a/doc/api/vm.md b/doc/api/vm.md
index 6ef084f8902ef0..d5dd824180ce94 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -179,7 +179,11 @@ added: v0.3.1
     **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. **Default:** `''`.
+    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.
@@ -373,7 +377,11 @@ added: v0.3.1
     **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. **Default:** `''`.
+    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
@@ -499,10 +507,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
diff --git a/lib/vm.js b/lib/vm.js
index 017f94e2a84591..ec05ad774afbf4 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -75,6 +75,11 @@ Script.prototype.runInContext = function(contextifiedSandbox, options) {
 };
 
 Script.prototype.runInNewContext = function(sandbox, options) {
+  const context = createContext(sandbox, getContextOptions(options));
+  return this.runInContext(context, options);
+};
+
+function getContextOptions(options) {
   const contextOptions = options ? {
     name: options.contextName,
     origin: options.contextOrigin
@@ -89,9 +94,8 @@ Script.prototype.runInNewContext = function(sandbox, options) {
     throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.contextOrigin',
                                'string', contextOptions.origin);
   }
-  const context = createContext(sandbox, contextOptions);
-  return this.runInContext(context, options);
-};
+  return contextOptions;
+}
 
 let defaultContextNameIndex = 1;
 function createContext(sandbox, options) {
@@ -170,21 +174,7 @@ function runInNewContext(code, sandbox, options) {
   if (typeof options === 'string') {
     options = { filename: 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);
-  }
-  sandbox = createContext(sandbox, contextOptions);
+  sandbox = createContext(sandbox, getContextOptions(options));
   options = Object.assign({}, options, {
     [kParsingContext]: sandbox
   });
diff --git a/src/env-inl.h b/src/env-inl.h
index 7ae5081947ea64..ae864fb08402f7 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -296,7 +296,7 @@ inline Environment::Environment(IsolateData* isolate_data,
 
   set_module_load_list_array(v8::Array::New(isolate()));
 
-  AssignToContext(context, ContextInfo(v8::String::Empty(isolate())));
+  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 fd6c029b6360bc..b0863aadb3f194 100644
--- a/src/env.h
+++ b/src/env.h
@@ -361,11 +361,9 @@ class IsolateData {
 };
 
 struct ContextInfo {
-  explicit ContextInfo(v8::Local<v8::String> name) : name(name) {
-    CHECK(!name.IsEmpty());
-  }
-  v8::Local<v8::String> name;
-  v8::Local<v8::String> origin;
+  explicit ContextInfo(std::string name) : name(name) {}
+  std::string name;
+  std::string origin;
   bool is_default = false;
 };
 
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
index 1a7042deff86b4..7ba1a144711524 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -29,7 +29,6 @@ using v8::Function;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
-using v8::NewStringType;
 using v8::Object;
 using v8::Persistent;
 using v8::String;
@@ -55,9 +54,8 @@ class StartIoTask : public v8::Task {
   Agent* agent;
 };
 
-template <typename T>
 std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
-                                               Local<T> value) {
+                                               Local<Value> value) {
   TwoByteValue buffer(isolate, value);
   return StringBuffer::create(StringView(*buffer, buffer.length()));
 }
@@ -307,10 +305,7 @@ class NodeInspectorClient : public V8InspectorClient {
         running_nested_loop_(false) {
     client_ = V8Inspector::create(env->isolate(), this);
     // TODO(bnoordhuis) Make name configurable from src/node.cc.
-    ContextInfo info(
-        String::NewFromUtf8(env->isolate(),
-                            GetHumanReadableProcessName().c_str(),
-                            NewStringType::kNormal).ToLocalChecked());
+    ContextInfo info(GetHumanReadableProcessName());
     info.is_default = true;
     contextCreated(env->context(), info);
   }
@@ -343,17 +338,13 @@ class NodeInspectorClient : public V8InspectorClient {
   }
 
   void contextCreated(Local<Context> context, const ContextInfo& info) {
-    std::unique_ptr<StringBuffer> name_buffer =
-        ToProtocolString(env_->isolate(), info.name);
-    std::unique_ptr<StringBuffer> origin_buffer;
+    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());
-    if (!info.origin.IsEmpty()) {
-      origin_buffer = ToProtocolString(env_->isolate(), info.origin);
-      v8info.origin = origin_buffer->string();
-    }
+    v8info.origin = origin_buffer->string();
 
     if (info.is_default) {
       aux_data_buffer = Utf8ToStringView("{\"isDefault\":true}");
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 1d181a8395a6db..db907c63336cf7 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -210,12 +210,12 @@ class ContextifyContext {
                             ctx->Global());
 
     Local<Value> name =
-        options_obj->Get(env->context(),
-                         FIXED_ONE_BYTE_STRING(env->isolate(), "name"))
+        options_obj->Get(env->context(), env->name_string())
             .ToLocalChecked();
     CHECK(name->IsString());
+    Utf8Value name_val(env->isolate(), name);
 
-    ContextInfo info(name.As<String>());
+    ContextInfo info(*name_val);
 
     Local<Value> origin =
         options_obj->Get(env->context(),
@@ -223,7 +223,8 @@ class ContextifyContext {
             .ToLocalChecked();
     if (!origin->IsUndefined()) {
       CHECK(origin->IsString());
-      info.origin = origin.As<String>();
+      Utf8Value origin_val(env->isolate(), origin);
+      info.origin = *origin_val;
     }
 
     env->AssignToContext(ctx, info);

From 596fa5a13ddda5af73d9fc85edc3e08df8c42923 Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
Date: Wed, 20 Dec 2017 06:38:28 -0800
Subject: [PATCH 4/4] More comments

---
 src/env.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/env.h b/src/env.h
index b0863aadb3f194..3cb95bf39220e2 100644
--- a/src/env.h
+++ b/src/env.h
@@ -361,8 +361,8 @@ class IsolateData {
 };
 
 struct ContextInfo {
-  explicit ContextInfo(std::string name) : name(name) {}
-  std::string name;
+  explicit ContextInfo(const std::string& name) : name(name) {}
+  const std::string name;
   std::string origin;
   bool is_default = false;
 };