Minor refactor to origin trials bindings

The bindings for origin trials had two utility functions to detect the
presence of the [OriginTrialEnabled] attribute on a definition. Both of
these functions were being used semi-interchangeably to check for origin
trials.

This CL merges the two utility functions to simplify the logic.

Also, there was some basic support for a [FeaturePolicy] attribute in
the utility function. This logic was removed, as the [FeaturePolicy]
was never used, and is not intended to be used in the future.

Bug: 788196
Change-Id: I2eda8e25147d30e251a1d9d076abe9a4f4b29c8a
Reviewed-on: https://chromium-review.googlesource.com/802015
Commit-Queue: Jason Chase <chasej@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520851}

third_party/WebKit/Source/bindings/scripts/code_generator.py

@@ -68,12 +68,12 @@ def secure_context_if(code, secure_context_test, test_result=None):
 
 
 # [OriginTrialEnabled]
-def origin_trial_enabled_if(code, origin_trial_function_name, execution_context=None):
-    if not origin_trial_function_name:
+def origin_trial_enabled_if(code, origin_trial_feature_name, execution_context=None):
+    if not origin_trial_feature_name:
         return code
 
     function = v8_utilities.origin_trial_function_call(
-        origin_trial_function_name, execution_context)
+        origin_trial_feature_name, execution_context)
     return generate_indented_conditional(code, function)
 
 

third_party/WebKit/Source/bindings/scripts/v8_attributes.py

@@ -172,7 +172,6 @@ def attribute_context(interface, attribute, interfaces):
         'on_instance': v8_utilities.on_instance(interface, attribute),
         'on_interface': v8_utilities.on_interface(interface, attribute),
         'on_prototype': v8_utilities.on_prototype(interface, attribute),
-        'origin_trial_enabled_function': v8_utilities.origin_trial_enabled_function_name(attribute),  # [OriginTrialEnabled]
         'origin_trial_feature_name': v8_utilities.origin_trial_feature_name(attribute),  # [OriginTrialEnabled]
         'use_output_parameter_for_result': idl_type.use_output_parameter_for_result,
         'measure_as': v8_utilities.measure_as(attribute, interface),  # [MeasureAs]
@@ -227,6 +226,10 @@ def runtime_call_stats_context(interface, attribute, context):
     })
 
 
+def is_origin_trial_enabled(attribute):
+    return bool(attribute['origin_trial_feature_name'])
+
+
 def is_secure_context(attribute):
     return bool(attribute['secure_context_test'])
 
@@ -236,7 +239,7 @@ def filter_accessors(attributes):
             not (attribute['exposed_test'] or
                  is_secure_context(attribute) or
                  attribute['context_enabled_feature_name'] or
-                 attribute['origin_trial_enabled_function'] or
+                 is_origin_trial_enabled(attribute) or
                  attribute['runtime_enabled_feature_name']) and
             not attribute['is_data_type_property']]
 
@@ -245,7 +248,7 @@ def is_data_attribute(attribute):
     return (not (attribute['exposed_test'] or
                  is_secure_context(attribute) or
                  attribute['context_enabled_feature_name'] or
-                 attribute['origin_trial_enabled_function'] or
+                 is_origin_trial_enabled(attribute) or
                  attribute['runtime_enabled_feature_name']) and
             attribute['is_data_type_property'])
 
@@ -277,7 +280,7 @@ def filter_conditionally_enabled(attributes):
     return [attribute for attribute in attributes if
             attribute['exposed_test'] or
             (is_secure_context(attribute) and
-             not attribute['origin_trial_feature_name'])]
+             not is_origin_trial_enabled(attribute))]
 
 
 ################################################################################

third_party/WebKit/Source/bindings/scripts/v8_dictionary.py

@@ -76,7 +76,7 @@ def dictionary_context(dictionary, interfaces_info):
 
     has_origin_trial_members = False
     for member in members:
-        if member['origin_trial_enabled_function']:
+        if member['origin_trial_feature_name']:
             has_origin_trial_members = True
             includes.add('core/origin_trials/origin_trials.h')
             break
@@ -151,7 +151,7 @@ def default_values():
         'is_object': unwrapped_idl_type.name == 'Object' or is_deprecated_dictionary,
         'is_required': member.is_required,
         'name': member.name,
-        'origin_trial_enabled_function': v8_utilities.origin_trial_enabled_function_name(member),  # [OriginTrialEnabled]
+        'origin_trial_feature_name': v8_utilities.origin_trial_feature_name(member),  # [OriginTrialEnabled]
         'runtime_enabled_feature_name': v8_utilities.runtime_enabled_feature_name(member),  # [RuntimeEnabled]
         'setter_name': setter_name_for_dictionary_member(member),
         'null_setter_name': null_setter_name_for_dictionary_member(member),

third_party/WebKit/Source/bindings/scripts/v8_interface.py

@@ -292,7 +292,7 @@ def interface_context(interface, interfaces):
         'is_typed_array_type': is_typed_array_type,
         'measure_as': v8_utilities.measure_as(interface, None),  # [MeasureAs]
         'needs_runtime_enabled_installer': needs_runtime_enabled_installer,
-        'origin_trial_enabled_function': v8_utilities.origin_trial_enabled_function_name(interface),
+        'origin_trial_feature_name': v8_utilities.origin_trial_feature_name(interface),
         'parent_interface': parent_interface,
         'pass_cpp_type': cpp_name(interface) + '*',
         'runtime_call_stats': runtime_call_stats_context(interface),
@@ -827,7 +827,6 @@ def constant_context(constant, interface):
         'idl_type': constant.idl_type.name,
         'measure_as': v8_utilities.measure_as(constant, interface),  # [MeasureAs]
         'name': constant.name,
-        'origin_trial_enabled_function': v8_utilities.origin_trial_enabled_function_name(constant),  # [OriginTrialEnabled]
         'origin_trial_feature_name': v8_utilities.origin_trial_feature_name(constant),  # [OriginTrialEnabled]
         # FIXME: use 'reflected_name' as correct 'name'
         'rcs_counter': 'Blink_' + v8_utilities.cpp_name(interface) + '_' + constant.name + '_ConstantGetter',

third_party/WebKit/Source/bindings/scripts/v8_methods.py

@@ -230,7 +230,6 @@ def method_context(interface, method, is_visible=True):
         'on_instance': v8_utilities.on_instance(interface, method),
         'on_interface': v8_utilities.on_interface(interface, method),
         'on_prototype': v8_utilities.on_prototype(interface, method),
-        'origin_trial_enabled_function': v8_utilities.origin_trial_enabled_function_name(method),  # [OriginTrialEnabled]
         'origin_trial_feature_name': v8_utilities.origin_trial_feature_name(method),  # [OriginTrialEnabled]
         'property_attributes': property_attributes(interface, method),
         'returns_promise': method.returns_promise,

third_party/WebKit/Source/bindings/scripts/v8_utilities.py

@@ -394,64 +394,31 @@ def measure_as(definition_or_member, interface):
 
 
 # [OriginTrialEnabled]
-def origin_trial_enabled_function_name(definition_or_member):
-    """Returns the name of the OriginTrials enabled function.
+def origin_trial_feature_name(definition_or_member):
+    """Returns the name of the feature for the OriginTrialEnabled attribute.
 
     An exception is raised if OriginTrialEnabled is used in conjunction with any
     of the following (which must be mutually exclusive with origin trials):
       - RuntimeEnabled
 
-    The returned function checks if the IDL member should be enabled.
-    Given extended attribute OriginTrialEnabled=FeatureName, return:
-        OriginTrials::{featureName}Enabled
-
     If the OriginTrialEnabled extended attribute is found, the includes are
     also updated as a side-effect.
     """
     extended_attributes = definition_or_member.extended_attributes
-    is_origin_trial_enabled = 'OriginTrialEnabled' in extended_attributes
+    feature_name = extended_attributes.get('OriginTrialEnabled')
 
-    if is_origin_trial_enabled and 'RuntimeEnabled' in extended_attributes:
+    if feature_name and 'RuntimeEnabled' in extended_attributes:
         raise Exception('[OriginTrialEnabled] and [RuntimeEnabled] must '
                         'not be specified on the same definition: %s'
                         % definition_or_member.name)
 
-    if is_origin_trial_enabled:
-        trial_name = extended_attributes['OriginTrialEnabled']
-        return 'OriginTrials::%sEnabled' % uncapitalize(trial_name)
-
-    is_feature_policy_enabled = 'FeaturePolicy' in extended_attributes
-
-    if is_feature_policy_enabled and 'RuntimeEnabled' in extended_attributes:
-        raise Exception('[FeaturePolicy] and [RuntimeEnabled] must '
-                        'not be specified on the same definition: %s'
-                        % definition_or_member.name)
-
-    if is_feature_policy_enabled and 'SecureContext' in extended_attributes:
-        raise Exception('[FeaturePolicy] and [SecureContext] must '
-                        'not be specified on the same definition '
-                        '(see https://crbug.com/695123 for workaround): %s'
-                        % definition_or_member.name)
-
-    if is_feature_policy_enabled:
-        includes.add('platform/bindings/ScriptState.h')
-        includes.add('platform/feature_policy/FeaturePolicy.h')
-
-        trial_name = extended_attributes['FeaturePolicy']
-        return 'FeaturePolicy::%sEnabled' % uncapitalize(trial_name)
-
-    return None
-
-
-def origin_trial_feature_name(definition_or_member):
-    extended_attributes = definition_or_member.extended_attributes
-    return extended_attributes.get('OriginTrialEnabled') or extended_attributes.get('FeaturePolicy')
+    return feature_name
 
 
-def origin_trial_function_call(function_name, execution_context=None):
+def origin_trial_function_call(feature_name, execution_context=None):
     """Returns a function call to determine if an origin trial is enabled."""
-    return '{function}({context})'.format(
-        function=function_name,
+    return 'OriginTrials::{feature_name}Enabled({context})'.format(
+        feature_name=uncapitalize(feature_name),
         context=execution_context if execution_context else "execution_context")
 
 

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

@@ -58,7 +58,7 @@ void {{v8_class}}::ToImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value, {{
   DCHECK(executionContext);
   {% endif %}{# has_origin_trial_members #}
   {% endif %}{# members #}
-  {% for origin_trial_test, origin_trial_member_list in members | groupby('origin_trial_enabled_function') %}
+  {% for origin_trial_test, origin_trial_member_list in members | groupby('origin_trial_feature_name') %}
   {% filter origin_trial_enabled(origin_trial_test, "executionContext") %}
   {% for feature_name, member_list in origin_trial_member_list | groupby('runtime_enabled_feature_name') %}
   {% filter runtime_enabled(feature_name) %}
@@ -133,7 +133,7 @@ bool toV8{{cpp_class}}(const {{cpp_class}}& impl, v8::Local<v8::Object> dictiona
   DCHECK(executionContext);
   {% endif %}{# has_origin_trial_members #}
   {% endif %}{# members #}
-  {% for origin_trial_test, origin_trial_member_list in members | groupby('origin_trial_enabled_function') %}
+  {% for origin_trial_test, origin_trial_member_list in members | groupby('origin_trial_feature_name') %}
   {% filter origin_trial_enabled(origin_trial_test, "executionContext") %}
   {% for feature_name, member_list in origin_trial_member_list | groupby('runtime_enabled_feature_name') %}
   {% filter runtime_enabled(feature_name) %}

third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp

@@ -744,35 +744,35 @@ void V8TestDictionary::ToImpl(v8::Isolate* isolate, v8::Local<v8::Value> v8Value
     }
   }
 
-  if (OriginTrials::featureName1Enabled(executionContext)) {
-    v8::Local<v8::Value> originTrialSecondMemberValue;
-    if (!v8Object->Get(context, keys[21].Get(isolate)).ToLocal(&originTrialSecondMemberValue)) {
+  if (OriginTrials::featureNameEnabled(executionContext)) {
+    v8::Local<v8::Value> originTrialMemberValue;
+    if (!v8Object->Get(context, keys[20].Get(isolate)).ToLocal(&originTrialMemberValue)) {
       exceptionState.RethrowV8Exception(block.Exception());
       return;
     }
-    if (originTrialSecondMemberValue.IsEmpty() || originTrialSecondMemberValue->IsUndefined()) {
+    if (originTrialMemberValue.IsEmpty() || originTrialMemberValue->IsUndefined()) {
       // Do nothing.
     } else {
-      bool originTrialSecondMemberCppValue = NativeValueTraits<IDLBoolean>::NativeValue(isolate, originTrialSecondMemberValue, exceptionState);
+      bool originTrialMemberCppValue = NativeValueTraits<IDLBoolean>::NativeValue(isolate, originTrialMemberValue, exceptionState);
       if (exceptionState.HadException())
         return;
-      impl.setOriginTrialSecondMember(originTrialSecondMemberCppValue);
+      impl.setOriginTrialMember(originTrialMemberCppValue);
     }
   }
 
-  if (OriginTrials::featureNameEnabled(executionContext)) {
-    v8::Local<v8::Value> originTrialMemberValue;
-    if (!v8Object->Get(context, keys[20].Get(isolate)).ToLocal(&originTrialMemberValue)) {
+  if (OriginTrials::featureName1Enabled(executionContext)) {
+    v8::Local<v8::Value> originTrialSecondMemberValue;
+    if (!v8Object->Get(context, keys[21].Get(isolate)).ToLocal(&originTrialSecondMemberValue)) {
       exceptionState.RethrowV8Exception(block.Exception());
       return;
     }
-    if (originTrialMemberValue.IsEmpty() || originTrialMemberValue->IsUndefined()) {
+    if (originTrialSecondMemberValue.IsEmpty() || originTrialSecondMemberValue->IsUndefined()) {
       // Do nothing.
     } else {
-      bool originTrialMemberCppValue = NativeValueTraits<IDLBoolean>::NativeValue(isolate, originTrialMemberValue, exceptionState);
+      bool originTrialSecondMemberCppValue = NativeValueTraits<IDLBoolean>::NativeValue(isolate, originTrialSecondMemberValue, exceptionState);
       if (exceptionState.HadException())
         return;
-      impl.setOriginTrialMember(originTrialMemberCppValue);
+      impl.setOriginTrialSecondMember(originTrialSecondMemberCppValue);
     }
   }
 }
@@ -1307,19 +1307,6 @@ bool toV8TestDictionary(const TestDictionary& impl, v8::Local<v8::Object> dictio
     }
   }
 
-  if (OriginTrials::featureName1Enabled(executionContext)) {
-    v8::Local<v8::Value> originTrialSecondMemberValue;
-    bool originTrialSecondMemberHasValueOrDefault = false;
-    if (impl.hasOriginTrialSecondMember()) {
-      originTrialSecondMemberValue = v8::Boolean::New(isolate, impl.originTrialSecondMember());
-      originTrialSecondMemberHasValueOrDefault = true;
-    }
-    if (originTrialSecondMemberHasValueOrDefault &&
-        !V8CallBoolean(dictionary->CreateDataProperty(context, keys[21].Get(isolate), originTrialSecondMemberValue))) {
-      return false;
-    }
-  }
-
   if (OriginTrials::featureNameEnabled(executionContext)) {
     v8::Local<v8::Value> originTrialMemberValue;
     bool originTrialMemberHasValueOrDefault = false;
@@ -1333,6 +1320,19 @@ bool toV8TestDictionary(const TestDictionary& impl, v8::Local<v8::Object> dictio
     }
   }
 
+  if (OriginTrials::featureName1Enabled(executionContext)) {
+    v8::Local<v8::Value> originTrialSecondMemberValue;
+    bool originTrialSecondMemberHasValueOrDefault = false;
+    if (impl.hasOriginTrialSecondMember()) {
+      originTrialSecondMemberValue = v8::Boolean::New(isolate, impl.originTrialSecondMember());
+      originTrialSecondMemberHasValueOrDefault = true;
+    }
+    if (originTrialSecondMemberHasValueOrDefault &&
+        !V8CallBoolean(dictionary->CreateDataProperty(context, keys[21].Get(isolate), originTrialSecondMemberValue))) {
+      return false;
+    }
+  }
+
   return true;
 }