more comments

Author: TimothyGu

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

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
   });

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);

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;
 };
 

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}");

src/node_contextify.cc

@@ -210,20 +210,21 @@ 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(),
                          FIXED_ONE_BYTE_STRING(env->isolate(), "origin"))
             .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);