First implementation of lazily cached accessor for DOM attributes.

third_party/WebKit/Source/bindings/templates/attributes.cpp.tmpl

 {% from 'utilities.cpp.tmpl' import declare_enum_validation_variable, v8_value_to_local_cpp_value %}
 
 {##############################################################################}
 {% macro attribute_getter(attribute, world_suffix) %}
 static void {{attribute.name}}AttributeGetter{{world_suffix}}(
 {%- if attribute.is_data_type_property %}
 const v8::PropertyCallbackInfo<v8::Value>& info
 {%- else %}
 const v8::FunctionCallbackInfo<v8::Value>& info
 {%- endif %}) {
@@ skipping 239 matching lines @@
 
 
 {##############################################################################}
 {% macro attribute_cached_property_key(attribute) %}
 v8::Local<v8::Private> {{v8_class_or_partial}}::{{attribute.name}}CachedPropertyKey(v8::Isolate* isolate)
 {
   return V8PrivateProperty::Get{{attribute.cached_accessor_name}}(isolate).GetPrivate();
 }
 {% endmacro %}
 
+{##############################################################################}
+{% macro attribute_cache_init_callback(attribute) %}
+void {{attribute.name}}CacheInitCallback(v8::Isolate* isolate, v8::Local<v8::Object> holder)
+{
+    auto prop = V8PrivateProperty::Get{{attribute.cached_accessor_name}}(isolate);

[Yuki, 2017/05/09 06:06:31]

We may want to use V8PrivateProperty::GetSymbol(isolate,
"{{interface_name}}#{{attribute.cached_accessor_name}}").  This is slower than
Get{{attribute.cached_accessor_name}}(), but CacheInitCallback should be rarely
called and it should be okay, I think.

+    {{cpp_class}}* impl = {{v8_class}}::toImpl(holder);
+    prop.Set(holder, ToV8({{attribute.cpp_value}}, holder, isolate));
+    //LOG(ERROR) << "{{attribute.name}}CacheInitCallback";

[Yuki, 2017/05/09 06:06:31]

nit: Remove this debugging code.

[nverne, 2017/05/10 02:11:49]

Done

+}
+{% endmacro %}
 
 {##############################################################################}
 {% macro constructor_getter_callback(attribute, world_suffix) %}
 void {{v8_class_or_partial}}::{{attribute.name}}ConstructorGetterCallback{{world_suffix}}(v8::Local<v8::Name> property, const v8::PropertyCallbackInfo<v8::Value>& info) {
   {% if attribute.deprecate_as %}
   Deprecation::CountDeprecation(CurrentExecutionContext(info.GetIsolate()), UseCounter::k{{attribute.deprecate_as}});
   {% endif %}
 
   {% if attribute.measure_as %}
   UseCounter::Count(CurrentExecutionContext(info.GetIsolate()), UseCounter::k{{attribute.measure_as('ConstructorGetter')}});
@@ skipping 131 matching lines @@
   {% endif %}
   {{attribute.cpp_setter}};
 
   {% if attribute.cached_attribute_validation_method %}
   // [CachedAttribute]
   // Invalidate the cached value.
   V8PrivateProperty::GetSymbol(
       isolate, "{{cpp_class}}#{{attribute.name.capitalize()}}")
       .DeleteProperty(holder, v8::Undefined(isolate));
   {% endif %}
+
+  {% if attribute.is_lazy_cached_accessor %}
+  // Call cache init callback to store the correct private property.
+  {{attribute.name}}CacheInitCallback(info.GetIsolate(), holder);
+  {% endif %}
+

[Yuki, 2017/05/09 06:06:31]

nit: Remove this empty line.

[nverne, 2017/05/10 02:11:49]

Done

 }
 {% endfilter %}{# format_remove_duplicates #}
 {% endmacro %}
 
 
 {##############################################################################}
 {% macro attribute_setter_callback(attribute, world_suffix) %}
 void {{v8_class_or_partial}}::{{attribute.name}}AttributeSetterCallback{{world_suffix}}(
 {%- if attribute.is_data_type_property %}
 v8::Local<v8::Name>, v8::Local<v8::Value> v8Value, const v8::PropertyCallbackInfo<void>& info
@@ skipping 41 matching lines @@
   {% elif attribute.has_cross_origin_setter %}
   {{cpp_class_or_partial}}V8Internal::{{attribute.name}}AttributeSetter(v8Value, V8CrossOriginSetterInfo(info.GetIsolate(), info.Holder()));
   {% else %}
   {{cpp_class_or_partial}}V8Internal::{{attribute.name}}AttributeSetter{{world_suffix}}(v8Value, info);
   {% endif %}
 }
 {% endmacro %}
 
 
 {##############################################################################}
-{% macro attribute_configuration(attribute) %}
+{% macro attribute_accessor_configuration(attribute, is_attribute) %}

[Yuki, 2017/05/09 06:06:30]

nit: s/is_attribute/is_data_property/

IDL attributes are defined to be ES accessor properties in Web IDL spec, but
Blink is using ES data properties for some limited cases.

"attribute" shouldn't mean "data property", so let's call it "data property".

[nverne, 2017/05/10 02:11:49]

This macro builds either an AccessorConfiguration or an AttributeConfiguration
from a python attribute object.  The meaning we want is now captured in the two
macros attribute_configuration() and accessor_configuration() calling this one.
Previously we only had the former macro and it forced us to include the same
fields in the underlying C++ structs, but this change allows them to safely
diverge. 

How about if I rename the macro build_configuration_from_attribute() and have
the change the second parameter to a string specifying the type of config?

 {% from 'utilities.cpp.tmpl' import property_location %}
 {% if attribute.constructor_type %}
   {% if attribute.needs_constructor_getter_callback %}
     {% set getter_callback = '%s::%sConstructorGetterCallback' % (v8_class_or_partial, attribute.name) %}
   {% else %}
     {% set getter_callback = 'V8%s::NamedConstructorAttributeGetter' % (attribute.constructor_type)
        if attribute.is_named_constructor else 'V8ConstructorAttributeGetter' %}
   {% endif %}
 {% set setter_callback = 'nullptr' %}
 {% else %}{# regular attributes #}
 {% set getter_callback = '%s::%sAttributeGetterCallback' %
            (v8_class_or_partial, attribute.name) %}
 {% set setter_callback = '%s::%sAttributeSetterCallback' %
            (v8_class_or_partial, attribute.name)
            if attribute.has_setter else 'nullptr' %}
 {% endif %}
 {% set wrapper_type_info =
        'const_cast<WrapperTypeInfo*>(&V8%s::wrapperTypeInfo)' % attribute.constructor_type
        if attribute.constructor_type else 'nullptr' %}
 {% set property_attribute = 'static_cast<v8::PropertyAttribute>(%s)' %
                             ' | '.join(attribute.property_attributes) %}
-{% set cached_accessor_callback =
+{% set cached_property_key =
        '%s::%sCachedPropertyKey' % (v8_class_or_partial, attribute.name)
        if attribute.is_cached_accessor else
        'nullptr' %}
+{% set cache_init_callback = 
+      '%sV8Internal::%sCacheInitCallback' % 
+          (cpp_class_or_partial, attribute.name)
+       if attribute.is_lazy_cached_accessor else
+       'nullptr' %}
 {% set holder_check = 'V8DOMConfiguration::kDoNotCheckHolder'
        if attribute.is_lenient_this else 'V8DOMConfiguration::kCheckHolder' %}
 {% if attribute.is_per_world_bindings %}
 {% set getter_callback_for_main_world = '%sForMainWorld' % getter_callback %}
 {% set setter_callback_for_main_world =
        '%sForMainWorld' % setter_callback
        if attribute.has_setter else 'nullptr' %}
-{"{{attribute.name}}", {{getter_callback_for_main_world}}, {{setter_callback_for_main_world}}, {{cached_accessor_callback}}, {{wrapper_type_info}}, {{property_attribute}}, {{property_location(attribute)}}, {{holder_check}}, V8DOMConfiguration::kMainWorld},
-{"{{attribute.name}}", {{getter_callback}}, {{setter_callback}}, {{cached_accessor_callback}}, {{wrapper_type_info}}, {{property_attribute}}, {{property_location(attribute)}}, {{holder_check}}, V8DOMConfiguration::kNonMainWorlds}
-{%- else %}
-{"{{attribute.name}}", {{getter_callback}}, {{setter_callback}}, {{cached_accessor_callback}}, {{wrapper_type_info}}, {{property_attribute}}, {{property_location(attribute)}}, {{holder_check}}, V8DOMConfiguration::kAllWorlds}
-{%- endif %}
-{%- endmacro %}
+{% endif %}
+
+
+{% set config_pre = {

[Yuki, 2017/05/09 06:06:31]

nit: I'd prefer naming this config_world_dependent or world_dependent_config.

+  "main" : [
+         '"%s"' % attribute.name,
+         getter_callback_for_main_world,
+         setter_callback_for_main_world,
+       ],
+  "other" : [       

[Yuki, 2017/05/09 06:06:31]

nit: "non-main" or something like that?
"non-main" is more specific and clearer than "other", I think.

+         '"%s"' % attribute.name,
+         getter_callback,
+         setter_callback,
+       ],
+} %}
+
+{% set accessor_only_fields = [] if is_attribute else [       

[Yuki, 2017/05/09 06:06:31]

nit: Can we fold this into config_post and rename it to
config_world_independent?

+      cached_property_key,
+      cache_init_callback,
+] %}
+
+{% set config_post = [       
+       wrapper_type_info,
+       property_attribute,
+       property_location(attribute),
+       holder_check,
+] %}
+
+{% if attribute.is_per_world_bindings %}

[Yuki, 2017/05/09 06:06:31]

Great improvement!  Can we go farther?

I'm wondering if we can define a constexpr function that takes world-dependent
config first and world-independent config last (or vice-versa) and return the
struct that we want, in order to make this code cleaner.

I vaguely think whether we can do something like the following.

  constexpr V8DOMConfiguration::AccessorConfiguration
  V8DOMConfiguration::BuildAccessorConfigration(...);

  V8DOMConfiguration::AccessorConfiguration accessor_config[] = {
    V8DOMConfiguration::BuildAccessorConfiguration(
      *{{world_dependent_config}}, *{{world_independent_config}}),
    ...,
  };

+  {% set main_config_list = config_pre["main"] + accessor_only_fields + 
+      config_post + ['V8DOMConfiguration::kMainWorld'] %}
+  {% set non_main_config_list = config_pre["other"] + accessor_only_fields + 
+      config_post + ['V8DOMConfiguration::kNonMainWorlds'] %}
+  {# Emit for main world then non-main.#}
+  {{'{'}}{{main_config_list | join(', ')}}{{'}'}}
+  {{','}}
+  {{'{'}}{{non_main_config_list | join(', ')}}{{'}'}}
+{% else %}
+  {% set all_worlds_config_list = config_pre["other"] + accessor_only_fields + 
+      config_post + ['V8DOMConfiguration::kAllWorlds'] %}
+  {# Emit only for all worlds #}
+  {{'{'}}{{all_worlds_config_list | join(', ')}}{{'}'}}
+{% endif %}
+{% endmacro %}
+
+{##############################################################################}
+{% macro attribute_configuration(attribute) %}
+{% set is_attribute = true %}

[Yuki, 2017/05/09 06:06:31]

nit: s/is_attribute/is_data_property/

+{{attribute_accessor_configuration(attribute, is_attribute)}}
+{% endmacro %}
+
+{##############################################################################}
+{% macro accessor_configuration(attribute) %}
+{% set is_attribute = false %}

[Yuki, 2017/05/09 06:06:31]

ditto

+{{attribute_accessor_configuration(attribute, is_attribute)}}
+{% endmacro %}
 
 {##############################################################################}
 {% macro install_conditionally_enabled_attributes_on_prototype() %}
 {% for attribute in attributes if (attribute.exposed_test or attribute.secure_context_test) and attribute.on_prototype %}
 {% filter exposed(attribute.exposed_test) %}
 {% filter secure_context(attribute.secure_context_test) %}
 {% filter runtime_enabled(attribute.runtime_enabled_feature_name) %}
 static const V8DOMConfiguration::AccessorConfiguration accessorConfiguration[] = {
-  {{attribute_configuration(attribute)}}
+  {{accessor_configuration(attribute)}}
 };
 for (const auto& accessorConfig : accessorConfiguration)
   V8DOMConfiguration::InstallAccessor(isolate, world, v8::Local<v8::Object>(), prototypeObject, interfaceObject, signature, accessorConfig);
 {% endfilter %}{# runtime_enabled #}
 {% endfilter %}{# secure_context #}
 {% endfilter %}{# exposed #}
 {% endfor %}
 {% endmacro %}