[keys] propagate PropertyFilter to proxy targets in KeyAccumulator

src/keys.cc

Index: src/keys.cc
diff --git a/src/keys.cc b/src/keys.cc
index 6d882663d78c18892376193616b6d22fcab51591..e72b869eaf4e8959ce37088f8bfaaae542bfe7de 100644
--- a/src/keys.cc
+++ b/src/keys.cc
@@ -38,7 +38,7 @@ MaybeHandle<FixedArray> KeyAccumulator::GetKeys(
   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);
 }
@@ -151,12 +151,14 @@ Maybe<bool> KeyAccumulator::AddKeysFromJSProxy(Handle<JSProxy> proxy,
 }
 
 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);
   }
@@ -170,7 +172,8 @@ Maybe<bool> KeyAccumulator::CollectKeys(Handle<JSReceiver> receiver,
         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));
@@ -703,8 +706,9 @@ Handle<FixedArray> KeyAccumulator::GetOwnEnumPropertyKeys(
 
 // 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_);
@@ -726,7 +730,7 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
   // 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;
@@ -849,11 +853,15 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
 }
 
 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);