Skip to content

Commit 8aea917

Browse files
isheludkotargos
isheludko
authored and
targos
committed
src: stop using v8::PropertyCallbackInfo<T>::This()
Refs: #60616 PR-URL: #61898 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent f3e0b79 commit 8aea917

5 files changed

Lines changed: 44 additions & 33 deletions

File tree

‎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
@@ -454,7 +454,7 @@ ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
454454
// args.GetIsolate()->GetCurrentContext() and take the pointer at
455455
// ContextEmbedderIndex::kContextifyContext, as V8 is supposed to
456456
// push the creation context before invoking these callbacks.
457-
return Get(args.This());
457+
return Get(args.HolderV2());
458458
}
459459

460460
ContextifyContext* ContextifyContext::Get(Local<Object> object) {
@@ -600,27 +600,38 @@ Intercepted ContextifyContext::PropertySetterCallback(
600600
return Intercepted::kNo;
601601
}
602602

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

612+
/*
613+
// true for x = 5
614+
// false for this.x = 5
615+
// false for Object.defineProperty(this, 'foo', ...)
616+
// false for vmResult.x = 5 where vmResult = vm.runInContext();
617+
618+
bool is_contextual_store = ctx->global_proxy() != args.This();
619+
620+
// Indicator to not return before setting (undeclared) function declarations
621+
// on the sandbox in strict mode, i.e. args.ShouldThrowOnError() = true.
622+
// True for 'function f() {}', 'this.f = function() {}',
623+
// 'var f = function()'.
624+
// In effect only for 'function f() {}' because
625+
// var f = function(), is_declared = true
626+
// this.f = function() {}, is_contextual_store = false.
627+
bool is_function = value->IsFunction();
628+
629+
bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox;
630+
if (!is_declared && args.ShouldThrowOnError() && is_contextual_store &&
631+
!is_function) {
632+
return Intercepted::kNo;
633+
}
634+
*/
624635
if (!is_declared && property->IsSymbol()) {
625636
return Intercepted::kNo;
626637
}

‎src/node_sqlite.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,7 @@ Intercepted DatabaseSyncLimits::LimitsGetter(
748748
}
749749

750750
DatabaseSyncLimits* limits;
751-
ASSIGN_OR_RETURN_UNWRAP(&limits, info.This(), Intercepted::kNo);
751+
ASSIGN_OR_RETURN_UNWRAP(&limits, info.HolderV2(), Intercepted::kNo);
752752

753753
Environment* env = limits->env();
754754
Isolate* isolate = env->isolate();
@@ -780,7 +780,7 @@ Intercepted DatabaseSyncLimits::LimitsSetter(
780780
}
781781

782782
DatabaseSyncLimits* limits;
783-
ASSIGN_OR_RETURN_UNWRAP(&limits, info.This(), Intercepted::kNo);
783+
ASSIGN_OR_RETURN_UNWRAP(&limits, info.HolderV2(), Intercepted::kNo);
784784

785785
Environment* env = limits->env();
786786
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
@@ -562,7 +562,7 @@ template <typename T>
562562
static bool ShouldIntercept(Local<Name> property,
563563
const PropertyCallbackInfo<T>& info) {
564564
Environment* env = Environment::GetCurrent(info);
565-
Local<Value> proto = info.This()->GetPrototypeV2();
565+
Local<Value> proto = info.HolderV2()->GetPrototypeV2();
566566

567567
if (proto->IsObject()) {
568568
bool has_prop;
@@ -586,7 +586,7 @@ static Intercepted StorageGetter(Local<Name> property,
586586
}
587587

588588
Storage* storage;
589-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
589+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
590590
Local<Value> result;
591591

592592
if (storage->Load(property).ToLocal(&result) && !result->IsNull()) {
@@ -600,7 +600,7 @@ static Intercepted StorageSetter(Local<Name> property,
600600
Local<Value> value,
601601
const PropertyCallbackInfo<void>& info) {
602602
Storage* storage;
603-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
603+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
604604

605605
if (storage->Store(property, value).IsNothing()) {
606606
info.GetReturnValue().SetFalse();
@@ -616,7 +616,7 @@ static Intercepted StorageQuery(Local<Name> property,
616616
}
617617

618618
Storage* storage;
619-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
619+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
620620
Local<Value> result;
621621
if (!storage->Load(property).ToLocal(&result) || result->IsNull()) {
622622
return Intercepted::kNo;
@@ -629,7 +629,7 @@ static Intercepted StorageQuery(Local<Name> property,
629629
static Intercepted StorageDeleter(Local<Name> property,
630630
const PropertyCallbackInfo<Boolean>& info) {
631631
Storage* storage;
632-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
632+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
633633

634634
info.GetReturnValue().Set(storage->Remove(property).IsJust());
635635

@@ -638,7 +638,7 @@ static Intercepted StorageDeleter(Local<Name> property,
638638

639639
static void StorageEnumerator(const PropertyCallbackInfo<Array>& info) {
640640
Storage* storage;
641-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This());
641+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2());
642642
Local<Array> result;
643643
if (!storage->Enumerate().ToLocal(&result)) {
644644
return;
@@ -650,7 +650,7 @@ static Intercepted StorageDefiner(Local<Name> property,
650650
const PropertyDescriptor& desc,
651651
const PropertyCallbackInfo<void>& info) {
652652
Storage* storage;
653-
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
653+
ASSIGN_OR_RETURN_UNWRAP(&storage, info.HolderV2(), Intercepted::kNo);
654654

655655
if (desc.has_value()) {
656656
return StorageSetter(property, desc.value(), info);

0 commit comments

Comments
 (0)