Support combination of [OriginTrialEnabled] and [SecureContext]

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

 # Copyright (C) 2013 Google Inc. All rights reserved.
 # coding=utf-8
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
@@ skipping 103 matching lines @@
         return sorted([member for member in members if member.get(KEY) and not member.get('exposed_test')])
 
     def member_filter_by_name(members, name):
         return [member for member in members if member[KEY] == name]
 
     # Collect all members visible on this interface with a defined origin trial
     origin_trial_constants = member_filter(constants)
     origin_trial_attributes = member_filter(attributes)
     origin_trial_methods = member_filter([method for method in methods
                                           if v8_methods.method_is_visible(method, interface.is_partial) and
-                                          not v8_methods.conditionally_exposed(method) and
                                           not v8_methods.custom_registration(method)])
 
     feature_names = set([member[KEY] for member in origin_trial_constants + origin_trial_attributes + origin_trial_methods])
 
     # Construct the list of dictionaries. 'needs_instance' will be true if any
     # member for the feature has 'on_instance' defined as true.
     features = [{'name': name,
                  'constants': member_filter_by_name(origin_trial_constants, name),
                  'attributes': member_filter_by_name(origin_trial_attributes, name),
                  'methods': member_filter_by_name(origin_trial_methods, name)}
                 for name in feature_names]
     for feature in features:
         members = feature['constants'] + feature['attributes'] + feature['methods']
-        feature['needs_instance'] = reduce(or_, (member.get('on_instance', False) for member in members))
+        feature['needs_instance'] = any(member.get('on_instance', False) for member in members)
+        # TODO(chasej): Need to handle method overloads? e.g.
+        # (method['overloads']['secure_context_test_all'] if 'overloads' in method else method['secure_context_test'])
+        feature['needs_secure_context'] = any(member.get('secure_context_test', False) for member in members)
 
     if features:
         includes.add('platform/bindings/ScriptState.h')
         includes.add('core/origin_trials/OriginTrials.h')
     return sorted(features)
 
 
+def conditionally_enabled_attributes(attributes):
+    """ Returns a list of attributes that are conditionally enabled on this
+     interface.
+
+    Attributes with both secure context and origin trial are excluded, as they
+    are handled separately under origin trial features.
+    """
+
+    return [attribute for attribute in attributes if
+            attribute['exposed_test'] or
+            (attribute['secure_context_test'] and
+             not attribute['origin_trial_feature_name'])]

[haraken, 2017/05/12 11:05:53]

Can we create a helper function like v8_attributes.filter_conditionally_exposed?

Then I'd prefer inlining conditionally_enabled_attributes to the caller site.

[chasej, 2017/05/16 02:21:39]

Done.

+
+
+def conditionally_enabled_methods(interface, methods):
+    """ Returns a list of methods that are conditionally enabled on this
+     interface.
+
+    Methods with origin trial are excluded, as they are handled separately under
+    origin trial features.
+    """
+
+    return [method for method in
+            v8_methods.filter_conditionally_exposed(methods, interface.is_partial) if
+            not method['origin_trial_feature_name']]
+

[Yuki, 2017/05/12 09:10:05]

nit: two empty lines

[chasej, 2017/05/16 02:21:39]

Done. Actually removed this function as well. The extra check for origin
trial was moved into the helper function in v8_methods.

 def interface_context(interface, interfaces):
     """Creates a Jinja template context for an interface.
 
     Args:
         interface: An interface to create the context for
         interfaces: A dict which maps an interface name to the definition
             which can be referred if needed
 
     Returns:
         A Jinja template context for |interface|
@@ skipping 188 matching lines @@
     })
 
     # Constants
     context.update({
         'constants': [constant_context(constant, interface) for constant in interface.constants],
         'do_not_check_constants': 'DoNotCheckConstants' in extended_attributes,
     })
 
     # Attributes
     attributes = attributes_context(interface, interfaces)
+
     context.update({
         'attributes': attributes,
         # Elements in attributes are broken in following members.
         'accessors': v8_attributes.filter_accessors(attributes),
         'data_attributes': v8_attributes.filter_data_attributes(attributes),
         'lazy_data_attributes': v8_attributes.filter_lazy_data_attributes(attributes),
         'origin_trial_attributes': v8_attributes.filter_origin_trial_enabled(attributes),
         'runtime_enabled_attributes': v8_attributes.filter_runtime_enabled(attributes),
     })
 
+    # Conditionally enabled attributes
+    conditional_enabled_attributes = conditionally_enabled_attributes(attributes)
+    has_conditional_attributes_on_prototype = any(  # pylint: disable=invalid-name
+        attribute['on_prototype'] for attribute in conditional_enabled_attributes)
+    context.update({
+        'has_conditional_attributes_on_prototype':
+            has_conditional_attributes_on_prototype,
+        'conditionally_enabled_attributes': conditional_enabled_attributes,
+    })
+
     # Methods
     methods, iterator_method = methods_context(interface)
     context.update({
         'has_origin_safe_method_setter': any(method['is_cross_origin'] and not method['is_unforgeable']
             for method in methods),
         'iterator_method': iterator_method,
         'methods': methods,
     })
 
+    # Conditionally enabled methods
+    conditional_enabled_methods = conditionally_enabled_methods(interface, methods)
+    has_conditional_methods = any(  # pylint: disable=invalid-name
+        method for method in conditional_enabled_methods)

[haraken, 2017/05/12 11:05:54]

Is this equivalent to len(conditional_enabled_methods)?

[Yuki, 2017/05/12 14:04:25]

Python evaluates [] (empty list) to False, so
    if (has_conditional_methods):
and
    if (conditionally_enabled_methods):
should yield the same result.  So, no need to define has_conditional_methods, I
think.

If we're going to define has_conditional_methods, then boolean type is
preferred, i.e. bool(x) is preferred to len(x).

[iclelland, 2017/05/12 15:11:49]

The template tags work the same way, so {% if conditionally_enabled_method %}
works as well as {% if has_conditional_methods %} in interface_base.cpp.tmpl

[chasej, 2017/05/16 02:21:39]

I've removed the has_conditional_methods variable entirely, both
here and in the templates.

+    context.update({
+        'has_conditional_methods':
+            has_conditional_methods,
+        'conditionally_enabled_methods': conditional_enabled_methods,
+    })
+
     # Window.idl in Blink has indexed properties, but the spec says Window
     # interface doesn't have indexed properties, instead the WindowProxy exotic
     # object has indexed properties.  Thus, Window interface must not support
     # iterators.
     has_array_iterator = (not interface.is_partial and
                           interface.has_indexed_elements and
                           interface.name != 'Window')
     context.update({
         'has_array_iterator': has_array_iterator,
         'iterable': interface.iterable,
     })
 
     # Conditionally enabled members
-    has_conditional_attributes_on_prototype = any(  # pylint: disable=invalid-name
-        (attribute['exposed_test'] or attribute['secure_context_test']) and attribute['on_prototype']
-        for attribute in attributes)
-    context.update({
-        'has_conditional_attributes_on_prototype':
-            has_conditional_attributes_on_prototype,
-    })
-
     prepare_prototype_and_interface_object_func = None  # pylint: disable=invalid-name
     if (unscopables or has_conditional_attributes_on_prototype or
-            v8_methods.filter_conditionally_exposed(methods, interface.is_partial)):
+            has_conditional_methods):
         prepare_prototype_and_interface_object_func = '%s::preparePrototypeAndInterfaceObject' % v8_class_name_or_partial  # pylint: disable=invalid-name
 
     context.update({
         'prepare_prototype_and_interface_object_func': prepare_prototype_and_interface_object_func,
     })
 
     context.update({
         'legacy_caller': legacy_caller(interface.legacy_caller, interface),
         'indexed_property_getter': property_getter(interface.indexed_property_getter, ['index']),
         'indexed_property_setter': property_setter(interface.indexed_property_setter, interface),
@@ skipping 1002 matching lines @@
     extended_attributes = deleter.extended_attributes
     is_call_with_script_state = v8_utilities.has_extended_attribute_value(deleter, 'CallWith', 'ScriptState')
     is_ce_reactions = 'CEReactions' in extended_attributes
     return {
         'is_call_with_script_state': is_call_with_script_state,
         'is_ce_reactions': is_ce_reactions,
         'is_custom': 'Custom' in extended_attributes,
         'is_raises_exception': 'RaisesException' in extended_attributes,
         'name': cpp_name(deleter),
     }