Changeset 186275 in webkit


Ignore:
Timestamp:
Jul 3, 2015 8:56:16 PM (9 years ago)
Author:
Chris Dumez
Message:

REGRESSION (r178097): HTMLSelectElement.add(option, undefined) prepends option to the list of options; should append to the end of the list of options
​https://bugs.webkit.org/show_bug.cgi?id=146566
<rdar://problem/21663919>

Reviewed by Ryosuke Niwa.

Source/WebCore:

HTMLSelectElement.add(X, undefined) is supposed to be equivalent to
HTMLSelectElement.add(X) which should *append* X. The same is true
for HTMLOptionsCollection.add(X, undefined).

However, due to a bug in our bindings generator for overloaded
operations, the actual behavior was not the expected one. The
second overload would be chosen: add(X, index) and undefined would
be converted as 0-index, which would *prepend* X.

This patch fixes the bindings generator so that undefined is allowed
for optional parameters of an overload operation, when doing the
overload resolution.

Tests:

  • fast/dom/HTMLSelectElement/add.html
  • fast/dom/HTMLSelectElement/options-collection-add.html
  • http/tests/websocket/tests/hybi/undefined-protocol.html
  • bindings/scripts/CodeGeneratorJS.pm:

(GenerateParametersCheckExpression):
Allow undefined value for optional parameters when doing the overload
resolution.

  • bindings/scripts/test/JS/JSTestObj.cpp:

(WebCore::jsTestObjPrototypeFunctionOverloadedMethod):
(WebCore::jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter1):
(WebCore::jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter2):
(WebCore::jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter):

  • bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp:

(WebCore::JSTestOverloadedConstructorsConstructor::constructJSTestOverloadedConstructors):

  • bindings/scripts/test/TestObj.idl:

Add bindings tests coverage and rebaseline.

LayoutTests:

  • fast/dom/HTMLSelectElement/add-expected.txt:
  • fast/dom/HTMLSelectElement/add.html:
  • fast/dom/HTMLSelectElement/options-collection-add-expected.txt:
  • fast/dom/HTMLSelectElement/options-collection-add.html:

Update tests so that calling add(X, undefined) is expected to append X,
not prepend it.

  • http/tests/websocket/tests/hybi/undefined-protocol-expected.txt: Added.
  • http/tests/websocket/tests/hybi/undefined-protocol.html: Added.

Add test coverage for "new WebSocket(url, undefined)" as WebSocket is
using constructor overloads with optional parameters. Previously, calling
new WebSocket(url, undefined) was equivalent to calling
new WebSocket(url, "undefined") even though it is supposed to be
equivalent to calling new WebSocket(url).

Location:
trunk
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r186266 r186275  
     12015-07-03  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION (r178097): HTMLSelectElement.add(option, undefined) prepends option to the list of options; should append to the end of the list of options
     4        https://bugs.webkit.org/show_bug.cgi?id=146566
     5        <rdar://problem/21663919>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        * fast/dom/HTMLSelectElement/add-expected.txt:
     10        * fast/dom/HTMLSelectElement/add.html:
     11        * fast/dom/HTMLSelectElement/options-collection-add-expected.txt:
     12        * fast/dom/HTMLSelectElement/options-collection-add.html:
     13        Update tests so that calling add(X, undefined) is expected to append X,
     14        not prepend it.
     15
     16        * http/tests/websocket/tests/hybi/undefined-protocol-expected.txt: Added.
     17        * http/tests/websocket/tests/hybi/undefined-protocol.html: Added.
     18        Add test coverage for "new WebSocket(url, undefined)" as WebSocket is
     19        using constructor overloads with optional parameters. Previously, calling
     20        new WebSocket(url, undefined) was equivalent to calling
     21        new WebSocket(url, "undefined") even though it is supposed to be
     22        equivalent to calling new WebSocket(url).
     23
    1242015-07-03  Chris Dumez  <cdumez@apple.com>
    225

  • trunk/LayoutTests/fast/dom/HTMLSelectElement/add-expected.txt

    r186265 r186275  
    3636PASS testAdd(createOption("Y21"), "foo") is "Y21,0,1,2"
    3737PASS testAdd(createOption("Y22"), NaN) is "Y22,0,1,2"
    38 PASS testAdd(createOption("Y23"), undefined) is "Y23,0,1,2"
     38PASS testAdd(createOption("Y23"), undefined) is "0,1,2,Y23"
    3939PASS testAdd(createOption("Y24"), -2) is "0,1,2,Y24"
    4040PASS testAdd(createOption("X"), mySelect.options[0]) is "X,0,1,2"

  • trunk/LayoutTests/fast/dom/HTMLSelectElement/add.html

    r186265 r186275  
    5050shouldBeEqualToString('testAdd(createOption("Y21"), "foo")', "Y21,0,1,2");
    5151shouldBeEqualToString('testAdd(createOption("Y22"), NaN)', "Y22,0,1,2");
    52 shouldBeEqualToString('testAdd(createOption("Y23"), undefined)', "Y23,0,1,2");
     52shouldBeEqualToString('testAdd(createOption("Y23"), undefined)', "0,1,2,Y23");
    5353shouldBeEqualToString('testAdd(createOption("Y24"), -2)', "0,1,2,Y24");
    5454shouldBeEqualToString('testAdd(createOption("X"), mySelect.options[0])', "X,0,1,2");

  • trunk/LayoutTests/fast/dom/HTMLSelectElement/options-collection-add-expected.txt

    r186265 r186275  
    4646PASS testAdd(createOption("Y21"), "foo") is "Y21,0,1,2"
    4747PASS testAdd(createOption("Y22"), NaN) is "Y22,0,1,2"
    48 PASS testAdd(createOption("Y23"), undefined) is "Y23,0,1,2"
     48PASS testAdd(createOption("Y23"), undefined) is "0,1,2,Y23"
    4949PASS testAdd(createOption("Y24"), -2) is "0,1,2,Y24"
    5050PASS testAdd(createOption("X"), mySelect.options[0]) is "X,0,1,2"

  • trunk/LayoutTests/fast/dom/HTMLSelectElement/options-collection-add.html

    r186265 r186275  
    7070shouldBeEqualToString('testAdd(createOption("Y21"), "foo")', "Y21,0,1,2");
    7171shouldBeEqualToString('testAdd(createOption("Y22"), NaN)', "Y22,0,1,2");
    72 shouldBeEqualToString('testAdd(createOption("Y23"), undefined)', "Y23,0,1,2");
     72shouldBeEqualToString('testAdd(createOption("Y23"), undefined)', "0,1,2,Y23");
    7373shouldBeEqualToString('testAdd(createOption("Y24"), -2)', "0,1,2,Y24");
    7474shouldBeEqualToString('testAdd(createOption("X"), mySelect.options[0])', "X,0,1,2");

  • trunk/Source/WebCore/ChangeLog

    r186272 r186275  
     12015-07-03  Chris Dumez  <cdumez@apple.com>
     2
     3        REGRESSION (r178097): HTMLSelectElement.add(option, undefined) prepends option to the list of options; should append to the end of the list of options
     4        https://bugs.webkit.org/show_bug.cgi?id=146566
     5        <rdar://problem/21663919>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        HTMLSelectElement.add(X, undefined) is supposed to be equivalent to
     10        HTMLSelectElement.add(X) which should *append* X. The same is true
     11        for HTMLOptionsCollection.add(X, undefined).
     12
     13        However, due to a bug in our bindings generator for overloaded
     14        operations, the actual behavior was not the expected one. The
     15        second overload would be chosen: add(X, index) and undefined would
     16        be converted as 0-index, which would *prepend* X.
     17
     18        This patch fixes the bindings generator so that undefined is allowed
     19        for optional parameters of an overload operation, when doing the
     20        overload resolution.
     21
     22        Tests:
     23        - fast/dom/HTMLSelectElement/add.html
     24        - fast/dom/HTMLSelectElement/options-collection-add.html
     25        - http/tests/websocket/tests/hybi/undefined-protocol.html
     26
     27        * bindings/scripts/CodeGeneratorJS.pm:
     28        (GenerateParametersCheckExpression):
     29        Allow undefined value for optional parameters when doing the overload
     30        resolution.
     31
     32        * bindings/scripts/test/JS/JSTestObj.cpp:
     33        (WebCore::jsTestObjPrototypeFunctionOverloadedMethod):
     34        (WebCore::jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter1):
     35        (WebCore::jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter2):
     36        (WebCore::jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter):
     37        * bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp:
     38        (WebCore::JSTestOverloadedConstructorsConstructor::constructJSTestOverloadedConstructors):
     39        * bindings/scripts/test/TestObj.idl:
     40        Add bindings tests coverage and rebaseline.
     41
    1422015-07-03  Dan Bernstein  <mitz@apple.com>
    243

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r186265 r186275  
    13841384            push(@andExpression, "(${value}.isNull() || ${value}.isFunction())");
    13851385            $usedArguments{$parameterIndex} = 1;
    1386         } elsif ($codeGenerator->GetArrayType($type) || $codeGenerator->GetSequenceType($type)) {
    1387             # FIXME: Add proper support for T[], T[]?, sequence<T>
    1388             if ($parameter->isNullable) {
    1389                 push(@andExpression, "(${value}.isNull() || (${value}.isObject() && isJSArray(${value})))");
     1386        } elsif (!IsNativeType($type)) {
     1387            my $condition = "";
     1388            $condition .= "${value}.isUndefined() || " if $parameter->isOptional;
     1389
     1390            # FIXME: WebIDL says that undefined is also acceptable for nullable parameters and
     1391            # should be converted to null:
     1392            # http://heycam.github.io/webidl/#es-nullable-type
     1393            $condition .= "${value}.isNull() || " if $parameter->isNullable;
     1394
     1395            if ($codeGenerator->GetArrayType($type) || $codeGenerator->GetSequenceType($type)) {
     1396                # FIXME: Add proper support for T[], T[]?, sequence<T>.
     1397                $condition .= "(${value}.isObject() && isJSArray(${value}))";
    13901398            } else {
    1391                 push(@andExpression, "(${value}.isObject() && isJSArray(${value}))");
    1392             }
    1393             $usedArguments{$parameterIndex} = 1;
    1394         } elsif (!IsNativeType($type)) {
    1395             if ($parameter->isNullable) {
    1396                 push(@andExpression, "(${value}.isNull() || (${value}.isObject() && asObject(${value})->inherits(JS${type}::info())))");
    1397             } else {
    1398                 push(@andExpression, "(${value}.isObject() && asObject(${value})->inherits(JS${type}::info()))");
    1399             }
     1399                $condition .= "(${value}.isObject() && asObject(${value})->inherits(JS${type}::info()))";
     1400            }
     1401            push(@andExpression, "(" . $condition . ")");
    14001402            $usedArguments{$parameterIndex} = 1;
    14011403        }

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp

    r186265 r186275  
    130130#endif
    131131JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethod(JSC::ExecState*);
     132JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter(JSC::ExecState*);
    132133JSC::EncodedJSValue JSC_HOST_CALL jsTestObjConstructorFunctionClassMethod(JSC::ExecState*);
    133134JSC::EncodedJSValue JSC_HOST_CALL jsTestObjConstructorFunctionClassMethodWithOptional(JSC::ExecState*);
    … …  
    634635#endif
    635636    { "overloadedMethod", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadedMethod), (intptr_t) (2) },
     637    { "overloadedMethodWithOptionalParameter", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter), (intptr_t) (1) },
    636638    { "classMethodWithClamp", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionClassMethodWithClamp), (intptr_t) (2) },
    637639    { "methodWithUnsignedLongSequence", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithUnsignedLongSequence), (intptr_t) (1) },
    … …  
    39563958    if ((argsCount == 1 && (arg0.isNull() || (arg0.isObject() && isJSArray(arg0)))))
    39573959        return jsTestObjPrototypeFunctionOverloadedMethod7(exec);
    3958     if ((argsCount == 1 && (arg0.isObject() && asObject(arg0)->inherits(JSTestObj::info()))))
     3960    if ((argsCount == 1 && ((arg0.isObject() && asObject(arg0)->inherits(JSTestObj::info())))))
    39593961        return jsTestObjPrototypeFunctionOverloadedMethod8(exec);
    3960     if ((argsCount == 1 && (arg0.isObject() && isJSArray(arg0))))
     3962    if ((argsCount == 1 && ((arg0.isObject() && isJSArray(arg0)))))
    39613963        return jsTestObjPrototypeFunctionOverloadedMethod9(exec);
    3962     if ((argsCount == 1 && (arg0.isObject() && isJSArray(arg0))))
     3964    if ((argsCount == 1 && ((arg0.isObject() && isJSArray(arg0)))))
    39633965        return jsTestObjPrototypeFunctionOverloadedMethod10(exec);
    39643966    if (argsCount == 1)
    … …  
    39663968    if ()
    39673969        return jsTestObjPrototypeFunctionOverloadedMethod12(exec);
     3970    if (argsCount < 1)
     3971        return throwVMError(exec, createNotEnoughArgumentsError(exec));
     3972    return throwVMTypeError(exec);
     3973}
     3974
     3975static EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter1(ExecState* exec)
     3976{
     3977    JSValue thisValue = exec->thisValue();
     3978    JSTestObj* castedThis = jsDynamicCast<JSTestObj*>(thisValue);
     3979    if (UNLIKELY(!castedThis))
     3980        return throwThisTypeError(*exec, "TestObj", "overloadedMethodWithOptionalParameter");
     3981    ASSERT_GC_OBJECT_INHERITS(castedThis, JSTestObj::info());
     3982    auto& impl = castedThis->impl();
     3983    if (UNLIKELY(exec->argumentCount() < 1))
     3984        return throwVMError(exec, createNotEnoughArgumentsError(exec));
     3985    TestObj* objArg1 = JSTestObj::toWrapped(exec->argument(0));
     3986    if (UNLIKELY(exec->hadException()))
     3987        return JSValue::encode(jsUndefined());
     3988
     3989    size_t argsCount = exec->argumentCount();
     3990    if (argsCount <= 1) {
     3991        impl.overloadedMethodWithOptionalParameter(objArg1);
     3992        return JSValue::encode(jsUndefined());
     3993    }
     3994
     3995    TestObj* objArg2 = JSTestObj::toWrapped(exec->argument(1));
     3996    if (UNLIKELY(exec->hadException()))
     3997        return JSValue::encode(jsUndefined());
     3998    impl.overloadedMethodWithOptionalParameter(objArg1, objArg2);
     3999    return JSValue::encode(jsUndefined());
     4000}
     4001
     4002static EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter2(ExecState* exec)
     4003{
     4004    JSValue thisValue = exec->thisValue();
     4005    JSTestObj* castedThis = jsDynamicCast<JSTestObj*>(thisValue);
     4006    if (UNLIKELY(!castedThis))
     4007        return throwThisTypeError(*exec, "TestObj", "overloadedMethodWithOptionalParameter");
     4008    ASSERT_GC_OBJECT_INHERITS(castedThis, JSTestObj::info());
     4009    auto& impl = castedThis->impl();
     4010    if (UNLIKELY(exec->argumentCount() < 1))
     4011        return throwVMError(exec, createNotEnoughArgumentsError(exec));
     4012    TestObj* objArg = JSTestObj::toWrapped(exec->argument(0));
     4013    if (UNLIKELY(exec->hadException()))
     4014        return JSValue::encode(jsUndefined());
     4015
     4016    size_t argsCount = exec->argumentCount();
     4017    if (argsCount <= 1) {
     4018        impl.overloadedMethodWithOptionalParameter(objArg);
     4019        return JSValue::encode(jsUndefined());
     4020    }
     4021
     4022    int longArg = toInt32(exec, exec->argument(1), NormalConversion);
     4023    if (UNLIKELY(exec->hadException()))
     4024        return JSValue::encode(jsUndefined());
     4025    impl.overloadedMethodWithOptionalParameter(objArg, longArg);
     4026    return JSValue::encode(jsUndefined());
     4027}
     4028
     4029EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter(ExecState* exec)
     4030{
     4031    size_t argsCount = std::min<size_t>(2, exec->argumentCount());
     4032    JSValue arg0(exec->argument(0));
     4033    JSValue arg1(exec->argument(1));
     4034    if ((argsCount == 1 && (arg0.isNull() || (arg0.isObject() && asObject(arg0)->inherits(JSTestObj::info())))) || (argsCount == 2 && (arg0.isNull() || (arg0.isObject() && asObject(arg0)->inherits(JSTestObj::info()))) && (arg1.isUndefined() || arg1.isNull() || (arg1.isObject() && asObject(arg1)->inherits(JSTestObj::info())))))
     4035        return jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter1(exec);
     4036    if ((argsCount == 1 && (arg0.isNull() || (arg0.isObject() && asObject(arg0)->inherits(JSTestObj::info())))) || (argsCount == 2 && (arg0.isNull() || (arg0.isObject() && asObject(arg0)->inherits(JSTestObj::info())))))
     4037        return jsTestObjPrototypeFunctionOverloadedMethodWithOptionalParameter2(exec);
    39684038    if (argsCount < 1)
    39694039        return throwVMError(exec, createNotEnoughArgumentsError(exec));

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp

    r186265 r186275  
    153153    size_t argsCount = std::min<size_t>(1, exec->argumentCount());
    154154    JSValue arg0(exec->argument(0));
    155     if ((argsCount == 1 && (arg0.isObject() && asObject(arg0)->inherits(JSArrayBuffer::info()))))
     155    if ((argsCount == 1 && ((arg0.isObject() && asObject(arg0)->inherits(JSArrayBuffer::info())))))
    156156        return JSTestOverloadedConstructorsConstructor::constructJSTestOverloadedConstructors1(exec);
    157     if ((argsCount == 1 && (arg0.isObject() && asObject(arg0)->inherits(JSArrayBufferView::info()))))
     157    if ((argsCount == 1 && ((arg0.isObject() && asObject(arg0)->inherits(JSArrayBufferView::info())))))
    158158        return JSTestOverloadedConstructorsConstructor::constructJSTestOverloadedConstructors2(exec);
    159     if ((argsCount == 1 && (arg0.isObject() && asObject(arg0)->inherits(JSBlob::info()))))
     159    if ((argsCount == 1 && ((arg0.isObject() && asObject(arg0)->inherits(JSBlob::info())))))
    160160        return JSTestOverloadedConstructorsConstructor::constructJSTestOverloadedConstructors3(exec);
    161161    if (argsCount == 1)

  • trunk/Source/WebCore/bindings/scripts/test/TestObj.idl

    r186265 r186275  
    191191    // FIXME: Implement support for overloaded functions with variadic arguments.
    192192    void    overloadedMethod(Blob... blobArgs);
     193
     194    void overloadedMethodWithOptionalParameter(TestObj? objArg1, optional TestObj? objArg2);
     195    void overloadedMethodWithOptionalParameter(TestObj? objArg, optional long longArg);
    193196#endif
    194197
Note: See TracChangeset for help on using the changeset viewer.