Add iterator support to FontFaceSet

Source/core/css/FontFaceSet.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 11 matching lines @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
  * DAMAGE.
  */
 
 #include "config.h"
 #include "core/css/FontFaceSet.h"
 
 #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"
 #include "core/css/FontFaceSetLoadEvent.h"
 #include "core/css/StylePropertySet.h"
 #include "core/css/parser/BisonCSSParser.h"
 #include "core/css/resolver/StyleResolver.h"
 #include "core/dom/Document.h"
 #include "core/dom/StyleEngine.h"
 #include "core/frame/FrameView.h"
@@ skipping 338 matching lines @@
 
     for (size_t i = 0; i < fontFaces.size(); ++i) {
         FontFace* face = fontFaces[i].get();
         if (thisArg)
             callback->handleItem(*thisArg, face, face, const_cast<FontFaceSet*>(this));
         else
             callback->handleItem(face, face, const_cast<FontFaceSet*>(this));
     }
 }
 
+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())
         return m_nonCSSConnectedFaces.size();
     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)
         return;
     if (!m_loadingFonts.isEmpty() || (!hasLoadedFonts() && m_readyResolvers.isEmpty()))
         return;
 
     // If the layout was invalidated in between when we thought layout
     // was updated and when we're ready to fire the event, just wait
     // until after the next layout before firing events.
@@ skipping 178 matching lines @@
 {
     visitor->trace(m_loadingFonts);
     visitor->trace(m_loadedFonts);
     visitor->trace(m_failedFonts);
     visitor->trace(m_nonCSSConnectedFaces);
     DocumentSupplement::trace(visitor);
     EventTargetWithInlineData::trace(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