[OriginTrials] Allow origin trials to be installed without an instance object.

third_party/WebKit/Source/bindings/templates/interface_base.cpp

Index: third_party/WebKit/Source/bindings/templates/interface_base.cpp
diff --git a/third_party/WebKit/Source/bindings/templates/interface_base.cpp b/third_party/WebKit/Source/bindings/templates/interface_base.cpp
index a04553663d6cddc66cdfef4faa90baef0f9c9350..a3cab8732a0b4e82f292b33688fce66777493fd9 100644
--- a/third_party/WebKit/Source/bindings/templates/interface_base.cpp
+++ b/third_party/WebKit/Source/bindings/templates/interface_base.cpp
@@ -368,25 +368,43 @@ static void install{{v8_class}}Template(v8::Isolate* isolate, const DOMWrapperWo
 {% from 'attributes.cpp' import attribute_configuration with context %}
 {% from 'constants.cpp' import constant_configuration with context %}
 {% from 'methods.cpp' import method_configuration with context %}
-{% for origin_trial_feature_name in origin_trial_feature_names %}{{newline}}
-void {{v8_class_or_partial}}::install{{origin_trial_feature_name}}(ScriptState* scriptState, v8::Local<v8::Object> instance)
+{% for origin_trial_feature in origin_trial_features %}{{newline}}
+{% if origin_trial_feature.needs_instance %}

[Yuki, 2016/07/08 13:08:24]

I'm uneasy with this pattern.  The current pattern seems
    {% if %}
    void foo() {
        do_something();
    {% else %}
    void foo(T) {
        do_something_else();
    {% endif %}
        do_common_thing();
    }
I don't like this pattern because open and close parentheses do not match and
hard to follow the code.

I think this part is not performance-sensitive, so I'd recommend the following
pattern.
    void foo(scriptState, instance) {
        do_common_things();
        if (instance.IsEmpty()) {
            do_something();
        } else {
            do_something_else();
        }
    }
and, if you want, we can optionally have the following thin wrapper.
    void foo(scriptState) {
        foo(scriptState, v8::Local<v8::Object>());
    }

Maybe, something will be redundant, but I don't care much.  I think this way is
easier to read the code.

[iclelland, 2016/07/08 14:20:49]

That's a fair criticism; I wasn't completely sure myself, but it seemed to
generate the cleanest code while minimizing duplication within the template.
sgtm; I'll rewrite it with that pattern. We can revisit if it turns out to be
performance-critical at some point.

[iclelland, 2016/07/08 14:48:25]

Done.

+void {{v8_class_or_partial}}::install{{origin_trial_feature.name}}(ScriptState* scriptState, v8::Local<v8::Object> instance)
 {
     v8::Local<v8::Object> prototype = instance->GetPrototype()->ToObject(scriptState->isolate());
 
-    {# Origin-Trial-enabled attributes #}
-    {% if attributes | for_origin_trial_feature(origin_trial_feature_name) or
-          methods | method_for_origin_trial_feature(origin_trial_feature_name, is_partial) %}
+    {% if attributes | for_origin_trial_feature(origin_trial_feature.name) or
+          methods | method_for_origin_trial_feature(origin_trial_feature.name, is_partial) %}
     V8PerIsolateData* perIsolateData = V8PerIsolateData::from(scriptState->isolate());
     v8::Local<v8::FunctionTemplate> interfaceTemplate = perIsolateData->findInterfaceTemplate(scriptState->world(), &{{v8_class}}::wrapperTypeInfo);
     v8::Local<v8::Signature> signature = v8::Signature::New(scriptState->isolate(), interfaceTemplate);
     ALLOW_UNUSED_LOCAL(signature);
     {% endif %}
-    {% if constants | for_origin_trial_feature(origin_trial_feature_name) or
-          methods | method_for_origin_trial_feature(origin_trial_feature_name, is_partial) %}
+    {% if constants | for_origin_trial_feature(origin_trial_feature.name) or
+          methods | method_for_origin_trial_feature(origin_trial_feature.name, is_partial) %}
     V8PerContextData* perContextData = V8PerContextData::from(scriptState->context());
     v8::Local<v8::Function> interface = perContextData->constructorForType(&{{v8_class}}::wrapperTypeInfo);
     {% endif %}
-    {% for attribute in attributes | for_origin_trial_feature(origin_trial_feature_name) | unique_by('name') | sort %}
+{% else %}
+void {{v8_class_or_partial}}::install{{origin_trial_feature.name}}(ScriptState* scriptState)
+{
+    {% if attributes | for_origin_trial_feature(origin_trial_feature.name) or
+          methods | method_for_origin_trial_feature(origin_trial_feature.name, is_partial) %}
+    v8::Local<v8::FunctionTemplate> interfaceTemplate = {{v8_class}}::wrapperTypeInfo.domTemplate(scriptState->isolate(), scriptState->world());

[Yuki, 2016/07/08 13:08:24]

Just FYI,

If the interface is already defined, you can retrieve it using
perIsolateData->findInterfaceTemplate().

Regardless of whether the interface is already defined or not yet defined, you
can retrieve it using {{v8_class}}::domTemplate().  You can use domTemplate()
for both cases that you have |instance| and you do not have |instance|.

[iclelland, 2016/07/08 14:20:49]

Yes. There are cases where that isn't possible, though -- I believe I used that
originally, and found it didn't work when I tried to retrieve the
OriginTrialsTest interface (this was when testing the static-attributes-cl).
Thanks for the pointer, though.

 
I will switch to use that in both cases, to combine more code.

[iclelland, 2016/07/08 14:48:25]

Done.

+    v8::Local<v8::Signature> signature = v8::Signature::New(scriptState->isolate(), interfaceTemplate);
+    ALLOW_UNUSED_LOCAL(signature);
+    {% endif %}
+    V8PerContextData* perContextData = V8PerContextData::from(scriptState->context());
+    {% if constants | for_origin_trial_feature(origin_trial_feature.name) or
+          methods | method_for_origin_trial_feature(origin_trial_feature.name, is_partial) %}
+    v8::Local<v8::Function> interface = perContextData->constructorForType(&{{v8_class}}::wrapperTypeInfo);
+    {% endif %}
+    v8::Local<v8::Object> prototype = perContextData->prototypeForType(&{{v8_class}}::wrapperTypeInfo);
+    v8::Local<v8::Object> instance; // unused; all members are OnInterface or OnPrototype
+{% endif %}
+    {# Origin-Trial-enabled attributes #}
+    {% for attribute in attributes | for_origin_trial_feature(origin_trial_feature.name) | unique_by('name') | sort %}
     {% if attribute.is_data_type_property %}
     const V8DOMConfiguration::AttributeConfiguration attribute{{attribute.name}}Configuration = \
         {{attribute_configuration(attribute)}};
@@ -398,13 +416,13 @@ void {{v8_class_or_partial}}::install{{origin_trial_feature_name}}(ScriptState*
     {% endif %}
     {% endfor %}
     {# Origin-Trial-enabled constants #}
-    {% for constant in constants | for_origin_trial_feature(origin_trial_feature_name) | unique_by('name') | sort %}
+    {% for constant in constants | for_origin_trial_feature(origin_trial_feature.name) | unique_by('name') | sort %}
     {% set constant_name = constant.name.title().replace('_', '') %}
     const V8DOMConfiguration::ConstantConfiguration constant{{constant_name}}Configuration = {{constant_configuration(constant)}};
     V8DOMConfiguration::installConstant(scriptState->isolate(), interface, prototype, constant{{constant_name}}Configuration);
     {% endfor %}
     {# Origin-Trial-enabled methods (no overloads) #}
-    {% for method in methods | method_for_origin_trial_feature(origin_trial_feature_name, is_partial) | unique_by('name') | sort %}
+    {% for method in methods | method_for_origin_trial_feature(origin_trial_feature.name, is_partial) | unique_by('name') | sort %}
     {% set method_name = method.name.title().replace('_', '') %}
     const V8DOMConfiguration::MethodConfiguration method{{method_name}}Configuration = {{method_configuration(method)}};
     V8DOMConfiguration::installMethod(scriptState->isolate(), scriptState->world(), instance, prototype, interface, signature, method{{method_name}}Configuration);