[callbacks] Clean up PropertyCallbackArguments

This CL removes the last method taking a raw callback as argument.

Bug: chromium:782550
Change-Id: Ia7f19120be7dcb2648f11cd3cdb8880f8818490d
Reviewed-on: https://chromium-review.googlesource.com/822198
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51695}

src/accessors.cc

@@ -97,6 +97,9 @@ MUST_USE_RESULT MaybeHandle<Object> ReplaceAccessorWithDataProperty(
 
 }  // namespace
 
+//
+// Accessors::ReconfigureToDataProperty
+//
 void Accessors::ReconfigureToDataProperty(
     v8::Local<v8::Name> key, v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<v8::Boolean>& info) {
@@ -118,6 +121,18 @@ void Accessors::ReconfigureToDataProperty(
   }
 }
 
+void Accessors::ReconfigureToDataPropertyGetter(
+    v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
+  UNREACHABLE();
+}
+
+Handle<AccessorInfo> Accessors::MakeReconfigureToDataPropertyInfo(
+    Isolate* isolate) {
+  Handle<Name> name = isolate->factory()->ReconfigureToDataProperty_string();
+  return MakeAccessor(isolate, name, &ReconfigureToDataPropertyGetter,
+                      &ReconfigureToDataProperty);
+}
+
 //
 // Accessors::ArgumentsIterator
 //
@@ -299,7 +314,6 @@ Handle<AccessorInfo> Accessors::MakeStringLengthInfo(Isolate* isolate) {
                       &StringLengthGetter, nullptr);
 }
 
-
 //
 // Accessors::ScriptColumnOffset
 //

src/accessors.h

@@ -33,6 +33,7 @@ class JavaScriptFrame;
   V(function_name, FunctionName)                                    \
   V(function_length, FunctionLength)                                \
   V(function_prototype, FunctionPrototype)                          \
+  V(reconfigure_to_data_property, ReconfigureToDataProperty)        \
   V(script_column_offset, ScriptColumnOffset)                       \
   V(script_compilation_type, ScriptCompilationType)                 \
   V(script_context_data, ScriptContextData)                         \

src/api-arguments-inl.h

@@ -17,6 +17,11 @@ namespace internal {
   F(Query, query, Object, v8::Integer) \
   F(Deleter, deleter, Object, v8::Boolean)
 
+#define DCHECK_NAME_COMPATIBLE(interceptor, name) \
+  DCHECK(interceptor->is_named());                \
+  DCHECK(!name->IsPrivate());                     \
+  DCHECK_IMPLIES(name->IsSymbol(), interceptor->can_intercept_symbols());
+
 #define PREPARE_CALLBACK_INFO(ISOLATE, F, RETURN_VALUE, API_RETURN_TYPE) \
   if (ISOLATE->needs_side_effect_check() &&                              \
       !PerformSideEffectCheck(ISOLATE, FUNCTION_ADDR(F))) {              \
@@ -29,13 +34,10 @@ namespace internal {
 #define CREATE_NAMED_CALLBACK(Function, type, ReturnType, ApiReturnType)      \
   Handle<ReturnType> PropertyCallbackArguments::CallNamed##Function(          \
       Handle<InterceptorInfo> interceptor, Handle<Name> name) {               \
-    DCHECK(interceptor->is_named());                                          \
-    DCHECK(!name->IsPrivate());                                               \
-    DCHECK_IMPLIES(name->IsSymbol(), interceptor->can_intercept_symbols());   \
+    DCHECK_NAME_COMPATIBLE(interceptor, name);                                \
     Isolate* isolate = this->isolate();                                       \
     RuntimeCallTimerScope timer(                                              \
         isolate, RuntimeCallCounterId::kNamed##Function##Callback);           \
-    DCHECK(!name->IsPrivate());                                               \
     GenericNamedProperty##Function##Callback f =                              \
         ToCData<GenericNamedProperty##Function##Callback>(                    \
             interceptor->type());                                             \
@@ -72,9 +74,7 @@ FOR_EACH_CALLBACK(CREATE_INDEXED_CALLBACK)
 
 Handle<Object> PropertyCallbackArguments::CallNamedGetter(
     Handle<InterceptorInfo> interceptor, Handle<Name> name) {
-  DCHECK(interceptor->is_named());
-  DCHECK_IMPLIES(name->IsSymbol(), interceptor->can_intercept_symbols());
-  DCHECK(!name->IsPrivate());
+  DCHECK_NAME_COMPATIBLE(interceptor, name);
   Isolate* isolate = this->isolate();
   RuntimeCallTimerScope timer(isolate,
                               RuntimeCallCounterId::kNamedGetterCallback);
@@ -87,8 +87,7 @@ Handle<Object> PropertyCallbackArguments::CallNamedGetter(
 
 Handle<Object> PropertyCallbackArguments::CallNamedDescriptor(
     Handle<InterceptorInfo> interceptor, Handle<Name> name) {
-  DCHECK(interceptor->is_named());
-  DCHECK_IMPLIES(name->IsSymbol(), interceptor->can_intercept_symbols());
+  DCHECK_NAME_COMPATIBLE(interceptor, name);
   Isolate* isolate = this->isolate();
   RuntimeCallTimerScope timer(isolate,
                               RuntimeCallCounterId::kNamedDescriptorCallback);
@@ -112,16 +111,9 @@ Handle<Object> PropertyCallbackArguments::BasicCallNamedGetterCallback(
 Handle<Object> PropertyCallbackArguments::CallNamedSetter(
     Handle<InterceptorInfo> interceptor, Handle<Name> name,
     Handle<Object> value) {
-  DCHECK_IMPLIES(name->IsSymbol(), interceptor->can_intercept_symbols());
+  DCHECK_NAME_COMPATIBLE(interceptor, name);
   GenericNamedPropertySetterCallback f =
       ToCData<GenericNamedPropertySetterCallback>(interceptor->setter());
-  return CallNamedSetterCallback(f, name, value);
-}
-
-Handle<Object> PropertyCallbackArguments::CallNamedSetterCallback(
-    GenericNamedPropertySetterCallback f, Handle<Name> name,
-    Handle<Object> value) {
-  DCHECK(!name->IsPrivate());
   Isolate* isolate = this->isolate();
   RuntimeCallTimerScope timer(isolate,
                               RuntimeCallCounterId::kNamedSetterCallback);
@@ -135,9 +127,7 @@ Handle<Object> PropertyCallbackArguments::CallNamedSetterCallback(
 Handle<Object> PropertyCallbackArguments::CallNamedDefiner(
     Handle<InterceptorInfo> interceptor, Handle<Name> name,
     const v8::PropertyDescriptor& desc) {
-  DCHECK(interceptor->is_named());
-  DCHECK(!name->IsPrivate());
-  DCHECK_IMPLIES(name->IsSymbol(), interceptor->can_intercept_symbols());
+  DCHECK_NAME_COMPATIBLE(interceptor, name);
   Isolate* isolate = this->isolate();
   RuntimeCallTimerScope timer(isolate,
                               RuntimeCallCounterId::kNamedDefinerCallback);
@@ -241,7 +231,7 @@ Handle<Object> PropertyCallbackArguments::CallAccessorGetter(
   return BasicCallNamedGetterCallback(f, name);
 }
 
-void PropertyCallbackArguments::CallAccessorSetter(
+Handle<Object> PropertyCallbackArguments::CallAccessorSetter(
     Handle<AccessorInfo> accessor_info, Handle<Name> name,
     Handle<Object> value) {
   Isolate* isolate = this->isolate();
@@ -249,9 +239,10 @@ void PropertyCallbackArguments::CallAccessorSetter(
                               RuntimeCallCounterId::kAccessorSetterCallback);
   AccessorNameSetterCallback f =
       ToCData<AccessorNameSetterCallback>(accessor_info->setter());
-  PREPARE_CALLBACK_INFO(isolate, f, void, void);
+  PREPARE_CALLBACK_INFO(isolate, f, Handle<Object>, void);
   LOG(isolate, ApiNamedPropertyAccess("accessor-setter", holder(), *name));
   f(v8::Utils::ToLocal(name), v8::Utils::ToLocal(value), callback_info);
+  return GetReturnValue<Object>(isolate);
 }
 
 #undef PREPARE_CALLBACK_INFO

src/api-arguments.h

@@ -103,8 +103,9 @@ class PropertyCallbackArguments
   // -------------------------------------------------------------------------
   // Accessor Callbacks
   // Also used for AccessorSetterCallback.
-  inline void CallAccessorSetter(Handle<AccessorInfo> info, Handle<Name> name,
-                                 Handle<Object> value);
+  inline Handle<Object> CallAccessorSetter(Handle<AccessorInfo> info,
+                                           Handle<Name> name,
+                                           Handle<Object> value);
   // Also used for AccessorGetterCallback, AccessorNameGetterCallback.
   inline Handle<Object> CallAccessorGetter(Handle<AccessorInfo> info,
                                            Handle<Name> name);
@@ -118,9 +119,6 @@ class PropertyCallbackArguments
   inline Handle<Object> CallNamedSetter(Handle<InterceptorInfo> interceptor,
                                         Handle<Name> name,
                                         Handle<Object> value);
-  inline Handle<Object> CallNamedSetterCallback(
-      GenericNamedPropertySetterCallback callback, Handle<Name> name,
-      Handle<Object> value);
   inline Handle<Object> CallNamedDefiner(Handle<InterceptorInfo> interceptor,
                                          Handle<Name> name,
                                          const v8::PropertyDescriptor& desc);

src/heap-symbols.h

@@ -161,6 +161,7 @@
   V(query_colon_string, "(?:)")                                    \
   V(RangeError_string, "RangeError")                               \
   V(raw_string, "raw")                                             \
+  V(ReconfigureToDataProperty_string, "ReconfigureToDataProperty") \
   V(ReferenceError_string, "ReferenceError")                       \
   V(RegExp_string, "RegExp")                                       \
   V(regexp_to_string, "[object RegExp]")                           \

src/objects-inl.h

@@ -2296,6 +2296,13 @@ bool AccessorInfo::has_getter() {
   return result;
 }
 
+bool AccessorInfo::has_setter() {
+  bool result = setter() != Smi::kZero;
+  DCHECK_EQ(result, setter() != Smi::kZero &&
+                        Foreign::cast(setter())->foreign_address() != nullptr);
+  return result;
+}
+
 ACCESSORS(AsyncGeneratorRequest, next, Object, kNextOffset)
 SMI_ACCESSORS(AsyncGeneratorRequest, resume_mode, kResumeModeOffset)
 ACCESSORS(AsyncGeneratorRequest, value, Object, kValueOffset)

src/objects.cc

@@ -1505,10 +1505,9 @@ MaybeHandle<Object> Object::GetPropertyWithAccessor(LookupIterator* it) {
     if (result.is_null()) return isolate->factory()->undefined_value();
     Handle<Object> reboxed_result = handle(*result, isolate);
     if (info->replace_on_access() && receiver->IsJSReceiver()) {
-      args.CallNamedSetterCallback(
-          reinterpret_cast<GenericNamedPropertySetterCallback>(
-              &Accessors::ReconfigureToDataProperty),
-          name, result);
+      args.CallAccessorSetter(
+          isolate->factory()->reconfigure_to_data_property_accessor(), name,
+          result);
       RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
     }
     return reboxed_result;
@@ -1594,15 +1593,7 @@ Maybe<bool> Object::SetPropertyWithAccessor(LookupIterator* it,
       return Nothing<bool>();
     }
 
-    // The actual type of call_fun is either v8::AccessorNameSetterCallback or
-    // i::Accesors::AccessorNameBooleanSetterCallback, depending on whether the
-    // AccessorInfo was created by the API or internally (see accessors.cc).
-    // Here we handle both cases using GenericNamedPropertySetterCallback and
-    // its Call method.
-    GenericNamedPropertySetterCallback call_fun =
-        v8::ToCData<GenericNamedPropertySetterCallback>(info->setter());
-
-    if (call_fun == nullptr) {
+    if (!info->has_setter()) {
       // TODO(verwaest): We should not get here anymore once all AccessorInfos
       // are marked as special_data_property. They cannot both be writable and
       // not have a setter.
@@ -1615,9 +1606,15 @@ Maybe<bool> Object::SetPropertyWithAccessor(LookupIterator* it,
           Nothing<bool>());
     }
 
+    // The actual type of setter callback is either
+    // v8::AccessorNameSetterCallback or
+    // i::Accesors::AccessorNameBooleanSetterCallback, depending on whether the
+    // AccessorInfo was created by the API or internally (see accessors.cc).
+    // Here we handle both cases using GenericNamedPropertySetterCallback and
+    // its Call method.
     PropertyCallbackArguments args(isolate, info->data(), *receiver, *holder,
                                    should_throw);
-    Handle<Object> result = args.CallNamedSetterCallback(call_fun, name, value);
+    Handle<Object> result = args.CallAccessorSetter(info, name, value);
     // In the case of AccessorNameSetterCallback, we know that the result value
     // cannot have been set, so the result of Call will be null.  In the case of
     // AccessorNameBooleanSetterCallback, the result will either be null

src/objects.h

@@ -4436,6 +4436,7 @@ class AccessorInfo: public Struct {
   DECL_ACCESSORS(getter, Object)
   inline bool has_getter();
   DECL_ACCESSORS(setter, Object)
+  inline bool has_setter();
   // This either points at the same as above, or a trampoline in case we are
   // running with the simulator. Use these entries from generated code.
   DECL_ACCESSORS(js_getter, Object)