From 8dc159658c97ff04dfc08ff5bfcd1a1a17b98430 Mon Sep 17 00:00:00 2001 From: Michaël Zasso Date: Sun, 19 Aug 2018 21:53:40 +0200 Subject: 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 Commit-Queue: Camillo Bruni Cr-Commit-Position: refs/heads/master@{#54821} Refs: https://github.com/v8/v8/commit/c608122b85238397a43910246f5ff218eb43fb24 PR-URL: https://github.com/nodejs/node/pull/21983 Reviewed-By: Refael Ackermann Reviewed-By: Gus Caplan Reviewed-By: Ujjwal Sharma Reviewed-By: Matteo Collina --- deps/v8/src/keys.cc | 48 +++++++++++------- deps/v8/src/keys.h | 14 ++++-- deps/v8/src/runtime/runtime-forin.cc | 3 +- deps/v8/test/cctest/test-api.cc | 97 +++++++++++++++++++++++++++++++++++- 4 files changed, 136 insertions(+), 26 deletions(-) (limited to 'deps') diff --git a/deps/v8/src/keys.cc b/deps/v8/src/keys.cc index 8ecbe0a1d7..689f4ac3df 100644 --- a/deps/v8/src/keys.cc +++ b/deps/v8/src/keys.cc @@ -38,10 +38,10 @@ static bool ContainsOnlyValidKeys(Handle array) { // static MaybeHandle KeyAccumulator::GetKeys( Handle object, KeyCollectionMode mode, PropertyFilter filter, - GetKeysConversion keys_conversion, bool is_for_in) { + GetKeysConversion keys_conversion, bool is_for_in, bool skip_indices) { Isolate* isolate = object->GetIsolate(); - FastKeyAccumulator accumulator(isolate, object, mode, filter); - accumulator.set_is_for_in(is_for_in); + FastKeyAccumulator accumulator(isolate, object, mode, filter, is_for_in, + skip_indices); return accumulator.GetKeys(keys_conversion); } @@ -355,7 +355,8 @@ Handle GetFastEnumPropertyKeys(Isolate* isolate, template MaybeHandle GetOwnKeysWithElements(Isolate* isolate, Handle object, - GetKeysConversion convert) { + GetKeysConversion convert, + bool skip_indices) { Handle keys; ElementsAccessor* accessor = object->GetElementsAccessor(); if (fast_properties) { @@ -364,8 +365,14 @@ MaybeHandle GetOwnKeysWithElements(Isolate* isolate, // TODO(cbruni): preallocate big enough array to also hold elements. keys = KeyAccumulator::GetOwnEnumPropertyKeys(isolate, object); } - MaybeHandle result = - accessor->PrependElementIndices(object, keys, convert, ONLY_ENUMERABLE); + + MaybeHandle result; + if (skip_indices) { + result = keys; + } else { + result = + accessor->PrependElementIndices(object, keys, convert, ONLY_ENUMERABLE); + } if (FLAG_trace_for_in_enumerate) { PrintF("| strings=%d symbols=0 elements=%u || prototypes>=1 ||\n", @@ -403,7 +410,8 @@ MaybeHandle FastKeyAccumulator::GetKeysFast( // Do not try to use the enum-cache for dict-mode objects. if (map->is_dictionary_map()) { - return GetOwnKeysWithElements(isolate_, object, keys_conversion); + return GetOwnKeysWithElements(isolate_, object, keys_conversion, + skip_indices_); } int enum_length = receiver_->map()->EnumLength(); if (enum_length == kInvalidEnumCacheSentinel) { @@ -421,7 +429,8 @@ MaybeHandle FastKeyAccumulator::GetKeysFast( } // The properties-only case failed because there were probably elements on the // receiver. - return GetOwnKeysWithElements(isolate_, object, keys_conversion); + return GetOwnKeysWithElements(isolate_, object, keys_conversion, + skip_indices_); } MaybeHandle @@ -450,6 +459,7 @@ MaybeHandle FastKeyAccumulator::GetKeysSlow( GetKeysConversion keys_conversion) { KeyAccumulator accumulator(isolate_, mode_, filter_); accumulator.set_is_for_in(is_for_in_); + accumulator.set_skip_indices(skip_indices_); accumulator.set_last_non_empty_prototype(last_non_empty_prototype_); MAYBE_RETURN(accumulator.CollectKeys(receiver_, receiver_), @@ -699,13 +709,15 @@ Maybe KeyAccumulator::CollectOwnPropertyNames(Handle receiver, Maybe KeyAccumulator::CollectAccessCheckInterceptorKeys( Handle access_check_info, Handle receiver, Handle object) { - MAYBE_RETURN((CollectInterceptorKeysInternal( - receiver, object, - handle(InterceptorInfo::cast( - access_check_info->indexed_interceptor()), - isolate_), - this, kIndexed)), - Nothing()); + if (!skip_indices_) { + MAYBE_RETURN((CollectInterceptorKeysInternal( + receiver, object, + handle(InterceptorInfo::cast( + access_check_info->indexed_interceptor()), + isolate_), + this, kIndexed)), + Nothing()); + } MAYBE_RETURN( (CollectInterceptorKeysInternal( receiver, object, @@ -942,9 +954,9 @@ Maybe KeyAccumulator::CollectOwnJSProxyTargetKeys( Handle keys; ASSIGN_RETURN_ON_EXCEPTION_VALUE( isolate_, keys, - KeyAccumulator::GetKeys(target, KeyCollectionMode::kOwnOnly, - ALL_PROPERTIES, - GetKeysConversion::kConvertToString, is_for_in_), + KeyAccumulator::GetKeys( + target, KeyCollectionMode::kOwnOnly, ALL_PROPERTIES, + GetKeysConversion::kConvertToString, is_for_in_, skip_indices_), Nothing()); Maybe result = AddKeysFromJSProxy(proxy, keys); return result; diff --git a/deps/v8/src/keys.h b/deps/v8/src/keys.h index 649d6a9599..5abbaac5cd 100644 --- a/deps/v8/src/keys.h +++ b/deps/v8/src/keys.h @@ -40,7 +40,7 @@ class KeyAccumulator final BASE_EMBEDDED { static MaybeHandle GetKeys( Handle object, KeyCollectionMode mode, PropertyFilter filter, GetKeysConversion keys_conversion = GetKeysConversion::kKeepNumbers, - bool is_for_in = false); + bool is_for_in = false, bool skip_indices = false); Handle GetKeys( GetKeysConversion convert = GetKeysConversion::kKeepNumbers); @@ -128,14 +128,19 @@ class KeyAccumulator final BASE_EMBEDDED { class FastKeyAccumulator { public: FastKeyAccumulator(Isolate* isolate, Handle receiver, - KeyCollectionMode mode, PropertyFilter filter) - : isolate_(isolate), receiver_(receiver), mode_(mode), filter_(filter) { + KeyCollectionMode mode, PropertyFilter filter, + bool is_for_in = false, bool skip_indices = false) + : isolate_(isolate), + receiver_(receiver), + mode_(mode), + filter_(filter), + is_for_in_(is_for_in), + skip_indices_(skip_indices) { Prepare(); } bool is_receiver_simple_enum() { return is_receiver_simple_enum_; } bool has_empty_prototype() { return has_empty_prototype_; } - void set_is_for_in(bool value) { is_for_in_ = value; } MaybeHandle GetKeys( GetKeysConversion convert = GetKeysConversion::kKeepNumbers); @@ -153,6 +158,7 @@ class FastKeyAccumulator { KeyCollectionMode mode_; PropertyFilter filter_; bool is_for_in_ = false; + bool skip_indices_ = false; bool is_receiver_simple_enum_ = false; bool has_empty_prototype_ = false; diff --git a/deps/v8/src/runtime/runtime-forin.cc b/deps/v8/src/runtime/runtime-forin.cc index 5df16faf46..ed1e226060 100644 --- a/deps/v8/src/runtime/runtime-forin.cc +++ b/deps/v8/src/runtime/runtime-forin.cc @@ -26,8 +26,7 @@ MaybeHandle Enumerate(Isolate* isolate, JSObject::MakePrototypesFast(receiver, kStartAtReceiver, isolate); FastKeyAccumulator accumulator(isolate, receiver, KeyCollectionMode::kIncludePrototypes, - ENUMERABLE_STRINGS); - accumulator.set_is_for_in(true); + ENUMERABLE_STRINGS, true); // Test if we have an enum cache for {receiver}. if (!accumulator.is_receiver_simple_enum()) { Handle keys; diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index d5d1ae101b..a018e12853 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -15360,14 +15360,107 @@ THREADED_TEST(PropertyEnumeration2) { } } -THREADED_TEST(PropertyNames) { +THREADED_TEST(GetPropertyNames) { LocalContext context; v8::Isolate* isolate = context->GetIsolate(); v8::HandleScope scope(isolate); v8::Local result = CompileRun( "var result = {0: 0, 1: 1, a: 2, b: 3};" "result[Symbol('symbol')] = true;" - "result.__proto__ = {2: 4, 3: 5, c: 6, d: 7};" + "result.__proto__ = {__proto__:null, 2: 4, 3: 5, c: 6, d: 7};" + "result;"); + v8::Local object = result.As(); + v8::PropertyFilter default_filter = + static_cast(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS); + v8::PropertyFilter include_symbols_filter = v8::ONLY_ENUMERABLE; + + v8::Local properties = + object->GetPropertyNames(context.local()).ToLocalChecked(); + const char* expected_properties1[] = {"0", "1", "a", "b", "2", "3", "c", "d"}; + CheckStringArray(isolate, properties, 8, expected_properties1); + + properties = + object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + default_filter, v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + CheckStringArray(isolate, properties, 8, expected_properties1); + + properties = object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + include_symbols_filter, + v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + const char* expected_properties1_1[] = {"0", "1", "a", "b", nullptr, + "2", "3", "c", "d"}; + CheckStringArray(isolate, properties, 9, expected_properties1_1); + CheckIsSymbolAt(isolate, properties, 4, "symbol"); + + properties = + object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + default_filter, v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties2[] = {"a", "b", "c", "d"}; + CheckStringArray(isolate, properties, 4, expected_properties2); + + properties = object + ->GetPropertyNames(context.local(), + v8::KeyCollectionMode::kIncludePrototypes, + include_symbols_filter, + v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties2_1[] = {"a", "b", nullptr, "c", "d"}; + CheckStringArray(isolate, properties, 5, expected_properties2_1); + CheckIsSymbolAt(isolate, properties, 2, "symbol"); + + properties = + object + ->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly, + default_filter, v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + const char* expected_properties3[] = {"0", "1", "a", "b"}; + CheckStringArray(isolate, properties, 4, expected_properties3); + + properties = object + ->GetPropertyNames( + context.local(), v8::KeyCollectionMode::kOwnOnly, + include_symbols_filter, v8::IndexFilter::kIncludeIndices) + .ToLocalChecked(); + const char* expected_properties3_1[] = {"0", "1", "a", "b", nullptr}; + CheckStringArray(isolate, properties, 5, expected_properties3_1); + CheckIsSymbolAt(isolate, properties, 4, "symbol"); + + properties = + object + ->GetPropertyNames(context.local(), v8::KeyCollectionMode::kOwnOnly, + default_filter, v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties4[] = {"a", "b"}; + CheckStringArray(isolate, properties, 2, expected_properties4); + + properties = object + ->GetPropertyNames( + context.local(), v8::KeyCollectionMode::kOwnOnly, + include_symbols_filter, v8::IndexFilter::kSkipIndices) + .ToLocalChecked(); + const char* expected_properties4_1[] = {"a", "b", nullptr}; + CheckStringArray(isolate, properties, 3, expected_properties4_1); + CheckIsSymbolAt(isolate, properties, 2, "symbol"); +} + +THREADED_TEST(ProxyGetPropertyNames) { + LocalContext context; + v8::Isolate* isolate = context->GetIsolate(); + v8::HandleScope scope(isolate); + v8::Local result = CompileRun( + "var target = {0: 0, 1: 1, a: 2, b: 3};" + "target[Symbol('symbol')] = true;" + "target.__proto__ = {__proto__:null, 2: 4, 3: 5, c: 6, d: 7};" + "var result = new Proxy(target, {});" "result;"); v8::Local object = result.As(); v8::PropertyFilter default_filter = -- cgit v1.2.3