Bindings: use |holder| local variable in attribute getters and setters

Source/bindings/templates/attributes.cpp

 {##############################################################################}
 {% macro attribute_getter(attribute, world_suffix) %}
 {% filter conditional(attribute.conditional_string) %}
 static void {{attribute.name}}AttributeGetter{{world_suffix}}(
 {%- if attribute.is_expose_js_accessors %}
 const v8::FunctionCallbackInfo<v8::Value>& info
 {%- else %}
 const v8::PropertyCallbackInfo<v8::Value>& info
 {%- endif %})
 {
     {% if attribute.is_reflect and not attribute.is_url and
           attribute.idl_type == 'DOMString' and is_node %}
     {% set cpp_class, v8_class = 'Element', 'V8Element' %}
     {% endif %}
-    {# impl #}
-    {# FIXME: use a local variable for holder more often and simplify below #}
-    {% if attribute.is_unforgeable or
-          interface_name == 'Window' and attribute.idl_type == 'EventHandler' %}
-    {% if interface_name == 'Window' %}
-    v8::Handle<v8::Object> holder = info.Holder();
-    {% else %}{# perform lookup first #}
+    {# holder #}
+    {% if attribute.is_unforgeable and interface_name != 'Window' %}
+    {# perform lookup first #}
     {# FIXME: can we remove this lookup? #}
     v8::Handle<v8::Object> holder = {{v8_class}}::findInstanceInPrototypeChain(info.This(), info.GetIsolate());
     if (holder.IsEmpty())
         return;
-    {% endif %}{# Window #}
+    {% elif not attribute.is_static %}
+    v8::Handle<v8::Object> holder = info.Holder();
+    {% endif %}
+    {# impl #}
+    {% if attribute.cached_attribute_validation_method %}
+    v8::Handle<v8::String> propertyName = v8AtomicString(info.GetIsolate(), "{{attribute.name}}");
     {{cpp_class}}* impl = {{v8_class}}::toNative(holder);
-    {% elif attribute.cached_attribute_validation_method %}
-    v8::Handle<v8::String> propertyName = v8AtomicString(info.GetIsolate(), "{{attribute.name}}");
-    {{cpp_class}}* impl = {{v8_class}}::toNative(info.Holder());
     if (!impl->{{attribute.cached_attribute_validation_method}}()) {
-        v8::Handle<v8::Value> jsValue = V8HiddenValue::getHiddenValue(info.GetIsolate(), info.Holder(), propertyName);
+        v8::Handle<v8::Value> jsValue = V8HiddenValue::getHiddenValue(info.GetIsolate(), holder, propertyName);
         if (!jsValue.IsEmpty()) {
             v8SetReturnValue(info, jsValue);
             return;
         }
     }
-    {% elif not (attribute.is_static or attribute.is_unforgeable) %}
-    {{cpp_class}}* impl = {{v8_class}}::toNative(info.Holder());
+    {% elif not attribute.is_static %}
+    {{cpp_class}}* impl = {{v8_class}}::toNative(holder);
     {% endif %}
     {% if attribute.is_implemented_by and not attribute.is_static %}
     ASSERT(impl);
     {% endif %}
     {% if interface_name == 'Window' and attribute.idl_type == 'EventHandler' %}
     if (!impl->document())
         return;
     {% endif %}
     {# Local variables #}
     {% if attribute.is_call_with_execution_context %}
     ExecutionContext* scriptContext = currentExecutionContext(info.GetIsolate());
     {% endif %}
     {% if attribute.is_check_security_for_node or
           attribute.is_getter_raises_exception %}
-    ExceptionState exceptionState(ExceptionState::GetterContext, "{{attribute.name}}", "{{interface_name}}", info.Holder(), info.GetIsolate());
+    ExceptionState exceptionState(ExceptionState::GetterContext, "{{attribute.name}}", "{{interface_name}}", holder, info.GetIsolate());
     {% endif %}
     {% if attribute.is_nullable %}
     bool isNull = false;
     {% endif %}
     {# FIXME: consider always using a local variable for value #}
     {% if attribute.cached_attribute_validation_method or
           attribute.is_getter_raises_exception or
           attribute.is_nullable or
           attribute.reflect_only or
           attribute.idl_type == 'EventHandler' %}
@@ skipping 17 matching lines @@
                          attribute.reflect_invalid, attribute.reflect_empty)
       | indent}}
     {% endif %}
     {% if attribute.is_nullable %}
     if (isNull) {
         v8SetReturnValueNull(info);
         return;
     }
     {% endif %}
     {% if attribute.cached_attribute_validation_method %}
-    V8HiddenValue::setHiddenValue(info.GetIsolate(), info.Holder(), propertyName, {{attribute.cpp_value}}.v8Value());
+    V8HiddenValue::setHiddenValue(info.GetIsolate(), holder, propertyName, {{attribute.cpp_value}}.v8Value());
     {% endif %}
     {# v8SetReturnValue #}
     {% if attribute.is_keep_alive_for_gc %}
     {# FIXME: merge local variable assignment with above #}
     {{attribute.cpp_type}} result({{attribute.cpp_value}});
     if (result && DOMDataStore::setReturnValueFromWrapper{{world_suffix}}<{{attribute.v8_type}}>(info.GetReturnValue(), result.get()))
         return;
-    v8::Handle<v8::Value> wrapper = toV8(result.get(), info.Holder(), info.GetIsolate());
+    v8::Handle<v8::Value> wrapper = toV8(result.get(), holder, info.GetIsolate());
     if (!wrapper.IsEmpty()) {
-        V8HiddenValue::setHiddenValue(info.GetIsolate(), info.Holder(), v8AtomicString(info.GetIsolate(), "{{attribute.name}}"), wrapper);
+        V8HiddenValue::setHiddenValue(info.GetIsolate(), holder, v8AtomicString(info.GetIsolate(), "{{attribute.name}}"), wrapper);
         {{attribute.v8_set_return_value}};
     }
     {% elif world_suffix %}
     {{attribute.v8_set_return_value_for_main_world}};
     {% else %}
     {{attribute.v8_set_return_value}};
     {% endif %}
 }
 {% endfilter %}
 {% endmacro %}
@@ skipping 92 matching lines @@
 {%- if attribute.is_expose_js_accessors %}
 v8::Local<v8::Value> jsValue, const v8::FunctionCallbackInfo<v8::Value>& info
 {%- else %}
 v8::Local<v8::Value> jsValue, const v8::PropertyCallbackInfo<void>& info
 {%- endif %})
 {
     {% if attribute.is_reflect and attribute.idl_type == 'DOMString' and
           is_node %}
     {% set cpp_class, v8_class = 'Element', 'V8Element' %}
     {% endif %}
+    {# Local variables #}
+    {% if not attribute.is_static %}
+    v8::Handle<v8::Object> holder = info.Holder();

[Nils Barth (inactive), 2014/03/27 05:19:46]

I've added this for consistency, and b/c it's short; WDYT?

+    {% endif %}
     {% if attribute.has_setter_exception_state %}
-    ExceptionState exceptionState(ExceptionState::SetterContext, "{{attribute.name}}", "{{interface_name}}", info.Holder(), info.GetIsolate());
+    ExceptionState exceptionState(ExceptionState::SetterContext, "{{attribute.name}}", "{{interface_name}}", holder, info.GetIsolate());
     {% endif %}
+    {# Type checking #}
     {% if attribute.has_strict_type_checking %}
     {# Type checking for interface types (if interface not implemented, throw
        TypeError), per http://www.w3.org/TR/WebIDL/#es-interface #}
     if (!isUndefinedOrNull(jsValue) && !V8{{attribute.idl_type}}::hasInstance(jsValue, info.GetIsolate())) {
         exceptionState.throwTypeError("The provided value is not of type '{{attribute.idl_type}}'.");
         exceptionState.throwIfNeeded();
         return;
     }
     {% endif %}
     {# impl #}
     {% if attribute.put_forwards %}
-    {{cpp_class}}* proxyImpl = {{v8_class}}::toNative(info.Holder());
+    {{cpp_class}}* proxyImpl = {{v8_class}}::toNative(holder);
     {{attribute.ref_ptr}}<{{attribute.idl_type}}> impl = WTF::getPtr(proxyImpl->{{attribute.name}}());
     if (!impl)
         return;
     {% elif not attribute.is_static %}
-    {{cpp_class}}* impl = {{v8_class}}::toNative(info.Holder());
+    {{cpp_class}}* impl = {{v8_class}}::toNative(holder);
     {% endif %}
     {% if attribute.is_implemented_by and not attribute.is_static %}
     ASSERT(impl);
     {% endif %}
     {% if attribute.idl_type == 'EventHandler' and interface_name == 'Window' %}
     if (!impl->document())
         return;
     {% endif %}
     {# Convert JS value to C++ value #}
     {% if attribute.idl_type != 'EventHandler' %}
     {{attribute.v8_value_to_local_cpp_value}};
     {% elif not is_node %}{# EventHandler hack #}
-    moveEventListenerToNewWrapper(info.Holder(), {{attribute.event_handler_getter_expression}}, jsValue, {{v8_class}}::eventListenerCacheIndex, info.GetIsolate());
+    moveEventListenerToNewWrapper(holder, {{attribute.event_handler_getter_expression}}, jsValue, {{v8_class}}::eventListenerCacheIndex, info.GetIsolate());
     {% endif %}
     {% if attribute.enum_validation_expression %}
     {# Setter ignores invalid enum values: http://www.w3.org/TR/WebIDL/#idl-enums #}
     String string = cppValue;
     if (!({{attribute.enum_validation_expression}}))
         return;
     {% endif %}
     {# Pre-set context #}
     {% if attribute.is_custom_element_callbacks or
           (attribute.is_reflect and
            not(attribute.idl_type == 'DOMString' and is_node)) %}
     {# Skip on compact node DOMString getters #}
     CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
     {% endif %}
     {% if attribute.is_call_with_execution_context or
           attribute.is_setter_call_with_execution_context %}
     ExecutionContext* scriptContext = currentExecutionContext(info.GetIsolate());
     {% endif %}
     {# Set #}
     {{attribute.cpp_setter}};
     {# Post-set #}
     {% if attribute.is_setter_raises_exception %}
     exceptionState.throwIfNeeded();
     {% endif %}
     {% if attribute.cached_attribute_validation_method %}
-    V8HiddenValue::deleteHiddenValue(info.GetIsolate(), info.Holder(), v8AtomicString(info.GetIsolate(), "{{attribute.name}}")); // Invalidate the cached value.
+    V8HiddenValue::deleteHiddenValue(info.GetIsolate(), holder, v8AtomicString(info.GetIsolate(), "{{attribute.name}}")); // Invalidate the cached value.
     {% endif %}
 }
 {% endfilter %}
 {% endmacro %}
 
 
 {##############################################################################}
 {% macro attribute_setter_callback(attribute, world_suffix) %}
 {% filter conditional(attribute.conditional_string) %}
 static void {{attribute.name}}AttributeSetterCallback{{world_suffix}}(
@@ skipping 25 matching lines @@
     {% endif %}
     {% if attribute.has_custom_setter %}
     {{v8_class}}::{{attribute.name}}AttributeSetterCustom(jsValue, info);
     {% else %}
     {{cpp_class}}V8Internal::{{attribute.name}}AttributeSetter{{world_suffix}}(jsValue, info);
     {% endif %}
     TRACE_EVENT_SET_SAMPLING_STATE("V8", "V8Execution");
 }
 {% endfilter %}
 {% endmacro %}