Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(254)

Issue 807263007: IDL: Add forEach() method to iterable<>/maplike<>/setlike<> interfaces (Closed)

Created:
5 years, 11 months ago by Jens Widell
Modified:
5 years, 10 months ago
Reviewers:
haraken, jsbell, yhirano
CC:
blink-reviews, arv+blink, sof, eae+blinkwatch, blink-reviews-dom_chromium.org, dglazkov+blink, blink-reviews-bindings_chromium.org, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

IDL: Add forEach() method to iterable<>/maplike<>/setlike<> interfaces Also add an implementation of forEach() in the Iterable<> helper class, so that current interfaces get the new method for free. BUG=432683 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189102

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : extend testing #

Total comments: 6

Patch Set 4 : use assert_equals() #

Patch Set 5 : drop context and handle scopes #

Patch Set 6 : rebased #

Patch Set 7 : remove FIXMEs about missing forEach() #

Total comments: 12

Patch Set 8 : rebased #

Patch Set 9 : #

Patch Set 10 : rebased #

Patch Set 11 : adjust test expectations #

Unified diffs Side-by-side diffs Delta from patch set Stats (+327 lines, -10 lines) Patch
M LayoutTests/fast/js/iterable-object.html View 1 2 1 chunk +24 lines, -0 lines 0 comments Download
M LayoutTests/fast/js/iterable-object-expected.txt View 1 2 1 chunk +33 lines, -0 lines 0 comments Download
M LayoutTests/http/tests/fetch/script-tests/headers.js View 1 2 3 1 chunk +12 lines, -0 lines 0 comments Download
M LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/webexposed/global-interface-listing-dedicated-worker-expected.txt View 1 2 3 4 5 2 chunks +2 lines, -0 lines 0 comments Download
M LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 3 chunks +3 lines, -0 lines 0 comments Download
M LayoutTests/webexposed/global-interface-listing-shared-worker-expected.txt View 1 2 3 4 5 2 chunks +2 lines, -0 lines 0 comments Download
M LayoutTests/webmidi/requestmidiaccess.html View 2 chunks +18 lines, -0 lines 0 comments Download
M LayoutTests/webmidi/requestmidiaccess-expected.txt View 2 chunks +8 lines, -0 lines 0 comments Download
M Source/bindings/scripts/idl_definitions.py View 1 2 3 4 5 2 chunks +11 lines, -5 lines 0 comments Download
M Source/bindings/scripts/v8_interface.py View 1 2 3 4 5 6 7 8 4 chunks +23 lines, -1 line 0 comments Download
M Source/bindings/scripts/v8_utilities.py View 1 2 3 4 5 6 7 2 chunks +2 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterface2.cpp View 1 2 3 4 5 3 chunks +37 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterface3.cpp View 3 chunks +42 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestObject.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +36 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/modules/V8TestInterface5.cpp View 3 chunks +37 lines, -0 lines 0 comments Download
M Source/core/dom/Iterable.h View 1 2 3 4 2 chunks +36 lines, -0 lines 0 comments Download
M Source/modules/webmidi/MIDIInputMap.idl View 1 2 3 4 5 6 1 chunk +0 lines, -2 lines 0 comments Download
M Source/modules/webmidi/MIDIOutputMap.idl View 1 2 3 4 5 6 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 34 (6 generated)
Jens Widell
PTAL I'm in particular not sure the actual forEach() implementation is done right, so please ...
5 years, 11 months ago (2015-01-22 15:11:14 UTC) #2
haraken
On 2015/01/22 15:11:14, Jens Widell wrote: > PTAL > > I'm in particular not sure ...
5 years, 11 months ago (2015-01-22 15:17:38 UTC) #3
haraken
5 years, 11 months ago (2015-01-22 15:27:58 UTC) #5
jsbell
https://codereview.chromium.org/807263007/diff/20001/LayoutTests/fast/js/iterable-object.html File LayoutTests/fast/js/iterable-object.html (right): https://codereview.chromium.org/807263007/diff/20001/LayoutTests/fast/js/iterable-object.html#newcode31 LayoutTests/fast/js/iterable-object.html:31: internals.forEach(function (value, key, object) { Can you add cases ...
5 years, 11 months ago (2015-01-22 19:53:54 UTC) #7
Jens Widell
https://codereview.chromium.org/807263007/diff/20001/LayoutTests/fast/js/iterable-object.html File LayoutTests/fast/js/iterable-object.html (right): https://codereview.chromium.org/807263007/diff/20001/LayoutTests/fast/js/iterable-object.html#newcode31 LayoutTests/fast/js/iterable-object.html:31: internals.forEach(function (value, key, object) { On 2015/01/22 19:53:54, jsbell ...
5 years, 11 months ago (2015-01-23 11:08:14 UTC) #8
yhirano
Can you remove forEach FIXMEs from modules/webmidi/MIDI{Input, Output}Map.idl? https://codereview.chromium.org/807263007/diff/40001/LayoutTests/http/tests/fetch/script-tests/headers.js File LayoutTests/http/tests/fetch/script-tests/headers.js (right): https://codereview.chromium.org/807263007/diff/40001/LayoutTests/http/tests/fetch/script-tests/headers.js#newcode135 LayoutTests/http/tests/fetch/script-tests/headers.js:135: assert_true(thisObject ...
5 years, 11 months ago (2015-01-23 11:49:03 UTC) #9
Jens Widell
https://codereview.chromium.org/807263007/diff/40001/LayoutTests/http/tests/fetch/script-tests/headers.js File LayoutTests/http/tests/fetch/script-tests/headers.js (right): https://codereview.chromium.org/807263007/diff/40001/LayoutTests/http/tests/fetch/script-tests/headers.js#newcode135 LayoutTests/http/tests/fetch/script-tests/headers.js:135: assert_true(thisObject === this); On 2015/01/23 11:49:03, yhirano wrote: > ...
5 years, 11 months ago (2015-01-23 11:53:55 UTC) #10
haraken
https://codereview.chromium.org/807263007/diff/40001/Source/core/dom/Iterable.h File Source/core/dom/Iterable.h (right): https://codereview.chromium.org/807263007/diff/40001/Source/core/dom/Iterable.h#newcode49 Source/core/dom/Iterable.h:49: v8::HandleScope handleScope(isolate); On 2015/01/23 11:53:55, Jens Widell wrote: > ...
5 years, 11 months ago (2015-01-23 11:55:39 UTC) #11
Jens Widell
https://codereview.chromium.org/807263007/diff/40001/Source/core/dom/Iterable.h File Source/core/dom/Iterable.h (right): https://codereview.chromium.org/807263007/diff/40001/Source/core/dom/Iterable.h#newcode49 Source/core/dom/Iterable.h:49: v8::HandleScope handleScope(isolate); On 2015/01/23 11:55:39, haraken wrote: > On ...
5 years, 11 months ago (2015-01-23 11:58:14 UTC) #12
Jens Widell
On 2015/01/23 11:49:03, yhirano wrote: > Can you remove forEach FIXMEs from modules/webmidi/MIDI{Input, Output}Map.idl? Done.
5 years, 11 months ago (2015-01-26 11:08:10 UTC) #13
yhirano
lgtm
5 years, 11 months ago (2015-01-26 11:19:27 UTC) #14
Jens Widell
On 2015/01/26 11:19:27, yhirano wrote: > lgtm Thanks! haraken@: Do you want to have another ...
5 years, 11 months ago (2015-01-26 13:07:31 UTC) #15
haraken
https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_interface.py File Source/bindings/scripts/v8_interface.py (right): https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_interface.py#newcode335 Source/bindings/scripts/v8_interface.py:335: def generated_argument(idl_type, name, is_optional=False, default_value=None, extended_attributes=None): It looks like ...
5 years, 11 months ago (2015-01-27 01:56:31 UTC) #16
Jens Widell
https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_interface.py File Source/bindings/scripts/v8_interface.py (right): https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_interface.py#newcode335 Source/bindings/scripts/v8_interface.py:335: def generated_argument(idl_type, name, is_optional=False, default_value=None, extended_attributes=None): On 2015/01/27 01:56:30, ...
5 years, 11 months ago (2015-01-27 07:19:23 UTC) #17
haraken
https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_utilities.py File Source/bindings/scripts/v8_utilities.py (right): https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_utilities.py#newcode204 Source/bindings/scripts/v8_utilities.py:204: 'ThisValue': 'ScriptValue(scriptState, info.This())', On 2015/01/27 07:19:23, Jens Widell wrote: ...
5 years, 11 months ago (2015-01-27 08:47:11 UTC) #18
Jens Widell
On 2015/01/27 08:47:11, haraken wrote: > https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_utilities.py > File Source/bindings/scripts/v8_utilities.py (right): > > https://codereview.chromium.org/807263007/diff/120001/Source/bindings/scripts/v8_utilities.py#newcode204 > ...
5 years, 11 months ago (2015-01-27 09:07:20 UTC) #19
Jens Widell
On 2015/01/27 09:07:20, Jens Widell wrote: > On 2015/01/27 08:47:11, haraken wrote: > > > ...
5 years, 11 months ago (2015-01-27 11:20:05 UTC) #20
haraken
On 2015/01/27 11:20:05, Jens Widell wrote: > On 2015/01/27 09:07:20, Jens Widell wrote: > > ...
5 years, 11 months ago (2015-01-27 22:50:00 UTC) #21
Jens Widell
On 2015/01/27 22:50:00, haraken wrote: > On 2015/01/27 11:20:05, Jens Widell wrote: > > On ...
5 years, 11 months ago (2015-01-28 07:44:00 UTC) #22
haraken
On 2015/01/28 07:44:00, Jens Widell wrote: > On 2015/01/27 22:50:00, haraken wrote: > > On ...
5 years, 11 months ago (2015-01-28 07:49:38 UTC) #23
Jens Widell
On 2015/01/28 07:49:38, haraken wrote: > On 2015/01/28 07:44:00, Jens Widell wrote: > > On ...
5 years, 11 months ago (2015-01-28 07:56:43 UTC) #24
haraken
On 2015/01/28 07:56:43, Jens Widell wrote: > On 2015/01/28 07:49:38, haraken wrote: > > On ...
5 years, 11 months ago (2015-01-28 08:09:55 UTC) #25
Jens Widell
On 2015/01/28 08:09:55, haraken wrote: > On 2015/01/28 07:56:43, Jens Widell wrote: > > On ...
5 years, 11 months ago (2015-01-28 08:11:38 UTC) #26
haraken
On 2015/01/28 08:11:38, Jens Widell wrote: > On 2015/01/28 08:09:55, haraken wrote: > > On ...
5 years, 11 months ago (2015-01-28 08:16:02 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/807263007/160001
5 years, 11 months ago (2015-01-28 08:16:02 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/44015)
5 years, 10 months ago (2015-01-28 09:47:55 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/807263007/200001
5 years, 10 months ago (2015-01-28 10:21:59 UTC) #33
commit-bot: I haz the power
5 years, 10 months ago (2015-01-28 11:35:53 UTC) #34
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189102

Powered by Google App Engine
This is Rietveld 408576698