vm: implement isContext() directly in JS land with private symbol

We are now directly checking the existence of a private symbol in the object to determine if an object is a ContextifyContext anyway, so there is no need to implement it in C++ anymore. PR-URL: #51685 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
nodejs · Feb 19, 2024 · ba444a9 · ba444a9
1 parent d7563a5
commit ba444a9
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 24 deletions.
diff --git a/lib/internal/vm.js b/lib/internal/vm.js
@@ -8,7 +8,6 @@ const {
 const {
   ContextifyScript,
   compileFunction,
-  isContext: _isContext,
 } = internalBinding('contextify');
 const {
   runInContext,
@@ -21,18 +20,19 @@ const {
 } = internalBinding('symbols');
 const {
   validateFunction,
-  validateObject,
-  kValidateObjectAllowArray,
 } = require('internal/validators');
 
 const {
   getOptionValue,
 } = require('internal/options');
+const {
+  privateSymbols: {
+    contextify_context_private_symbol,
+  },
+} = internalBinding('util');
 
 function isContext(object) {
-  validateObject(object, 'object', kValidateObjectAllowArray);
-
-  return _isContext(object);
+  return object[contextify_context_private_symbol] !== undefined;
 }
 
 function getHostDefinedOptionId(importModuleDynamically, hint) {

diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js
@@ -17,7 +17,6 @@ const {
   TypeError,
 } = primordials;
 
-const { isContext } = internalBinding('contextify');
 const {
   isModuleNamespaceObject,
 } = require('internal/util/types');
@@ -75,6 +74,8 @@ const kContext = Symbol('kContext');
 const kPerContextModuleId = Symbol('kPerContextModuleId');
 const kLink = Symbol('kLink');
 
+const { isContext } = require('internal/vm');
+
 class Module {
   constructor(options) {
     emitExperimentalWarning('VM Modules');

diff --git a/lib/vm.js b/lib/vm.js
@@ -49,6 +49,7 @@ const {
   validateString,
   validateStringArray,
   validateUint32,
+  kValidateObjectAllowArray,
   kValidateObjectAllowNullable,
 } = require('internal/validators');
 const {
@@ -59,14 +60,26 @@ const {
 const {
   getHostDefinedOptionId,
   internalCompileFunction,
-  isContext,
+  isContext: _isContext,
   registerImportModuleDynamically,
 } = require('internal/vm');
 const {
   vm_dynamic_import_main_context_default,
 } = internalBinding('symbols');
 const kParsingContext = Symbol('script parsing context');
 
+/**
+ * Check if object is a context object created by vm.createContext().
+ * @throws {TypeError} If object is not an object in the first place, throws TypeError.
+ * @param {object} object Object to check.
+ * @returns {boolean}
+ */
+function isContext(object) {
+  validateObject(object, 'object', kValidateObjectAllowArray);
+
+  return _isContext(object);
+}
+
 class Script extends ContextifyScript {
   constructor(code, options = kEmptyObject) {
     code = `${code}`;

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -342,15 +342,13 @@ void ContextifyContext::CreatePerIsolateProperties(
     IsolateData* isolate_data, Local<ObjectTemplate> target) {
   Isolate* isolate = isolate_data->isolate();
   SetMethod(isolate, target, "makeContext", MakeContext);
-  SetMethod(isolate, target, "isContext", IsContext);
   SetMethod(isolate, target, "compileFunction", CompileFunction);
   SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax);
 }
 
 void ContextifyContext::RegisterExternalReferences(
     ExternalReferenceRegistry* registry) {
   registry->Register(MakeContext);
-  registry->Register(IsContext);
   registry->Register(CompileFunction);
   registry->Register(ContainsModuleSyntax);
   registry->Register(PropertyGetterCallback);
@@ -415,20 +413,6 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-
-void ContextifyContext::IsContext(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
-  CHECK(args[0]->IsObject());
-  Local<Object> sandbox = args[0].As<Object>();
-
-  Maybe<bool> result =
-      sandbox->HasPrivate(env->context(),
-                          env->contextify_context_private_symbol());
-  args.GetReturnValue().Set(result.FromJust());
-}
-
-
 void ContextifyContext::WeakCallback(
     const WeakCallbackInfo<ContextifyContext>& data) {
   ContextifyContext* context = data.GetParameter();