Skip to content

Commit 8dc1596

Browse files
targostargos
committed
deps: cherry-pick c608122 from upstream V8
Original commit message: [api][keys] Allow skipping indices for Proxies with GetPropertyNames Bug: v8:7942 Change-Id: I7b3740b04cbcaa56dc809150900ab8d821b054ce Reviewed-on: https://chromium-review.googlesource.com/1156544 Reviewed-by: Toon Verwaest <verwaest@chromium.org> Commit-Queue: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/master@{#54821} Refs: v8/v8@c608122 PR-URL: #21983 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 127e703 commit 8dc1596

File tree

5 files changed

+137
-27
lines changed

5 files changed

+137
-27
lines changed

‎common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
# Reset this number to 0 on major V8 upgrades.
3131
# Increment by one for each non-official patch applied to deps/v8.
32-
'v8_embedder_string': '-node.3',
32+
'v8_embedder_string': '-node.4',
3333

3434
# Enable disassembler for `--print-code` v8 options
3535
'v8_enable_disassembler': 1,

‎deps/v8/src/keys.cc

+30-18
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ static bool ContainsOnlyValidKeys(Handle<FixedArray> array) {
3838
// static
3939
MaybeHandle<FixedArray> KeyAccumulator::GetKeys(
4040
Handle<JSReceiver> object, KeyCollectionMode mode, PropertyFilter filter,
41-
GetKeysConversion keys_conversion, bool is_for_in) {
41+
GetKeysConversion keys_conversion, bool is_for_in, bool skip_indices) {
4242
Isolate* isolate = object->GetIsolate();
43-
FastKeyAccumulator accumulator(isolate, object, mode, filter);
44-
accumulator.set_is_for_in(is_for_in);
43+
FastKeyAccumulator accumulator(isolate, object, mode, filter, is_for_in,
44+
skip_indices);
4545
return accumulator.GetKeys(keys_conversion);
4646
}
4747

@@ -355,7 +355,8 @@ Handle<FixedArray> GetFastEnumPropertyKeys(Isolate* isolate,
355355
template <bool fast_properties>
356356
MaybeHandle<FixedArray> GetOwnKeysWithElements(Isolate* isolate,
357357
Handle<JSObject> object,
358-
GetKeysConversion convert) {
358+
GetKeysConversion convert,
359+
bool skip_indices) {
359360
Handle<FixedArray> keys;
360361
ElementsAccessor* accessor = object->GetElementsAccessor();
361362
if (fast_properties) {
@@ -364,8 +365,14 @@ MaybeHandle<FixedArray> GetOwnKeysWithElements(Isolate* isolate,
364365
// TODO(cbruni): preallocate big enough array to also hold elements.
365366
keys = KeyAccumulator::GetOwnEnumPropertyKeys(isolate, object);
366367
}
367-
MaybeHandle<FixedArray> result =
368-
accessor->PrependElementIndices(object, keys, convert, ONLY_ENUMERABLE);
368+
369+
MaybeHandle<FixedArray> result;
370+
if (skip_indices) {
371+
result = keys;
372+
} else {
373+
result =
374+
accessor->PrependElementIndices(object, keys, convert, ONLY_ENUMERABLE);
375+
}
369376

370377
if (FLAG_trace_for_in_enumerate) {
371378
PrintF("| strings=%d symbols=0 elements=%u || prototypes>=1 ||\n",
@@ -403,7 +410,8 @@ MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysFast(
403410

404411
// Do not try to use the enum-cache for dict-mode objects.
405412
if (map->is_dictionary_map()) {
406-
return GetOwnKeysWithElements<false>(isolate_, object, keys_conversion);
413+
return GetOwnKeysWithElements<false>(isolate_, object, keys_conversion,
414+
skip_indices_);
407415
}
408416
int enum_length = receiver_->map()->EnumLength();
409417
if (enum_length == kInvalidEnumCacheSentinel) {
@@ -421,7 +429,8 @@ MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysFast(
421429
}
422430
// The properties-only case failed because there were probably elements on the
423431
// receiver.
424-
return GetOwnKeysWithElements<true>(isolate_, object, keys_conversion);
432+
return GetOwnKeysWithElements<true>(isolate_, object, keys_conversion,
433+
skip_indices_);
425434
}
426435

427436
MaybeHandle<FixedArray>
@@ -450,6 +459,7 @@ MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysSlow(
450459
GetKeysConversion keys_conversion) {
451460
KeyAccumulator accumulator(isolate_, mode_, filter_);
452461
accumulator.set_is_for_in(is_for_in_);
462+
accumulator.set_skip_indices(skip_indices_);
453463
accumulator.set_last_non_empty_prototype(last_non_empty_prototype_);
454464

455465
MAYBE_RETURN(accumulator.CollectKeys(receiver_, receiver_),
@@ -699,13 +709,15 @@ Maybe<bool> KeyAccumulator::CollectOwnPropertyNames(Handle<JSReceiver> receiver,
699709
Maybe<bool> KeyAccumulator::CollectAccessCheckInterceptorKeys(
700710
Handle<AccessCheckInfo> access_check_info, Handle<JSReceiver> receiver,
701711
Handle<JSObject> object) {
702-
MAYBE_RETURN((CollectInterceptorKeysInternal(
703-
receiver, object,
704-
handle(InterceptorInfo::cast(
705-
access_check_info->indexed_interceptor()),
706-
isolate_),
707-
this, kIndexed)),
708-
Nothing<bool>());
712+
if (!skip_indices_) {
713+
MAYBE_RETURN((CollectInterceptorKeysInternal(
714+
receiver, object,
715+
handle(InterceptorInfo::cast(
716+
access_check_info->indexed_interceptor()),
717+
isolate_),
718+
this, kIndexed)),
719+
Nothing<bool>());
720+
}
709721
MAYBE_RETURN(
710722
(CollectInterceptorKeysInternal(
711723
receiver, object,
@@ -942,9 +954,9 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyTargetKeys(
942954
Handle<FixedArray> keys;
943955
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
944956
isolate_, keys,
945-
KeyAccumulator::GetKeys(target, KeyCollectionMode::kOwnOnly,
946-
ALL_PROPERTIES,
947-
GetKeysConversion::kConvertToString, is_for_in_),
957+
KeyAccumulator::GetKeys(
958+
target, KeyCollectionMode::kOwnOnly, ALL_PROPERTIES,
959+
GetKeysConversion::kConvertToString, is_for_in_, skip_indices_),
948960
Nothing<bool>());
949961
Maybe<bool> result = AddKeysFromJSProxy(proxy, keys);
950962
return result;

‎deps/v8/src/keys.h

+10-4
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class KeyAccumulator final BASE_EMBEDDED {
4040
static MaybeHandle<FixedArray> GetKeys(
4141
Handle<JSReceiver> object, KeyCollectionMode mode, PropertyFilter filter,
4242
GetKeysConversion keys_conversion = GetKeysConversion::kKeepNumbers,
43-
bool is_for_in = false);
43+
bool is_for_in = false, bool skip_indices = false);
4444

4545
Handle<FixedArray> GetKeys(
4646
GetKeysConversion convert = GetKeysConversion::kKeepNumbers);
@@ -128,14 +128,19 @@ class KeyAccumulator final BASE_EMBEDDED {
128128
class FastKeyAccumulator {
129129
public:
130130
FastKeyAccumulator(Isolate* isolate, Handle<JSReceiver> receiver,
131-
KeyCollectionMode mode, PropertyFilter filter)
132-
: isolate_(isolate), receiver_(receiver), mode_(mode), filter_(filter) {
131+
KeyCollectionMode mode, PropertyFilter filter,
132+
bool is_for_in = false, bool skip_indices = false)
133+
: isolate_(isolate),
134+
receiver_(receiver),
135+
mode_(mode),
136+
filter_(filter),
137+
is_for_in_(is_for_in),
138+
skip_indices_(skip_indices) {
133139
Prepare();
134140
}
135141

136142
bool is_receiver_simple_enum() { return is_receiver_simple_enum_; }
137143
bool has_empty_prototype() { return has_empty_prototype_; }
138-
void set_is_for_in(bool value) { is_for_in_ = value; }
139144

140145
MaybeHandle<FixedArray> GetKeys(
141146
GetKeysConversion convert = GetKeysConversion::kKeepNumbers);
@@ -153,6 +158,7 @@ class FastKeyAccumulator {
153158
KeyCollectionMode mode_;
154159
PropertyFilter filter_;
155160
bool is_for_in_ = false;
161+
bool skip_indices_ = false;
156162
bool is_receiver_simple_enum_ = false;
157163
bool has_empty_prototype_ = false;
158164

‎deps/v8/src/runtime/runtime-forin.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ MaybeHandle<HeapObject> Enumerate(Isolate* isolate,
2626
JSObject::MakePrototypesFast(receiver, kStartAtReceiver, isolate);
2727
FastKeyAccumulator accumulator(isolate, receiver,
2828
KeyCollectionMode::kIncludePrototypes,
29-
ENUMERABLE_STRINGS);
30-
accumulator.set_is_for_in(true);
29+
ENUMERABLE_STRINGS, true);
3130
// Test if we have an enum cache for {receiver}.
3231
if (!accumulator.is_receiver_simple_enum()) {
3332
Handle<FixedArray> keys;

‎deps/v8/test/cctest/test-api.cc

+95-2
Original file line numberDiff line numberDiff line change
@@ -15360,14 +15360,107 @@ THREADED_TEST(PropertyEnumeration2) {
1536015360
}
1536115361
}
1536215362

15363-
THREADED_TEST(PropertyNames) {
15363+
THREADED_TEST(GetPropertyNames) {
1536415364
LocalContext context;
1536515365
v8::Isolate* isolate = context->GetIsolate();
1536615366
v8::HandleScope scope(isolate);
1536715367
v8::Local<v8::Value> result = CompileRun(
1536815368
"var result = {0: 0, 1: 1, a: 2, b: 3};"
1536915369
"result[Symbol('symbol')] = true;"
15370-
"result.__proto__ = {2: 4, 3: 5, c: 6, d: 7};"
15370+
"result.__proto__ = {__proto__:null, 2: 4, 3: 5, c: 6, d: 7};"
15371+
"result;");
15372+
v8::Local<v8::Object> object = result.As<v8::Object>();
15373+
v8::PropertyFilter default_filter =
15374+
static_cast<v8::PropertyFilter>(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS);
15375+
v8::PropertyFilter include_symbols_filter = v8::ONLY_ENUMERABLE;
15376+
15377+
v8::Local<v8::Array> properties =
15378+
object->GetPropertyNames(context.local()).ToLocalChecked();
15379+
const char* expected_properties1[] = {"0", "1", "a", "b", "2", "3", "c", "d"};
15380+
CheckStringArray(isolate, properties, 8, expected_properties1);
15381+
15382+
properties =
15383+
object
15384+
->GetPropertyNames(context.local(),
15385+
v8::KeyCollectionMode::kIncludePrototypes,
15386+
default_filter, v8::IndexFilter::kIncludeIndices)
15387+
.ToLocalChecked();
15388+
CheckStringArray(isolate, properties, 8, expected_properties1);
15389+
15390+
properties = object
15391+
->GetPropertyNames(context.local(),
15392+
v8::KeyCollectionMode::kIncludePrototypes,
15393+
include_symbols_filter,
15394+
v8::IndexFilter::kIncludeIndices)
15395+
.ToLocalChecked();
15396+
const char* expected_properties1_1[] = {"0", "1", "a", "b", nullptr,
15397+
"2", "3", "c", "d"};
15398+
CheckStringArray(isolate, properties, 9, expected_properties1_1);
15399+
CheckIsSymbolAt(isolate, properties, 4, "symbol");
15400+
15401+
properties =
15402+
object
15403+
->GetPropertyNames(context.local(),
15404+
v8::KeyCollectionMode::kIncludePrototypes,
15405+
default_filter, v8::IndexFilter::kSkipIndices)
15406+
.ToLocalChecked();
15407+
const char* expected_properties2[] = {"a", "b", "c", "d"};
15408+
CheckStringArray(isolate, properties, 4, expected_properties2);
15409+
15410+
properties = object
15411+
->GetPropertyNames(context.local(),
15412+
v8::KeyCollectionMode::kIncludePrototypes,
15413+
include_symbols_filter,
15414+
v8::IndexFilter::kSkipIndices)
15415+
.ToLocalChecked();
15416+
const char* expected_properties2_1[] = {"a", "b", nullptr, "c", "d"};
15417+
CheckStringArray(isolate, properties, 5, expected_properties2_1);
15418+
CheckIsSymbolAt(isolate, properties, 2, "symbol");
15419+
15420+
properties =
15421+
object
15422+
->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly,
15423+
default_filter, v8::IndexFilter::kIncludeIndices)
15424+
.ToLocalChecked();
15425+
const char* expected_properties3[] = {"0", "1", "a", "b"};
15426+
CheckStringArray(isolate, properties, 4, expected_properties3);
15427+
15428+
properties = object
15429+
->GetPropertyNames(
15430+
context.local(), v8::KeyCollectionMode::kOwnOnly,
15431+
include_symbols_filter, v8::IndexFilter::kIncludeIndices)
15432+
.ToLocalChecked();
15433+
const char* expected_properties3_1[] = {"0", "1", "a", "b", nullptr};
15434+
CheckStringArray(isolate, properties, 5, expected_properties3_1);
15435+
CheckIsSymbolAt(isolate, properties, 4, "symbol");
15436+
15437+
properties =
15438+
object
15439+
->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly,
15440+
default_filter, v8::IndexFilter::kSkipIndices)
15441+
.ToLocalChecked();
15442+
const char* expected_properties4[] = {"a", "b"};
15443+
CheckStringArray(isolate, properties, 2, expected_properties4);
15444+
15445+
properties = object
15446+
->GetPropertyNames(
15447+
context.local(), v8::KeyCollectionMode::kOwnOnly,
15448+
include_symbols_filter, v8::IndexFilter::kSkipIndices)
15449+
.ToLocalChecked();
15450+
const char* expected_properties4_1[] = {"a", "b", nullptr};
15451+
CheckStringArray(isolate, properties, 3, expected_properties4_1);
15452+
CheckIsSymbolAt(isolate, properties, 2, "symbol");
15453+
}
15454+
15455+
THREADED_TEST(ProxyGetPropertyNames) {
15456+
LocalContext context;
15457+
v8::Isolate* isolate = context->GetIsolate();
15458+
v8::HandleScope scope(isolate);
15459+
v8::Local<v8::Value> result = CompileRun(
15460+
"var target = {0: 0, 1: 1, a: 2, b: 3};"
15461+
"target[Symbol('symbol')] = true;"
15462+
"target.__proto__ = {__proto__:null, 2: 4, 3: 5, c: 6, d: 7};"
15463+
"var result = new Proxy(target, {});"
1537115464
"result;");
1537215465
v8::Local<v8::Object> object = result.As<v8::Object>();
1537315466
v8::PropertyFilter default_filter =

0 commit comments

Comments
 (0)