[proxies] Use JSReceiver::GetKeys() for more purposes

src/objects.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/objects.h"
 
 #include <cmath>
 #include <iomanip>
 #include <sstream>
 
@@ skipping 8157 matching lines @@
         Handle<JSObject>::cast(v8::Utils::OpenHandle(*result)));
   } else {
     accumulator->AddKeys(
         Handle<JSObject>::cast(v8::Utils::OpenHandle(*result)));
   }
   return true;
 }
 
 
 static bool GetKeysFromJSObject(Isolate* isolate, Handle<JSReceiver> receiver,
-                                Handle<JSObject> object, PropertyFilter filter,
+                                Handle<JSObject> object, PropertyFilter* filter,

[Jakob Kummerow, 2015/12/04 13:39:29]

If you have any better ideas than passing |filter| in as a pointer, I'm all
ears. The issue is that when a global proxy has access checks, we have to modify
the filter that's applied when looking at the global object behind it (the
global proxy's hidden prototype).

[Camillo Bruni, 2015/12/04 14:37:16]

we might want to push all of this onto the KeyAccumulator, but honestly this is
a minor issue :)

                                 JSReceiver::KeyCollectionType type,
                                 KeyAccumulator* accumulator) {
   accumulator->NextPrototype();
-  bool keep_going = true;
   // 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 == JSReceiver::INCLUDE_PROTOS) {
       return false;
     }
     // ...whereas [[OwnPropertyKeys]] shall return whitelisted properties.
     DCHECK(type == JSReceiver::OWN_ONLY);
-    filter = static_cast<PropertyFilter>(filter | ONLY_ALL_CAN_READ);
-    keep_going = false;
-
-    // TODO(jkummerow): LayoutTests need adaptation before we can be spec

[Jakob Kummerow, 2015/12/04 13:39:29]

Waiting for https://codereview.chromium.org/1503453002/ to land.

-    // compliant.
-    // Let [[OwnPropertyKeys]] also return an empty list for now.
-    return false;
+    *filter = static_cast<PropertyFilter>(*filter | ONLY_ALL_CAN_READ);
   }
 
-  JSObject::CollectOwnElementKeys(object, accumulator, filter);
+  JSObject::CollectOwnElementKeys(object, accumulator, *filter);
 
   // Add the element keys from the interceptor.
   if (!GetKeysFromInterceptor<v8::IndexedPropertyEnumeratorCallback, kIndexed>(
-          isolate, receiver, object, filter, accumulator)) {
+          isolate, receiver, object, *filter, accumulator)) {
     DCHECK(isolate->has_pending_exception());
     return false;
   }
 
-  if (filter == ENUMERABLE_STRINGS) {
+  if (*filter == ENUMERABLE_STRINGS) {
     // We can cache the computed property keys if access checks are
     // not needed and no interceptors are involved.
     //
     // We do not use the cache if the object has elements and
     // therefore it does not make sense to cache the property names
     // for arguments objects.  Arguments objects will always have
     // elements.
     // Wrapped strings have elements, but don't have an elements
     // array or dictionary.  So the fast inline test for whether to
     // use the cache says yes, so we should not create a cache.
     Handle<JSFunction> arguments_function(
         JSFunction::cast(isolate->sloppy_arguments_map()->GetConstructor()));
     bool cache_enum_length =
         ((object->map()->GetConstructor() != *arguments_function) &&
          !object->IsJSValue() && !object->IsAccessCheckNeeded() &&
          !object->HasNamedInterceptor() && !object->HasIndexedInterceptor());
     // Compute the property keys and cache them if possible.
     Handle<FixedArray> enum_keys =
         JSObject::GetEnumPropertyKeys(object, cache_enum_length);
     accumulator->AddKeys(enum_keys);
   } else {
-    object->CollectOwnPropertyNames(accumulator, filter);
+    object->CollectOwnPropertyNames(accumulator, *filter);
   }
 
   // Add the property keys from the interceptor.
   if (!GetKeysFromInterceptor<v8::GenericNamedPropertyEnumeratorCallback,
-                              kNamed>(isolate, receiver, object, filter,
+                              kNamed>(isolate, receiver, object, *filter,
                                       accumulator)) {
     DCHECK(isolate->has_pending_exception());
     return false;
   }
-  return keep_going;
+  return true;
 }
 
 
 // Helper function for JSReceiver::GetKeys() below. Can be called recursively.
 // Returns false iff an exception was thrown.
 static bool GetKeys_Internal(Isolate* isolate, Handle<JSReceiver> receiver,
                              Handle<JSReceiver> object,
                              JSReceiver::KeyCollectionType type,
                              PropertyFilter filter,
                              KeyAccumulator* accumulator) {
@@ skipping 12 matching lines @@
                                           Handle<JSProxy>::cast(current),
                                           filter, accumulator);
       } else {
         DCHECK(type == JSReceiver::INCLUDE_PROTOS);
         result = JSProxy::Enumerate(
             isolate, receiver, Handle<JSProxy>::cast(current), accumulator);
       }
     } else {
       DCHECK(current->IsJSObject());
       result = GetKeysFromJSObject(isolate, receiver,
-                                   Handle<JSObject>::cast(current), filter,
+                                   Handle<JSObject>::cast(current), &filter,
                                    type, accumulator);
     }
     if (!result) {
       if (isolate->has_pending_exception()) {
         return false;
       }
       // If there was no exception, then "false" means "stop iterating".
       break;
     }
   }
@@ skipping 7360 matching lines @@
   RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
   if (result.IsEmpty()) return isolate->factory()->undefined_value();
   Handle<Object> result_internal = v8::Utils::OpenHandle(*result);
   result_internal->VerifyApiCallResultType();
   *done = true;
   // Rebox handle before return
   return handle(*result_internal, isolate);
 }
 
 
-// Compute the property keys from the interceptor.
-// TODO(jkummerow): Deprecated.
-MaybeHandle<JSObject> JSObject::GetKeysForNamedInterceptor(
-    Handle<JSObject> object, Handle<JSReceiver> receiver) {
-  Isolate* isolate = receiver->GetIsolate();
-  Handle<InterceptorInfo> interceptor(object->GetNamedInterceptor());
-  PropertyCallbackArguments
-      args(isolate, interceptor->data(), *receiver, *object);
-  v8::Local<v8::Object> result;
-  if (!interceptor->enumerator()->IsUndefined()) {
-    v8::GenericNamedPropertyEnumeratorCallback enum_fun =
-        v8::ToCData<v8::GenericNamedPropertyEnumeratorCallback>(
-            interceptor->enumerator());
-    LOG(isolate, ApiObjectAccess("interceptor-named-enum", *object));
-    result = args.Call(enum_fun);
-  }
-  if (result.IsEmpty()) return MaybeHandle<JSObject>();
-  DCHECK(v8::Utils::OpenHandle(*result)->IsJSArray() ||
-         (v8::Utils::OpenHandle(*result)->IsJSObject() &&
-          Handle<JSObject>::cast(v8::Utils::OpenHandle(*result))
-              ->HasSloppyArgumentsElements()));
-  // Rebox before returning.
-  return handle(JSObject::cast(*v8::Utils::OpenHandle(*result)), isolate);
-}
-
-
-// Compute the element keys from the interceptor.
-// TODO(jkummerow): Deprecated.
-MaybeHandle<JSObject> JSObject::GetKeysForIndexedInterceptor(
-    Handle<JSObject> object, Handle<JSReceiver> receiver) {
-  Isolate* isolate = receiver->GetIsolate();
-  Handle<InterceptorInfo> interceptor(object->GetIndexedInterceptor());
-  PropertyCallbackArguments
-      args(isolate, interceptor->data(), *receiver, *object);
-  v8::Local<v8::Object> result;
-  if (!interceptor->enumerator()->IsUndefined()) {
-    v8::IndexedPropertyEnumeratorCallback enum_fun =
-        v8::ToCData<v8::IndexedPropertyEnumeratorCallback>(
-            interceptor->enumerator());
-    LOG(isolate, ApiObjectAccess("interceptor-indexed-enum", *object));
-    result = args.Call(enum_fun);
-  }
-  if (result.IsEmpty()) return MaybeHandle<JSObject>();
-  DCHECK(v8::Utils::OpenHandle(*result)->IsJSArray() ||
-         (v8::Utils::OpenHandle(*result)->IsJSObject() &&
-          Handle<JSObject>::cast(v8::Utils::OpenHandle(*result))
-              ->HasSloppyArgumentsElements()));
-  // Rebox before returning.
-  return handle(JSObject::cast(*v8::Utils::OpenHandle(*result)), isolate);
-}
-
-
 Maybe<bool> JSObject::HasRealNamedProperty(Handle<JSObject> object,
                                            Handle<Name> name) {
   LookupIterator it = LookupIterator::PropertyOrElement(
       name->GetIsolate(), object, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
   return HasProperty(&it);
 }
 
 
 Maybe<bool> JSObject::HasRealElementProperty(Handle<JSObject> object,
                                              uint32_t index) {
@@ skipping 2183 matching lines @@
     Dictionary<Derived, Shape, Key>* raw_dict = *dictionary;
     for (int i = 0; i < capacity; i++) {
       Object* k = raw_dict->KeyAt(i);
       if (!raw_dict->IsKey(k) || k->FilterKey(filter)) continue;
       if (raw_dict->IsDeleted(i)) continue;
       PropertyDetails details = raw_dict->DetailsAt(i);
       if ((details.attributes() & filter) != 0) continue;
       if (filter & ONLY_ALL_CAN_READ) {
         if (details.kind() != kAccessor) continue;
         Object* accessors = raw_dict->ValueAt(i);
+        if (accessors->IsPropertyCell()) {

[Jakob Kummerow, 2015/12/04 13:39:29]

cf. line 17883.

+          accessors = PropertyCell::cast(accessors)->value();
+        }
         if (!accessors->IsAccessorInfo()) continue;
         if (!AccessorInfo::cast(accessors)->all_can_read()) continue;
       }
       array->set(array_size++, Smi::FromInt(i));
     }
 
     EnumIndexComparator<Derived> cmp(static_cast<Derived*>(raw_dict));
     Smi** start = reinterpret_cast<Smi**>(array->GetFirstElementAddress());
     std::sort(start, start + array_size, cmp);
   }
@@ skipping 1207 matching lines @@
   if (cell->value() != *new_value) {
     cell->set_value(*new_value);
     Isolate* isolate = cell->GetIsolate();
     cell->dependent_code()->DeoptimizeDependentCodeGroup(
         isolate, DependentCode::kPropertyCellChangedGroup);
   }
 }
 
 }  // namespace internal
 }  // namespace v8