[api][runtime] Support all-in ctors of {Named,Indexed}PropertyHandlerConfiguration

- Explicitly allows construction of
{Named,Indexed}PropertyHandlerConfiguration with all the members filled.

Bug: v8:7612
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I426ea33846b5dbf2b3482c722c963a6e4b0abded
Reviewed-on: https://chromium-review.googlesource.com/1163882Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55142}

include/v8.h

@@ -5893,6 +5893,26 @@ enum class PropertyHandlerFlags {
 };
 
 struct NamedPropertyHandlerConfiguration {
+  NamedPropertyHandlerConfiguration(
+      GenericNamedPropertyGetterCallback getter,
+      GenericNamedPropertySetterCallback setter,
+      GenericNamedPropertyQueryCallback query,
+      GenericNamedPropertyDeleterCallback deleter,
+      GenericNamedPropertyEnumeratorCallback enumerator,
+      GenericNamedPropertyDefinerCallback definer,
+      GenericNamedPropertyDescriptorCallback descriptor,
+      Local<Value> data = Local<Value>(),
+      PropertyHandlerFlags flags = PropertyHandlerFlags::kNone)
+      : getter(getter),
+        setter(setter),
+        query(query),
+        deleter(deleter),
+        enumerator(enumerator),
+        definer(definer),
+        descriptor(descriptor),
+        data(data),
+        flags(flags) {}
+
   NamedPropertyHandlerConfiguration(
       /** Note: getter is required */
       GenericNamedPropertyGetterCallback getter = 0,
@@ -5944,6 +5964,25 @@ struct NamedPropertyHandlerConfiguration {
 
 
 struct IndexedPropertyHandlerConfiguration {
+  IndexedPropertyHandlerConfiguration(
+      IndexedPropertyGetterCallback getter,
+      IndexedPropertySetterCallback setter, IndexedPropertyQueryCallback query,
+      IndexedPropertyDeleterCallback deleter,
+      IndexedPropertyEnumeratorCallback enumerator,
+      IndexedPropertyDefinerCallback definer,
+      IndexedPropertyDescriptorCallback descriptor,
+      Local<Value> data = Local<Value>(),
+      PropertyHandlerFlags flags = PropertyHandlerFlags::kNone)
+      : getter(getter),
+        setter(setter),
+        query(query),
+        deleter(deleter),
+        enumerator(enumerator),
+        definer(definer),
+        descriptor(descriptor),
+        data(data),
+        flags(flags) {}
+
   IndexedPropertyHandlerConfiguration(
       /** Note: getter is required */
       IndexedPropertyGetterCallback getter = 0,

src/api.cc

@@ -1760,10 +1760,6 @@ static i::Handle<i::InterceptorInfo> CreateInterceptorInfo(
     i::Isolate* isolate, Getter getter, Setter setter, Query query,
     Descriptor descriptor, Deleter remover, Enumerator enumerator,
     Definer definer, Local<Value> data, PropertyHandlerFlags flags) {
-  // Either intercept attributes or descriptor.
-  DCHECK(query == nullptr || descriptor == nullptr);
-  // Only use descriptor callback with definer callback.
-  DCHECK(query == nullptr || definer == nullptr);
   auto obj = i::Handle<i::InterceptorInfo>::cast(
       isolate->factory()->NewStruct(i::INTERCEPTOR_INFO_TYPE, i::TENURED));
   obj->set_flags(0);

src/objects.cc

@@ -7833,41 +7833,42 @@ Maybe<bool> GetPropertyDescriptorWithInterceptor(LookupIterator* it,
     }
   }
 
-  if (it->state() == LookupIterator::INTERCEPTOR) {
-    Isolate* isolate = it->isolate();
-    Handle<InterceptorInfo> interceptor = it->GetInterceptor();
-    if (!interceptor->descriptor()->IsUndefined(isolate)) {
-      Handle<Object> result;
-      Handle<JSObject> holder = it->GetHolder<JSObject>();
+  if (it->state() != LookupIterator::INTERCEPTOR) return Just(false);
 
-      Handle<Object> receiver = it->GetReceiver();
-      if (!receiver->IsJSReceiver()) {
-        ASSIGN_RETURN_ON_EXCEPTION_VALUE(
-            isolate, receiver, Object::ConvertReceiver(isolate, receiver),
-            Nothing<bool>());
-      }
+  Isolate* isolate = it->isolate();
+  Handle<InterceptorInfo> interceptor = it->GetInterceptor();
+  if (interceptor->descriptor()->IsUndefined(isolate)) return Just(false);
 
-      PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
-                                     *holder, kDontThrow);
-      if (it->IsElement()) {
-        result = args.CallIndexedDescriptor(interceptor, it->index());
-      } else {
-        result = args.CallNamedDescriptor(interceptor, it->name());
-      }
-      if (!result.is_null()) {
-        // Request successfully intercepted, try to set the property
-        // descriptor.
-        Utils::ApiCheck(
-            PropertyDescriptor::ToPropertyDescriptor(isolate, result, desc),
-            it->IsElement() ? "v8::IndexedPropertyDescriptorCallback"
-                            : "v8::NamedPropertyDescriptorCallback",
-            "Invalid property descriptor.");
+  Handle<Object> result;
+  Handle<JSObject> holder = it->GetHolder<JSObject>();
 
-        return Just(true);
-      }
-      it->Next();
-    }
+  Handle<Object> receiver = it->GetReceiver();
+  if (!receiver->IsJSReceiver()) {
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, receiver,
+                                     Object::ConvertReceiver(isolate, receiver),
+                                     Nothing<bool>());
+  }
+
+  PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
+                                 *holder, kDontThrow);
+  if (it->IsElement()) {
+    result = args.CallIndexedDescriptor(interceptor, it->index());
+  } else {
+    result = args.CallNamedDescriptor(interceptor, it->name());
   }
+  if (!result.is_null()) {
+    // Request successfully intercepted, try to set the property
+    // descriptor.
+    Utils::ApiCheck(
+        PropertyDescriptor::ToPropertyDescriptor(isolate, result, desc),
+        it->IsElement() ? "v8::IndexedPropertyDescriptorCallback"
+                        : "v8::NamedPropertyDescriptorCallback",
+        "Invalid property descriptor.");
+
+    return Just(true);
+  }
+
+  it->Next();
   return Just(false);
 }
 }  // namespace

test/unittests/api/interceptor-unittest.cc

@@ -29,4 +29,180 @@ TEST_F(InterceptorTest, FreezeApiObjectWithInterceptor) {
 }
 
 }  // namespace
+
+namespace internal {
+namespace {
+
+class InterceptorLoggingTest : public TestWithNativeContext {
+ public:
+  InterceptorLoggingTest() {}
+
+  static const int kTestIndex = 0;
+
+  static void NamedPropertyGetter(Local<v8::Name> name,
+                                  const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "named getter");
+  }
+
+  static void NamedPropertySetter(Local<v8::Name> name, Local<v8::Value> value,
+                                  const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "named setter");
+  }
+
+  static void NamedPropertyQuery(
+      Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Integer>& info) {
+    LogCallback(info, "named query");
+  }
+
+  static void NamedPropertyDeleter(
+      Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Boolean>& info) {
+    LogCallback(info, "named deleter");
+  }
+
+  static void NamedPropertyEnumerator(
+      const v8::PropertyCallbackInfo<Array>& info) {
+    LogCallback(info, "named enumerator");
+  }
+
+  static void NamedPropertyDefiner(
+      Local<v8::Name> name, const v8::PropertyDescriptor& desc,
+      const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "named definer");
+  }
+
+  static void NamedPropertyDescriptor(
+      Local<v8::Name> name, const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "named descriptor");
+  }
+
+  static void IndexedPropertyGetter(
+      uint32_t index, const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "indexed getter");
+  }
+
+  static void IndexedPropertySetter(
+      uint32_t index, Local<v8::Value> value,
+      const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "indexed setter");
+  }
+
+  static void IndexedPropertyQuery(
+      uint32_t index, const v8::PropertyCallbackInfo<v8::Integer>& info) {
+    LogCallback(info, "indexed query");
+  }
+
+  static void IndexedPropertyDeleter(
+      uint32_t index, const v8::PropertyCallbackInfo<v8::Boolean>& info) {
+    LogCallback(info, "indexed deleter");
+  }
+
+  static void IndexedPropertyEnumerator(
+      const v8::PropertyCallbackInfo<Array>& info) {
+    LogCallback(info, "indexed enumerator");
+  }
+
+  static void IndexedPropertyDefiner(
+      uint32_t index, const v8::PropertyDescriptor& desc,
+      const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "indexed definer");
+  }
+
+  static void IndexedPropertyDescriptor(
+      uint32_t index, const v8::PropertyCallbackInfo<Value>& info) {
+    LogCallback(info, "indexed descriptor");
+  }
+
+  template <class T>
+  static void LogCallback(const v8::PropertyCallbackInfo<T>& info,
+                          const char* callback_name) {
+    InterceptorLoggingTest* test = reinterpret_cast<InterceptorLoggingTest*>(
+        info.This()->GetAlignedPointerFromInternalField(kTestIndex));
+    test->Log(callback_name);
+  }
+
+  void Log(const char* callback_name) {
+    if (log_is_empty_) {
+      log_is_empty_ = false;
+    } else {
+      log_ << ", ";
+    }
+    log_ << callback_name;
+  }
+
+ protected:
+  void SetUp() override {
+    // Set up the object that supports full interceptors.
+    v8::Local<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(v8_isolate());
+    templ->SetInternalFieldCount(1);
+    templ->SetHandler(v8::NamedPropertyHandlerConfiguration(
+        NamedPropertyGetter, NamedPropertySetter, NamedPropertyQuery,
+        NamedPropertyDeleter, NamedPropertyEnumerator, NamedPropertyDefiner,
+        NamedPropertyDescriptor));
+    templ->SetHandler(v8::IndexedPropertyHandlerConfiguration(
+        IndexedPropertyGetter, IndexedPropertySetter, IndexedPropertyQuery,
+        IndexedPropertyDeleter, IndexedPropertyEnumerator,
+        IndexedPropertyDefiner, IndexedPropertyDescriptor));
+    v8::Local<v8::Object> instance =
+        templ->NewInstance(context()).ToLocalChecked();
+    instance->SetAlignedPointerInInternalField(kTestIndex, this);
+    SetGlobalProperty("obj", instance);
+  }
+
+  std::string Run(const char* script) {
+    log_is_empty_ = true;
+    log_.str(std::string());
+    log_.clear();
+
+    RunJS(script);
+    return log_.str();
+  }
+
+ private:
+  bool log_is_empty_ = false;
+  std::stringstream log_;
+};
+
+TEST_F(InterceptorLoggingTest, DispatchTest) {
+  EXPECT_EQ(Run("for (var p in obj) {}"),
+            "indexed enumerator, named enumerator");
+  EXPECT_EQ(Run("Object.keys(obj)"), "indexed enumerator, named enumerator");
+
+  EXPECT_EQ(Run("obj.foo"), "named getter");
+  EXPECT_EQ(Run("obj[42]"), "indexed getter");
+
+  EXPECT_EQ(Run("obj.foo = null"), "named setter");
+  EXPECT_EQ(Run("obj[42] = null"), "indexed setter");
+
+  EXPECT_EQ(Run("Object.getOwnPropertyDescriptor(obj, 'foo')"),
+            "named descriptor");
+
+  EXPECT_EQ(Run("Object.getOwnPropertyDescriptor(obj, 42)"),
+            "indexed descriptor");
+
+  EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {value: 42})"),
+            "named descriptor, named definer, named setter");
+  EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {get(){} })"),
+            "named descriptor, named definer");
+  EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {set(value){}})"),
+            "named descriptor, named definer");
+  EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {get(){}, set(value){}})"),
+            "named descriptor, named definer");
+
+  EXPECT_EQ(Run("Object.defineProperty(obj, 42, {value: 'foo'})"),
+            "indexed descriptor, "
+            // then attempt definer first and fallback to setter.
+            "indexed definer, indexed setter");
+
+  EXPECT_EQ(Run("Object.prototype.propertyIsEnumerable.call(obj, 'a')"),
+            "named query");
+  EXPECT_EQ(Run("Object.prototype.propertyIsEnumerable.call(obj, 42)"),
+            "indexed query");
+
+  EXPECT_EQ(Run("Object.prototype.hasOwnProperty.call(obj, 'a')"),
+            "named query");
+  // TODO(cbruni): Fix once hasOnwProperty is fixed (https://crbug.com/872628)
+  EXPECT_EQ(Run("Object.prototype.hasOwnProperty.call(obj, '42')"), "");
+}
+}  // namespace
+}  // namespace internal
 }  // namespace v8