Add iterator support to FontFaceSet

Source/core/css/FontFaceSet.cpp

Index: Source/core/css/FontFaceSet.cpp
diff --git a/Source/core/css/FontFaceSet.cpp b/Source/core/css/FontFaceSet.cpp
index 7de6b7767b337a4fbd63a42f9d335aa1c360d6c2..b745eb83209f4f81ac940099b6189fbb3c94ccf3 100644
--- a/Source/core/css/FontFaceSet.cpp
+++ b/Source/core/css/FontFaceSet.cpp
@@ -29,6 +29,7 @@
 #include "bindings/core/v8/Dictionary.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
+#include "bindings/core/v8/V8FontFace.h"
 #include "core/css/CSSFontSelector.h"
 #include "core/css/CSSSegmentedFontFace.h"
 #include "core/css/FontFaceCache.h"
@@ -387,6 +388,21 @@ void FontFaceSet::forEachInternal(PassOwnPtrWillBeRawPtr<FontFaceSetForEachCallb
     }
 }
 
+Iterator* FontFaceSet::keys()
+{
+    return new FontFaceSetIterator(this, FontFaceSetIterator::Keys);
+}
+
+Iterator* FontFaceSet::values()
+{
+    return new FontFaceSetIterator(this, FontFaceSetIterator::Values);
+}
+
+Iterator* FontFaceSet::entries()
+{
+    return new FontFaceSetIterator(this, FontFaceSetIterator::Entries);
+}
+
 unsigned long FontFaceSet::size() const
 {
     if (!inActiveDocumentContext())
@@ -394,6 +410,27 @@ unsigned long FontFaceSet::size() const
     return cssConnectedFontFaceList().size() + m_nonCSSConnectedFaces.size();
 }
 
+FontFace* FontFaceSet::fontFaceAt(size_t index) const
+{
+    if (inActiveDocumentContext()) {
+        const WillBeHeapListHashSet<RefPtrWillBeMember<FontFace> >& cssConnectedFaces = cssConnectedFontFaceList();
+        if (index < cssConnectedFaces.size()) {
+            WillBeHeapListHashSet<RefPtrWillBeMember<FontFace> >::const_iterator it = cssConnectedFaces.begin();
+            for (size_t i = 0; i < index; i++)
+                ++it;
+            return it->get();
+        }
+        index -= cssConnectedFaces.size();
+    }
+    if (index < m_nonCSSConnectedFaces.size()) {
+        WillBeHeapListHashSet<RefPtrWillBeMember<FontFace> >::const_iterator it =  m_nonCSSConnectedFaces.begin();
+        for (size_t i = 0; i < index; i++)
+            ++it;
+        return it->get();
+    }
+    return 0;
+}
+
 void FontFaceSet::fireDoneEventIfPossible()
 {
     if (m_shouldFireLoadingEvent)
@@ -592,4 +629,20 @@ void FontFaceSet::trace(Visitor* visitor)
 }
 #endif
 
+ScriptValue FontFaceSet::FontFaceSetIterator::next(ScriptState* scriptState, ExceptionState&)
+{
+    FontFace* fontFace = m_fontFaceSet->fontFaceAt(m_index);

[haraken, 2014/09/11 12:06:07]

Each next() method is O(n), and thus it takes O(n^2) to iterate all font faces.
Is this intended?

[Kunihiko Sakamoto, 2014/09/12 09:01:02]

That's definitely not desirable, but wouldn't be a big problem in usual use
cases.
I've added a FIXME comment and will revisit later.

+    if (!fontFace)
+        return ScriptValue(scriptState, v8DoneIteratorResult(scriptState->isolate()));
+    m_index++;
+    if (m_kind == FontFaceSetIterator::Entries) {
+        Vector<ScriptValue> entry;
+        ScriptValue value = ScriptValue(scriptState, toV8(fontFace, scriptState->context()->Global(), scriptState->isolate()));

[haraken, 2014/09/11 12:06:07]

yhirano-san: In the Iterator pattern, is it guaranteed that we're in a correct
context in the next() method? I mean, don't we need to enter
ScriptState::Scope(scriptState) here?

[yhirano, 2014/09/11 12:29:06]

I think so, next is called just like other methods.
Is there any specific concern?

[haraken, 2014/09/11 12:32:28]

No :) I just wanted to confirm that we don't need to enter a ScriptState before
calling toV8().

+        entry.append(value);
+        entry.append(value);
+        return ScriptValue(scriptState, v8IteratorResult(scriptState, entry));
+    }
+    return ScriptValue(scriptState, v8IteratorResult(scriptState, fontFace));
+}
+
 } // namespace blink