Bug 204847 - Add support for WebIDL set-like forEach
Summary: Add support for WebIDL set-like forEach
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 159140
Blocks:
  Show dependency treegraph
 
Reported: 2019-12-04 03:22 PST by youenn fablet
Modified: 2019-12-13 12:50 PST (History)
7 users (show)

See Also:


Attachments
Patch (17.08 KB, patch)
2019-12-04 10:03 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (19.61 KB, patch)
2019-12-06 00:32 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (19.67 KB, patch)
2019-12-08 08:15 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (25.70 KB, patch)
2019-12-09 00:05 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2019-12-04 03:22:44 PST
Add support for WebIDL set-like forEach
Comment 1 youenn fablet 2019-12-04 10:03:08 PST
Created attachment 384823 [details]
Patch
Comment 2 youenn fablet 2019-12-04 13:08:36 PST
 inspector/debugger/tail-deleted-frames/tail-deleted-frames-this-value.html is failing in Mac-debug-wk1, not Mac-wk1, so probably is fine.
Comment 3 youenn fablet 2019-12-06 00:32:20 PST
Created attachment 384993 [details]
Patch
Comment 4 Megan Gardner 2019-12-06 14:47:07 PST
Comment on attachment 384993 [details]
Patch

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

This is not my area, but lgtm, but we should probably get Chris or Darin or someone else to review this as well...

> Source/WebCore/bindings/js/JSDOMBindingInternals.js:39
> +    @getByIdDirectPrivate(this, "backingSet").forEach((value, key, map) => {

map? should this be 'set'?

> LayoutTests/platform/mac-wk1/TestExpectations:34
> +

I'm confused how this was affected at all
Comment 5 youenn fablet 2019-12-08 08:13:56 PST
> > Source/WebCore/bindings/js/JSDOMBindingInternals.js:39
> > +    @getByIdDirectPrivate(this, "backingSet").forEach((value, key, map) => {
> 
> map? should this be 'set'?

Right, sounds better, should not change the behaviour.

> > LayoutTests/platform/mac-wk1/TestExpectations:34
> > +
> 
> I'm confused how this was affected at all

I don't know either, I asked Joe and Devin and apparently this depends on which methods are executed or not before or as part of the test.

If I remove the JS-built-in for the setlike foreach, the test passes again.
Maybe the test should be rewritten to be more robust.
Comment 6 youenn fablet 2019-12-08 08:15:53 PST
Created attachment 385123 [details]
Patch
Comment 7 youenn fablet 2019-12-09 00:05:19 PST
Created attachment 385136 [details]
Patch
Comment 8 youenn fablet 2019-12-09 01:47:17 PST
Comment on attachment 385136 [details]
Patch

Worked around the inspector test failure by making the JS builtin working for both setlike and maplike.
Comment 9 youenn fablet 2019-12-13 09:51:06 PST
ping review
Comment 10 WebKit Commit Bot 2019-12-13 12:48:22 PST
The commit-queue encountered the following flaky tests while processing attachment 385136 [details]:

imported/w3c/web-platform-tests/content-security-policy/reporting/report-same-origin-with-cookies.html bug 205216 (author: dbates@webkit.org)
The commit-queue is continuing to process your patch.
Comment 11 WebKit Commit Bot 2019-12-13 12:49:17 PST
Comment on attachment 385136 [details]
Patch

Clearing flags on attachment: 385136

Committed r253494: <https://trac.webkit.org/changeset/253494>
Comment 12 WebKit Commit Bot 2019-12-13 12:49:19 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2019-12-13 12:50:24 PST
<rdar://problem/57921779>