Bug 217612

Summary: [WebIDL] convertRecord() should handle duplicate keys for USVString records
Product: WebKit Reporter: Alexey Shvayka <ashvayka>
Component: BindingsAssignee: Alexey Shvayka <ashvayka>
Status: RESOLVED FIXED    
Severity: Trivial CC: achristensen, cdumez, clopez, ews-watchlist, sam, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://github.com/web-platform-tests/wpt/pull/26126
Attachments:
Description Flags
Patch
none
Patch none

Description Alexey Shvayka 2020-10-12 09:34:11 PDT
[WebIDL] convertRecord() should handle duplicate keys for USVString records
Comment 1 Alexey Shvayka 2020-10-12 11:13:25 PDT
Created attachment 411134 [details]
Patch
Comment 2 EWS Watchlist 2020-10-12 11:14:20 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 Sam Weinig 2020-10-12 11:43:30 PDT
Comment on attachment 411134 [details]
Patch

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

> LayoutTests/imported/w3c/ChangeLog:9
> +        * web-platform-tests/url/urlsearchparams-constructor.any-expected.txt:
> +        * web-platform-tests/url/urlsearchparams-constructor.any.js:

Usually we don't modify the imported tests directly, or if we do, try to get that change integrated upstream as well. Is this something you plan to do with this change?
Comment 4 Alexey Shvayka 2020-10-12 11:58:31 PDT
(In reply to Sam Weinig from comment #3)

Thank you for review, Sam.

> Usually we don't modify the imported tests directly, or if we do, try to get
> that change integrated upstream as well. Is this something you plan to do
> with this change?

I've got acquainted with WPT export process and make sure that tests are merged into upstream before landing a patch.
Comment 5 Sam Weinig 2020-10-12 13:08:30 PDT
(In reply to Alexey Shvayka from comment #4)
> (In reply to Sam Weinig from comment #3)
> 
> Thank you for review, Sam.
> 
> > Usually we don't modify the imported tests directly, or if we do, try to get
> > that change integrated upstream as well. Is this something you plan to do
> > with this change?
> 
> I've got acquainted with WPT export process and make sure that tests are
> merged into upstream before landing a patch.

Great. Thanks.
Comment 6 Radar WebKit Bug Importer 2020-10-19 09:35:25 PDT
<rdar://problem/70443299>
Comment 7 Alexey Shvayka 2020-10-19 13:30:07 PDT
Created attachment 411786 [details]
Patch

Adjust .worker subtest expectations.
Comment 8 EWS 2020-10-19 19:25:22 PDT
Committed r268709: <https://trac.webkit.org/changeset/268709>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411786 [details].
Comment 9 Alex Christensen 2020-10-21 16:53:59 PDT
Comment on attachment 411786 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMConvertRecord.h:134
> +                        auto index = result.findMatching([&](auto& entry) { return entry.key == typedKey; });

This is an O(n^2) algorithm.  See https://bugs.webkit.org/show_bug.cgi?id=218062