bindings: Supports [Replaceable] accessors.

Source/bindings/scripts/v8_attributes.py

Index: Source/bindings/scripts/v8_attributes.py
diff --git a/Source/bindings/scripts/v8_attributes.py b/Source/bindings/scripts/v8_attributes.py
index 8e3baff3c820a7a4f3d162c5947cb757d1e325bd..7c6ac9f0248a9bdea1d676b77d4cdcfb7dd26e2d 100644
--- a/Source/bindings/scripts/v8_attributes.py
+++ b/Source/bindings/scripts/v8_attributes.py
@@ -59,18 +59,19 @@ def attribute_context(interface, attribute):
     is_reflect = 'Reflect' in extended_attributes
     if is_custom_element_callbacks or is_reflect:
         includes.add('core/dom/custom/CustomElementProcessingStack.h')
-    # [PerWorldBindings]
-    if 'PerWorldBindings' in extended_attributes:
-        assert idl_type.is_wrapper_type or 'LogActivity' in extended_attributes, '[PerWorldBindings] should only be used with wrapper types: %s.%s' % (interface.name, attribute.name)
+    # [ExposeJSAccessors]
+    is_expose_js_accessors = 'ExposeJSAccessors' in extended_attributes
     # [ImplementedInPrivateScript]
     is_implemented_in_private_script = 'ImplementedInPrivateScript' in extended_attributes
     if is_implemented_in_private_script:
         includes.add('bindings/core/v8/PrivateScriptRunner.h')
         includes.add('core/frame/LocalFrame.h')
         includes.add('platform/ScriptForbiddenScope.h')
-
     # [OnlyExposedToPrivateScript]
     is_only_exposed_to_private_script = 'OnlyExposedToPrivateScript' in extended_attributes
+    # [PerWorldBindings]
+    if 'PerWorldBindings' in extended_attributes:
+        assert idl_type.is_wrapper_type or 'LogActivity' in extended_attributes, '[PerWorldBindings] should only be used with wrapper types: %s.%s' % (interface.name, attribute.name)
 
     if (base_idl_type == 'EventHandler' and
         interface.name in ['Window', 'WorkerGlobalScope'] and
@@ -101,7 +102,7 @@ def attribute_context(interface, attribute):
         'is_call_with_script_state': v8_utilities.has_extended_attribute_value(attribute, 'CallWith', 'ScriptState'),
         'is_check_security_for_node': is_check_security_for_node,
         'is_custom_element_callbacks': is_custom_element_callbacks,
-        'is_expose_js_accessors': 'ExposeJSAccessors' in extended_attributes,
+        'is_expose_js_accessors': is_expose_js_accessors,
         'is_getter_raises_exception':  # [RaisesException]
             'RaisesException' in extended_attributes and
             extended_attributes['RaisesException'] in (None, 'Getter'),
@@ -134,7 +135,7 @@ def attribute_context(interface, attribute):
         'reflect_missing': extended_attributes.get('ReflectMissing'),
         'reflect_only': extended_attribute_value_as_list(attribute, 'ReflectOnly'),
         'runtime_enabled_function': v8_utilities.runtime_enabled_function_name(attribute),  # [RuntimeEnabled]
-        'setter_callback': setter_callback_name(interface, attribute),
+        'setter_callback': setter_callback_name(interface, attribute, is_expose_js_accessors),
         'should_be_exposed_to_script': not (is_implemented_in_private_script and is_only_exposed_to_private_script),
         'world_suffixes': ['', 'ForMainWorld']
                           if 'PerWorldBindings' in extended_attributes
@@ -427,18 +428,20 @@ def scoped_content_attribute_name(interface, attribute):
 # Attribute configuration
 ################################################################################
 
-# [Replaceable]
-def setter_callback_name(interface, attribute):
+# [PutForwards], [Replaceable]
+def setter_callback_name(interface, attribute, is_expose_js_accessors):
     cpp_class_name = cpp_name(interface)
     cpp_class_name_or_partial = cpp_name_or_partial(interface)
 
     extended_attributes = attribute.extended_attributes
-    if (('Replaceable' in extended_attributes and
-         'PutForwards' not in extended_attributes) or
-        is_constructor_attribute(attribute)):
+    if (not is_expose_js_accessors and
+        ('Replaceable' in extended_attributes or

[haraken, 2015/02/23 14:48:52]

Why can we remove "'PutForwards' not in extended_attributes" ?

[Yuki, 2015/02/23 15:18:42]

[Replaceable] and [PutForwards] conflict each other, and I assume that both of
them are never sepecified at a time.  Validation of IDLs should be done
somewhere else.

[haraken, 2015/02/23 17:34:00]

Sounds like a good idea.

+         is_constructor_attribute(attribute))):
         return '%sV8Internal::%sForceSetAttributeOnThisCallback' % (

[haraken, 2015/02/23 14:48:52]

Help me understand: What's a difference between a [Replaceable] attribute on a
prototype chain and a [Replaceable] attribute on an instance? I'm wondering why
you don't need to call ForceSet for a [Replaceable] attribute on a prototype
chain.

[Yuki, 2015/02/23 15:18:42]

Basically we have the same implementation for "replaceable on prototype" and
"replaceable on instance".  We call v8::Object::ForceSet to set an own property
in either case.

The point is that
a) for accessor version, the callback function takes only
v8::FunctionCallbackInfo, and there is no way to tell the property name from the
argument.  So, in this CL, we're going to create
  %sAttributeSetterCallback(v8::FunctionCallbackInfo)
where we can tell the property name as "{{attribute.name}}".

b) for attribute (non-accessor) version, the callback function takes two
arguments: property name as v8::String and the v8::PropertyCallbackInfo.  So we
can re-use the same function, named %sForceSetAttributeOnThisCallback, for each
property.

[haraken, 2015/02/23 17:33:59]

Ah, now I understand what this CL is doing :)

It seems a bit confusing to me to use different names between attributes on
prototype chains and attributes on instances. Can we use the same method name?

For example, what happens if we just remove the if branch at line 437 ? (What
happens if we unconditionally use '%sV8Internal::%sAttributeSetterCallback'?)

At the very least, by using the same method name for attributes on prototype
chains and attributes on instances, I guess we can remove a bunch of
'is_expose_js_accessors' checks from this CL, because we no longer need to
distinguish the two methods.

[Yuki, 2015/02/25 14:19:29]

Done.

             cpp_class_name_or_partial, cpp_class_name)
-    if attribute.is_read_only and 'PutForwards' not in extended_attributes:
+    if (attribute.is_read_only and
+            'PutForwards' not in extended_attributes and
+            'Replaceable' not in extended_attributes):

[haraken, 2015/02/23 14:48:52]

Fix indentation.

[Yuki, 2015/02/23 15:18:42]

We need this indentation.  Otherwise, I got a lint warning:
"visually indented line with same indent as next logical line".

I followed this way:
http://stackoverflow.com/questions/21592569/how-to-use-continuation-line-over...

         return '0'
     return '%sV8Internal::%sAttributeSetterCallback' % (cpp_class_name_or_partial, attribute.name)