[keys] Cleanup: removing unused parameters in elements.cc and keys.cc

- Cleanup CL to remove unused parameters in elements.cc. Additionally use fields
  from the KeyAccumulator wherever possible.
- Make method names in KeyAccumulator more consistent

BUG=

Review-Url: https://codereview.chromium.org/1944703002
Cr-Commit-Position: refs/heads/master@{#36073}

src/elements.cc

@@ -865,8 +865,7 @@ class ElementsAccessorBase : public ElementsAccessor {
     KeyAccumulator accumulator(isolate, OWN_ONLY, ALL_PROPERTIES);
     accumulator.NextPrototype();
     Subclass::CollectElementIndicesImpl(
-        object, handle(object->elements(), isolate), &accumulator, kMaxUInt32,
-        ALL_PROPERTIES, 0);
+        object, handle(object->elements(), isolate), &accumulator);
     Handle<FixedArray> keys = accumulator.GetKeys();
 
     for (int i = 0; i < keys->length(); ++i) {
@@ -900,23 +899,19 @@ class ElementsAccessorBase : public ElementsAccessor {
 
   void CollectElementIndices(Handle<JSObject> object,
                              Handle<FixedArrayBase> backing_store,
-                             KeyAccumulator* keys, uint32_t range,
-                             PropertyFilter filter, uint32_t offset) final {
-    if (filter & ONLY_ALL_CAN_READ) return;
-    Subclass::CollectElementIndicesImpl(object, backing_store, keys, range,
-                                        filter, offset);
+                             KeyAccumulator* keys) final {
+    if (keys->filter() & ONLY_ALL_CAN_READ) return;
+    Subclass::CollectElementIndicesImpl(object, backing_store, keys);
   }
 
   static void CollectElementIndicesImpl(Handle<JSObject> object,
                                         Handle<FixedArrayBase> backing_store,
-                                        KeyAccumulator* keys, uint32_t range,
-                                        PropertyFilter filter,
-                                        uint32_t offset) {
+                                        KeyAccumulator* keys) {
     DCHECK_NE(DICTIONARY_ELEMENTS, kind());
     // Non-dictionary elements can't have all-can-read accessors.
     uint32_t length = GetIterationLength(*object, *backing_store);
-    if (range < length) length = range;
-    for (uint32_t i = offset; i < length; i++) {
+    PropertyFilter filter = keys->filter();
+    for (uint32_t i = 0; i < length; i++) {
       if (Subclass::HasElementImpl(object, i, backing_store, filter)) {
         keys->AddKey(i);
       }
@@ -1316,16 +1311,15 @@ class DictionaryElementsAccessor
 
   static void CollectElementIndicesImpl(Handle<JSObject> object,
                                         Handle<FixedArrayBase> backing_store,
-                                        KeyAccumulator* keys, uint32_t range,
-                                        PropertyFilter filter,
-                                        uint32_t offset) {
-    if (filter & SKIP_STRINGS) return;
+                                        KeyAccumulator* keys) {
+    if (keys->filter() & SKIP_STRINGS) return;
     Isolate* isolate = keys->isolate();
     Handle<Object> undefined = isolate->factory()->undefined_value();
     Handle<Object> the_hole = isolate->factory()->the_hole_value();
     Handle<SeededNumberDictionary> dictionary =
         Handle<SeededNumberDictionary>::cast(backing_store);
     int capacity = dictionary->Capacity();
+    PropertyFilter filter = keys->filter();
     for (int i = 0; i < capacity; i++) {
       uint32_t key =
           GetKeyForEntryImpl(dictionary, i, filter, *undefined, *the_hole);
@@ -2391,22 +2385,17 @@ class SloppyArgumentsElementsAccessor
 
   static void CollectElementIndicesImpl(Handle<JSObject> object,
                                         Handle<FixedArrayBase> backing_store,
-                                        KeyAccumulator* keys, uint32_t range,
-                                        PropertyFilter filter,
-                                        uint32_t offset) {
+                                        KeyAccumulator* keys) {
     FixedArray* parameter_map = FixedArray::cast(*backing_store);
     uint32_t length = parameter_map->length() - 2;
-    if (range < length) length = range;
-
-    for (uint32_t i = offset; i < length; ++i) {
+    for (uint32_t i = 0; i < length; ++i) {
       if (!parameter_map->get(i + 2)->IsTheHole()) {
         keys->AddKey(i);
       }
     }
 
     Handle<FixedArrayBase> store(FixedArrayBase::cast(parameter_map->get(1)));
-    ArgumentsAccessor::CollectElementIndicesImpl(object, store, keys, range,
-                                                 filter, offset);
+    ArgumentsAccessor::CollectElementIndicesImpl(object, store, keys);
     if (Subclass::kind() == FAST_SLOPPY_ARGUMENTS_ELEMENTS) {
       keys->SortCurrentElementsList();
     }
@@ -2754,15 +2743,13 @@ class StringWrapperElementsAccessor
 
   static void CollectElementIndicesImpl(Handle<JSObject> object,
                                         Handle<FixedArrayBase> backing_store,
-                                        KeyAccumulator* keys, uint32_t range,
-                                        PropertyFilter filter,
-                                        uint32_t offset) {
+                                        KeyAccumulator* keys) {
     uint32_t length = GetString(*object)->length();
     for (uint32_t i = 0; i < length; i++) {
       keys->AddKey(i);
     }
-    BackingStoreAccessor::CollectElementIndicesImpl(object, backing_store, keys,
-                                                    range, filter, offset);
+    BackingStoreAccessor::CollectElementIndicesImpl(object, backing_store,
+                                                    keys);
   }
 
   static void GrowCapacityAndConvertImpl(Handle<JSObject> object,

src/elements.h

@@ -81,18 +81,12 @@ class ElementsAccessor {
   // whose PropertyAttribute match |filter|.
   virtual void CollectElementIndices(Handle<JSObject> object,
                                      Handle<FixedArrayBase> backing_store,
-                                     KeyAccumulator* keys,
-                                     uint32_t range = kMaxUInt32,
-                                     PropertyFilter filter = ALL_PROPERTIES,
-                                     uint32_t offset = 0) = 0;
+                                     KeyAccumulator* keys) = 0;
 
   inline void CollectElementIndices(Handle<JSObject> object,
-                                    KeyAccumulator* keys,
-                                    uint32_t range = kMaxUInt32,
-                                    PropertyFilter filter = ALL_PROPERTIES,
-                                    uint32_t offset = 0) {
-    CollectElementIndices(object, handle(object->elements()), keys, range,
-                          filter, offset);
+                                    KeyAccumulator* keys) {
+    CollectElementIndices(object, handle(object->elements(), keys->isolate()),
+                          keys);
   }
 
   virtual Maybe<bool> CollectValuesOrEntries(
@@ -100,7 +94,6 @@ class ElementsAccessor {
       Handle<FixedArray> values_or_entries, bool get_entries, int* nof_items,
       PropertyFilter filter = ALL_PROPERTIES) = 0;
 
-  //
   virtual Handle<FixedArray> PrependElementIndices(
       Handle<JSObject> object, Handle<FixedArrayBase> backing_store,
       Handle<FixedArray> keys, GetKeysConversion convert,

src/keys.cc

@@ -314,20 +314,18 @@ void KeyAccumulator::NextPrototype() {
   level_symbol_length_ = 0;
 }
 
-Maybe<bool> KeyAccumulator::GetKeys_Internal(Handle<JSReceiver> receiver,
-                                             Handle<JSReceiver> object,
-                                             KeyCollectionType type) {
+Maybe<bool> KeyAccumulator::CollectKeys(Handle<JSReceiver> receiver,
+                                        Handle<JSReceiver> object) {
   // Proxies have no hidden prototype and we should not trigger the
   // [[GetPrototypeOf]] trap on the last iteration when using
   // AdvanceFollowingProxies.
-  if (type == OWN_ONLY && object->IsJSProxy()) {
-    MAYBE_RETURN(
-        JSProxyOwnPropertyKeys(receiver, Handle<JSProxy>::cast(object)),
-        Nothing<bool>());
+  if (type_ == OWN_ONLY && object->IsJSProxy()) {
+    MAYBE_RETURN(CollectOwnJSProxyKeys(receiver, Handle<JSProxy>::cast(object)),
+                 Nothing<bool>());
     return Just(true);
   }
 
-  PrototypeIterator::WhereToEnd end = type == OWN_ONLY
+  PrototypeIterator::WhereToEnd end = type_ == OWN_ONLY
                                           ? PrototypeIterator::END_AT_NON_HIDDEN
                                           : PrototypeIterator::END_AT_NULL;
   for (PrototypeIterator iter(isolate_, object,
@@ -337,16 +335,15 @@ Maybe<bool> KeyAccumulator::GetKeys_Internal(Handle<JSReceiver> receiver,
         PrototypeIterator::GetCurrent<JSReceiver>(iter);
     Maybe<bool> result = Just(false);  // Dummy initialization.
     if (current->IsJSProxy()) {
-      result = JSProxyOwnPropertyKeys(receiver, Handle<JSProxy>::cast(current));
+      result = CollectOwnJSProxyKeys(receiver, Handle<JSProxy>::cast(current));
     } else {
       DCHECK(current->IsJSObject());
-      result =
-          GetKeysFromJSObject(receiver, Handle<JSObject>::cast(current), type);
+      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 GetKeysFromJSObject.
+    // case on API objects for OWN_ONLY keys handled in CollectOwnKeys.
     if (!iter.AdvanceFollowingProxiesIgnoringAccessChecks()) {
       return Nothing<bool>();
     }
@@ -629,10 +626,10 @@ static Maybe<bool> GetKeysFromInterceptor(Handle<JSReceiver> receiver,
   return Just(true);
 }
 
-void KeyAccumulator::CollectOwnElementKeys(Handle<JSObject> object) {
+void KeyAccumulator::CollectOwnElementIndices(Handle<JSObject> object) {
   if (filter_ & SKIP_STRINGS) return;
   ElementsAccessor* accessor = object->GetElementsAccessor();
-  accessor->CollectElementIndices(object, this, kMaxUInt32, filter_, 0);
+  accessor->CollectElementIndices(object, this);
 }
 
 void KeyAccumulator::CollectOwnPropertyNames(Handle<JSObject> object) {
@@ -664,24 +661,23 @@ void KeyAccumulator::CollectOwnPropertyNames(Handle<JSObject> object) {
 
 // Returns |true| on success, |false| if prototype walking should be stopped,
 // |nothing| if an exception was thrown.
-Maybe<bool> KeyAccumulator::GetKeysFromJSObject(Handle<JSReceiver> receiver,
-                                                Handle<JSObject> object,
-                                                KeyCollectionType type) {
+Maybe<bool> KeyAccumulator::CollectOwnKeys(Handle<JSReceiver> receiver,
+                                           Handle<JSObject> object) {
   this->NextPrototype();
   // Check access rights if required.
   if (object->IsAccessCheckNeeded() &&
       !isolate_->MayAccess(handle(isolate_->context()), object)) {
     // The cross-origin spec says that [[Enumerate]] shall return an empty
     // iterator when it doesn't have access...
-    if (type == INCLUDE_PROTOS) {
+    if (type_ == INCLUDE_PROTOS) {
       return Just(false);
     }
     // ...whereas [[OwnPropertyKeys]] shall return whitelisted properties.
-    DCHECK_EQ(OWN_ONLY, type);
+    DCHECK_EQ(OWN_ONLY, type_);
     filter_ = static_cast<PropertyFilter>(filter_ | ONLY_ALL_CAN_READ);
   }
 
-  this->CollectOwnElementKeys(object);
+  this->CollectOwnElementIndices(object);
 
   // Add the element keys from the interceptor.
   Maybe<bool> success =
@@ -732,8 +728,8 @@ Handle<FixedArray> KeyAccumulator::GetEnumPropertyKeys(
 
 // ES6 9.5.12
 // Returns |true| on success, |nothing| in case of exception.
-Maybe<bool> KeyAccumulator::JSProxyOwnPropertyKeys(Handle<JSReceiver> receiver,
-                                                   Handle<JSProxy> proxy) {
+Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
+                                                  Handle<JSProxy> proxy) {
   STACK_CHECK(isolate_, Nothing<bool>());
   // 1. Let handler be the value of the [[ProxyHandler]] internal slot of O.
   Handle<Object> handler(proxy->handler(), isolate_);
@@ -755,7 +751,11 @@ Maybe<bool> KeyAccumulator::JSProxyOwnPropertyKeys(Handle<JSReceiver> receiver,
   // 6. If trap is undefined, then
   if (trap->IsUndefined()) {
     // 6a. Return target.[[OwnPropertyKeys]]().
-    return this->GetKeys_Internal(receiver, target, OWN_ONLY);
+    KeyCollectionType previous_type = type_;
+    type_ = OWN_ONLY;
+    Maybe<bool> result = this->CollectKeys(receiver, target);
+    type_ = previous_type;
+    return result;
   }
   // 7. Let trapResultArray be Call(trap, handler, «target»).
   Handle<Object> trap_result_array;

src/keys.h

@@ -36,6 +36,15 @@ class KeyAccumulator final BASE_EMBEDDED {
       : isolate_(isolate), type_(type), filter_(filter) {}
   ~KeyAccumulator();
 
+  Handle<FixedArray> GetKeys(GetKeysConversion convert = KEEP_NUMBERS);
+  Maybe<bool> CollectKeys(Handle<JSReceiver> receiver,
+                          Handle<JSReceiver> object);
+  void CollectOwnElementIndices(Handle<JSObject> object);
+  void CollectOwnPropertyNames(Handle<JSObject> object);
+
+  static Handle<FixedArray> GetEnumPropertyKeys(Isolate* isolate,
+                                                Handle<JSObject> object);
+
   bool AddKey(uint32_t key);
   bool AddKey(Object* key, AddKeyConversion convert);
   bool AddKey(Handle<Object> key, AddKeyConversion convert);
@@ -44,38 +53,27 @@ class KeyAccumulator final BASE_EMBEDDED {
   void AddKeysFromProxy(Handle<JSObject> array);
   Maybe<bool> AddKeysFromProxy(Handle<JSProxy> proxy, Handle<FixedArray> keys);
   void AddElementKeysFromInterceptor(Handle<JSObject> array);
+
   // Jump to the next level, pushing the current |levelLength_| to
   // |levelLengths_| and adding a new list to |elements_|.
   void NextPrototype();
   // Sort the integer indices in the last list in |elements_|
   void SortCurrentElementsList();
-  Handle<FixedArray> GetKeys(GetKeysConversion convert = KEEP_NUMBERS);
   int length() { return length_; }
   Isolate* isolate() { return isolate_; }
-  void set_filter_proxy_keys(bool filter) { filter_proxy_keys_ = filter; }
   PropertyFilter filter() { return filter_; }
-
-  Maybe<bool> GetKeys_Internal(Handle<JSReceiver> receiver,
-                               Handle<JSReceiver> object,
-                               KeyCollectionType type);
-  static Handle<FixedArray> GetEnumPropertyKeys(Isolate* isolate,
-                                                Handle<JSObject> object);
-
-  void CollectOwnElementKeys(Handle<JSObject> object);
-  void CollectOwnPropertyNames(Handle<JSObject> object);
+  void set_filter_proxy_keys(bool filter) { filter_proxy_keys_ = filter; }
 
  private:
+  Maybe<bool> CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
+                                    Handle<JSProxy> proxy);
+  Maybe<bool> CollectOwnKeys(Handle<JSReceiver> receiver,
+                             Handle<JSObject> object);
   bool AddIntegerKey(uint32_t key);
   bool AddStringKey(Handle<Object> key, AddKeyConversion convert);
   bool AddSymbolKey(Handle<Object> array);
   void SortCurrentElementsListRemoveDuplicates();
 
-  Maybe<bool> JSProxyOwnPropertyKeys(Handle<JSReceiver> receiver,
-                                     Handle<JSProxy> proxy);
-  Maybe<bool> GetKeysFromJSObject(Handle<JSReceiver> receiver,
-                                  Handle<JSObject> object,
-                                  KeyCollectionType type);
-
   Isolate* isolate_;
   KeyCollectionType type_;
   PropertyFilter filter_;

src/objects.cc

@@ -8065,7 +8065,7 @@ MaybeHandle<FixedArray> JSReceiver::GetKeys(Handle<JSReceiver> object,
   Isolate* isolate = object->GetIsolate();
   KeyAccumulator accumulator(isolate, type, filter);
   accumulator.set_filter_proxy_keys(filter_proxy_keys);
-  MAYBE_RETURN(accumulator.GetKeys_Internal(object, object, type),
+  MAYBE_RETURN(accumulator.CollectKeys(object, object),
                MaybeHandle<FixedArray>());
   Handle<FixedArray> keys = accumulator.GetKeys(keys_conversion);
   DCHECK(ContainsOnlyValidKeys(keys));
@@ -8163,7 +8163,7 @@ MaybeHandle<FixedArray> GetOwnValuesOrEntries(Isolate* isolate,
   PropertyFilter key_filter =
       static_cast<PropertyFilter>(filter & ~ONLY_ENUMERABLE);
   KeyAccumulator accumulator(isolate, OWN_ONLY, key_filter);
-  MAYBE_RETURN(accumulator.GetKeys_Internal(object, object, OWN_ONLY),
+  MAYBE_RETURN(accumulator.CollectKeys(object, object),
                MaybeHandle<FixedArray>());
   Handle<FixedArray> keys = accumulator.GetKeys(CONVERT_TO_STRING);
   DCHECK(ContainsOnlyValidKeys(keys));

src/runtime/runtime-array.cc

@@ -213,7 +213,7 @@ RUNTIME_FUNCTION(Runtime_GetArrayKeys) {
     }
     accumulator.NextPrototype();
     Handle<JSObject> current = PrototypeIterator::GetCurrent<JSObject>(iter);
-    accumulator.CollectOwnElementKeys(current);
+    accumulator.CollectOwnElementIndices(current);
   }
   // Erase any keys >= length.
   Handle<FixedArray> keys = accumulator.GetKeys(KEEP_NUMBERS);