Bug 189136 - Omitting an optional argument when calling a WebIDL operation results in a "required member" TypeError if the argument type is a dictionary with required members
Summary: Omitting an optional argument when calling a WebIDL operation results in a "r...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-29 18:12 PDT by Andy Estes
Modified: 2018-08-29 20:08 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2018-08-29 18:12:09 PDT
Payment Request defines a dictionary that has a required member:

    dictionary PaymentMethodChangeEventInit : PaymentRequestUpdateEventInit {
        required DOMString methodName;
        object? methodDetails;
    };

It also defines an interface (PaymentMethodChangeEvent) with a constructor that takes an optional PaymentMethodChangeEventInit:

    Constructor(DOMString type, optional PaymentMethodChangeEventInit eventInitDict)

If you call the constructor, omitting the second argument, you get a TypeError about a missing required field (methodName).

Despite eventInitDict being optional, WebKit still generates code that tries to convert undefined to a PaymentMethodChangeEventInit struct:

    auto eventInitDict = convert<IDLDictionary<PaymentMethodChangeEventInit>>(*state, state->argument(1));

And that code throws when converting undefined with a required member:

    JSValue methodNameValue;
    if (isNullOrUndefined)
        methodNameValue = jsUndefined();
    else {
        methodNameValue = object->get(&state, Identifier::fromString(&state, "methodName"));
        RETURN_IF_EXCEPTION(throwScope, { });
    }
    if (!methodNameValue.isUndefined()) {
        result.methodName = convert<IDLDOMString>(state, methodNameValue);
        RETURN_IF_EXCEPTION(throwScope, { });
    } else {
        throwRequiredMemberTypeError(state, throwScope, "methodName", "PaymentMethodChangeEventInit", "DOMString");
        return { };
    }

I don't know if it's legal per WebIDL to have optional arguments to structs with required members, but if it is then this seems like a bug in our bindings generation.
Comment 1 Andy Estes 2018-08-29 18:55:47 PDT
CodeGeneratorJS.pm thinks that an optional dictionary without a default value has an implicit default value of []:

    if ($argument->isOptional && !defined($argument->default)) {
        # As per Web IDL, optional dictionary arguments are always considered to have a default value of an empty dictionary, unless otherwise specified.
        $argument->default("[]") if $codeGenerator->IsDictionaryType($type);

But [] is an invalid value for a dictionary with required members.

I can't find where in the WebIDL spec it says this should be the default, but I'm not super familiar with it.
Comment 2 Darin Adler 2018-08-29 20:05:33 PDT
The rule in question in the latest WebIDL draft SAYS this:

"If the type of an argument is a dictionary type or a union type that has a dictionary as one of its flattened member types, and that dictionary type and its ancestors have no required members, and the argument is either the final argument or is followed only by optional arguments, then the argument must be specified as optional. Such arguments are always considered to have a default value of an empty dictionary, unless otherwise specified."

Note that this rule does not apply to a dictionary with required members. So I think in C++ we need to represent such an argument using std::optional rather than by relying on a default value.
Comment 3 Chris Dumez 2018-08-29 20:08:14 PDT
I filed:
https://github.com/w3c/payment-request/issues/770

I do not think we should have optional dictionary parameters with required members.