IDL: restructure logic handling registration of methods

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 233 matching lines @@
         'has_per_context_enabled_attributes': any(attribute['per_context_enabled_function'] for attribute in attributes),
         'has_replaceable_attributes': any(attribute['is_replaceable'] for attribute in attributes),
     })
 
     # Methods
     methods = [v8_methods.generate_method(interface, method)
                for method in interface.operations
                if method.name]  # Skip anonymous special operations (methods)
     generate_method_overloads(methods)
     for method in methods:
-        method['do_generate_method_configuration'] = (
-            method['do_not_check_signature'] and
-            not method['per_context_enabled_function'] and
-            # For overloaded methods, only generate one accessor
-            ('overload_index' not in method or method['overload_index'] == 1))
+        if 'overloads' in method:

[Nils Barth (inactive), 2014/06/11 06:28:28]

Comment:
# For overloaded methods, only generate one accessor

+            overloads = method['overloads']
+            method['do_generate_method_configuration'] = (
+                overloads['do_not_check_signature_any'] and

[Jens Widell, 2014/06/10 09:42:47]

I'm not 100 % sure about using "any" instead of "all" here, but it's correct for
these cases that I'm at least somewhat familiar with:

- Overloaded static methods: 'do_not_check_signature' is guaranteed false on all
overloads, and thus 'do_not_check_signature_any' is false, and
'do_generate_method_configuration' is false.

- Partially [RuntimeEnabled] overloaded methods: 'do_not_check_signature' will
be true on those that aren't runtime enabled, and so
'do_not_check_signature_any' is true, and 'do_generate_method_configuration' is
true (which is good; a method that is only partially runtime enabled is
unconditionally configured.)

Where this might be wrong would be overloads where only some overloads have any
of the attributes [DoNotCheckSecurity], [DoNotCheckSignature], [NotEnumerable],
[ReadOnly] or [Unforgeable]. Having overloads with different [NotEnumerable],
[ReadOnly] or [Unforgeable] makes no sense I think, and the others I'm not
really familiar with.

Anyway: with the current set of overloads in current IDL files, this code does
not change the generated code.

[Nils Barth (inactive), 2014/06/11 06:28:28]

haraken:
Thoughts on do_not_check_signature on overloaded methods?
I'm not clear on what check signature actually does, or why we're skipping it;
are there security or type-checking implications?

Jens: I think your code and reasoning are correct;
could you put a comment somewhere that this is fiddly?
(I think in generate_overloads; see below.)

[DoNotCheckSecurity] is quite rare (only used in Location and Window)
http://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-DoNotC...

...and [DoNotCheckSignature] is only used one place:
(for what should be a stringifier in Window):
http://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-Immuta...

[haraken, 2014/06/11 06:38:08]

Good question.

I think it doesn't make sense at all to specify [DoNotCheckSecurity] and/or
[DoNotCheckSignature] on a part of overloaded methods (I cannot imagine use
cases, and in fact there is no use case at the moment). We should specify
[DoNotCheckSecurity] and/or [DoNotCheckSignature] on either all of the
overloaded methods or none of the overloaded methods.

Shall we add the check about it, instead of worrying about "any" or "all"?

+                not overloads['per_context_enabled_function_all'])

[Nils Barth (inactive), 2014/06/11 06:28:28]

I'd slightly prefer |continue| to |else|
(slightly flatter, clearly exceptional case).

+        else:
+            method['do_generate_method_configuration'] = (
+                method['do_not_check_signature'] and
+                not method['per_context_enabled_function'] and
+                # Ignore any overload not handled by the case above

[Nils Barth (inactive), 2014/06/11 06:28:28]

Comment tweak:
# Overloaded methods are handled above (only one accessor)

+                'overload_index' not in method)
 
         # The value of the Function object’s “length” property is a Number
         # determined as follows:
         # 1. Let S be the effective overload set for regular operations (if the
         # operation is a regular operation) or for static operations (if the
         # operation is a static operation) with identifier id on interface I and
         # with argument count 0.
         # 2. Return the length of the shortest argument list of the entries in S.
         # FIXME: This calculation doesn't take into account whether runtime
         # enabled overloads are actually enabled, so length may be incorrect.
@@ skipping 109 matching lines @@
     # controlled by the same runtime enabled feature, in which case there would
     # be no function object at all if it is not enabled.
     shortest_overloads = effective_overloads_by_length[0][1]
     if (all(method.get('runtime_enabled_function')
             for method, _, _ in shortest_overloads) and
         not common_value(overloads, 'runtime_enabled_function')):
         raise ValueError('Function.length of %s depends on runtime enabled features' % overloads[0]['name'])
 
     return {
         'deprecate_all_as': common_value(overloads, 'deprecate_as'),  # [DeprecateAs]
+        'do_not_check_signature_any': any(method.get('do_not_check_signature')

[Nils Barth (inactive), 2014/06/11 06:28:28]

Maybe a comment on this being potentially wrong if weird mix of overloads?

+                                          for method in overloads),
         'length_tests_methods': length_tests_methods(effective_overloads_by_length),
         'minarg': lengths[0],
         # 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]
+        'per_context_enabled_function_all': common_value(overloads, 'per_context_enabled_function'),  # [PerContextEnabled]
         '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,
     }
 
 
 def effective_overload_set(F):
     """Returns the effective overload set of an overloaded function.
@@ skipping 629 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)