Revert "[api] Use query interceptor in Object.keys()."

It caused crashes in the extension process on Canary.

This reverts commit b6059a67.

Also revert followup test CL:

  "[api] Add test for EnumeratorCallback and for...in."

as it depends on the logic in the reverted change.

This reverts commit 56772de7.

Bug: chromium:757371, v8:6627
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Id110128e6dc858a5a60ffc0175e8bb927b90bfc5
Reviewed-on: https://chromium-review.googlesource.com/626720Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47522}

include/v8.h

@@ -5120,8 +5120,6 @@ typedef void (*NamedPropertyDeleterCallback)(
 /**
  * Returns an array containing the names of the properties the named
  * property getter intercepts.
- *
- * Note: The values in the array must be of type v8::Name.
  */
 typedef void (*NamedPropertyEnumeratorCallback)(
     const PropertyCallbackInfo<Array>& info);
@@ -5245,8 +5243,6 @@ typedef void (*GenericNamedPropertyDeleterCallback)(
 /**
  * Returns an array containing the names of the properties the named
  * property getter intercepts.
- *
- * Note: The values in the array must be of type v8::Name.
  */
 typedef void (*GenericNamedPropertyEnumeratorCallback)(
     const PropertyCallbackInfo<Array>& info);
@@ -5327,10 +5323,7 @@ typedef void (*IndexedPropertyDeleterCallback)(
     const PropertyCallbackInfo<Boolean>& info);
 
 /**
- * Returns an array containing the indices of the properties the indexed
- * property getter intercepts.
- *
- * Note: The values in the array must be uint32_t.
+ * See `v8::GenericNamedPropertyEnumeratorCallback`.
  */
 typedef void (*IndexedPropertyEnumeratorCallback)(
     const PropertyCallbackInfo<Array>& info);

src/elements.cc

@@ -615,16 +615,6 @@ class ElementsAccessorBase : public ElementsAccessor {
                                           filter) != kMaxUInt32;
   }
 
-  bool HasEntry(JSObject* holder, uint32_t entry) final {
-    return Subclass::HasEntryImpl(holder->GetIsolate(), holder->elements(),
-                                  entry);
-  }
-
-  static bool HasEntryImpl(Isolate* isolate, FixedArrayBase* backing_store,
-                           uint32_t entry) {
-    UNIMPLEMENTED();
-  }
-
   bool HasAccessors(JSObject* holder) final {
     return Subclass::HasAccessorsImpl(holder, holder->elements());
   }

src/elements.h

@@ -50,10 +50,6 @@ class ElementsAccessor {
     return HasElement(holder, index, holder->elements(), filter);
   }
 
-  // Note: this is currently not implemented for string wrapper and
-  // typed array elements.
-  virtual bool HasEntry(JSObject* holder, uint32_t entry) = 0;
-
   virtual Handle<Object> Get(Handle<JSObject> holder, uint32_t entry) = 0;
 
   virtual PropertyDetails GetDetails(JSObject* holder, uint32_t entry) = 0;

src/keys.cc

@@ -4,7 +4,7 @@
 
 #include "src/keys.h"
 
-#include "src/api-arguments-inl.h"
+#include "src/api-arguments.h"
 #include "src/elements.h"
 #include "src/factory.h"
 #include "src/identity-map.h"
@@ -392,7 +392,7 @@ MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysFast(
     return MaybeHandle<FixedArray>();
   }
 
-  // From this point on we are certain to only collect own keys.
+  // From this point on we are certiain to only collect own keys.
   DCHECK(receiver_->IsJSObject());
   Handle<JSObject> object = Handle<JSObject>::cast(receiver_);
 
@@ -456,41 +456,6 @@ namespace {
 
 enum IndexedOrNamed { kIndexed, kNamed };
 
-template <IndexedOrNamed type>
-void FilterForEnumerableProperties(PropertyCallbackArguments* args,
-                                   Object* query_fun, Handle<JSObject> result,
-                                   KeyAccumulator* accumulator) {
-  DCHECK(result->IsJSArray() || result->HasSloppyArgumentsElements());
-
-  ElementsAccessor* accessor = result->GetElementsAccessor();
-
-  uint32_t length = accessor->GetCapacity(*result, result->elements());
-  for (uint32_t i = 0; i < length; i++) {
-    if (!accessor->HasEntry(*result, i)) continue;
-    Handle<Object> element = accessor->Get(result, i);
-    Handle<Object> attributes;
-    if (type == kIndexed) {
-      uint32_t number;
-      CHECK(element->ToUint32(&number));
-      attributes = args->Call(
-          v8::ToCData<v8::IndexedPropertyQueryCallback>(query_fun), number);
-    } else {
-      CHECK(element->IsName());
-      attributes = args->Call(
-          v8::ToCData<v8::GenericNamedPropertyQueryCallback>(query_fun),
-          Handle<Name>::cast(element));
-    }
-
-    if (!attributes.is_null()) {
-      int32_t value;
-      CHECK(attributes->ToInt32(&value));
-      if ((value & DONT_ENUM) == 0) {
-        accumulator->AddKey(element, DO_NOT_CONVERT);
-      }
-    }
-  }
-}
-
 // Returns |true| on success, |nothing| on exception.
 template <class Callback, IndexedOrNamed type>
 Maybe<bool> CollectInterceptorKeysInternal(Handle<JSReceiver> receiver,
@@ -498,30 +463,20 @@ Maybe<bool> CollectInterceptorKeysInternal(Handle<JSReceiver> receiver,
                                            Handle<InterceptorInfo> interceptor,
                                            KeyAccumulator* accumulator) {
   Isolate* isolate = accumulator->isolate();
-  PropertyCallbackArguments enum_args(isolate, interceptor->data(), *receiver,
-                                      *object, Object::DONT_THROW);
+  PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
+                                 *object, Object::DONT_THROW);
   Handle<JSObject> result;
   if (!interceptor->enumerator()->IsUndefined(isolate)) {
     Callback enum_fun = v8::ToCData<Callback>(interceptor->enumerator());
     const char* log_tag = type == kIndexed ? "interceptor-indexed-enum"
                                            : "interceptor-named-enum";
     LOG(isolate, ApiObjectAccess(log_tag, *object));
-    result = enum_args.Call(enum_fun);
+    result = args.Call(enum_fun);
   }
   RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate, Nothing<bool>());
   if (result.is_null()) return Just(true);
-
-  if ((accumulator->filter() & ONLY_ENUMERABLE) &&
-      !interceptor->query()->IsUndefined(isolate)) {
-    PropertyCallbackArguments query_args(
-        isolate, interceptor->data(), *receiver, *object, Object::DONT_THROW);
-
-    FilterForEnumerableProperties<type>(&query_args, interceptor->query(),
-                                        result, accumulator);
-  } else {
-    accumulator->AddKeys(
-        result, type == kIndexed ? CONVERT_TO_ARRAY_INDEX : DO_NOT_CONVERT);
-  }
+  accumulator->AddKeys(
+      result, type == kIndexed ? CONVERT_TO_ARRAY_INDEX : DO_NOT_CONVERT);
   return Just(true);
 }
 

test/cctest/test-api-interceptors.cc

@@ -3326,18 +3326,12 @@ static void NamedEnum(const v8::PropertyCallbackInfo<v8::Array>& info) {
   ApiTestFuzzer::Fuzz();
   v8::Local<v8::Array> result = v8::Array::New(info.GetIsolate(), 3);
   v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
-  CHECK(
-      result
-          ->Set(context, v8::Integer::New(info.GetIsolate(), 0), v8_str("foo"))
-          .FromJust());
-  CHECK(
-      result
-          ->Set(context, v8::Integer::New(info.GetIsolate(), 1), v8_str("bar"))
-          .FromJust());
-  CHECK(
-      result
-          ->Set(context, v8::Integer::New(info.GetIsolate(), 2), v8_str("baz"))
-          .FromJust());
+  result->Set(context, v8::Integer::New(info.GetIsolate(), 0), v8_str("foo"))
+      .FromJust();
+  result->Set(context, v8::Integer::New(info.GetIsolate(), 1), v8_str("bar"))
+      .FromJust();
+  result->Set(context, v8::Integer::New(info.GetIsolate(), 2), v8_str("baz"))
+      .FromJust();
   info.GetReturnValue().Set(result);
 }
 
@@ -3346,12 +3340,10 @@ static void IndexedEnum(const v8::PropertyCallbackInfo<v8::Array>& info) {
   ApiTestFuzzer::Fuzz();
   v8::Local<v8::Array> result = v8::Array::New(info.GetIsolate(), 2);
   v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
-  CHECK(
-      result->Set(context, v8::Integer::New(info.GetIsolate(), 0), v8_str("0"))
-          .FromJust());
-  CHECK(
-      result->Set(context, v8::Integer::New(info.GetIsolate(), 1), v8_str("1"))
-          .FromJust());
+  result->Set(context, v8::Integer::New(info.GetIsolate(), 0), v8_str("0"))
+      .FromJust();
+  result->Set(context, v8::Integer::New(info.GetIsolate(), 1), v8_str("1"))
+      .FromJust();
   info.GetReturnValue().Set(result);
 }
 
@@ -4972,221 +4964,6 @@ THREADED_TEST(NonMaskingInterceptorPrototypePropertyIC) {
   ExpectInt32("f(outer)", 4);
 }
 
-namespace {
-
-void ConcatNamedPropertyGetter(
-    Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
-  info.GetReturnValue().Set(
-      // Return the property name concatenated with itself.
-      String::Concat(name.As<String>(), name.As<String>()));
-}
-
-void ConcatIndexedPropertyGetter(
-    uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& info) {
-  info.GetReturnValue().Set(
-      // Return the double value of the index.
-      v8_num(index + index));
-}
-
-void EnumCallbackWithNames(const v8::PropertyCallbackInfo<v8::Array>& info) {
-  ApiTestFuzzer::Fuzz();
-  v8::Local<v8::Array> result = v8::Array::New(info.GetIsolate(), 4);
-  v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
-  CHECK(
-      result
-          ->Set(context, v8::Integer::New(info.GetIsolate(), 0), v8_str("foo"))
-          .FromJust());
-  CHECK(
-      result
-          ->Set(context, v8::Integer::New(info.GetIsolate(), 1), v8_str("bar"))
-          .FromJust());
-  CHECK(
-      result
-          ->Set(context, v8::Integer::New(info.GetIsolate(), 2), v8_str("baz"))
-          .FromJust());
-  CHECK(
-      result->Set(context, v8::Integer::New(info.GetIsolate(), 3), v8_str("10"))
-          .FromJust());
-
-  //  Create a holey array.
-  CHECK(result->Delete(context, v8::Integer::New(info.GetIsolate(), 1))
-            .FromJust());
-  info.GetReturnValue().Set(result);
-}
-
-void EnumCallbackWithIndices(const v8::PropertyCallbackInfo<v8::Array>& info) {
-  ApiTestFuzzer::Fuzz();
-  v8::Local<v8::Array> result = v8::Array::New(info.GetIsolate(), 4);
-  v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
-
-  CHECK(result->Set(context, v8::Integer::New(info.GetIsolate(), 0), v8_num(10))
-            .FromJust());
-  CHECK(result->Set(context, v8::Integer::New(info.GetIsolate(), 1), v8_num(11))
-            .FromJust());
-  CHECK(result->Set(context, v8::Integer::New(info.GetIsolate(), 2), v8_num(12))
-            .FromJust());
-  CHECK(result->Set(context, v8::Integer::New(info.GetIsolate(), 3), v8_num(14))
-            .FromJust());
-
-  //  Create a holey array.
-  CHECK(result->Delete(context, v8::Integer::New(info.GetIsolate(), 1))
-            .FromJust());
-  info.GetReturnValue().Set(result);
-}
-
-void RestrictiveNamedQuery(Local<Name> property,
-                           const v8::PropertyCallbackInfo<v8::Integer>& info) {
-  // Only "foo" is enumerable.
-  if (v8_str("foo")
-          ->Equals(info.GetIsolate()->GetCurrentContext(), property)
-          .FromJust()) {
-    info.GetReturnValue().Set(v8::PropertyAttribute::None);
-    return;
-  }
-  info.GetReturnValue().Set(v8::PropertyAttribute::DontEnum);
-}
-
-void RestrictiveIndexedQuery(
-    uint32_t index, const v8::PropertyCallbackInfo<v8::Integer>& info) {
-  // Only index 2 and 12 are enumerable.
-  if (index == 2 || index == 12) {
-    info.GetReturnValue().Set(v8::PropertyAttribute::None);
-    return;
-  }
-  info.GetReturnValue().Set(v8::PropertyAttribute::DontEnum);
-}
-}  // namespace
-
-// Regression test for V8 bug 6627.
-// Object.keys() must return enumerable keys only.
-THREADED_TEST(EnumeratorsAndUnenumerableNamedProperties) {
-  // The enumerator interceptor returns a list
-  // of items which are filtered according to the
-  // properties defined in the query interceptor.
-  v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-  v8::Local<v8::ObjectTemplate> obj = ObjectTemplate::New(isolate);
-  obj->SetHandler(v8::NamedPropertyHandlerConfiguration(
-      ConcatNamedPropertyGetter, NULL, RestrictiveNamedQuery, NULL,
-      EnumCallbackWithNames));
-  LocalContext context;
-  context->Global()
-      ->Set(context.local(), v8_str("obj"),
-            obj->NewInstance(context.local()).ToLocalChecked())
-      .FromJust();
-
-  ExpectInt32("Object.getOwnPropertyNames(obj).length", 3);
-  ExpectString("Object.getOwnPropertyNames(obj)[0]", "foo");
-  ExpectString("Object.getOwnPropertyNames(obj)[1]", "baz");
-  ExpectString("Object.getOwnPropertyNames(obj)[2]", "10");
-
-  ExpectTrue("Object.getOwnPropertyDescriptor(obj, 'foo').enumerable");
-  ExpectFalse("Object.getOwnPropertyDescriptor(obj, 'baz').enumerable");
-
-  ExpectInt32("Object.entries(obj).length", 1);
-  ExpectString("Object.entries(obj)[0][0]", "foo");
-  ExpectString("Object.entries(obj)[0][1]", "foofoo");
-
-  ExpectInt32("Object.keys(obj).length", 1);
-  ExpectString("Object.keys(obj)[0]", "foo");
-
-  ExpectInt32("Object.values(obj).length", 1);
-  ExpectString("Object.values(obj)[0]", "foofoo");
-}
-
-THREADED_TEST(EnumeratorsAndUnenumerableIndexedPropertiesArgumentsElements) {
-  v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-  v8::Local<v8::ObjectTemplate> obj = ObjectTemplate::New(isolate);
-  obj->SetHandler(v8::IndexedPropertyHandlerConfiguration(
-      ConcatIndexedPropertyGetter, NULL, RestrictiveIndexedQuery, NULL,
-      SloppyArgsIndexedPropertyEnumerator));
-  LocalContext context;
-  context->Global()
-      ->Set(context.local(), v8_str("obj"),
-            obj->NewInstance(context.local()).ToLocalChecked())
-      .FromJust();
-
-  ExpectInt32("Object.getOwnPropertyNames(obj).length", 4);
-  ExpectString("Object.getOwnPropertyNames(obj)[0]", "0");
-  ExpectString("Object.getOwnPropertyNames(obj)[1]", "1");
-  ExpectString("Object.getOwnPropertyNames(obj)[2]", "2");
-  ExpectString("Object.getOwnPropertyNames(obj)[3]", "3");
-
-  ExpectTrue("Object.getOwnPropertyDescriptor(obj, '2').enumerable");
-
-  ExpectInt32("Object.entries(obj).length", 1);
-  ExpectString("Object.entries(obj)[0][0]", "2");
-  ExpectInt32("Object.entries(obj)[0][1]", 4);
-
-  ExpectInt32("Object.keys(obj).length", 1);
-  ExpectString("Object.keys(obj)[0]", "2");
-
-  ExpectInt32("Object.values(obj).length", 1);
-  ExpectInt32("Object.values(obj)[0]", 4);
-}
-
-THREADED_TEST(EnumeratorsAndUnenumerableIndexedProperties) {
-  // The enumerator interceptor returns a list
-  // of items which are filtered according to the
-  // properties defined in the query interceptor.
-  v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-  v8::Local<v8::ObjectTemplate> obj = ObjectTemplate::New(isolate);
-  obj->SetHandler(v8::IndexedPropertyHandlerConfiguration(
-      ConcatIndexedPropertyGetter, NULL, RestrictiveIndexedQuery, NULL,
-      EnumCallbackWithIndices));
-  LocalContext context;
-  context->Global()
-      ->Set(context.local(), v8_str("obj"),
-            obj->NewInstance(context.local()).ToLocalChecked())
-      .FromJust();
-
-  ExpectInt32("Object.getOwnPropertyNames(obj).length", 3);
-  ExpectString("Object.getOwnPropertyNames(obj)[0]", "10");
-  ExpectString("Object.getOwnPropertyNames(obj)[1]", "12");
-  ExpectString("Object.getOwnPropertyNames(obj)[2]", "14");
-
-  ExpectFalse("Object.getOwnPropertyDescriptor(obj, '10').enumerable");
-  ExpectTrue("Object.getOwnPropertyDescriptor(obj, '12').enumerable");
-
-  ExpectInt32("Object.entries(obj).length", 1);
-  ExpectString("Object.entries(obj)[0][0]", "12");
-  ExpectInt32("Object.entries(obj)[0][1]", 24);
-
-  ExpectInt32("Object.keys(obj).length", 1);
-  ExpectString("Object.keys(obj)[0]", "12");
-
-  ExpectInt32("Object.values(obj).length", 1);
-  ExpectInt32("Object.values(obj)[0]", 24);
-}
-
-THREADED_TEST(EnumeratorsAndForIn) {
-  v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-  v8::Local<v8::ObjectTemplate> obj = ObjectTemplate::New(isolate);
-  obj->SetHandler(v8::NamedPropertyHandlerConfiguration(
-      ConcatNamedPropertyGetter, NULL, RestrictiveNamedQuery, NULL, NamedEnum));
-  LocalContext context;
-  context->Global()
-      ->Set(context.local(), v8_str("obj"),
-            obj->NewInstance(context.local()).ToLocalChecked())
-      .FromJust();
-
-  ExpectInt32("Object.getOwnPropertyNames(obj).length", 3);
-  ExpectString("Object.getOwnPropertyNames(obj)[0]", "foo");
-
-  ExpectTrue("Object.getOwnPropertyDescriptor(obj, 'foo').enumerable");
-
-  CompileRun(
-      "let concat = '';"
-      "for(var prop in obj) {"
-      "  concat += `key:${prop}:value:${obj[prop]}`;"
-      "}");
-
-  // Check that for...in only iterates over enumerable properties.
-  ExpectString("concat", "key:foo:value:foofoo");
-}
 
 namespace {