Skip to content

Commit 5202995

Browse files
joyeecheungrichardlau
authored andcommitted
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]>
1 parent 7e3ee82 commit 5202995

File tree

4 files changed

+22
-24
lines changed

4 files changed

+22
-24
lines changed

lib/internal/vm.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ const {
88
const {
99
ContextifyScript,
1010
compileFunction,
11-
isContext: _isContext,
1211
} = internalBinding('contextify');
1312
const {
1413
runInContext,
@@ -21,18 +20,19 @@ const {
2120
} = internalBinding('symbols');
2221
const {
2322
validateFunction,
24-
validateObject,
25-
kValidateObjectAllowArray,
2623
} = require('internal/validators');
2724

2825
const {
2926
getOptionValue,
3027
} = require('internal/options');
28+
const {
29+
privateSymbols: {
30+
contextify_context_private_symbol,
31+
},
32+
} = internalBinding('util');
3133

3234
function isContext(object) {
33-
validateObject(object, 'object', kValidateObjectAllowArray);
34-
35-
return _isContext(object);
35+
return object[contextify_context_private_symbol] !== undefined;
3636
}
3737

3838
function getHostDefinedOptionId(importModuleDynamically, hint) {

lib/internal/vm/module.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ const {
1616
TypeError,
1717
} = primordials;
1818

19-
const { isContext } = internalBinding('contextify');
2019
const {
2120
isModuleNamespaceObject,
2221
} = require('internal/util/types');
@@ -74,6 +73,8 @@ const kContext = Symbol('kContext');
7473
const kPerContextModuleId = Symbol('kPerContextModuleId');
7574
const kLink = Symbol('kLink');
7675

76+
const { isContext } = require('internal/vm');
77+
7778
class Module {
7879
constructor(options) {
7980
emitExperimentalWarning('VM Modules');

lib/vm.js

+14-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const {
4949
validateString,
5050
validateStringArray,
5151
validateUint32,
52+
kValidateObjectAllowArray,
5253
kValidateObjectAllowNullable,
5354
} = require('internal/validators');
5455
const {
@@ -59,14 +60,26 @@ const {
5960
const {
6061
getHostDefinedOptionId,
6162
internalCompileFunction,
62-
isContext,
63+
isContext: _isContext,
6364
registerImportModuleDynamically,
6465
} = require('internal/vm');
6566
const {
6667
vm_dynamic_import_main_context_default,
6768
} = internalBinding('symbols');
6869
const kParsingContext = Symbol('script parsing context');
6970

71+
/**
72+
* Check if object is a context object created by vm.createContext().
73+
* @throws {TypeError} If object is not an object in the first place, throws TypeError.
74+
* @param {object} object Object to check.
75+
* @returns {boolean}
76+
*/
77+
function isContext(object) {
78+
validateObject(object, 'object', kValidateObjectAllowArray);
79+
80+
return _isContext(object);
81+
}
82+
7083
class Script extends ContextifyScript {
7184
constructor(code, options = kEmptyObject) {
7285
code = `${code}`;

src/node_contextify.cc

-16
Original file line numberDiff line numberDiff line change
@@ -342,15 +342,13 @@ void ContextifyContext::CreatePerIsolateProperties(
342342
IsolateData* isolate_data, Local<ObjectTemplate> target) {
343343
Isolate* isolate = isolate_data->isolate();
344344
SetMethod(isolate, target, "makeContext", MakeContext);
345-
SetMethod(isolate, target, "isContext", IsContext);
346345
SetMethod(isolate, target, "compileFunction", CompileFunction);
347346
SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax);
348347
}
349348

350349
void ContextifyContext::RegisterExternalReferences(
351350
ExternalReferenceRegistry* registry) {
352351
registry->Register(MakeContext);
353-
registry->Register(IsContext);
354352
registry->Register(CompileFunction);
355353
registry->Register(ContainsModuleSyntax);
356354
registry->Register(PropertyGetterCallback);
@@ -415,20 +413,6 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
415413
}
416414
}
417415

418-
419-
void ContextifyContext::IsContext(const FunctionCallbackInfo<Value>& args) {
420-
Environment* env = Environment::GetCurrent(args);
421-
422-
CHECK(args[0]->IsObject());
423-
Local<Object> sandbox = args[0].As<Object>();
424-
425-
Maybe<bool> result =
426-
sandbox->HasPrivate(env->context(),
427-
env->contextify_context_private_symbol());
428-
args.GetReturnValue().Set(result.FromJust());
429-
}
430-
431-
432416
void ContextifyContext::WeakCallback(
433417
const WeakCallbackInfo<ContextifyContext>& data) {
434418
ContextifyContext* context = data.GetParameter();

0 commit comments

Comments
 (0)