v8binding: Fix a conversion rule of optional argument of IDL operation

func(undefined, true) in JS should be treated as
func("missing", IDL true) in Web IDL, and this conversion rule
hasn't been implemented correctly in Blink-V8 bindings.

This patch introduces a new trait IDLOptional to represent this
conversion rule without changing the current behavior.  For
example,

  NativeValueTraits<IDLOptional<IDLBoolean>>::NativeValue(
    v8::Undefined(isolate))

returns `false` of type bool for now, but we can make it return
base::nullopt of type base::Optional<bool> in future.

Diff of the test expectations come from a fix of off-by-one error
about argument index.

Bug: 1151949
Change-Id: Ia7a478442d3c11e5d3dfe39a6e08182bd718ed90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2563181Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834905}

third_party/blink/renderer/bindings/core/v8/idl_types.h

@@ -298,6 +298,20 @@ struct IDLOnBeforeUnloadEventHandler final
     : public IDLBaseHelper<EventListener*> {};
 struct IDLOnErrorEventHandler final : public IDLBaseHelper<EventListener*> {};
 
+// IDL optional types
+//
+// IDLOptional represents an optional argument and supports a conversion from
+// ES undefined to "missing" special value.  The "missing" value might be
+// represented in Blink as base::nullopt, nullptr, 0, etc. depending on a Blink
+// type.
+//
+// Note that IDLOptional is not meant to represent an optional dictionary
+// member.
+template <typename T>
+struct IDLOptional final : public IDLBase {
+  using ImplType = void;
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_IDL_TYPES_H_

third_party/blink/renderer/bindings/scripts/bind_gen/blink_v8_bridge.py

@@ -242,7 +242,21 @@ def blink_type_info(idl_type, use_new_union=False):
     assert False, "Unknown type: {}".format(idl_type.syntactic_form)
 
 
-def native_value_tag(idl_type):
+def native_value_tag(idl_type, argument=None):
+    """Returns the tag type of NativeValueTraits."""
+    assert isinstance(idl_type, web_idl.IdlType)
+    assert argument is None or isinstance(argument, web_idl.Argument)
+
+    if (idl_type.is_optional
+            and not (idl_type.is_nullable or
+                     (argument and argument.default_value) or
+                     (argument and argument == argument.owner.arguments[-1]))):
+        return "IDLOptional<{}>".format(_native_value_tag_impl(idl_type))
+
+    return _native_value_tag_impl(idl_type)
+
+
+def _native_value_tag_impl(idl_type):
     """Returns the tag type of NativeValueTraits."""
     assert isinstance(idl_type, web_idl.IdlType)
 
@@ -280,15 +294,16 @@ def native_value_tag(idl_type):
 
     if real_type.is_sequence:
         return "IDLSequence<{}>".format(
-            native_value_tag(real_type.element_type))
+            _native_value_tag_impl(real_type.element_type))
 
     if real_type.is_frozen_array:
-        return "IDLArray<{}>".format(native_value_tag(real_type.element_type))
+        return "IDLArray<{}>".format(
+            _native_value_tag_impl(real_type.element_type))
 
     if real_type.is_record:
         return "IDLRecord<{}, {}>".format(
-            native_value_tag(real_type.key_type),
-            native_value_tag(real_type.value_type))
+            _native_value_tag_impl(real_type.key_type),
+            _native_value_tag_impl(real_type.value_type))
 
     if real_type.is_promise:
         return "IDLPromise"
@@ -300,7 +315,8 @@ def native_value_tag(idl_type):
         return "IDLUnionNotINT<{}>".format(class_name)
 
     if real_type.is_nullable:
-        return "IDLNullable<{}>".format(native_value_tag(real_type.inner_type))
+        return "IDLNullable<{}>".format(
+            _native_value_tag_impl(real_type.inner_type))
 
     assert False, "Unknown type: {}".format(idl_type.syntactic_form)
 
@@ -369,6 +385,9 @@ def make_default_value_expr(idl_type, default_value):
       else
         var = ${assignment_value};
     """
+    assert isinstance(idl_type, web_idl.IdlType)
+    assert (default_value is None
+            or isinstance(default_value, web_idl.LiteralConstant))
     assert default_value.is_type_compatible_with(idl_type)
 
     class DefaultValueExpr:
@@ -520,8 +539,7 @@ def make_default_value_expr(idl_type, default_value):
 def make_v8_to_blink_value(blink_var_name,
                            v8_value_expr,
                            idl_type,
-                           argument_index=None,
-                           default_value=None,
+                           argument=None,
                            cg_context=None):
     """
     Returns a SymbolNode whose definition converts a v8::Value to a Blink value.
@@ -529,9 +547,7 @@ def make_v8_to_blink_value(blink_var_name,
     assert isinstance(blink_var_name, str)
     assert isinstance(v8_value_expr, str)
     assert isinstance(idl_type, web_idl.IdlType)
-    assert (argument_index is None or isinstance(argument_index, (int, long)))
-    assert (default_value is None
-            or isinstance(default_value, web_idl.LiteralConstant))
+    assert argument is None or isinstance(argument, web_idl.Argument)
 
     T = TextNode
     F = lambda *args, **kwargs: T(_format(*args, **kwargs))
@@ -565,26 +581,28 @@ def make_v8_to_blink_value(blink_var_name,
             v8_value_expr)
 
     def create_definition(symbol_node):
-        if argument_index is None:
+        if argument is None:
             func_name = "NativeValue"
             arguments = ["${isolate}", v8_value_expr, "${exception_state}"]
         else:
             func_name = "ArgumentValue"
             arguments = [
                 "${isolate}",
-                str(argument_index),
+                str(argument.index),
                 v8_value_expr,
                 "${exception_state}",
             ]
         if "StringContext" in idl_type.effective_annotations:
             arguments.append("${execution_context_of_document_tree}")
-        blink_value_expr = _format(
-            "NativeValueTraits<{_1}>::{_2}({_3})",
-            _1=native_value_tag(idl_type),
+        blink_value_expr = _format("NativeValueTraits<{_1}>::{_2}({_3})",
+                                   _1=native_value_tag(idl_type, argument),
                                    _2=func_name,
                                    _3=", ".join(arguments))
-        default_expr = (make_default_value_expr(idl_type, default_value)
-                        if default_value else None)
+        if argument and argument.default_value:
+            default_expr = make_default_value_expr(idl_type,
+                                                   argument.default_value)
+        else:
+            default_expr = None
         exception_exit_node = CxxUnlikelyIfNode(
             cond="${exception_state}.HadException()", body=T("return;"))
 
@@ -598,7 +616,8 @@ def make_v8_to_blink_value(blink_var_name,
             "decltype(NativeValueTraits<{}>::NativeValue("
             "std::declval<v8::Isolate*>(), "
             "std::declval<v8::Local<v8::Value>>(), "
-            "std::declval<ExceptionState&>()))", native_value_tag(idl_type))
+            "std::declval<ExceptionState&>()))",
+            native_value_tag(idl_type, argument))
         if default_expr and default_expr.is_initialization_lightweight:
             pattern = "{} ${{{}}}{{{}}};"
             args = [

third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

@@ -244,11 +244,13 @@ const auto ${arg1_value} = arg1_value_maybe_enum.value();
                                    cg_context.attribute.idl_type))
         return
 
-    for index, argument in enumerate(cg_context.function_like.arguments):
-        name = name_style.arg_f("arg{}_{}", index + 1, argument.identifier)
+    for argument in cg_context.function_like.arguments:
+        name = name_style.arg_f("arg{}_{}", argument.index + 1,
+                                argument.identifier)
         if argument.is_variadic:
             code_node.register_code_symbol(
-                make_v8_to_blink_value_variadic(name, "${info}", index,
+                make_v8_to_blink_value_variadic(name, "${info}",
+                                                argument.index,
                                                 argument.idl_type))
         else:
             v8_value = "${{info}}[{}]".format(argument.index)
@@ -256,8 +258,7 @@ const auto ${arg1_value} = arg1_value_maybe_enum.value();
                 make_v8_to_blink_value(name,
                                        v8_value,
                                        argument.idl_type,
-                                       argument_index=index,
-                                       default_value=argument.default_value,
+                                       argument=argument,
                                        cg_context=cg_context))
 
 
@@ -2435,7 +2436,7 @@ if (${info}.ShouldThrowOnError()) {
             "blink_property_value",
             "${v8_property_value}",
             cg_context.indexed_property_setter.arguments[1].idl_type,
-            argument_index=2))
+            argument=cg_context.indexed_property_setter.arguments[1]))
 
     body.extend([
         TextNode("""\
@@ -2797,7 +2798,7 @@ if (!is_creating) {
             "blink_property_value",
             "${v8_property_value}",
             cg_context.named_property_setter.arguments[1].idl_type,
-            argument_index=2))
+            argument=cg_context.named_property_setter.arguments[1]))
 
     if "Custom" in cg_context.named_property_setter.extended_attributes:
         text = _format(

third_party/blink/renderer/bindings/scripts/web_idl/__init__.py

@@ -35,6 +35,7 @@ def _setup_sys_path():
 _setup_sys_path()
 
 from . import file_io
+from .argument import Argument
 from .ast_group import AstGroup
 from .attribute import Attribute
 from .callback_function import CallbackFunction

third_party/blink/web_tests/fast/forms/select/collection-setter-getter-expected.txt

@@ -12,7 +12,7 @@ PASS set_options.length is 3
 4) trying to set an element to a non-Option value: null
 PASS set_options.length is 3
 5) trying to set an element to a non-Option value: form (object of incorrect type)
-PASS set_options[10] = my_form threw exception TypeError: Failed to set an indexed property on 'HTMLOptionsCollection': parameter 3 is not of type 'HTMLOptionElement'..
+PASS set_options[10] = my_form threw exception TypeError: Failed to set an indexed property on 'HTMLOptionsCollection': parameter 2 is not of type 'HTMLOptionElement'..
 PASS set_options.length is 3
 PASS successfullyParsed is true
 

third_party/blink/web_tests/fast/forms/select/exceptions-expected.txt

@@ -2,7 +2,7 @@ This test should trigger the single exception on HTMLSelectElement, and verify t
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
-PASS el[1] = 0; threw exception TypeError: Failed to set an indexed property on 'HTMLSelectElement': parameter 3 is not of type 'HTMLOptionElement'..
+PASS el[1] = 0; threw exception TypeError: Failed to set an indexed property on 'HTMLSelectElement': parameter 2 is not of type 'HTMLOptionElement'..
 PASS el.namedItem() threw exception TypeError: Failed to execute 'namedItem' on 'HTMLSelectElement': 1 argument required, but only 0 present..
 PASS successfullyParsed is true
 

third_party/blink/web_tests/fast/forms/select/select-selectedIndex-expected.txt

@@ -59,7 +59,7 @@ PASS mySelect.selectedIndex is -1
 PASS mySelect.options.length is 10
 PASS mySelect.selectedIndex is -1
 19) trying to set an element that's not an option: select element
-PASS mySelect.options[10] = mySelect; threw exception TypeError: Failed to set an indexed property on 'HTMLOptionsCollection': parameter 3 is not of type 'HTMLOptionElement'..
+PASS mySelect.options[10] = mySelect; threw exception TypeError: Failed to set an indexed property on 'HTMLOptionsCollection': parameter 2 is not of type 'HTMLOptionElement'..
 PASS mySelect.options.length is 10
 PASS mySelect.selectedIndex is -1
 20) trying to set a option element using an invalid index: negative infinity

third_party/blink/web_tests/fast/forms/select/select-selectedIndex-multiple-expected.txt

@@ -59,7 +59,7 @@ PASS mySelect.selectedIndex is 0
 PASS mySelect.options.length is 10
 PASS mySelect.selectedIndex is 0
 19) trying to set an element that's not an option: select element
-PASS mySelect.options[10] = mySelect; threw exception TypeError: Failed to set an indexed property on 'HTMLOptionsCollection': parameter 3 is not of type 'HTMLOptionElement'..
+PASS mySelect.options[10] = mySelect; threw exception TypeError: Failed to set an indexed property on 'HTMLOptionsCollection': parameter 2 is not of type 'HTMLOptionElement'..
 PASS mySelect.options.length is 10
 PASS mySelect.selectedIndex is 0
 20) trying to set a option element using an invalid index: negative infinity

third_party/blink/web_tests/svg/dom/SVGNumberList-basics-expected.txt

@@ -90,10 +90,10 @@ PASS text1.rotate.baseVal.replaceItem(null, 0) threw exception TypeError: Failed
 
 
 Test uncommon values for indexed setter
-PASS text1.rotate.baseVal[0] = 30 threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 3 is not of type 'SVGNumber'..
-PASS text1.rotate.baseVal[0] = 'aString' threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 3 is not of type 'SVGNumber'..
-PASS text1.rotate.baseVal[0] = text1 threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 3 is not of type 'SVGNumber'..
-PASS text1.rotate.baseVal[0] = null threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 3 is not of type 'SVGNumber'..
+PASS text1.rotate.baseVal[0] = 30 threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 2 is not of type 'SVGNumber'..
+PASS text1.rotate.baseVal[0] = 'aString' threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 2 is not of type 'SVGNumber'..
+PASS text1.rotate.baseVal[0] = text1 threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 2 is not of type 'SVGNumber'..
+PASS text1.rotate.baseVal[0] = null threw exception TypeError: Failed to set an indexed property on 'SVGNumberList': parameter 2 is not of type 'SVGNumber'..
 PASS text1.rotate.baseVal.replaceItem(text1.rotate.baseVal.getItem(0), 0) is text1.rotate.baseVal.getItem(0)
 PASS text1.rotate.baseVal.numberOfItems is 4
 PASS text1.rotate.baseVal.getItem(0).value is 1

third_party/blink/web_tests/svg/dom/SVGPointList-basics-expected.txt

@@ -103,10 +103,10 @@ PASS poly1.points.replaceItem(1, 0) threw exception TypeError: Failed to execute
 
 
 Test uncommon values for indexed setter
-PASS poly1.points[0] = 30 threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 3 is not of type 'SVGPoint'..
-PASS poly1.points[0] = 'aString' threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 3 is not of type 'SVGPoint'..
-PASS poly1.points[0] = poly1 threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 3 is not of type 'SVGPoint'..
-PASS poly1.points[0] = null threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 3 is not of type 'SVGPoint'..
+PASS poly1.points[0] = 30 threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 2 is not of type 'SVGPoint'..
+PASS poly1.points[0] = 'aString' threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 2 is not of type 'SVGPoint'..
+PASS poly1.points[0] = poly1 threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 2 is not of type 'SVGPoint'..
+PASS poly1.points[0] = null threw exception TypeError: Failed to set an indexed property on 'SVGPointList': parameter 2 is not of type 'SVGPoint'..
 
 
 Test uncommon arguments for replaceItem() and xml-dom synchronization