Bug 172218 - URLSearchParams / Headers objects @@iterator is not as per Web IDL spec
Summary: URLSearchParams / Headers objects @@iterator is not as per Web IDL spec
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://heycam.github.io/webidl/#es-i...
Keywords:
Depends on: 172247 172333 172342
Blocks:
  Show dependency treegraph
 
Reported: 2017-05-17 07:12 PDT by Anne van Kesteren
Modified: 2017-05-19 16:07 PDT (History)
6 users (show)

See Also:


Attachments
WIP Patch (41.33 KB, patch)
2017-05-17 12:56 PDT, Chris Dumez
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.17 MB, application/zip)
2017-05-17 13:48 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-elcapitan (2.25 MB, application/zip)
2017-05-17 14:13 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (14.78 MB, application/zip)
2017-05-17 14:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews101 for mac-elcapitan (1.55 MB, application/zip)
2017-05-17 14:56 PDT, Build Bot
no flags Details
WIP Patch (41.33 KB, patch)
2017-05-18 21:38 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (2.79 KB, patch)
2017-05-18 21:39 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-elcapitan (1.84 MB, application/zip)
2017-05-18 22:48 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews101 for mac-elcapitan (948.58 KB, application/zip)
2017-05-18 22:51 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (869.23 KB, application/zip)
2017-05-18 22:54 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (744.19 KB, application/zip)
2017-05-18 23:16 PDT, Build Bot
no flags Details
Patch (17.00 KB, patch)
2017-05-19 14:29 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (18.77 KB, patch)
2017-05-19 15:26 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Chris Dumez 2017-05-17 12:56:20 PDT
(In reply to Anne van Kesteren from comment #0)
> Tests:
> 
>   https://w3c-test.org/url/interfaces.any.html
>   https://w3c-test.org/url/interfaces.any.worker.html

Hmm, when I import this test locally, I get:
Harness Error (FAIL), message = Unterminated generic type record, line 20 (tokens: ', USVString> ')
[
    {
        "type": "other",
        "value": ","
    },
    {
        "type": "whitespace",
        "value": " "
    },
    {
        "type": "identifier",
        "value": "USVString"
    },
    {
        "type": "other",
        "value": ">"
    },
    {
        "type": "whitespace",
        "value": " "
    }
]

PASS Untitled


It is running fine on w3c-test.org though :/ I tried updating our idlharness.js but it did not help.
Comment 2 Chris Dumez 2017-05-17 12:56:52 PDT
Created attachment 310431 [details]
WIP Patch
Comment 3 Build Bot 2017-05-17 12:58:53 PDT
Comment on attachment 310431 [details]
WIP Patch

Attachment 310431 [details] did not pass bindings-ews (mac):
Output: http://webkit-queues.webkit.org/results/3764419

New failing tests:
(JS) JSTestIterable.cpp
(JS) JSMapLike.cpp
(JS) JSTestNode.cpp
(JS) JSReadOnlyMapLike.cpp
Comment 4 Build Bot 2017-05-17 13:48:15 PDT
Comment on attachment 310431 [details]
WIP Patch

Attachment 310431 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3764525

New failing tests:
fast/text/unicode-range-javascript.html
Comment 5 Build Bot 2017-05-17 13:48:17 PDT
Created attachment 310437 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 6 Build Bot 2017-05-17 14:13:11 PDT
Comment on attachment 310431 [details]
WIP Patch

Attachment 310431 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3764537

New failing tests:
imported/w3c/web-platform-tests/url/a-element-origin-xhtml.xhtml
imported/w3c/web-platform-tests/XMLHttpRequest/interfaces.html
imported/w3c/web-platform-tests/media-source/interfaces.html
imported/w3c/web-platform-tests/url/failure.html
imported/w3c/web-platform-tests/url/a-element-origin.html
imported/w3c/web-platform-tests/fetch/api/headers/headers-idl.html
imported/w3c/web-platform-tests/hr-time/idlharness.html
imported/w3c/web-platform-tests/encoding/idlharness.html
imported/w3c/web-platform-tests/WebCryptoAPI/idlharness.html
imported/w3c/web-platform-tests/url/url-origin.html
imported/w3c/web-platform-tests/fetch/api/request/request-idl.html
imported/w3c/web-platform-tests/FileAPI/idlharness.worker.html
imported/w3c/web-platform-tests/fetch/api/response/response-idl.html
imported/w3c/web-platform-tests/url/url-constructor.html
imported/w3c/web-platform-tests/url/a-element-xhtml.xhtml
imported/w3c/web-platform-tests/IndexedDB/interfaces.worker.html
imported/w3c/web-platform-tests/resource-timing/idlharness.html
imported/w3c/web-platform-tests/WebCryptoAPI/idlharness.worker.html
imported/w3c/web-platform-tests/background-fetch/interfaces.worker.html
imported/w3c/web-platform-tests/url/historical.worker.html
imported/w3c/web-platform-tests/IndexedDB/interfaces.html
imported/w3c/web-platform-tests/notifications/interfaces.html
fast/text/unicode-range-javascript.html
imported/w3c/web-platform-tests/url/a-element.html
imported/w3c/web-platform-tests/FileAPI/idlharness.html
imported/w3c/web-platform-tests/url/urlsearchparams-constructor.html
Comment 7 Build Bot 2017-05-17 14:13:12 PDT
Created attachment 310444 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 8 Build Bot 2017-05-17 14:25:53 PDT
Comment on attachment 310431 [details]
WIP Patch

Attachment 310431 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3764574

New failing tests:
fast/text/unicode-range-javascript.html
Comment 9 Build Bot 2017-05-17 14:25:55 PDT
Created attachment 310445 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 10 Build Bot 2017-05-17 14:56:25 PDT
Comment on attachment 310431 [details]
WIP Patch

Attachment 310431 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3764824

New failing tests:
imported/w3c/web-platform-tests/url/a-element-origin-xhtml.xhtml
imported/w3c/web-platform-tests/XMLHttpRequest/interfaces.html
imported/w3c/web-platform-tests/media-source/interfaces.html
imported/w3c/web-platform-tests/url/failure.html
imported/w3c/web-platform-tests/url/a-element-origin.html
imported/w3c/web-platform-tests/fetch/api/headers/headers-idl.html
imported/w3c/web-platform-tests/hr-time/idlharness.html
imported/w3c/web-platform-tests/encoding/idlharness.html
imported/w3c/web-platform-tests/WebCryptoAPI/idlharness.html
imported/w3c/web-platform-tests/url/url-origin.html
imported/w3c/web-platform-tests/fetch/api/request/request-idl.html
imported/w3c/web-platform-tests/FileAPI/idlharness.worker.html
imported/w3c/web-platform-tests/fetch/api/response/response-idl.html
imported/w3c/web-platform-tests/url/url-constructor.html
imported/w3c/web-platform-tests/url/a-element-xhtml.xhtml
imported/w3c/web-platform-tests/IndexedDB/interfaces.worker.html
imported/w3c/web-platform-tests/dom/interfaces.html
imported/w3c/web-platform-tests/resource-timing/idlharness.html
imported/w3c/web-platform-tests/WebCryptoAPI/idlharness.worker.html
imported/w3c/web-platform-tests/background-fetch/interfaces.worker.html
imported/w3c/web-platform-tests/url/historical.worker.html
imported/w3c/web-platform-tests/IndexedDB/interfaces.html
imported/w3c/web-platform-tests/notifications/interfaces.html
fast/text/unicode-range-javascript.html
imported/w3c/web-platform-tests/url/a-element.html
imported/w3c/web-platform-tests/FileAPI/idlharness.html
imported/w3c/web-platform-tests/url/urlsearchparams-constructor.html
Comment 11 Build Bot 2017-05-17 14:56:26 PDT
Created attachment 310448 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 12 Chris Dumez 2017-05-17 15:19:20 PDT
(In reply to Chris Dumez from comment #1)
> (In reply to Anne van Kesteren from comment #0)
> > Tests:
> > 
> >   https://w3c-test.org/url/interfaces.any.html
> >   https://w3c-test.org/url/interfaces.any.worker.html
> 
> Hmm, when I import this test locally, I get:
> Harness Error (FAIL), message = Unterminated generic type record, line 20
> (tokens: ', USVString> ')
> [
>     {
>         "type": "other",
>         "value": ","
>     },
>     {
>         "type": "whitespace",
>         "value": " "
>     },
>     {
>         "type": "identifier",
>         "value": "USVString"
>     },
>     {
>         "type": "other",
>         "value": ">"
>     },
>     {
>         "type": "whitespace",
>         "value": " "
>     }
> ]
> 
> PASS Untitled
> 
> 
> It is running fine on w3c-test.org though :/ I tried updating our
> idlharness.js but it did not help.

Tentatively re-syncing our WPT tools via Bug 172247 in hope that it addresses this issue.
Comment 13 Chris Dumez 2017-05-18 21:38:51 PDT
Created attachment 310605 [details]
WIP Patch
Comment 14 Chris Dumez 2017-05-18 21:39:37 PDT
Created attachment 310606 [details]
WIP Patch
Comment 15 Build Bot 2017-05-18 21:41:35 PDT
Comment on attachment 310606 [details]
WIP Patch

Attachment 310606 [details] did not pass bindings-ews (mac):
Output: http://webkit-queues.webkit.org/results/3773846

New failing tests:
(JS) JSTestIterable.cpp
(JS) JSMapLike.cpp
(JS) JSTestNode.cpp
(JS) JSReadOnlyMapLike.cpp
Comment 16 Chris Dumez 2017-05-18 22:26:51 PDT
(In reply to Chris Dumez from comment #1)
> (In reply to Anne van Kesteren from comment #0)
> > Tests:
> > 
> >   https://w3c-test.org/url/interfaces.any.html
> >   https://w3c-test.org/url/interfaces.any.worker.html
> 
> Hmm, when I import this test locally, I get:
> Harness Error (FAIL), message = Unterminated generic type record, line 20
> (tokens: ', USVString> ')
> [
>     {
>         "type": "other",
>         "value": ","
>     },
>     {
>         "type": "whitespace",
>         "value": " "
>     },
>     {
>         "type": "identifier",
>         "value": "USVString"
>     },
>     {
>         "type": "other",
>         "value": ">"
>     },
>     {
>         "type": "whitespace",
>         "value": " "
>     }
> ]
> 
> PASS Untitled
> 
> 
> It is running fine on w3c-test.org though :/ I tried updating our
> idlharness.js but it did not help.

Found the issue. It is webbidl2.js that is outdated. Updating it via Bug 172342.
Comment 17 Build Bot 2017-05-18 22:48:41 PDT
Comment on attachment 310606 [details]
WIP Patch

Attachment 310606 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3773931

New failing tests:
fast/text/unicode-range-javascript.html
Comment 18 Build Bot 2017-05-18 22:48:42 PDT
Created attachment 310620 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 19 Build Bot 2017-05-18 22:50:59 PDT
Comment on attachment 310606 [details]
WIP Patch

Attachment 310606 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3774004

New failing tests:
fast/text/unicode-range-javascript.html
Comment 20 Build Bot 2017-05-18 22:51:00 PDT
Created attachment 310621 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 21 Build Bot 2017-05-18 22:54:07 PDT
Comment on attachment 310606 [details]
WIP Patch

Attachment 310606 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3774007

New failing tests:
fast/text/unicode-range-javascript.html
Comment 22 Build Bot 2017-05-18 22:54:08 PDT
Created attachment 310622 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 23 Build Bot 2017-05-18 23:16:40 PDT
Comment on attachment 310606 [details]
WIP Patch

Attachment 310606 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3774041

New failing tests:
fast/text/unicode-range-javascript.html
Comment 24 Build Bot 2017-05-18 23:16:42 PDT
Created attachment 310623 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 25 Chris Dumez 2017-05-19 14:29:09 PDT
Created attachment 310701 [details]
Patch
Comment 26 youenn fablet 2017-05-19 15:02:02 PDT
Comment on attachment 310701 [details]
Patch

Looks better indeed!
Might want to do some additional clean-up, see below.
Maybe we do not need to add Symbol.Iterator at all in IDLParser.pm and handle it directly in CodeGeneratorJS.pm

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

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5439
> +            $iterationKind = "Value" if $propertyName eq "entries" and not $interface->iterable->isKeyValue;

We should not need to do anything with Symbol.Iterator here since we are reusing entries or values for that.
We are generating code for nothing probably. Can we remove that case?
Probably also for these function declarations as well.

> Source/WebCore/bindings/scripts/test/JS/JSTestNode.cpp:300
>  

Yep, jsTestNodePrototypeFunctionSymbolIterator is probably not needed.
Comment 27 Chris Dumez 2017-05-19 15:26:47 PDT
Created attachment 310709 [details]
Patch
Comment 28 WebKit Commit Bot 2017-05-19 16:07:44 PDT
Comment on attachment 310709 [details]
Patch

Clearing flags on attachment: 310709

Committed r217166: <http://trac.webkit.org/changeset/217166>
Comment 29 WebKit Commit Bot 2017-05-19 16:07:46 PDT
All reviewed patches have been landed.  Closing bug.