Support per-overload [RuntimeEnabled] extended attribute

Source/bindings/scripts/v8_interface.py

Index: Source/bindings/scripts/v8_interface.py
diff --git a/Source/bindings/scripts/v8_interface.py b/Source/bindings/scripts/v8_interface.py
index 80049e1be1461fe5fc32d11eb13d9fac2311c69a..36c4b0ada023ff6082593fbad0b7aaa2ba0cd514 100644
--- a/Source/bindings/scripts/v8_interface.py
+++ b/Source/bindings/scripts/v8_interface.py
@@ -354,6 +354,15 @@ def generate_overloads(overloads):
     for index, method in enumerate(overloads, 1):
         method['overload_index'] = index
 
+    runtime_enabled_features = set(method.get('runtime_enabled_function')

[Nils Barth (inactive), 2014/05/27 06:33:20]

You don't need dict.get() here, do you?

[Jens Widell, 2014/05/27 06:46:20]

Needed for overloaded constructors currently; generate_constructor() doesn't set
'runtime_enabled_function'. Maybe it should (i.e. maybe we should support
runtime enabled constructor overloads too) and either way, it could of course at
least set it to None to allow us to assume it's there.

Which do you prefer? :-)

[Nils Barth (inactive), 2014/05/27 07:24:17]

Oh, good point.
get() is a bit better, since having long lists of default None/False for the
constructor context is fragile and gross.
Thanks!

+                                   for method in overloads)
+    if len(runtime_enabled_features) > 1:

[Nils Barth (inactive), 2014/05/27 06:33:20]

Shouldn't you use the utility common_value() function instead?

[Jens Widell, 2014/05/27 06:46:20]

It returns None both if there is no common value and when the common value is
None. I handle those two cases differently here.

[Nils Barth (inactive), 2014/05/27 07:24:17]

Oh, good point.

I'd prefer consistency with the other [DeprecateAs] and [MeasureAs] code,
however you do it.
Those treat it by having a separate *_all_as variable and branching in Jinja,
instead of changing the variables in Python.

I think you can handle these uniformly via something like:
Python:
'runtime_enabled_function_all': common_value(overloads,
'runtime_enabled_function'),

Jinja:
{% filter runtime_enabled(not overloads.runtime_enabled_function_all and
method.runtime_enabled_function) %}

WDYT?

[Jens Widell, 2014/05/27 07:39:17]

That works nicely in the overload_resolution_method() macro, but in the
configure_class_template block, where the method is registered, I don't have the
'overloads' dict accessible. Almost, though. It's stored on the last method, and
the method registration is generated for the first method:

  {% if not method.overload_index or method.overload_index == 1 %}
  {# For overloaded methods, only generate one accessor #}

I could change that condition to

  {% if not method.overload_index or method.overloads %}

I think, and then use

  {% filter runtime_enabled(method.overloads.runtime_enabled_function_all) %}

to make the registration runtime enabled.

Good, you think? (I haven't tested it.)

[Jens Widell, 2014/05/27 07:42:15]

But that of course doesn't work for non-overloaded methods.

[Nils Barth (inactive), 2014/05/27 07:50:42]

That sounds great!

[Nils Barth (inactive), 2014/05/27 07:50:42]

...why not? (confused)

+        # More than one different value for [RuntimeEnabled] among the

[Nils Barth (inactive), 2014/05/27 06:33:20]

Could you clarify that this also works for *missing* values?

E.g.,
f();
[RuntimeEnabled=Foo] f(long x);

You have a test case, but None makes it a bit weird.

[Jens Widell, 2014/05/27 06:46:20]

Yeah, I'm essentially treating None as any other value for the RuntimeEnabled
attribute. In your example, 'runtime_enabled_features' is set([None, 'Foo'])
which looks like more than one different value and triggers per-overload
handling.

I can add a comment about it. I could also rewrite the code to handle None
explicitly, if you like.

[Nils Barth (inactive), 2014/05/27 07:24:17]

None is confusing enough that I'd appreciate a comment, if you'd be so kind :)
(The value of no value is itself a value. It's so Zen.)
I think you can avoid special-casing None by using a more Jinja-y style, as
outlined above.

+        # overloads; need to handle as part of overload resoultion.
+        for method in overloads:
+            method['overload_runtime_enabled_function'] = method['runtime_enabled_function']
+            method['runtime_enabled_function'] = None
+
     effective_overloads_by_length = effective_overload_set_by_length(overloads)
     lengths = [length for length, _ in effective_overloads_by_length]
 
@@ -697,7 +706,10 @@ def resolution_tests_methods(effective_overloads):
         method = next(method for idl_type, method in idl_types_methods
                       if idl_type.name == 'String' or idl_type.is_enum)
         yield 'true', method
-        return
+        # Stop here unless this overload (and not all overloads) is runtime

[Jens Widell, 2014/05/23 14:55:16]

A simpler fix here would of course be to just remove the 'return' here and let
the compiler eliminate the resulting dead code that would be produced in cases
where no [RuntimeEnabled] is involved and there are string+number overloads:

  if (true) {
    foo1Method(info); // string
    return;
  }
  if (true) {
    foo2Method(info); // number
    return;
  }

[Nils Barth (inactive), 2014/05/27 06:33:20]

That's fine to do: CG simplicity is a higher priority than generated source code
legibility, per:
http://www.chromium.org/developers/design-documents/idl-compiler#TOC-Goals

Could you remove the early returns here and below, and add a note about the dead
code elimination in the comment beginning:
# (Perform automatic type conversion, in order.

Something like: "We rely on compiler dead code elimination to remove redundant
branches."

+        # enabled.
+        if not method.get('overload_runtime_enabled_function'):
+            return
     except StopIteration:
         pass