IDL: Support optional argument default value syntax

Source/bindings/scripts/v8_methods.py

Index: Source/bindings/scripts/v8_methods.py
diff --git a/Source/bindings/scripts/v8_methods.py b/Source/bindings/scripts/v8_methods.py
index 7f7b83b348f2525957d2cc759bf2e2329044ffc3..d94bf1bd20288370f8f2f29babe65847abfed248 100644
--- a/Source/bindings/scripts/v8_methods.py
+++ b/Source/bindings/scripts/v8_methods.py
@@ -181,8 +181,10 @@ def generate_argument(interface, method, argument, index):
                                            used_as_argument=True,
                                            used_as_variadic_argument=argument.is_variadic),
         'cpp_value': this_cpp_value,
+        'default_value': str(argument.default_value) if argument.default_value else None,

[Nils Barth (inactive), 2014/06/16 09:18:17]

Could you add a FIXME here to check that the type of the default value is
compatible with the type of the argument?
(Currently fails due to |null| in DOMString)

(This is a bit too complicated to do in the parser,
but it's useful validation.)

Concretely, it might be good to add a property |v8_default_value|
which returns str(argument.default_value) or None,
or raises if invalid, which will keep this code simple.

         'enum_validation_expression': idl_type.enum_validation_expression,
-        'has_default': 'Default' in extended_attributes,
+        # FIXME: remove once [Default] removed and just use argument.default_value
+        'has_default': 'Default' in extended_attributes or argument.default_value,
         'has_event_listener_argument': any(
             argument_so_far for argument_so_far in method.arguments[:index]
             if argument_so_far.idl_type.name == 'EventListener'),
@@ -298,9 +300,10 @@ def v8_value_to_local_cpp_value(argument, index):
     name = argument.name
     if argument.is_variadic:
         return v8_value_to_local_cpp_variadic_value(argument, index)
-    # [Default=NullString]
+    # FIXME: This special way of handling string arguments with null defaults
+    # can go away once we fully support default values.
     if (argument.is_optional and idl_type.name in ('String', 'ByteString') and
-        extended_attributes.get('Default') == 'NullString'):
+        argument.default_value and argument.default_value.is_null):

[haraken, 2014/06/13 13:46:52]

Just to confirm: If you write:

  void foo(optional DOMString value = null);

and call:

  foo();

then a null String (instead of a String "null") is expected to be passed to
Blink, right?

I'm asking this because if you write:

  void foo(DOMString);

and call:

  foo(null);

then a String "null" is passed to Blink.

[Jens Widell, 2014/06/13 14:14:52]

Yes. The argumentOrNull() function returns an empty v8::Handle if the index is
out of range (IOW, not a Javascript null value as one might think given the
function name) and that gets converted into a null string
(WTF::String::isNull()==true) by V8StringResource<>.

[haraken, 2014/06/13 14:32:53]

Sorry for not being clear. I understand that your CL does the conversion. My
question is whether the behavior aligns with the IDL spec or not.

  [a] foo(DOMString str) & foo(null) => a String "null"
  [b] foo(optional DOMString str = null) & foo() => a null String
  [c] foo(optional DOMString str = null) & foo(null) => a String "null"

I'm confident about [a] and [c], but not confident about [b]. (Sorry I'm asking
this question without reading the spec seriously...)

[Jens Widell, 2014/06/13 14:49:46]

I see. :-) Well, if I read the spec correctly, your [b] is correct because
default values bypass value conversion, which is where "", "null" and
"undefined" would appear.

See http://heycam.github.io/webidl/#dfn-overload-resolution-algorithm, 9.4.1 vs
9.5 and 12.4.1 vs 12.5.

[haraken, 2014/06/13 14:59:03]

Makes sense, thanks!

[Nils Barth (inactive), 2014/06/16 09:18:17]

Same conclusion as Joshua
(i.e., go ahead with this change, fix illegal |null| defaults in followup);
just adding some details.

Properly,
optional DOMString s = null
is *illegal*, as |null| is not a valid value for a DOMString
(as Joshua notes); it's like writing
optional DOMString s = 1

Formally the correct thing would be something like:
  optional DOMString? s = null
...or:
  [TreatNullAs=NullString] optional DOMString s = null
...per:
http://www.chromium.org/blink/webidl/blink-idl-extended-attributes#TOC-TreatN...
...our take on the standard [TreatNullAs]
http://heycam.github.io/webidl/#TreatNullAs
...but that would change the behavior of *explicitly* passing |null|.

Also, regarding argument conversion, the correct section is 13.1:
13. While i is less than the number of arguments callable is declared to take:
1. If callable’s argument at index i is declared with a default value, then
append to values that default value.

That's the relevant section when trailing optional arguments are *omitted*;
the sections you cited are when |undefined| is passed as an optional argument.
(>.<)

[Jens Widell, 2014/06/16 09:37:14]

You are (both) right, naturally. And it was of course always the intent to have
the per-spec default values in the IDL, and they will not be null since that's
not allowed. Our null strings (whether as [Default=Null] or "= null") was
always, and still is with this patch, a way to work around lacking support for
default values in the IDL. Soon, though... :-)

         v8_value = 'argumentOrNull(info, %s)' % index
     else:
         v8_value = 'info[%s]' % index