815502 - Implement variadic arguments for WebIDL methods

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

This is needed for things like the recent changes to ClassList.add/remove.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Implement support for variadic arguments in WebIDL.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 685723 [details] [diff] [review]
Implement support for variadic arguments in WebIDL.

Review of attachment 685723 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on the WebIDL.py bits.

Comment 3

[Peter Van der Beken [:peterv]]

Comment on attachment 685723 [details] [diff] [review]
Implement support for variadic arguments in WebIDL.

Review of attachment 685723 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Codegen.py
@@ +3068,5 @@
> +
> +        # NOTE: Keep this in sync with sequence conversions as needed
> +        variadicConversion = string.Template("""const ${seqType} ${declName};
> +if (${argc} > ${index}) {
> +  ${seqType} &arr = const_cast< ${seqType}& >(${declName});

I think we usually do |${seqType}& arr|.

@@ +3074,5 @@
> +  if (!arr.SetCapacity(${argc} - ${index})) {
> +    JS_ReportOutOfMemory(cx);
> +    return false;
> +  }
> +  for (uint32_t variadicArg = 0; variadicArg < length; ++variadicArg) {

How do you feel about making this go from ${index} to ${argc} (and val would just be ${argv}[variadicArg]). I think that would be slightly clearer.

@@ +6819,5 @@
>  
>  class CGNativeMember(ClassMethod):
>      def __init__(self, descriptor, member, name, signature, extendedAttrs,
>                   breakAfter=True, passCxAsNeeded=True, visibility="public",
> +                 jsObjectsArePtr=False, variadicIsSequence=False):

Could you explain variadicIsSequence, when would one pass in (the default) False?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> I think we usually do |${seqType}& arr|.

Done.

> How do you feel about making this go from ${index} to ${argc}

Done.

> Could you explain variadicIsSequence, when would one pass in (the default) False?

And have it actually matter?  Right now, for callback function codegen, which takes an nsTArray for variadics, not a Sequence.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Interdiff for the review comments

Comment 6

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7a5e83065d9f

Comment 7

[Boris Zbarsky [:bzbarsky]]

Documented at https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#typemapping

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a5e83065d9f