[keys] propagate PropertyFilter to proxy targets in KeyAccumulator

src/keys.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/keys.h"
 
 #include "src/api-arguments.h"
 #include "src/elements.h"
 #include "src/factory.h"
 #include "src/identity-map.h"
@@ skipping 20 matching lines @@
 }
 
 }  // namespace
 MaybeHandle<FixedArray> KeyAccumulator::GetKeys(
     Handle<JSReceiver> object, KeyCollectionMode mode, PropertyFilter filter,
     GetKeysConversion keys_conversion, bool filter_proxy_keys, bool is_for_in) {
   Isolate* isolate = object->GetIsolate();
   KeyAccumulator accumulator(isolate, mode, filter);
   accumulator.set_filter_proxy_keys(filter_proxy_keys);
   accumulator.set_is_for_in(is_for_in);
-  MAYBE_RETURN(accumulator.CollectKeys(object, object),
+  MAYBE_RETURN(accumulator.CollectKeys(object, object, keys_conversion),
                MaybeHandle<FixedArray>());
   return accumulator.GetKeys(keys_conversion);
 }
 
 Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
   if (keys_.is_null()) {
     return isolate_->factory()->empty_fixed_array();
   }
   if (mode_ == KeyCollectionMode::kOwnOnly &&
       keys_->map() == isolate_->heap()->fixed_array_map()) {
@@ skipping 92 matching lines @@
   if (mode_ == KeyCollectionMode::kOwnOnly && !is_for_in_) {
     // If we collect only the keys from a JSProxy do not sort or deduplicate it.
     keys_ = keys;
     return Just(true);
   }
   AddKeys(keys, is_for_in_ ? CONVERT_TO_ARRAY_INDEX : DO_NOT_CONVERT);
   return Just(true);
 }
 
 Maybe<bool> KeyAccumulator::CollectKeys(Handle<JSReceiver> receiver,
-                                        Handle<JSReceiver> object) {
+                                        Handle<JSReceiver> object,
+                                        GetKeysConversion keys_conversion) {
   // Proxies have no hidden prototype and we should not trigger the
   // [[GetPrototypeOf]] trap on the last iteration when using
   // AdvanceFollowingProxies.
   if (mode_ == KeyCollectionMode::kOwnOnly && object->IsJSProxy()) {
-    MAYBE_RETURN(CollectOwnJSProxyKeys(receiver, Handle<JSProxy>::cast(object)),
+    MAYBE_RETURN(CollectOwnJSProxyKeys(receiver, Handle<JSProxy>::cast(object),
+                                       keys_conversion),
                  Nothing<bool>());
     return Just(true);
   }
 
   PrototypeIterator::WhereToEnd end = mode_ == KeyCollectionMode::kOwnOnly
                                           ? PrototypeIterator::END_AT_NON_HIDDEN
                                           : PrototypeIterator::END_AT_NULL;
   for (PrototypeIterator iter(isolate_, object, kStartAtReceiver, end);
        !iter.IsAtEnd();) {
     Handle<JSReceiver> current =
         PrototypeIterator::GetCurrent<JSReceiver>(iter);
     Maybe<bool> result = Just(false);  // Dummy initialization.
     if (current->IsJSProxy()) {
-      result = CollectOwnJSProxyKeys(receiver, Handle<JSProxy>::cast(current));
+      result = CollectOwnJSProxyKeys(receiver, Handle<JSProxy>::cast(current),
+                                     keys_conversion);
     } else {
       DCHECK(current->IsJSObject());
       result = CollectOwnKeys(receiver, Handle<JSObject>::cast(current));
     }
     MAYBE_RETURN(result, Nothing<bool>());
     if (!result.FromJust()) break;  // |false| means "stop iterating".
     // Iterate through proxies but ignore access checks for the ALL_CAN_READ
     // case on API objects for OWN_ONLY keys handled in CollectOwnKeys.
     if (!iter.AdvanceFollowingProxiesIgnoringAccessChecks()) {
       return Nothing<bool>();
@@ skipping 512 matching lines @@
         object->global_dictionary());
   } else {
     return GetOwnEnumPropertyDictionaryKeys(
         isolate, KeyCollectionMode::kOwnOnly, nullptr, object,
         object->property_dictionary());
   }
 }
 
 // ES6 9.5.12
 // Returns |true| on success, |nothing| in case of exception.
-Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
-                                                  Handle<JSProxy> proxy) {
+Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(
+    Handle<JSReceiver> receiver, Handle<JSProxy> proxy,
+    GetKeysConversion keys_conversion) {
   STACK_CHECK(isolate_, Nothing<bool>());
   // 1. Let handler be the value of the [[ProxyHandler]] internal slot of O.
   Handle<Object> handler(proxy->handler(), isolate_);
   // 2. If handler is null, throw a TypeError exception.
   // 3. Assert: Type(handler) is Object.
   if (proxy->IsRevoked()) {
     isolate_->Throw(*isolate_->factory()->NewTypeError(
         MessageTemplate::kProxyRevoked, isolate_->factory()->ownKeys_string()));
     return Nothing<bool>();
   }
   // 4. Let target be the value of the [[ProxyTarget]] internal slot of O.
   Handle<JSReceiver> target(proxy->target(), isolate_);
   // 5. Let trap be ? GetMethod(handler, "ownKeys").
   Handle<Object> trap;
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
       isolate_, trap, Object::GetMethod(Handle<JSReceiver>::cast(handler),
                                         isolate_->factory()->ownKeys_string()),
       Nothing<bool>());
   // 6. If trap is undefined, then
   if (trap->IsUndefined(isolate_)) {
     // 6a. Return target.[[OwnPropertyKeys]]().
-    return CollectOwnJSProxyTargetKeys(proxy, target);
+    return CollectOwnJSProxyTargetKeys(proxy, target, keys_conversion);
   }
   // 7. Let trapResultArray be Call(trap, handler, «target»).
   Handle<Object> trap_result_array;
   Handle<Object> args[] = {target};
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
       isolate_, trap_result_array,
       Execution::Call(isolate_, trap, handler, arraysize(args), args),
       Nothing<bool>());
   // 8. Let trapResult be ? CreateListFromArrayLike(trapResultArray,
   //    «String, Symbol»).
@@ skipping 102 matching lines @@
     DCHECK_GT(unchecked_result_keys_size, 0);
     isolate_->Throw(*isolate_->factory()->NewTypeError(
         MessageTemplate::kProxyOwnKeysNonExtensible));
     return Nothing<bool>();
   }
   // 21. Return trapResult.
   return AddKeysFromJSProxy(proxy, trap_result);
 }
 
 Maybe<bool> KeyAccumulator::CollectOwnJSProxyTargetKeys(
-    Handle<JSProxy> proxy, Handle<JSReceiver> target) {
+    Handle<JSProxy> proxy, Handle<JSReceiver> target,
+    GetKeysConversion keys_conversion) {
   // TODO(cbruni): avoid creating another KeyAccumulator
   Handle<FixedArray> keys;
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
-      isolate_, keys, JSReceiver::OwnPropertyKeys(target), Nothing<bool>());
+      isolate_, keys,
+      KeyAccumulator::GetKeys(target, KeyCollectionMode::kOwnOnly, filter_,

[Dan Ehrenberg, 2016/07/08 22:02:07]

It looks like the relevant change here is passing the filter_, where previously,
ALL_PROPERTIES was passed by JSReciever::OwnPropertyKeys.

What is the function of getting the key conversion to be passed through? I don't
see the harm in converting numeric keys to strings here, and Object.keys
parameterizes it that way anyway.

+                              keys_conversion, filter_proxy_keys_, is_for_in_),
+      Nothing<bool>());
   bool prev_filter_proxy_keys_ = filter_proxy_keys_;
   filter_proxy_keys_ = false;
   Maybe<bool> result = AddKeysFromJSProxy(proxy, keys);
   filter_proxy_keys_ = prev_filter_proxy_keys_;
   return result;
 }
 
 }  // namespace internal
 }  // namespace v8