IDL compiler: [Reflect] for getters (+ remove unnecessary WebCore::)

Source/bindings/scripts/unstable/v8_attributes.py

Index: Source/bindings/scripts/unstable/v8_attributes.py
diff --git a/Source/bindings/scripts/unstable/v8_attributes.py b/Source/bindings/scripts/unstable/v8_attributes.py
index 029d5d20e68d6e74c29dd0a4c925d6c5067c7cb7..511e93d1b4485fd5f35f3aee30cb0d8097d5c63f 100644
--- a/Source/bindings/scripts/unstable/v8_attributes.py
+++ b/Source/bindings/scripts/unstable/v8_attributes.py
@@ -81,7 +81,8 @@ def generate_attribute_and_includes(interface, attribute):
         contents['is_custom_getter'] = True
         return contents, set()
 
-    cpp_value = getter_expression(interface, attribute, contents)
+    includes = set()
+    cpp_value = getter_expression(interface, attribute, contents, includes)
     # Normally we can inline the function call into the return statement to
     # avoid the overhead of using a Ref<> temporary, but for some cases
     # (nullable types, EventHandler, CachedAttribute, or if there are
@@ -97,10 +98,11 @@ def generate_attribute_and_includes(interface, attribute):
 
     if this_is_keep_alive_for_gc:
         return_v8_value_statement = 'v8SetReturnValue(info, wrapper);'
-        includes = v8_types.includes_for_type(idl_type)
+        type_includes = v8_types.includes_for_type(idl_type)
         includes.add('bindings/v8/V8HiddenPropertyName.h')
     else:
-        return_v8_value_statement, includes = v8_types.v8_set_return_value(idl_type, cpp_value, callback_info='info', isolate='info.GetIsolate()', extended_attributes=extended_attributes, script_wrappable='imp')
+        return_v8_value_statement, type_includes = v8_types.v8_set_return_value(idl_type, cpp_value, callback_info='info', isolate='info.GetIsolate()', extended_attributes=extended_attributes, script_wrappable='imp')
+    includes.update(type_includes)
     contents['return_v8_value_statement'] = return_v8_value_statement
 
     if (idl_type == 'EventHandler' and
@@ -128,21 +130,50 @@ def generate_attribute_and_includes(interface, attribute):
     return contents, includes
 
 
-def getter_expression(interface, attribute, contents):
-    this_getter_name = getter_name(interface, attribute)
-    arguments = v8_utilities.call_with_arguments(attribute, contents)
+def getter_expression(interface, attribute, contents, includes):
+    arguments = []
+    if 'Reflect' in attribute.extended_attributes:
+        getter_base_name = content_attribute_getter_base_name(attribute, includes, arguments)
+    else:
+        getter_base_name = uncapitalize(attribute.name)
+
+    if attribute.is_static:
+        getter_name = '%s::%s' % (interface.name, getter_base_name)
+    else:
+        getter_name = 'imp->%s' % getter_base_name
+
+    arguments.extend(v8_utilities.call_with_arguments(attribute, contents))
     if attribute.is_nullable:
         arguments.append('isNull')
     if attribute.data_type == 'EventHandler':
         arguments.append('isolatedWorldForIsolate(info.GetIsolate())')
-    return '%s(%s)' % (this_getter_name, ', '.join(arguments))
+    return '%s(%s)' % (getter_name, ', '.join(arguments))
 
 
-def getter_name(interface, attribute):
-    getter_method_name = uncapitalize(attribute.name)
-    if attribute.is_static:
-        return '%s::%s' % (interface.name, getter_method_name)
-    return 'imp->%s' % getter_method_name
+CONTENT_ATTRIBUTE_GETTER_NAMES = {

[haraken, 2013/10/15 02:14:30]

I don't fully understand what "content" means.

CONTENT_ATTRIBUTE_GETTER_NAMES => CPP_ATTRIBUTE_GETTER_NAMES ?

[Nils Barth (inactive), 2013/10/15 09:48:15]

The IDL attribute is reflecting an HTML attribute ("content attribute"), so the
getter is a getter for the underlying content attribute.

Sorry, forgot to link to docs:
https://sites.google.com/a/chromium.org/dev/blink/webidl/blink-idl-extended-a...
http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interf...

"Some IDL attributes are defined to reflect a particular content attribute."

"Content attribute" is a term in the HTML5 spec (WhatWG), meaning "HTML
attribute, not IDL attribute":
http://www.whatwg.org/specs/web-apps/current-work/multipage/infrastructure.ht...
"This specification refers to both HTML and XML attributes and IDL attributes,
often in the same context. When it is not clear which is being referred to, they
are referred to as content attributes for HTML and XML attributes, and IDL
attributes for those defined on IDL interfaces."

+    'boolean': 'fastHasAttribute',
+    'long': 'getIntegralAttribute',
+    'unsigned long': 'getUnsignedIntegralAttribute',
+}
+
+
+def content_attribute_getter_base_name(attribute, includes, arguments):

[haraken, 2013/10/15 02:14:30]

content_attribute_getter_base_name => cpp_attribute_getter_name ?

+    default_content_attribute_name = attribute.name.lower()
+    content_attribute_name = attribute.extended_attributes['Reflect'] or default_content_attribute_name

[haraken, 2013/10/15 02:14:30]

Nit: default_content_attribute_name is not necessary.

[Nils Barth (inactive), 2013/10/15 09:48:15]

Removed.

+
+    namespace = 'HTMLNames'  # FIXME: can be SVG too
+    includes.add('%s.h' % namespace)
+
+    if content_attribute_name in ['class', 'id', 'name']:

[haraken, 2013/10/15 02:14:30]

Add a comment:

  # For performance optimization.

[Nils Barth (inactive), 2013/10/15 09:48:15]

Done.

+        return 'get%sAttribute' % content_attribute_name.capitalize()
+
+    scoped_name = 'WebCore::%s::%sAttr' % (namespace, content_attribute_name)

[haraken, 2013/10/15 02:14:30]

Is WebCore:: necessary?

[Nils Barth (inactive), 2013/10/15 09:48:15]

Doesn't look like it!
Removed here and in Perl.

+    arguments.append(scoped_name)
+
+    idl_type = attribute.data_type
+    if idl_type in CONTENT_ATTRIBUTE_GETTER_NAMES:
+        return CONTENT_ATTRIBUTE_GETTER_NAMES[idl_type]
+    return 'fastGetAttribute'
 
 
 def is_keep_alive_for_gc(attribute):