Simplify the interface of PropertyCallbackArguments

Use internal handles as API, and move boilerplate code into the call wrappers.
BUG=

Review URL: https://codereview.chromium.org/1776913005

Cr-Commit-Position: refs/heads/master@{#34684}

src/api-arguments.cc

@@ -4,82 +4,28 @@
 
 #include "src/api-arguments.h"
 
-#include "src/api.h"
-#include "src/vm-state-inl.h"
-
 namespace v8 {
 namespace internal {
 
-template <typename T>
-template <typename V>
-v8::Local<V> CustomArguments<T>::GetReturnValue(Isolate* isolate) {
-  // Check the ReturnValue.
-  Object** handle = &this->begin()[kReturnValueOffset];
-  // Nothing was set, return empty handle as per previous behaviour.
-  if ((*handle)->IsTheHole()) return v8::Local<V>();
-  return Utils::Convert<Object, V>(Handle<Object>(handle));
-}
-
-v8::Local<v8::Value> FunctionCallbackArguments::Call(FunctionCallback f) {
+Handle<Object> FunctionCallbackArguments::Call(FunctionCallback f) {
   Isolate* isolate = this->isolate();
   VMState<EXTERNAL> state(isolate);
   ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));
   FunctionCallbackInfo<v8::Value> info(begin(), argv_, argc_,
                                        is_construct_call_);
   f(info);
-  return GetReturnValue<v8::Value>(isolate);
+  return GetReturnValue<Object>(isolate);
 }
 
-#define WRITE_CALL_0(Function, ReturnValue)                            \
-  v8::Local<ReturnValue> PropertyCallbackArguments::Call(Function f) { \
-    Isolate* isolate = this->isolate();                                \
-    VMState<EXTERNAL> state(isolate);                                  \
-    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));       \
-    PropertyCallbackInfo<ReturnValue> info(begin());                   \
-    f(info);                                                           \
-    return GetReturnValue<ReturnValue>(isolate);                       \
-  }
-
-#define WRITE_CALL_1(Function, ReturnValue, Arg1)                     \
-  v8::Local<ReturnValue> PropertyCallbackArguments::Call(Function f,  \
-                                                         Arg1 arg1) { \
-    Isolate* isolate = this->isolate();                               \
-    VMState<EXTERNAL> state(isolate);                                 \
-    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));      \
-    PropertyCallbackInfo<ReturnValue> info(begin());                  \
-    f(arg1, info);                                                    \
-    return GetReturnValue<ReturnValue>(isolate);                      \
-  }
-
-#define WRITE_CALL_2(Function, ReturnValue, Arg1, Arg2)          \
-  v8::Local<ReturnValue> PropertyCallbackArguments::Call(        \
-      Function f, Arg1 arg1, Arg2 arg2) {                        \
-    Isolate* isolate = this->isolate();                          \
-    VMState<EXTERNAL> state(isolate);                            \
-    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f)); \
-    PropertyCallbackInfo<ReturnValue> info(begin());             \
-    f(arg1, arg2, info);                                         \
-    return GetReturnValue<ReturnValue>(isolate);                 \
-  }
-
-#define WRITE_CALL_2_VOID(Function, ReturnValue, Arg1, Arg2)               \
-  void PropertyCallbackArguments::Call(Function f, Arg1 arg1, Arg2 arg2) { \
-    Isolate* isolate = this->isolate();                                    \
-    VMState<EXTERNAL> state(isolate);                                      \
-    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));           \
-    PropertyCallbackInfo<ReturnValue> info(begin());                       \
-    f(arg1, arg2, info);                                                   \
-  }
-
-FOR_EACH_CALLBACK_TABLE_MAPPING_0(WRITE_CALL_0)
-FOR_EACH_CALLBACK_TABLE_MAPPING_1(WRITE_CALL_1)
-FOR_EACH_CALLBACK_TABLE_MAPPING_2(WRITE_CALL_2)
-FOR_EACH_CALLBACK_TABLE_MAPPING_2_VOID_RETURN(WRITE_CALL_2_VOID)
-
-#undef WRITE_CALL_0
-#undef WRITE_CALL_1
-#undef WRITE_CALL_2
-#undef WRITE_CALL_2_VOID
+Handle<JSObject> PropertyCallbackArguments::Call(
+    IndexedPropertyEnumeratorCallback f) {
+  Isolate* isolate = this->isolate();
+  VMState<EXTERNAL> state(isolate);
+  ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));
+  PropertyCallbackInfo<v8::Array> info(begin());
+  f(info);
+  return GetReturnValue<JSObject>(isolate);
+}
 
 }  // namespace internal
 }  // namespace v8

src/api-arguments.h

@@ -5,37 +5,14 @@
 #ifndef V8_API_ARGUMENTS_H_
 #define V8_API_ARGUMENTS_H_
 
+#include "src/api.h"
 #include "src/isolate.h"
 #include "src/tracing/trace-event.h"
+#include "src/vm-state-inl.h"
 
 namespace v8 {
 namespace internal {
 
-// For each type of callback, we have a list of arguments
-// They are used to generate the Call() functions below
-// These aren't included in the list as they have duplicate signatures
-// F(GenericNamedPropertyEnumeratorCallback, ...)
-// F(GenericNamedPropertyGetterCallback, ...)
-
-#define FOR_EACH_CALLBACK_TABLE_MAPPING_0(F) \
-  F(IndexedPropertyEnumeratorCallback, v8::Array)
-
-#define FOR_EACH_CALLBACK_TABLE_MAPPING_1(F)                               \
-  F(AccessorNameGetterCallback, v8::Value, v8::Local<v8::Name>)            \
-  F(GenericNamedPropertyQueryCallback, v8::Integer, v8::Local<v8::Name>)   \
-  F(GenericNamedPropertyDeleterCallback, v8::Boolean, v8::Local<v8::Name>) \
-  F(IndexedPropertyGetterCallback, v8::Value, uint32_t)                    \
-  F(IndexedPropertyQueryCallback, v8::Integer, uint32_t)                   \
-  F(IndexedPropertyDeleterCallback, v8::Boolean, uint32_t)
-
-#define FOR_EACH_CALLBACK_TABLE_MAPPING_2(F)                            \
-  F(GenericNamedPropertySetterCallback, v8::Value, v8::Local<v8::Name>, \
-    v8::Local<v8::Value>)                                               \
-  F(IndexedPropertySetterCallback, v8::Value, uint32_t, v8::Local<v8::Value>)
-
-#define FOR_EACH_CALLBACK_TABLE_MAPPING_2_VOID_RETURN(F) \
-  F(AccessorNameSetterCallback, void, v8::Local<v8::Name>, v8::Local<v8::Value>)
-
 // Custom arguments replicate a small segment of stack that can be
 // accessed through an Arguments object the same way the actual stack
 // can.
@@ -68,13 +45,25 @@ class CustomArguments : public CustomArgumentsBase<T::kArgsLength> {
   explicit inline CustomArguments(Isolate* isolate) : Super(isolate) {}
 
   template <typename V>
-  v8::Local<V> GetReturnValue(Isolate* isolate);
+  Handle<V> GetReturnValue(Isolate* isolate);
 
   inline Isolate* isolate() {
     return reinterpret_cast<Isolate*>(this->begin()[T::kIsolateIndex]);
   }
 };
 
+template <typename T>
+template <typename V>
+Handle<V> CustomArguments<T>::GetReturnValue(Isolate* isolate) {
+  // Check the ReturnValue.
+  Object** handle = &this->begin()[kReturnValueOffset];
+  // Nothing was set, return empty handle as per previous behaviour.
+  if ((*handle)->IsTheHole()) return Handle<V>();
+  Handle<V> result = Handle<V>::cast(Handle<Object>(handle));
+  result->VerifyApiCallResultType();
+  return result;
+}
+
 class PropertyCallbackArguments
     : public CustomArguments<PropertyCallbackInfo<Value> > {
  public:
@@ -117,27 +106,91 @@ class PropertyCallbackArguments
  * and used if it's been set to anything inside the callback.
  * New style callbacks always use the return value.
  */
-#define WRITE_CALL_0(Function, ReturnValue) \
-  v8::Local<ReturnValue> Call(Function f);
+  Handle<JSObject> Call(IndexedPropertyEnumeratorCallback f);
+
+#define FOR_EACH_CALLBACK_TABLE_MAPPING_1_NAME(F)                  \
+  F(AccessorNameGetterCallback, "get", v8::Value, Object)          \
+  F(GenericNamedPropertyQueryCallback, "has", v8::Integer, Object) \
+  F(GenericNamedPropertyDeleterCallback, "delete", v8::Boolean, Object)
+
+#define WRITE_CALL_1_NAME(Function, type, ApiReturn, InternalReturn)         \
+  Handle<InternalReturn> Call(Function f, Handle<Name> name) {               \
+    Isolate* isolate = this->isolate();                                      \
+    VMState<EXTERNAL> state(isolate);                                        \
+    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));             \
+    PropertyCallbackInfo<ApiReturn> info(begin());                           \
+    LOG(isolate,                                                             \
+        ApiNamedPropertyAccess("interceptor-named-" type, holder(), *name)); \
+    f(v8::Utils::ToLocal(name), info);                                       \
+    return GetReturnValue<InternalReturn>(isolate);                          \
+  }
 
-#define WRITE_CALL_1(Function, ReturnValue, Arg1) \
-  v8::Local<ReturnValue> Call(Function f, Arg1 arg1);
+  FOR_EACH_CALLBACK_TABLE_MAPPING_1_NAME(WRITE_CALL_1_NAME)
+
+#undef FOR_EACH_CALLBACK_TABLE_MAPPING_1_NAME
+#undef WRITE_CALL_1_NAME
+
+#define FOR_EACH_CALLBACK_TABLE_MAPPING_1_INDEX(F)            \
+  F(IndexedPropertyGetterCallback, "get", v8::Value, Object)  \
+  F(IndexedPropertyQueryCallback, "has", v8::Integer, Object) \
+  F(IndexedPropertyDeleterCallback, "delete", v8::Boolean, Object)
+
+#define WRITE_CALL_1_INDEX(Function, type, ApiReturn, InternalReturn)  \
+  Handle<InternalReturn> Call(Function f, uint32_t index) {            \
+    Isolate* isolate = this->isolate();                                \
+    VMState<EXTERNAL> state(isolate);                                  \
+    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));       \
+    PropertyCallbackInfo<ApiReturn> info(begin());                     \
+    LOG(isolate, ApiIndexedPropertyAccess("interceptor-indexed-" type, \
+                                          holder(), index));           \
+    f(index, info);                                                    \
+    return GetReturnValue<InternalReturn>(isolate);                    \
+  }
 
-#define WRITE_CALL_2(Function, ReturnValue, Arg1, Arg2) \
-  v8::Local<ReturnValue> Call(Function f, Arg1 arg1, Arg2 arg2);
+  FOR_EACH_CALLBACK_TABLE_MAPPING_1_INDEX(WRITE_CALL_1_INDEX)
+
+#undef FOR_EACH_CALLBACK_TABLE_MAPPING_1_INDEX
+#undef WRITE_CALL_1_INDEX
+
+  Handle<Object> Call(GenericNamedPropertySetterCallback f, Handle<Name> name,
+                      Handle<Object> value) {
+    Isolate* isolate = this->isolate();
+    VMState<EXTERNAL> state(isolate);
+    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));
+    PropertyCallbackInfo<v8::Value> info(begin());
+    LOG(isolate,
+        ApiNamedPropertyAccess("interceptor-named-set", holder(), *name));
+    f(v8::Utils::ToLocal(name), v8::Utils::ToLocal(value), info);
+    return GetReturnValue<Object>(isolate);
+  }
 
-#define WRITE_CALL_2_VOID(Function, ReturnValue, Arg1, Arg2) \
-  void Call(Function f, Arg1 arg1, Arg2 arg2);
+  Handle<Object> Call(IndexedPropertySetterCallback f, uint32_t index,
+                      Handle<Object> value) {
+    Isolate* isolate = this->isolate();
+    VMState<EXTERNAL> state(isolate);
+    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));
+    PropertyCallbackInfo<v8::Value> info(begin());
+    LOG(isolate,
+        ApiIndexedPropertyAccess("interceptor-indexed-set", holder(), index));
+    f(index, v8::Utils::ToLocal(value), info);
+    return GetReturnValue<Object>(isolate);
+  }
 
-  FOR_EACH_CALLBACK_TABLE_MAPPING_0(WRITE_CALL_0)
-  FOR_EACH_CALLBACK_TABLE_MAPPING_1(WRITE_CALL_1)
-  FOR_EACH_CALLBACK_TABLE_MAPPING_2(WRITE_CALL_2)
-  FOR_EACH_CALLBACK_TABLE_MAPPING_2_VOID_RETURN(WRITE_CALL_2_VOID)
+  void Call(AccessorNameSetterCallback f, Handle<Name> name,
+            Handle<Object> value) {
+    Isolate* isolate = this->isolate();
+    VMState<EXTERNAL> state(isolate);
+    ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f));
+    PropertyCallbackInfo<void> info(begin());
+    LOG(isolate,
+        ApiNamedPropertyAccess("interceptor-named-set", holder(), *name));
+    f(v8::Utils::ToLocal(name), v8::Utils::ToLocal(value), info);
+  }
 
-#undef WRITE_CALL_0
-#undef WRITE_CALL_1
-#undef WRITE_CALL_2
-#undef WRITE_CALL_2_VOID
+ private:
+  inline JSObject* holder() {
+    return JSObject::cast(this->begin()[T::kHolderIndex]);
+  }
 };
 
 class FunctionCallbackArguments
@@ -187,7 +240,7 @@ class FunctionCallbackArguments
    * and used if it's been set to anything inside the callback.
    * New style callbacks always use the return value.
    */
-  v8::Local<v8::Value> Call(FunctionCallback f);
+  Handle<Object> Call(FunctionCallback f);
 
  private:
   internal::Object** argv_;

src/builtins.cc

@@ -3970,14 +3970,8 @@ MUST_USE_RESULT MaybeHandle<Object> HandleApiCallHelper(
                                      args.length() - 1,
                                      is_construct);
 
-    v8::Local<v8::Value> value = custom.Call(callback);
-    Handle<Object> result;
-    if (value.IsEmpty()) {
-      result = isolate->factory()->undefined_value();
-    } else {
-      result = v8::Utils::OpenHandle(*value);
-      result->VerifyApiCallResultType();
-    }
+    Handle<Object> result = custom.Call(callback);
+    if (result.is_null()) result = isolate->factory()->undefined_value();
 
     RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
     if (!is_construct || result->IsJSObject()) {
@@ -4144,8 +4138,6 @@ MaybeHandle<Object> Builtins::InvokeApiFunction(Handle<HeapObject> function,
 MUST_USE_RESULT static Object* HandleApiCallAsFunctionOrConstructor(
     Isolate* isolate, bool is_construct_call,
     BuiltinArguments<BuiltinExtraArguments::kNone> args) {
-  Heap* heap = isolate->heap();
-
   Handle<Object> receiver = args.receiver();
 
   // Get the object called.
@@ -4180,12 +4172,11 @@ MUST_USE_RESULT static Object* HandleApiCallAsFunctionOrConstructor(
                                      &args[0] - 1,
                                      args.length() - 1,
                                      is_construct_call);
-    v8::Local<v8::Value> value = custom.Call(callback);
-    if (value.IsEmpty()) {
-      result = heap->undefined_value();
+    Handle<Object> result_handle = custom.Call(callback);
+    if (result_handle.is_null()) {
+      result = isolate->heap()->undefined_value();
     } else {
-      result = *reinterpret_cast<Object**>(*value);
-      result->VerifyApiCallResultType();
+      result = *result_handle;
     }
   }
   // Check for exceptions and return result.

src/ic/ic.cc

@@ -2761,12 +2761,11 @@ RUNTIME_FUNCTION(Runtime_StoreCallbackProperty) {
       FUNCTION_CAST<v8::AccessorNameSetterCallback>(setter_address);
   DCHECK(fun != NULL);
 
-  LOG(isolate, ApiNamedPropertyAccess("store", *receiver, *name));
   Object::ShouldThrow should_throw =
       is_sloppy(language_mode) ? Object::DONT_THROW : Object::THROW_ON_ERROR;
   PropertyCallbackArguments custom_args(isolate, callback->data(), *receiver,
                                         *holder, should_throw);
-  custom_args.Call(fun, v8::Utils::ToLocal(name), v8::Utils::ToLocal(value));
+  custom_args.Call(fun, name, value);
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
   return *value;
 }
@@ -2796,19 +2795,15 @@ RUNTIME_FUNCTION(Runtime_LoadPropertyWithInterceptorOnly) {
   v8::GenericNamedPropertyGetterCallback getter =
       v8::ToCData<v8::GenericNamedPropertyGetterCallback>(
           interceptor->getter());
-  LOG(isolate, ApiNamedPropertyAccess("interceptor-named-get", *holder, *name));
-  v8::Local<v8::Value> result =
-      arguments.Call(getter, v8::Utils::ToLocal(name));
+  Handle<Object> result = arguments.Call(getter, name);
 
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
 
   Handle<Object> result_internal;
-  if (result.IsEmpty()) {
+  if (result.is_null()) {
     return isolate->heap()->no_interceptor_result_sentinel();
   }
-  result_internal = v8::Utils::OpenHandle(*result);
-  result_internal->VerifyApiCallResultType();
-  return *result_internal;
+  return *result;
 }
 
 
@@ -2886,31 +2881,24 @@ RUNTIME_FUNCTION(Runtime_LoadElementWithInterceptor) {
   uint32_t index = args.smi_at(1);
 
   InterceptorInfo* interceptor = receiver->GetIndexedInterceptor();
-  v8::Local<v8::Value> result;
   PropertyCallbackArguments arguments(isolate, interceptor->data(), *receiver,
                                       *receiver, Object::DONT_THROW);
 
   v8::IndexedPropertyGetterCallback getter =
       v8::ToCData<v8::IndexedPropertyGetterCallback>(interceptor->getter());
-  LOG(isolate,
-      ApiIndexedPropertyAccess("interceptor-indexed-get", *receiver, index));
-  result = arguments.Call(getter, index);
+  Handle<Object> result = arguments.Call(getter, index);
 
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
 
-  Handle<Object> result_internal;
-  if (result.IsEmpty()) {
+  if (result.is_null()) {
     LookupIterator it(isolate, receiver, index, receiver);
     DCHECK_EQ(LookupIterator::INTERCEPTOR, it.state());
     it.Next();
-    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result_internal,
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result,
                                        Object::GetProperty(&it));
-  } else {
-    result_internal = v8::Utils::OpenHandle(*result);
-    result_internal->VerifyApiCallResultType();
   }
 
-  return *result_internal;
+  return *result;
 }