Skip to content

Commit 113b5cf

Browse files
isheludkotargos
isheludko
authored and
targos
committed
src: stop using v8::PropertyCallbackInfo<T>::This()
Refs: #60616
1 parent e94fd87 commit 113b5cf

File tree

5 files changed

+44
-33
lines changed

5 files changed

+44
-33
lines changed

‎src/module_wrap.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,7 @@ void ModuleWrap::HasAsyncGraph(Local<Name> property,
10321032
Isolate* isolate = args.GetIsolate();
10331033
Environment* env = Environment::GetCurrent(isolate);
10341034
ModuleWrap* obj;
1035-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
1035+
ASSIGN_OR_RETURN_UNWRAP(&obj, args.HolderV2());
10361036

10371037
Local<Module> module = obj->module_.Get(isolate);
10381038
if (module->GetStatus() < Module::kInstantiated) {
@@ -1248,7 +1248,7 @@ void ModuleWrap::SetImportMetaResolveInitializer(
12481248
static void ImportMetaResolveLazyGetter(
12491249
Local<v8::Name> name, const PropertyCallbackInfo<Value>& info) {
12501250
Isolate* isolate = info.GetIsolate();
1251-
Local<Value> receiver_val = info.This();
1251+
Local<Value> receiver_val = info.HolderV2();
12521252
if (!receiver_val->IsObject()) {
12531253
THROW_ERR_INVALID_INVOCATION(isolate);
12541254
return;
@@ -1289,7 +1289,7 @@ static void PathHelpersLazyGetter(Local<v8::Name> name,
12891289
// When this getter is invoked in a vm context, the `Realm::GetCurrent(info)`
12901290
// returns a nullptr and retrieve the creation context via `this` object and
12911291
// get the creation Realm.
1292-
Local<Value> receiver_val = info.This();
1292+
Local<Value> receiver_val = info.HolderV2();
12931293
if (!receiver_val->IsObject()) {
12941294
THROW_ERR_INVALID_INVOCATION(isolate);
12951295
return;

‎src/node_contextify.cc‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
453453
// args.GetIsolate()->GetCurrentContext() and take the pointer at
454454
// ContextEmbedderIndex::kContextifyContext, as V8 is supposed to
455455
// push the creation context before invoking these callbacks.
456-
return Get(args.This());
456+
return Get(args.HolderV2());
457457
}
458458

459459
ContextifyContext* ContextifyContext::Get(Local<Object> object) {
@@ -587,27 +587,38 @@ Intercepted ContextifyContext::PropertySetterCallback(
587587
return Intercepted::kNo;
588588
}
589589

590-
// true for x = 5
591-
// false for this.x = 5
592-
// false for Object.defineProperty(this, 'foo', ...)
593-
// false for vmResult.x = 5 where vmResult = vm.runInContext();
594-
bool is_contextual_store = ctx->global_proxy() != args.This();
595-
596-
// Indicator to not return before setting (undeclared) function declarations
597-
// on the sandbox in strict mode, i.e. args.ShouldThrowOnError() = true.
598-
// True for 'function f() {}', 'this.f = function() {}',
599-
// 'var f = function()'.
600-
// In effect only for 'function f() {}' because
601-
// var f = function(), is_declared = true
602-
// this.f = function() {}, is_contextual_store = false.
603-
bool is_function = value->IsFunction();
604-
590+
// V8 comment: As long as the context is not detached the contextual accesses
591+
// are the same as regular accesses to `context->Global()`s data property.
592+
// The only difference is that after detaching `args.Holder()` will
593+
// become a new identity and will no longer be equal to `context->Global()`.
594+
// TODO(Node.js): revise the code below as the "contextual"-ness of the
595+
// store is not actually relevant here. Also, new variable declaration is
596+
// reported by V8 via PropertyDefinerCallback.
605597
bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox;
606-
if (!is_declared && args.ShouldThrowOnError() && is_contextual_store &&
607-
!is_function) {
608-
return Intercepted::kNo;
609-
}
610598

599+
/*
600+
// true for x = 5
601+
// false for this.x = 5
602+
// false for Object.defineProperty(this, 'foo', ...)
603+
// false for vmResult.x = 5 where vmResult = vm.runInContext();
604+
605+
bool is_contextual_store = ctx->global_proxy() != args.This();
606+
607+
// Indicator to not return before setting (undeclared) function declarations
608+
// on the sandbox in strict mode, i.e. args.ShouldThrowOnError() = true.
609+
// True for 'function f() {}', 'this.f = function() {}',
610+
// 'var f = function()'.
611+
// In effect only for 'function f() {}' because
612+
// var f = function(), is_declared = true
613+
// this.f = function() {}, is_contextual_store = false.
614+
bool is_function = value->IsFunction();
615+
616+
bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox;
617+
if (!is_declared && args.ShouldThrowOnError() && is_contextual_store &&
618+
!is_function) {
619+
return Intercepted::kNo;
620+
}
621+
*/
611622
if (!is_declared && property->IsSymbol()) {
612623
return Intercepted::kNo;
613624
}

‎src/node_sqlite.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ Intercepted DatabaseSyncLimits::LimitsGetter(
729729
}
730730

731731
DatabaseSyncLimits* limits;
732-
ASSIGN_OR_RETURN_UNWRAP(&limits, info.This(), Intercepted::kNo);
732+
ASSIGN_OR_RETURN_UNWRAP(&limits, info.HolderV2(), Intercepted::kNo);
733733

734734
Environment* env = limits->env();
735735
Isolate* isolate = env->isolate();
@@ -761,7 +761,7 @@ Intercepted DatabaseSyncLimits::LimitsSetter(
761761
}
762762

763763
DatabaseSyncLimits* limits;
764-
ASSIGN_OR_RETURN_UNWRAP(&limits, info.This(), Intercepted::kNo);
764+
ASSIGN_OR_RETURN_UNWRAP(&limits, info.HolderV2(), Intercepted::kNo);
765765

766766
Environment* env = limits->env();
767767
Isolate* isolate = env->isolate();

‎src/node_util.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ static void DefineLazyPropertiesGetter(
366366
// When this getter is invoked in a vm context, the `Realm::GetCurrent(info)`
367367
// returns a nullptr and retrieve the creation context via `this` object and
368368
// get the creation Realm.
369-
Local<Value> receiver_val = info.This();
369+
Local<Value> receiver_val = info.HolderV2();
370370
if (!receiver_val->IsObject()) {
371371
THROW_ERR_INVALID_INVOCATION(isolate);
372372
return;

‎src/node_webstorage.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ template <typename T>
530530
static bool ShouldIntercept(Local<Name> property,
531531
const PropertyCallbackInfo<T>& info) {
532532
Environment* env = Environment::GetCurrent(info);
533-
Local<Value> proto = info.This()->GetPrototypeV2();
533+
Local<Value> proto = info.HolderV2()->GetPrototypeV2();
534534

535535
if (proto->IsObject()) {
536536
bool has_prop;
@@ -554,7 +554,7 @@ static Intercepted StorageGetter(Local<Name> property,
554554
}
555555

556556
Storage* storage;
557-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
557+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
558558
Local<Value> result;
559559

560560
if (storage->Load(property).ToLocal(&result) && !result->IsNull()) {
@@ -568,7 +568,7 @@ static Intercepted StorageSetter(Local<Name> property,
568568
Local<Value> value,
569569
const PropertyCallbackInfo<void>& info) {
570570
Storage* storage;
571-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
571+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
572572

573573
if (storage->Store(property, value).IsNothing()) {
574574
info.GetReturnValue().SetFalse();
@@ -584,7 +584,7 @@ static Intercepted StorageQuery(Local<Name> property,
584584
}
585585

586586
Storage* storage;
587-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
587+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
588588
Local<Value> result;
589589
if (!storage->Load(property).ToLocal(&result) || result->IsNull()) {
590590
return Intercepted::kNo;
@@ -597,7 +597,7 @@ static Intercepted StorageQuery(Local<Name> property,
597597
static Intercepted StorageDeleter(Local<Name> property,
598598
const PropertyCallbackInfo<Boolean>& info) {
599599
Storage* storage;
600-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
600+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
601601

602602
info.GetReturnValue().Set(storage->Remove(property).IsJust());
603603

@@ -606,7 +606,7 @@ static Intercepted StorageDeleter(Local<Name> property,
606606

607607
static void StorageEnumerator(const PropertyCallbackInfo<Array>& info) {
608608
Storage* storage;
609-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This());
609+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2());
610610
Local<Array> result;
611611
if (!storage->Enumerate().ToLocal(&result)) {
612612
return;
@@ -618,7 +618,7 @@ static Intercepted StorageDefiner(Local<Name> property,
618618
const PropertyDescriptor& desc,
619619
const PropertyCallbackInfo<void>& info) {
620620
Storage* storage;
621-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
621+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
622622

623623
if (desc.has_value()) {
624624
return StorageSetter(property, desc.value(), info);

0 commit comments

Comments
 (0)