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

Issue 557763003: Add iterator support to FontFaceSet (Closed)

Created:
6 years, 3 months ago by Kunihiko Sakamoto
Modified:
5 years, 2 months ago
Reviewers:
haraken, bashi, yhirano, eseidel
CC:
blink-reviews, arv+blink, blink-reviews-css, ed+blinkwatch_opera.com, abarth-chromium, dglazkov+blink, blink-reviews-bindings_chromium.org, Inactive, darktears, apavlov+blink_chromium.org, rwlbuis, rune+blink, arv (Not doing code reviews)
Project:
blink
Visibility:
Public.

Description

Add iterator support to FontFaceSet This patch adds keys(), values(), and entries() methods to FontFaceSet [1], and makes FontFaceSet interface Iterable. These methods should behave like ES6 Set objects' [2]. [1] http://dev.w3.org/csswg/css-font-loading/#FontFaceSet-interface [2] https://people.mozilla.org/~jorendorff/es6-draft.html#sec-set-objects BUG=392075 TEST=LayoutTests/fast/css/fontfaceset-iterator.html

Patch Set 1 : #

Total comments: 4

Patch Set 2 : #

Total comments: 5

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+156 lines, -0 lines) Patch
A LayoutTests/fast/css/fontfaceset-iterator.html View 1 chunk +47 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/fontfaceset-iterator-expected.txt View 1 chunk +16 lines, -0 lines 0 comments Download
M Source/core/css/FontFaceSet.h View 1 3 chunks +34 lines, -0 lines 0 comments Download
M Source/core/css/FontFaceSet.cpp View 1 2 4 chunks +55 lines, -0 lines 0 comments Download
M Source/core/css/FontFaceSet.idl View 2 chunks +4 lines, -0 lines 0 comments Download

Messages

Total messages: 26 (11 generated)
Kunihiko Sakamoto
6 years, 3 months ago (2014-09-11 06:31:54 UTC) #7
Kunihiko Sakamoto
+yhirano for Iterator usage
6 years, 3 months ago (2014-09-11 06:34:07 UTC) #9
yhirano
https://codereview.chromium.org/557763003/diff/80001/Source/core/css/FontFaceSet.cpp File Source/core/css/FontFaceSet.cpp (right): https://codereview.chromium.org/557763003/diff/80001/Source/core/css/FontFaceSet.cpp#newcode639 Source/core/css/FontFaceSet.cpp:639: ScriptValue value = ScriptValue(scriptState, V8ValueTraits<FontFace*>::toV8Value(fontFace, scriptState->context()->Global(), scriptState->isolate())); You can ...
6 years, 3 months ago (2014-09-11 08:04:45 UTC) #10
Kunihiko Sakamoto
https://codereview.chromium.org/557763003/diff/80001/Source/core/css/FontFaceSet.cpp File Source/core/css/FontFaceSet.cpp (right): https://codereview.chromium.org/557763003/diff/80001/Source/core/css/FontFaceSet.cpp#newcode639 Source/core/css/FontFaceSet.cpp:639: ScriptValue value = ScriptValue(scriptState, V8ValueTraits<FontFace*>::toV8Value(fontFace, scriptState->context()->Global(), scriptState->isolate())); On 2014/09/11 ...
6 years, 3 months ago (2014-09-11 10:47:36 UTC) #11
yhirano
lgtm
6 years, 3 months ago (2014-09-11 10:51:04 UTC) #12
haraken
This CL needs API owner review. https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp File Source/core/css/FontFaceSet.cpp (right): https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp#newcode634 Source/core/css/FontFaceSet.cpp:634: FontFace* fontFace = ...
6 years, 3 months ago (2014-09-11 12:06:08 UTC) #14
yhirano
https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp File Source/core/css/FontFaceSet.cpp (right): https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp#newcode640 Source/core/css/FontFaceSet.cpp:640: ScriptValue value = ScriptValue(scriptState, toV8(fontFace, scriptState->context()->Global(), scriptState->isolate())); On 2014/09/11 ...
6 years, 3 months ago (2014-09-11 12:29:06 UTC) #15
haraken
https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp File Source/core/css/FontFaceSet.cpp (right): https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp#newcode640 Source/core/css/FontFaceSet.cpp:640: ScriptValue value = ScriptValue(scriptState, toV8(fontFace, scriptState->context()->Global(), scriptState->isolate())); On 2014/09/11 ...
6 years, 3 months ago (2014-09-11 12:32:28 UTC) #16
eseidel
Are we shipping any other ES6 Set-compatible objects at this time? Should we ship them ...
6 years, 3 months ago (2014-09-11 16:01:40 UTC) #18
eseidel
Do we have to use duck-typing and copy/paste these 3 methods into every set-like or ...
6 years, 3 months ago (2014-09-11 16:03:03 UTC) #19
Kunihiko Sakamoto
https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp File Source/core/css/FontFaceSet.cpp (right): https://codereview.chromium.org/557763003/diff/100001/Source/core/css/FontFaceSet.cpp#newcode634 Source/core/css/FontFaceSet.cpp:634: FontFace* fontFace = m_fontFaceSet->fontFaceAt(m_index); On 2014/09/11 12:06:07, haraken wrote: ...
6 years, 3 months ago (2014-09-12 09:01:02 UTC) #21
yhirano
On 2014/09/11 16:03:03, eseidel wrote: > Do we have to use duck-typing and copy/paste these ...
6 years, 3 months ago (2014-09-12 09:21:01 UTC) #22
Kunihiko Sakamoto
On 2014/09/11 16:01:40, eseidel wrote: > Are we shipping any other ES6 Set-compatible objects at ...
6 years, 3 months ago (2014-09-17 08:44:55 UTC) #23
yhirano
On 2014/09/17 08:44:55, Kunihiko Sakamoto wrote: > On 2014/09/11 16:01:40, eseidel wrote: > > Are ...
6 years, 3 months ago (2014-09-17 08:50:47 UTC) #24
yhirano
5 years, 3 months ago (2015-08-31 05:56:40 UTC) #26
Removing myself from the reviewers list.

Powered by Google App Engine
This is Rietveld 408576698