[bindings] partial interfaces should not violate componentization

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 59 matching lines @@
     'wtf/GetPtr.h',
     'wtf/RefPtr.h',
 ])
 
 
 def interface_context(interface):
     includes.clear()
     includes.update(INTERFACE_CPP_INCLUDES)
     header_includes = set(INTERFACE_H_INCLUDES)
 
-    parent_interface = interface.parent
-    if parent_interface:
-        header_includes.update(v8_types.includes_for_interface(parent_interface))
+    if interface.is_partial:
+        # A partial interface definition cannot specify that the interface inherits
+        # from another interface. Inheritance must be specified on the original interface
+        # definition.
+        parent_interface = None

[bashi, 2014/10/15 05:29:23]

nit: you can omit this.

+        is_audio_buffer = False

[bashi, 2014/10/15 05:29:23]

unused?

[tasak, 2014/10/15 11:24:18]

Done.

+        is_document = False
+        is_event_target = False

[bashi, 2014/10/15 05:29:23]

nit: you can omit assigning False.

[tasak, 2014/10/15 11:24:18]

I saw the following errors:
    'is_document': is_document,
UnboundLocalError: local variable 'is_document' referenced before assignment
    'is_event_target': is_event_target,
UnboundLocalError: local variable 'is_event_target' referenced before assignment
    'parent_interface': parent_interface,
UnboundLocalError: local variable 'parent_interface' referenced before
assignment

[bashi, 2014/10/17 02:23:24]

BTW, shouldn't we raise an exception rather than assigning false?

+        # partial interface needs the definition of its original interface.
+        includes.add('bindings/core/v8/V8%s.h' % interface.name)
+    else:
+        parent_interface = interface.parent
+        if parent_interface:
+            header_includes.update(v8_types.includes_for_interface(parent_interface))
+        is_document = inherits_interface(interface.name, 'Document')
+        is_event_target = inherits_interface(interface.name, 'EventTarget')
+
     extended_attributes = interface.extended_attributes
 
     # [ActiveDOMObject]
     is_active_dom_object = 'ActiveDOMObject' in extended_attributes
 
     # [CheckSecurity]
     is_check_security = 'CheckSecurity' in extended_attributes
     if is_check_security:
         includes.add('bindings/core/v8/BindingSecurity.h')
 
     # [DependentLifetime]
     is_dependent_lifetime = 'DependentLifetime' in extended_attributes
 
     # [Iterable]
     iterator_method = None
-    if 'Iterable' in extended_attributes:
+    # FIXME: support Iterable in partial interfaces. However, we don't
+    # need to support iterator overloads between interface and
+    # partial interface definitions.
+    # http://heycam.github.io/webidl/#idl-overloading
+    if 'Iterable' in extended_attributes and not interface.is_partial:
         iterator_operation = IdlOperation(interface.idl_name)
         iterator_operation.name = 'iterator'
         iterator_operation.idl_type = IdlType('Iterator')
         iterator_operation.extended_attributes['RaisesException'] = None
         iterator_operation.extended_attributes['CallWith'] = 'ScriptState'
         iterator_method = v8_methods.method_context(interface,
                                                     iterator_operation)
 
     # [MeasureAs]
     is_measure_as = 'MeasureAs' in extended_attributes
@@ skipping 26 matching lines @@
     # [Custom=Wrap], [SetWrapperReferenceFrom]
     has_visit_dom_wrapper = (
         has_extended_attribute_value(interface, 'Custom', 'VisitDOMWrapper') or
         reachable_node_function or
         set_wrapper_reference_to_list)
 
     this_gc_type = gc_type(interface)
 
     wrapper_class_id = ('NodeClassId' if inherits_interface(interface.name, 'Node') else 'ObjectClassId')
 
+    v8_class_name = v8_utilities.v8_class_name(interface)
+    cpp_class_name = cpp_name(interface)
+
     context = {
         'conditional_string': conditional_string(interface),  # [Conditional]
-        'cpp_class': cpp_name(interface),
+        'cpp_class': cpp_class_name,
+        'actual_cpp_class': cpp_class_name + 'Partial' if interface.is_partial else cpp_class_name,
         'gc_type': this_gc_type,
         # FIXME: Remove 'EventTarget' special handling, http://crbug.com/383699
         'has_access_check_callbacks': (is_check_security and
                                        interface.name != 'Window' and
                                        interface.name != 'EventTarget'),
         'has_custom_legacy_call_as_function': has_extended_attribute_value(interface, 'Custom', 'LegacyCallAsFunction'),  # [Custom=LegacyCallAsFunction]
         'has_custom_to_v8': has_extended_attribute_value(interface, 'Custom', 'ToV8'),  # [Custom=ToV8]
         'has_custom_wrap': has_extended_attribute_value(interface, 'Custom', 'Wrap'),  # [Custom=Wrap]
         'has_visit_dom_wrapper': has_visit_dom_wrapper,
         'header_includes': header_includes,
         'interface_name': interface.name,
         'is_active_dom_object': is_active_dom_object,
         'is_check_security': is_check_security,
         'is_dependent_lifetime': is_dependent_lifetime,
-        'is_event_target': inherits_interface(interface.name, 'EventTarget'),
+        'is_document': is_document,
+        'is_event_target': is_event_target,
         'is_exception': interface.is_exception,
         'is_node': inherits_interface(interface.name, 'Node'),
         'is_script_wrappable': is_script_wrappable,
         'iterator_method': iterator_method,
         'lifetime': 'Dependent'
             if (has_visit_dom_wrapper or
                 is_active_dom_object or
                 is_dependent_lifetime)
             else 'Independent',
+        'is_partial': interface.is_partial,
+        'has_partial_interface': len(interface.partial_interfaces) > 0,
         'measure_as': v8_utilities.measure_as(interface),  # [MeasureAs]
         'parent_interface': parent_interface,
         'pass_cpp_type': cpp_template_type(
             cpp_ptr_type('PassRefPtr', 'RawPtr', this_gc_type),
             cpp_name(interface)),
         'reachable_node_function': reachable_node_function,
         'runtime_enabled_function': runtime_enabled_function_name(interface),  # [RuntimeEnabled]
         'set_wrapper_reference_to_list': set_wrapper_reference_to_list,
-        'v8_class': v8_utilities.v8_class_name(interface),
+        'v8_class': v8_class_name,
+        'actual_v8_class': v8_class_name + 'Partial' if interface.is_partial else v8_class_name,

[bashi, 2014/10/15 05:29:23]

nit: alphabetical order. (I'd like to call this |v8_partial_class| though)

[tasak, 2014/10/15 11:24:18]

Done.
I would like to call. However, if a given interface is not partial, we can use
|v8_partial_class| in jinja templates?

[bashi, 2014/10/17 02:23:24]

Acknowledged.

         'wrapper_class_id': wrapper_class_id,
     }
 
     # Constructors
-    constructors = [constructor_context(interface, constructor)
-                    for constructor in interface.constructors
-                    # FIXME: shouldn't put named constructors with constructors
-                    # (currently needed for Perl compatibility)
-                    # Handle named constructors separately
-                    if constructor.name == 'Constructor']
-    if len(constructors) > 1:
-        context['constructor_overloads'] = overloads_context(constructors)
-
-    # [CustomConstructor]
-    custom_constructors = [{  # Only needed for computing interface length
-        'number_of_required_arguments':
-            number_of_required_arguments(constructor),
-    } for constructor in interface.custom_constructors]
-
-    # [EventConstructor]
-    has_event_constructor = 'EventConstructor' in extended_attributes
-    any_type_attributes = [attribute for attribute in interface.attributes
-                           if attribute.idl_type.name == 'Any']
-    if has_event_constructor:
-        includes.add('bindings/core/v8/Dictionary.h')
-        if any_type_attributes:
-            includes.add('bindings/core/v8/SerializedScriptValue.h')
-
-    # [NamedConstructor]
-    named_constructor = named_constructor_context(interface)
-
-    if (constructors or custom_constructors or has_event_constructor or
-        named_constructor):
-        includes.add('bindings/core/v8/V8ObjectConstructor.h')
-        includes.add('core/frame/LocalDOMWindow.h')
-
-    context.update({
-        'any_type_attributes': any_type_attributes,
-        'constructors': constructors,
-        'has_custom_constructor': bool(custom_constructors),
-        'has_event_constructor': has_event_constructor,
-        'interface_length':
-            interface_length(interface, constructors + custom_constructors),
-        'is_constructor_raises_exception': extended_attributes.get('RaisesException') == 'Constructor',  # [RaisesException=Constructor]
-        'named_constructor': named_constructor,
-    })
+    update_interface_constructor_context(interface, context)
 
     constants = [constant_context(constant) for constant in interface.constants]
 
     special_getter_constants = []
     runtime_enabled_constants = []
     constant_configuration_constants = []
 
     for constant in constants:
         if constant['measure_as'] or constant['deprecate_as']:
             special_getter_constants.append(constant)
@@ skipping 27 matching lines @@
                   attribute['runtime_enabled_function'] or
                   attribute['per_context_enabled_function'])
              and attribute['should_be_exposed_to_script']
              for attribute in attributes),
         'has_conditional_attributes': any(attribute['per_context_enabled_function'] or attribute['exposed_test'] for attribute in attributes),
         'has_constructor_attributes': any(attribute['constructor_type'] for attribute in attributes),
         'has_replaceable_attributes': any(attribute['is_replaceable'] for attribute in attributes),
     })
 
     # Methods
-    methods = [v8_methods.method_context(interface, method)
-               for method in interface.operations
-               if method.name]  # Skip anonymous special operations (methods)
-    compute_method_overloads_context(methods)
+    methods = []
+    if interface.original_interface:
+        for operation in interface.original_interface.operations:
+            if not operation.name:
+                continue
+            method = v8_methods.method_context(interface, operation)

[bashi, 2014/10/15 05:29:23]

How about adding an optional argument |is_visible|, of which default value is
True, to method_context()?
This way, we can write this like below:

methods.extends([v8_methods.method_context(interface, method, is_visible=False)
    for method in interface.original_interface.operations])

[tasak, 2014/10/15 11:24:19]

Done.

+            method['visible'] = False
+            methods.append(method)
+    methods.extend([v8_methods.method_context(interface, method)
+                    for method in interface.operations
+                    if method.name])  # Skip anonymous special operations (methods)
+    if interface.partial_interfaces:
+        assert len(interface.partial_interfaces) == len(set(interface.partial_interfaces))
+        for partial_interface in interface.partial_interfaces:
+            for operation in partial_interface.operations:
+                if not operation.name:
+                    continue
+                partial_method = v8_methods.method_context(interface, operation)
+                partial_method['visible'] = False
+                methods.append(partial_method)
+    compute_method_overloads_context(interface, methods)
 
     # Stringifier
     if interface.stringifier:
         stringifier = interface.stringifier
         method = IdlOperation(interface.idl_name)
         method.name = 'toString'
         method.idl_type = IdlType('DOMString')
         method.extended_attributes.update(stringifier.extended_attributes)
         if stringifier.attribute:
             method.extended_attributes['ImplementedAs'] = stringifier.attribute.name
         elif stringifier.operation:
             method.extended_attributes['ImplementedAs'] = stringifier.operation.name
         methods.append(v8_methods.method_context(interface, method))
 
     conditionally_enabled_methods = []
     custom_registration_methods = []
     method_configuration_methods = []
 
     for method in methods:
         # Skip all but one method in each set of overloaded methods.
         if 'overload_index' in method and 'overloads' not in method:
             continue
 
         if 'overloads' in method:
             overloads = method['overloads']
+            if not overloads['visible']:
+                continue
+            # original interface will register instead of partial interface.
+            if overloads['has_partial_overloads'] and interface.is_partial:
+                continue
             per_context_enabled_function = overloads['per_context_enabled_function_all']
             conditionally_exposed_function = overloads['exposed_test_all']
             runtime_enabled_function = overloads['runtime_enabled_function_all']
             has_custom_registration = overloads['has_custom_registration_all']
         else:
+            if not method['visible']:
+                continue
             per_context_enabled_function = method['per_context_enabled_function']
             conditionally_exposed_function = method['exposed_test']
             runtime_enabled_function = method['runtime_enabled_function']
             has_custom_registration = method['has_custom_registration']
 
         if per_context_enabled_function or conditionally_exposed_function:
             conditionally_enabled_methods.append(method)
             continue
         if runtime_enabled_function or has_custom_registration:
             custom_registration_methods.append(method)
@@ skipping 34 matching lines @@
         'indexed_property_deleter': indexed_property_deleter(interface),
         'is_override_builtins': 'OverrideBuiltins' in extended_attributes,
         'named_property_getter': named_property_getter(interface),
         'named_property_setter': named_property_setter(interface),
         'named_property_deleter': named_property_deleter(interface),
     })
 
     return context
 
 
+def update_interface_constructor_context(interface, context):

[bashi, 2014/10/15 05:29:23]

Question: is there any change except for factoring out as a method? It's
somewhat difficult to check what has been changed without diff..

[tasak, 2014/10/15 11:24:19]

Done.

+    constructors = [constructor_context(interface, constructor)
+                    for constructor in interface.constructors
+                    # FIXME: shouldn't put named constructors with constructors
+                    # (currently needed for Perl compatibility)
+                    # Handle named constructors separately
+                    if constructor.name == 'Constructor']
+    if len(constructors) > 1:
+        context['constructor_overloads'] = overloads_context(interface, constructors)
+
+    # [CustomConstructor]
+    custom_constructors = [{  # Only needed for computing interface length
+        'number_of_required_arguments':
+            number_of_required_arguments(constructor),
+    } for constructor in interface.custom_constructors]
+
+    # [EventConstructor]
+    has_event_constructor = 'EventConstructor' in interface.extended_attributes
+    # [NamedConstructor]
+    named_constructor = named_constructor_context(interface)
+
+    if constructors or custom_constructors or has_event_constructor or named_constructor:
+        if interface.is_partial:
+            raise Exception('[Constructor] and [NamedConstructor] MUST NOT be'
+                            ' specified on partial interface definitions:'
+                            '%s' % interface.name)
+
+        includes.add('bindings/core/v8/V8ObjectConstructor.h')
+        includes.add('core/frame/LocalDOMWindow.h')
+
+    any_type_attributes = [attribute for attribute in interface.attributes
+                           if attribute.idl_type.name == 'Any']
+    if has_event_constructor:
+        includes.add('bindings/core/v8/Dictionary.h')
+        if any_type_attributes:
+            includes.add('bindings/core/v8/SerializedScriptValue.h')
+
+    context.update({
+        'any_type_attributes': any_type_attributes,
+        'constructors': constructors,
+        'has_custom_constructor': bool(custom_constructors),
+        'has_event_constructor': has_event_constructor,
+        'interface_length':
+            interface_length(interface, constructors + custom_constructors),
+        'is_constructor_raises_exception':
+            interface.extended_attributes.get('RaisesException') == 'Constructor',  # [RaisesException=Constructor]
+        'named_constructor': named_constructor,
+    })
+
+
 # [DeprecateAs], [Reflect], [RuntimeEnabled]
 def constant_context(constant):
     extended_attributes = constant.extended_attributes
     return {
         'cpp_class': extended_attributes.get('PartialInterfaceImplementedAs'),
         'deprecate_as': v8_utilities.deprecate_as(constant),  # [DeprecateAs]
         'idl_type': constant.idl_type.name,
         'measure_as': v8_utilities.measure_as(constant),  # [MeasureAs]
         'name': constant.name,
         # FIXME: use 'reflected_name' as correct 'name'
         'reflected_name': extended_attributes.get('Reflect', constant.name),
         'runtime_enabled_function': runtime_enabled_function_name(constant),
         'value': constant.value,
     }
 
 
 ################################################################################
 # Overloads
 ################################################################################
 
-def compute_method_overloads_context(methods):
+def compute_method_overloads_context(interface, methods):
     # Regular methods
-    compute_method_overloads_context_by_type([method for method in methods
-                                              if not method['is_static']])
+    compute_method_overloads_context_by_type(
+        interface, [method for method in methods if not method['is_static']])
     # Static methods
-    compute_method_overloads_context_by_type([method for method in methods
-                                              if method['is_static']])
+    compute_method_overloads_context_by_type(
+        interface, [method for method in methods if method['is_static']])
 
 
-def compute_method_overloads_context_by_type(methods):
+def compute_method_overloads_context_by_type(interface, methods):
     """Computes |method.overload*| template values.
 
     Called separately for static and non-static (regular) methods,
     as these are overloaded separately.
     Modifies |method| in place for |method| in |methods|.
     Doesn't change the |methods| list itself (only the values, i.e. individual
     methods), so ok to treat these separately.
     """
     # Add overload information only to overloaded methods, so template code can
     # easily verify if a function is overloaded
     for name, overloads in method_overloads_by_name(methods):
         # Resolution function is generated after last overloaded function;
         # package necessary information into |method.overloads| for that method.
-        overloads[-1]['overloads'] = overloads_context(overloads)
+        overloads[-1]['overloads'] = overloads_context(interface, overloads)
         overloads[-1]['overloads']['name'] = name
 
 
 def method_overloads_by_name(methods):
     """Returns generator of overloaded methods by name: [name, [method]]"""
     # Filter to only methods that are actually overloaded
     method_counts = Counter(method['name'] for method in methods)
     overloaded_method_names = set(name
                                   for name, count in method_counts.iteritems()
                                   if count > 1)
     overloaded_methods = [method for method in methods
                           if method['name'] in overloaded_method_names]
 
     # Group by name (generally will be defined together, but not necessarily)
     return sort_and_groupby(overloaded_methods, itemgetter('name'))
 
 
-def overloads_context(overloads):
+def overloads_context(interface, overloads):
     """Returns |overloads| template values for a single name.
 
     Sets |method.overload_index| in place for |method| in |overloads|
     and returns dict of overall overload template values.
     """
+    # partial overloads breaks this assumption.

[bashi, 2014/10/15 05:29:23]

Should we remove or change the assert then?

[tasak, 2014/10/15 11:24:19]

Removed the comment. This assert is correct now.

     assert len(overloads) > 1  # only apply to overloaded names
     for index, method in enumerate(overloads, 1):
         method['overload_index'] = index
 
     effective_overloads_by_length = effective_overload_set_by_length(overloads)
     lengths = [length for length, _ in effective_overloads_by_length]
     name = overloads[0].get('name', '<constructor>')
 
     # Check and fail if all overloads with the shortest acceptable arguments
     # list are runtime enabled, since we would otherwise set 'length' on the
@@ skipping 20 matching lines @@
                 raise ValueError('Overloads of %s have conflicting extended attribute %s'
                                  % (name, extended_attribute))
 
     # Check and fail if overloads disagree about whether the return type
     # is a Promise or not.
     promise_overload_count = sum(1 for method in overloads if method.get('idl_type') == 'Promise')
     if promise_overload_count not in (0, len(overloads)):
         raise ValueError('Overloads of %s have conflicting Promise/non-Promise types'
                          % (name))
 
+    overloads_visibles = set([method.get('visible', True) for method in overloads])

[bashi, 2014/10/15 05:29:23]

Use method['visible'] instead of get()? It seems that |method| always has
'visible'.

[tasak, 2014/10/15 11:24:19]

|method| always has. However, |constructor| never has.

constructors = [constructor_context ...]
if len(constructors) > 1:
        context['constructor_overloads'] = overloads_context(interface,
constructors)

So we should modify constructor_context?

[bashi, 2014/10/17 02:23:24]

It will help make code simple, IMO.

+    if len(overloads_visibles) > 1:
+        overloads_visible = True
+        has_partial_overloads = True
+    else:
+        overloads_visible = overloads_visibles.pop()
+        has_partial_overloads = False
+
     return {
         'deprecate_all_as': common_value(overloads, 'deprecate_as'),  # [DeprecateAs]
         'exposed_test_all': common_value(overloads, 'exposed_test'),  # [Exposed]
         'has_custom_registration_all': common_value(overloads, 'has_custom_registration'),
         'length_tests_methods': length_tests_methods(effective_overloads_by_length),
         # 1. Let maxarg be the length of the longest type list of the
         # entries in S.
         'maxarg': lengths[-1],
         'measure_all_as': common_value(overloads, 'measure_as'),  # [MeasureAs]
         'minarg': lengths[0],
         'per_context_enabled_function_all': common_value(overloads, 'per_context_enabled_function'),  # [PerContextEnabled]
         'runtime_enabled_function_all': common_value(overloads, 'runtime_enabled_function'),  # [RuntimeEnabled]
         'valid_arities': lengths
             # Only need to report valid arities if there is a gap in the
             # sequence of possible lengths, otherwise invalid length means
             # "not enough arguments".
             if lengths[-1] - lengths[0] != len(lengths) - 1 else None,
+        'visible': overloads_visible,
+        'has_partial_overloads': has_partial_overloads,
     }
 
 
 def effective_overload_set(F):
     """Returns the effective overload set of an overloaded function.
 
     An effective overload set is the set of overloaded functions + signatures
     (type list of arguments, with optional and variadic arguments included or
     not), and is used in the overload resolution algorithm.
 
@@ skipping 637 matching lines @@
         deleter = next(
             method
             for method in interface.operations
             if ('deleter' in method.specials and
                 len(method.arguments) == 1 and
                 str(method.arguments[0].idl_type) == 'DOMString'))
     except StopIteration:
         return None
 
     return property_deleter(deleter)