bindings: Reduces the binary size by reducing # of callback functions.

Source/bindings/templates/interface_base.cpp

Index: Source/bindings/templates/interface_base.cpp
diff --git a/Source/bindings/templates/interface_base.cpp b/Source/bindings/templates/interface_base.cpp
index be36312858b3e19abaa21ee71c068f0887e38f82..1777a9a5a52b2c5dade51116af42ea085cf8c725 100644
--- a/Source/bindings/templates/interface_base.cpp
+++ b/Source/bindings/templates/interface_base.cpp
@@ -25,7 +25,7 @@ namespace blink {
 
 {% set wrapper_type_info_const = '' if has_partial_interface else 'const ' %}
 {% if not is_partial %}
-{{wrapper_type_info_const}}WrapperTypeInfo {{v8_class}}::wrapperTypeInfo = { gin::kEmbedderBlink, {{dom_template}}, {{v8_class}}::refObject, {{v8_class}}::derefObject, {{v8_class}}::trace, {{to_active_dom_object}}, {{visit_dom_wrapper}}, {{v8_class}}::installConditionallyEnabledMethods, {{v8_class}}::installConditionallyEnabledProperties, {{parent_wrapper_type_info}}, WrapperTypeInfo::{{wrapper_type_prototype}}, WrapperTypeInfo::{{wrapper_class_id}}, WrapperTypeInfo::{{event_target_inheritance}}, WrapperTypeInfo::{{lifetime}}, WrapperTypeInfo::{{gc_type}} };
+{{wrapper_type_info_const}}WrapperTypeInfo {{v8_class}}::wrapperTypeInfo = { gin::kEmbedderBlink, {{dom_template}}, {{v8_class}}::refObject, {{v8_class}}::derefObject, {{v8_class}}::trace, {{to_active_dom_object}}, {{visit_dom_wrapper}}, {{v8_class}}::installConditionallyEnabledMethods, {{v8_class}}::installConditionallyEnabledProperties, "{{interface_name}}", {{parent_wrapper_type_info}}, WrapperTypeInfo::{{wrapper_type_prototype}}, WrapperTypeInfo::{{wrapper_class_id}}, WrapperTypeInfo::{{event_target_inheritance}}, WrapperTypeInfo::{{lifetime}}, WrapperTypeInfo::{{gc_type}} };
 
 // This static member must be declared by DEFINE_WRAPPERTYPEINFO in {{cpp_class}}.h.
 // For details, see the comment of DEFINE_WRAPPERTYPEINFO in
@@ -69,6 +69,55 @@ static void {{cpp_class}}ForceSetAttributeOnThis(v8::Local<v8::String> name, v8:
     v8::Local<v8::Object>::Cast(info.This())->ForceSet(name, v8Value);
 }
 
+{% for attribute in attributes
+       if (attribute.constructor_type and
+           attribute.name == attribute.constructor_type and
+           not attribute.is_expose_js_accessors) %}
+{% if loop.first %}

[haraken, 2015/03/17 00:06:07]

Instead of using loop.first, can we introduce
has_constructor_attribute_setter_for_properties? (We normally introduce has_xxx
variables like this.)

[Yuki, 2015/03/17 08:35:19]

Done.

+static void {{cpp_class}}ConstructorAttributeSetterCallbackAsProperty(v8::Local<v8::String>, v8::Local<v8::Value> v8Value, const v8::PropertyCallbackInfo<void>& info)

[haraken, 2015/03/17 00:06:07]

I'm just curious, but how many constructor attributes still need the
PropertyCallbackInfo version?

[Yuki, 2015/03/17 08:35:19]

We've not yet moved any of constructor attribute to prototype chains.  I've
removed accessor version in this CL to make it look simple.

+{
+    TRACE_EVENT_SET_SAMPLING_STATE("blink", "DOMSetter");
+    do {

[haraken, 2015/03/17 00:06:08]

What is the do...while for?

[Yuki, 2015/03/17 08:35:19]

Added a comment.  It's for use of 'break'.
We need to call TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution") at the end.

+        v8::Local<v8::Value> data = info.Data();

[haraken, 2015/03/17 00:06:08]

Not related to your CL, we want to deprecate info.Data(). It would be great if
there is a way to remove info.Data() from the constructor attribute setter.

[Yuki, 2015/03/17 08:35:19]

We could use "hidden value" instead of info.Data(), but it doesn't help much
since hidden values are also discouraged...  I'll keep it in mind.

+        ASSERT(data->IsExternal());
+        V8PerContextData* perContextData = V8PerContextData::from(info.Holder()->CreationContext());
+        if (!perContextData)
+            break;
+        const WrapperTypeInfo* wrapperTypeInfo = WrapperTypeInfo::unwrap(data);
+        if (!wrapperTypeInfo)
+            break;
+        {{cpp_class}}ForceSetAttributeOnThis(v8String(info.GetIsolate(), wrapperTypeInfo->interfaceName), v8Value, info);

[haraken, 2015/03/17 00:06:07]

Not related to your CL, I want to understand why this is ForceSet (which is
discouraged) instead of Set. Can we change the ForceSet to Set and see what
tests fail?

[Yuki, 2015/03/17 08:35:19]

My understanding is:
  Set = regular assignment, which means named/indexed properties are supported.
  ForceSet = bypass interceptors such as named/indexed properties.

We shouldn't use regular Set inside a setter callback, which may cause an
infinite loop.  Since we know 1) the attribute is a replaceable attribute or
constructor attribute, and 2) V8 (JS) found this is the proper setter callback,
thus we should simply set the value without looking for a setter again.

+    } while (false);
+    TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution");
+}
+
+{% endif %}
+{% endfor %}
+{% for attribute in attributes
+       if (attribute.constructor_type and
+           attribute.name == attribute.constructor_type and
+           attribute.is_expose_js_accessors) %}
+{% if loop.first %}

[haraken, 2015/03/17 00:06:08]

Ditto. I'd prefer has_constructor_attribute_setter_for_accessors.

[Yuki, 2015/03/17 08:35:19]

Done.

+static void {{cpp_class}}ConstructorAttributeSetterCallbackAsAccessor(const v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    TRACE_EVENT_SET_SAMPLING_STATE("blink", "DOMSetter");
+    do {

[haraken, 2015/03/17 00:06:07]

What is the do...while for?

[Yuki, 2015/03/17 08:35:19]

Done.

+        v8::Local<v8::Value> v8Value = info[0];
+        v8::Local<v8::Value> data = info.Data();
+        ASSERT(data->IsExternal());
+        V8PerContextData* perContextData = V8PerContextData::from(info.Holder()->CreationContext());
+        if (!perContextData)
+            break;
+        const WrapperTypeInfo* wrapperTypeInfo = WrapperTypeInfo::unwrap(data);
+        if (!wrapperTypeInfo)
+            break;
+        {{cpp_class}}ForceSetAttributeOnThis(v8String(info.GetIsolate(), wrapperTypeInfo->interfaceName), v8Value, info);
+    } while (false);
+    TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution");
+}
+
+{% endif %}
+{% endfor %}
 {% endif %}
 {% endblock %}
 {##############################################################################}
@@ -87,11 +136,14 @@ static void {{cpp_class}}ForceSetAttributeOnThis(v8::Local<v8::String> name, v8:
 {{attribute_getter_callback(attribute, world_suffix)}}
 {% endif %}
 {% if attribute.has_setter %}
-{% if not attribute.has_custom_setter %}
+{% if not (attribute.has_custom_setter or
+           (attribute.constructor_type and attribute.name == attribute.constructor_type)) %}

[haraken, 2015/03/17 00:06:08]

Maybe I'm wrong, but can we avoid introducing the 'attribute.name ==
attribute.constructor_type' check by storing a "proper" interface name (e.g., a
"WebKit"-prefixed name) on wrapperTypeInfo->interfaceName?

[Yuki, 2015/03/17 08:35:19]

No, we cannot.  At least there is case that 1) the proper interface name is
unprefixed name, and 2) we have both of prefixed and unprefixed constructor
attributes.

For example, we have
  "WebKitTransitionEvent" attribute to create a TransitionEvent, and
  "TransitionEvent" attribute, too, to create a TransitionEvent.

It's better for compatibility.

 {{attribute_setter(attribute, world_suffix)}}
 {% endif %}
+{% if not (attribute.constructor_type and attribute.name == attribute.constructor_type) %}
 {{attribute_setter_callback(attribute, world_suffix)}}
 {% endif %}
+{% endif %}
 {% endfor %}
 {% endfor %}
 {##############################################################################}