src: merge RunInThisContext() with RunInContext()

This commit resolves TODO in `RunInThisContext()` by merging `RunInThisContext()` with `RunInContext()`. Signed-off-by: Daeyeon Jeong [email protected]
nodejs · May 28, 2022 · 86207bc · 86207bc
1 parent 331088f
commit 86207bc
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 56 deletions.
diff --git a/lib/vm.js b/lib/vm.js
@@ -23,7 +23,6 @@
 
 const {
   ArrayPrototypeForEach,
-  ArrayPrototypeUnshift,
   Symbol,
   PromiseReject,
   ReflectApply,
@@ -122,17 +121,19 @@ class Script extends ContextifyScript {
   }
 
   runInThisContext(options) {
-    const { breakOnSigint, args } = getRunInContextArgs(options);
+    const { breakOnSigint, args } = getRunInContextArgs(null, options);
     if (breakOnSigint && process.listenerCount('SIGINT') > 0) {
-      return sigintHandlersWrap(super.runInThisContext, this, args);
+      return sigintHandlersWrap(super.runInContext, this, args);
     }
-    return ReflectApply(super.runInThisContext, this, args);
+    return ReflectApply(super.runInContext, this, args);
   }
 
   runInContext(contextifiedObject, options) {
     validateContext(contextifiedObject);
-    const { breakOnSigint, args } = getRunInContextArgs(options);
-    ArrayPrototypeUnshift(args, contextifiedObject);
+    const { breakOnSigint, args } = getRunInContextArgs(
+      contextifiedObject,
+      options,
+    );
     if (breakOnSigint && process.listenerCount('SIGINT') > 0) {
       return sigintHandlersWrap(super.runInContext, this, args);
     }
@@ -152,7 +153,7 @@ function validateContext(contextifiedObject) {
   }
 }
 
-function getRunInContextArgs(options = {}) {
+function getRunInContextArgs(contextifiedObject, options = {}) {
   validateObject(options, 'options');
 
   let timeout = options.timeout;
@@ -173,7 +174,13 @@ function getRunInContextArgs(options = {}) {
 
   return {
     breakOnSigint,
-    args: [timeout, displayErrors, breakOnSigint, breakFirstLine]
+    args: [
+      contextifiedObject,
+      timeout,
+      displayErrors,
+      breakOnSigint,
+      breakFirstLine,
+    ],
   };
 }
 

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -663,7 +663,6 @@ void ContextifyScript::Init(Environment* env, Local<Object> target) {
   script_tmpl->SetClassName(class_name);
   env->SetProtoMethod(script_tmpl, "createCachedData", CreateCachedData);
   env->SetProtoMethod(script_tmpl, "runInContext", RunInContext);
-  env->SetProtoMethod(script_tmpl, "runInThisContext", RunInThisContext);
 
   Local<Context> context = env->context();
 
@@ -677,7 +676,6 @@ void ContextifyScript::RegisterExternalReferences(
   registry->Register(New);
   registry->Register(CreateCachedData);
   registry->Register(RunInContext);
-  registry->Register(RunInThisContext);
 }
 
 void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
@@ -844,59 +842,35 @@ void ContextifyScript::CreateCachedData(
   }
 }
 
-void ContextifyScript::RunInThisContext(
-    const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
-  ContextifyScript* wrapped_script;
-  ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder());
-
-  TRACE_EVENT0(TRACING_CATEGORY_NODE2(vm, script), "RunInThisContext");
-
-  // TODO(addaleax): Use an options object or otherwise merge this with
-  // RunInContext().
-  CHECK_EQ(args.Length(), 4);
-
-  CHECK(args[0]->IsNumber());
-  int64_t timeout = args[0]->IntegerValue(env->context()).FromJust();
-
-  CHECK(args[1]->IsBoolean());
-  bool display_errors = args[1]->IsTrue();
-
-  CHECK(args[2]->IsBoolean());
-  bool break_on_sigint = args[2]->IsTrue();
-
-  CHECK(args[3]->IsBoolean());
-  bool break_on_first_line = args[3]->IsTrue();
-
-  // Do the eval within this context
-  EvalMachine(env,
-              timeout,
-              display_errors,
-              break_on_sigint,
-              break_on_first_line,
-              nullptr,  // microtask_queue
-              args);
-}
-
 void ContextifyScript::RunInContext(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   ContextifyScript* wrapped_script;
   ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder());
 
   CHECK_EQ(args.Length(), 5);
+  CHECK(args[0]->IsObject() || args[0]->IsNull());
 
-  CHECK(args[0]->IsObject());
-  Local<Object> sandbox = args[0].As<Object>();
-  // Get the context from the sandbox
-  ContextifyContext* contextify_context =
-      ContextifyContext::ContextFromContextifiedSandbox(env, sandbox);
-  CHECK_NOT_NULL(contextify_context);
+  Local<Context> context;
+  Environment* eval_env = nullptr;
+  std::shared_ptr<v8::MicrotaskQueue> microtask_queue;
 
-  Local<Context> context = contextify_context->context();
-  if (context.IsEmpty())
-    return;
+  if (args[0]->IsObject()) {
+    Local<Object> sandbox = args[0].As<Object>();
+    // Get the context from the sandbox
+    ContextifyContext* contextify_context =
+        ContextifyContext::ContextFromContextifiedSandbox(env, sandbox);
+    CHECK_NOT_NULL(contextify_context);
+
+    context = contextify_context->context();
+    if (context.IsEmpty()) return;
+
+    eval_env = contextify_context->env();
+    microtask_queue = contextify_context->microtask_queue();
+  } else {
+    eval_env = env;
+    context = env->context();
+  }
 
   TRACE_EVENT0(TRACING_CATEGORY_NODE2(vm, script), "RunInContext");
 
@@ -914,12 +888,12 @@ void ContextifyScript::RunInContext(const FunctionCallbackInfo<Value>& args) {
 
   // Do the eval within the context
   Context::Scope context_scope(context);
-  EvalMachine(contextify_context->env(),
+  EvalMachine(eval_env,
               timeout,
               display_errors,
               break_on_sigint,
               break_on_first_line,
-              contextify_context->microtask_queue(),
+              microtask_queue,
               args);
 }