Support combination of [OriginTrialEnabled] and [SecureContext]

This CL fixes the bindings logic to allow the [OriginTrialEnabled] and
[SecureContext] to be used together on the same IDL members.

The bug was found when trying to use [SecureContext] for WebUSB.
The IDL definition for WebUSB is corrected to use [SecureContext]
as well.

Other cleanups in this CL:
- Remove unused check_origin_trial macro
- Remove obsolete attribute variable, and fix reference for generating include
- Apply some consistency to attribute and method helper functions

BUG=695123

Review-Url: https://codereview.chromium.org/2880713002
Cr-Commit-Position: refs/heads/master@{#472062}

third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origintrials.js

@@ -148,23 +148,18 @@ expect_always_bindings = (insecure_context, opt_description_suffix) => {
     }, 'Constant should exist on interface and return value, regardless of trial' + description_suffix);
 
   if (insecure_context) {
-    // TODO(crbug.com/695123): Uncomment test when fixed so [SecureContext] and
-    //   [OriginTrialEnabled] extended attributes work correctly together.
-    /*
     test(() => {
         expect_member_fails('secureUnconditionalAttribute');
       }, 'Secure attribute should not exist, regardless of trial' + description_suffix);
-    */
+
     test(() => {
         expect_member_fails('secureStaticUnconditionalAttribute');
       }, 'Secure static attribute should not exist, regardless of trial' + description_suffix);
-    // TODO(crbug.com/695123): Uncomment test when fixed so [SecureContext] and
-    //   [OriginTrialEnabled] extended attributes work correctly together.
-    /*
+
     test(() => {
         expect_member_fails('secureUnconditionalMethod');
       }, 'Secure method should not exist, regardless of trial' + description_suffix);
-    */
+
     test(() => {
         expect_member_fails('secureStaticUnconditionalMethod');
       }, 'Secure static method should not exist, regardless of trial' + description_suffix);

third_party/WebKit/Source/bindings/modules/v8/ConditionalFeaturesForModules.cpp

@@ -50,10 +50,7 @@ void InstallConditionalFeaturesForModules(
   v8::Isolate* isolate = script_state->GetIsolate();
   const DOMWrapperWorld& world = script_state->World();
   if (wrapper_type_info == &V8Navigator::wrapperTypeInfo) {
-    // Mimics the [SecureContext] extended attribute. Work-around for
-    // https://crbug.com/695123.
-    if (OriginTrials::installedAppEnabled(execution_context) &&
-        execution_context->IsSecureContext()) {
+    if (OriginTrials::installedAppEnabled(execution_context)) {
       V8NavigatorPartial::installInstalledApp(
           isolate, world, v8::Local<v8::Object>(), prototype_object,
           interface_object);
@@ -63,10 +60,7 @@ void InstallConditionalFeaturesForModules(
                                           v8::Local<v8::Object>(),
                                           prototype_object, interface_object);
     }
-    // Mimics the [SecureContext] extended attribute. Work-around for
-    // https://crbug.com/695123.
-    if (OriginTrials::webUSBEnabled(execution_context) &&
-        execution_context->IsSecureContext()) {
+    if (OriginTrials::webUSBEnabled(execution_context)) {
       V8NavigatorPartial::installWebUSB(isolate, world, v8::Local<v8::Object>(),
                                         prototype_object, interface_object);
     }

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

@@ -58,9 +58,11 @@ def exposed_if(code, exposed_test):
 
 
 # [SecureContext]
-def secure_context_if(code, secure_context_test):
+def secure_context_if(code, secure_context_test, test_result=None):
     if not secure_context_test:
         return code
+    if test_result:
+        return generate_indented_conditional(code, test_result)
     return generate_indented_conditional(code, 'executionContext && (%s)' % secure_context_test)
 
 
@@ -72,7 +74,6 @@ def runtime_enabled_if(code, name):
     function = v8_utilities.runtime_enabled_function(name)
     return generate_indented_conditional(code, function)
 
-
 def initialize_jinja_env(cache_dir):
     jinja_env = jinja2.Environment(
         loader=jinja2.FileSystemLoader(TEMPLATES_DIR),

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

@@ -241,19 +241,20 @@ def filter_lazy_data_attributes(attributes):
     return [attribute for attribute in attributes if is_data_attribute(attribute) and is_lazy_data_attribute(attribute)]
 
 
-def filter_origin_trial_enabled(attributes):
-    return [attribute for attribute in attributes if
-            attribute['origin_trial_feature_name'] and
-            not attribute['exposed_test']]
-
-
 def filter_runtime_enabled(attributes):
     return [attribute for attribute in attributes if
             not (attribute['exposed_test'] or
                  attribute['secure_context_test']) and
             attribute['runtime_enabled_feature_name']]
 
 
+def filter_conditionally_enabled(attributes):
+    return [attribute for attribute in attributes if
+            attribute['exposed_test'] or
+            (attribute['secure_context_test'] and
+             not attribute['origin_trial_feature_name'])]
+
+
 ################################################################################
 # Getter
 ################################################################################

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

@@ -121,7 +121,6 @@ def member_filter_by_name(members, name):
     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 member_filter_by_name(members, 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')
@@ -351,23 +353,34 @@ def interface_context(interface, interfaces):
 
     # 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 = v8_attributes.filter_conditionally_enabled(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': v8_methods.filter_conditionally_enabled(methods, interface.is_partial),
     })
 
     # Window.idl in Blink has indexed properties, but the spec says Window
@@ -383,17 +396,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)):
+            context['conditionally_enabled_methods']):
         prepare_prototype_and_interface_object_func = '%s::preparePrototypeAndInterfaceObject' % v8_class_name_or_partial  # pylint: disable=invalid-name
 
     context.update({

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

@@ -49,15 +49,21 @@ def method_is_visible(method, interface_is_partial):
     return method['visible'] and 'overload_index' not in method
 
 
-def conditionally_exposed(method):
+def is_origin_trial_enabled(method):
+    return bool(method['origin_trial_feature_name'])
+
+
+def is_conditionally_enabled(method):
     exposed = method['overloads']['exposed_test_all'] if 'overloads' in method else method['exposed_test']
     secure_context = method['overloads']['secure_context_test_all'] if 'overloads' in method else method['secure_context_test']
     return exposed or secure_context
 
 
-def filter_conditionally_exposed(methods, interface_is_partial):
+def filter_conditionally_enabled(methods, interface_is_partial):
     return [method for method in methods if (
-        method_is_visible(method, interface_is_partial) and conditionally_exposed(method))]
+        method_is_visible(method, interface_is_partial) and
+        is_conditionally_enabled(method) and
+        not is_origin_trial_enabled(method))]
 
 
 def custom_registration(method):
@@ -68,8 +74,8 @@ def custom_registration(method):
         return True
     if 'overloads' in method:
         return (method['overloads']['runtime_determined_lengths'] or
-                (method['overloads']['runtime_enabled_all'] and not conditionally_exposed(method)))
-    return method['runtime_enabled_feature_name'] and not conditionally_exposed(method)
+                (method['overloads']['runtime_enabled_all'] and not is_conditionally_enabled(method)))
+    return method['runtime_enabled_feature_name'] and not is_conditionally_enabled(method)
 
 
 def filter_custom_registration(methods, interface_is_partial):
@@ -80,14 +86,13 @@ def filter_custom_registration(methods, interface_is_partial):
 def filter_method_configuration(methods, interface_is_partial):
     return [method for method in methods if
             method_is_visible(method, interface_is_partial) and
-            not method['origin_trial_feature_name'] and
-            not conditionally_exposed(method) and
+            not is_origin_trial_enabled(method) and
+            not is_conditionally_enabled(method) and
             not custom_registration(method)]
 
 
 def method_filters():
-    return {'conditionally_exposed': filter_conditionally_exposed,
-            'custom_registration': filter_custom_registration,
+    return {'custom_registration': filter_custom_registration,
             'has_method_configuration': filter_method_configuration}
 
 

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

@@ -389,7 +389,6 @@ def origin_trial_enabled_function_name(definition_or_member):
     An exception is raised if OriginTrialEnabled is used in conjunction with any
     of the following (which must be mutually exclusive with origin trials):
       - RuntimeEnabled
-      - SecureContext
 
     The returned function checks if the IDL member should be enabled.
     Given extended attribute OriginTrialEnabled=FeatureName, return:
@@ -406,12 +405,6 @@ def origin_trial_enabled_function_name(definition_or_member):
                         'not be specified on the same definition: %s'
                         % definition_or_member.name)
 
-    if is_origin_trial_enabled and 'SecureContext' in extended_attributes:
-        raise Exception('[OriginTrialEnabled] and [SecureContext] must '
-                        'not be specified on the same definition '
-                        '(see https://crbug.com/695123 for workaround): %s'
-                        % definition_or_member.name)
-
     if is_origin_trial_enabled:
         trial_name = extended_attributes['OriginTrialEnabled']
         return 'OriginTrials::%sEnabled' % uncapitalize(trial_name)
@@ -471,7 +464,7 @@ def on_instance(interface, member):
     """Returns True if the interface's member needs to be defined on every
     instance object.
 
-    The following members must be defiend on an instance object.
+    The following members must be defined on an instance object.
     - [Unforgeable] members
     - regular members of [Global] or [PrimaryGlobal] interfaces
     """

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

@@ -572,7 +572,7 @@ const v8::FunctionCallbackInfo<v8::Value>& info
 
 {##############################################################################}
 {% macro install_conditionally_enabled_attributes_on_prototype() %}
-{% for attribute in attributes if (attribute.exposed_test or attribute.secure_context_test) and attribute.on_prototype %}
+{% for attribute in conditionally_enabled_attributes if attribute.on_prototype %}
 {% filter exposed(attribute.exposed_test) %}
 {% filter secure_context(attribute.secure_context_test) %}
 {% filter runtime_enabled(attribute.runtime_enabled_feature_name) %}

third_party/WebKit/Source/bindings/templates/interface.h.tmpl

@@ -13,7 +13,7 @@ namespace blink {
 {% if has_event_constructor %}
 class Dictionary;
 {% endif %}
-{% if origin_trial_attributes %}
+{% if origin_trial_features %}
 class ScriptState;
 {% endif %}
 {% if named_constructor %}

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

@@ -704,8 +704,13 @@ void {{v8_class_or_partial}}::install{{feature.name}}(v8::Isolate* isolate, cons
   v8::Local<v8::Signature> signature = v8::Signature::New(isolate, interfaceTemplate);
   ALLOW_UNUSED_LOCAL(signature);
   {% endif %}
+  {% if feature.needs_secure_context %}
+  ExecutionContext* executionContext = ToExecutionContext(isolate->GetCurrentContext());
+  bool isSecureContext = (executionContext && executionContext->IsSecureContext());
+  {% endif %}{# needs secure context #}
   {# Origin-Trial-enabled attributes #}
   {% for attribute in feature.attributes %}
+  {% filter secure_context(attribute.secure_context_test, 'isSecureContext') %}
   {% if attribute.is_data_type_property %}
   static const V8DOMConfiguration::AttributeConfiguration attribute{{attribute.name}}Configuration[] = {
     {{attribute_configuration(attribute) | indent(2)}}
@@ -719,6 +724,7 @@ void {{v8_class_or_partial}}::install{{feature.name}}(v8::Isolate* isolate, cons
   for (const auto& accessorConfig : accessor{{attribute.name}}Configuration)
     V8DOMConfiguration::InstallAccessor(isolate, world, instance, prototype, interface, signature, accessorConfig);
   {% endif %}
+  {% endfilter %}{# secure_context #}
   {% endfor %}
   {# Origin-Trial-enabled constants #}
   {% for constant in feature.constants %}
@@ -728,12 +734,14 @@ void {{v8_class_or_partial}}::install{{feature.name}}(v8::Isolate* isolate, cons
   {% endfor %}
   {# Origin-Trial-enabled methods (no overloads) #}
   {% for method in feature.methods %}
+  {% filter secure_context(method.secure_context_test, 'isSecureContext') %}
   {% set method_name = method.name.title().replace('_', '') %}
   static const V8DOMConfiguration::MethodConfiguration method{{method_name}}Configuration[] = {
     {{method_configuration(method) | indent(2)}}
   };
   for (const auto& methodConfig : method{{method_name}}Configuration)
     V8DOMConfiguration::InstallMethod(isolate, world, instance, prototype, interface, signature, methodConfig);
+  {% endfilter %}{# secure_context #}
   {% endfor %}
 }
 
@@ -771,7 +779,7 @@ void {{v8_class_or_partial}}::preparePrototypeAndInterfaceObject(v8::Local<v8::C
   {% endif %}
 
   v8::Isolate* isolate = context->GetIsolate();
-  {% if has_conditional_attributes_on_prototype or methods | conditionally_exposed(is_partial) %}
+  {% if has_conditional_attributes_on_prototype or conditionally_enabled_methods %}
   v8::Local<v8::Signature> signature = v8::Signature::New(isolate, interfaceTemplate);
   ExecutionContext* executionContext = ToExecutionContext(context);
   DCHECK(executionContext);
@@ -794,7 +802,7 @@ void {{v8_class_or_partial}}::preparePrototypeAndInterfaceObject(v8::Local<v8::C
 {% if has_conditional_attributes_on_prototype %}
   {{install_conditionally_enabled_attributes_on_prototype() | indent(2)}}
 {% endif %}
-{% if methods | conditionally_exposed(is_partial) %}
+{% if conditionally_enabled_methods %}
   {{install_conditionally_enabled_methods() | indent(2)}}
 {% endif %}
 }

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

@@ -636,8 +636,7 @@ for (const auto& methodConfig : {{method.name}}MethodConfiguration)
 
 {######################################}
 {% macro install_conditionally_enabled_methods() %}
-{% if methods | conditionally_exposed(is_partial) %}
-{% for method in methods | conditionally_exposed(is_partial) %}
+{% for method in conditionally_enabled_methods %}
 {% filter secure_context(method.overloads.secure_context_test_all
                          if method.overloads else
                          method.secure_context_test) %}
@@ -656,5 +655,4 @@ for (const auto& methodConfig : {{method.name}}MethodConfiguration)
 {% endfilter %}{# exposed() #}
 {% endfilter %}{# secure_context() #}
 {% endfor %}
-{% endif %}
 {%- endmacro %}

third_party/WebKit/Source/bindings/templates/partial_interface.h.tmpl

@@ -10,7 +10,7 @@
 
 namespace blink {
 
-{% if origin_trial_attributes %}
+{% if origin_trial_features %}
 class ScriptState;
 {% endif %}
 

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

@@ -53,16 +53,3 @@ const char* {{enum_variable}}[] = {
 {% endif %}
 {{property_location_list | join(' | ')}}
 {%- endmacro %}
-
-
-{% macro check_origin_trial(member, isolate="info.GetIsolate()") -%}
-ExecutionContext* executionContext = CurrentExecutionContext({{isolate}});
-String errorMessage;
-if (!{{member.origin_trial_enabled_function}}(executionContext, errorMessage)) {
-  V8SetReturnValue(info, v8::Undefined(info.GetIsolate()));
-  if (!errorMessage.IsEmpty()) {
-    executionContext->AddConsoleMessage(ConsoleMessage::Create(kJSMessageSource, kErrorMessageLevel, errorMessage));
-  }
-  return;
-}
-{% endmacro %}

third_party/WebKit/Source/bindings/tests/results/core/V8TestConstants.h

@@ -25,6 +25,7 @@
 
 namespace blink {
 
+class ScriptState;
 class V8TestConstants {
   STATIC_ONLY(V8TestConstants);
  public: