Support combination of [OriginTrialEnabled] and [SecureContext]

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

Index: third_party/WebKit/Source/bindings/scripts/v8_interface.py
diff --git a/third_party/WebKit/Source/bindings/scripts/v8_interface.py b/third_party/WebKit/Source/bindings/scripts/v8_interface.py
index 48e93125de99a3db581f0d989e63d9bc583f50cf..e4c301abddd6b7d390f95b06a5399447feabe443 100644
--- a/third_party/WebKit/Source/bindings/scripts/v8_interface.py
+++ b/third_party/WebKit/Source/bindings/scripts/v8_interface.py
@@ -121,7 +121,6 @@ def origin_trial_features(interface, constants, attributes, methods):
     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])
@@ -135,7 +134,10 @@ def origin_trial_features(interface, constants, attributes, methods):
                 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')
@@ -143,6 +145,32 @@ def origin_trial_features(interface, constants, attributes, methods):
     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.
 
@@ -351,6 +379,7 @@ def interface_context(interface, interfaces):
 
     # Attributes
     attributes = attributes_context(interface, interfaces)
+
     context.update({
         'attributes': attributes,
         # Elements in attributes are broken in following members.
@@ -361,6 +390,16 @@ def interface_context(interface, interfaces):
         '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({
@@ -370,6 +409,16 @@ def interface_context(interface, interfaces):
         '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
@@ -383,17 +432,9 @@ def interface_context(interface, interfaces):
     })
 
     # 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({