Bug 172410 – Value for iterator property is wrong for maplike interfaces

• Component: Bindings
• Status: RESOLVED FIXED

[Chris Dumez]

Value for iterator property is wrong for maplike interfaces.

Value for iterator property is wrong for maplike interfaces. The iterator property is supposed to have the same value as the 'entries' property but we currently use the value of the 'values' property.

Specification:
- https://heycam.github.io/webidl/#es-map-entries
- https://heycam.github.io/webidl/#es-iterator

[Chris Dumez]

Created attachment 310760 [details]
WIP Patch

This should be testable via the RTCStatsReport interface. I'll look into it tomorrow.

[Build Bot]

Attachment 310760 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Confirmed that:
Object.getOwnPropertyDescriptor(RTCStatsReport.prototype, Symbol.iterator).value === Object.getOwnPropertyDescriptor(RTCStatsReport.prototype, 'entries').value

in Firefox.

This does not work in WebKit because RTCStatsReport is wrongly marked as NoInterfaceObject :/

[Chris Dumez]

Created attachment 310772 [details]
Patch

[Sam Weinig]

Comment on attachment 310772 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310772&action=review

> Source/WebCore/Modules/mediastream/RTCStatsReport.idl:27
> +// FIXME: This should not be marked as [NoInterfaceObject].
>  [

Why not just do it now?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3469
>          if (InterfaceNeedsIterator($interface)) {
>              AddToImplIncludes("<builtins/BuiltinNames.h>");
> -            if (IsKeyValueIterableInterface($interface)) {
> +            if (IsKeyValueIterableInterface($interface) or $interface->mapLike) {

While you are in the neighborhood (not really, but whatever), consider removing the comment:

    # FIXME: This should return 1 for maplike once we support them.
    return 1 if $interface->mapLike;

in InterfaceNeedsIterator()

[Chris Dumez]

(In reply to Sam Weinig from comment #5)
> Comment on attachment 310772 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310772&action=review
> 
> > Source/WebCore/Modules/mediastream/RTCStatsReport.idl:27
> > +// FIXME: This should not be marked as [NoInterfaceObject].
> >  [
> 
> Why not just do it now?

Seems unrelated but I will follow up on this ASAP.

> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3469
> >          if (InterfaceNeedsIterator($interface)) {
> >              AddToImplIncludes("<builtins/BuiltinNames.h>");
> > -            if (IsKeyValueIterableInterface($interface)) {
> > +            if (IsKeyValueIterableInterface($interface) or $interface->mapLike) {
> 
> While you are in the neighborhood (not really, but whatever), consider
> removing the comment:
> 
>     # FIXME: This should return 1 for maplike once we support them.
>     return 1 if $interface->mapLike;
> 
> in InterfaceNeedsIterator()

Oh right, this should actually say setlike, not maplike.

[Chris Dumez]

Committed r217188: <http://trac.webkit.org/changeset/217188>