Tweak the TextTrackCue "cue index" management

Source/core/html/track/TextTrackCueList.cpp

Index: Source/core/html/track/TextTrackCueList.cpp
diff --git a/Source/core/html/track/TextTrackCueList.cpp b/Source/core/html/track/TextTrackCueList.cpp
index 40052b9b4aef97ea42f7f0571ba537dd3492b12b..2f0b055fd33d1837661a411d6d8cc2b247188577 100644
--- a/Source/core/html/track/TextTrackCueList.cpp
+++ b/Source/core/html/track/TextTrackCueList.cpp
@@ -31,6 +31,7 @@
 namespace blink {
 
 TextTrackCueList::TextTrackCueList()
+    : m_firstInvalidIndex(0)

[philipj_slow, 2015/02/26 15:43:12]

After seeing how m_firstInvalidIndex is used I understand why this makes sense
(an index past the end of the list is greater than any valid index and will
short-circuit validateCueIndexes) but it might be worth documenting, perhaps
with assertions, the invariant.

[fs, 2015/02/26 16:09:34]

I added some comments around the invalidate/validate methods. Not sure where a
good place to validate would be (validateCueIndexes is not such a place - it'll
blow up horribly if it fails - or need a "raw" accessor.)

 {
 }
 
@@ -41,11 +42,6 @@ unsigned long TextTrackCueList::length() const
     return m_list.size();
 }
 
-unsigned long TextTrackCueList::getCueIndex(TextTrackCue* cue) const
-{
-    return m_list.find(cue);
-}
-
 TextTrackCue* TextTrackCueList::item(unsigned index) const
 {
     if (index < m_list.size())
@@ -90,7 +86,7 @@ bool TextTrackCueList::add(PassRefPtrWillBeRawPtr<TextTrackCue> cue)
         return false;
 
     m_list.insert(index, cue);
-    invalidateCueIndexes(index);
+    invalidateCueIndex(index);
     return true;
 }
 
@@ -117,9 +113,7 @@ bool TextTrackCueList::remove(TextTrackCue* cue)
         return false;
 
     m_list.remove(index);
-    // FIXME: While removing a cue does not invalidate the cue order, it does
-    // make it more difficult to maintain the invariant, so should probably
-    // just invalidate here as well.
+    invalidateCueIndex(index);
     return true;
 }
 
@@ -131,10 +125,6 @@ void TextTrackCueList::updateCueIndex(TextTrackCue* cue)
     cue->setIsActive(false);
     cue->removeDisplayTree();
 
-    // FIXME: If moving the cue such that its index in list increases, then
-    // what happens with the cached index on cues in the range [oldIndex,
-    // newIndex)? (Some of the indices will be "safe", but there'll be a risk
-    // that the lazy update via cueIndex() yields duplicates/incorrect order.)
     add(cue);
 }
 
@@ -143,11 +133,16 @@ void TextTrackCueList::clear()
     m_list.clear();
 }
 
-void TextTrackCueList::invalidateCueIndexes(size_t start)
+void TextTrackCueList::invalidateCueIndex(size_t index)
+{
+    m_firstInvalidIndex = std::min(m_firstInvalidIndex, index);
+}
+
+void TextTrackCueList::validateCueIndexes()
 {
-    // FIXME: When iterating cues we could as well update their cached indices too.
-    for (size_t i = start; i < m_list.size(); ++i)
-        m_list[i]->invalidateCueIndex();
+    for (size_t i = m_firstInvalidIndex; i < m_list.size(); ++i)
+        m_list[i]->updateCueIndex(safeCast<unsigned>(i));

[philipj_slow, 2015/02/26 15:43:12]

Yeah, size_t might not be too bad for the cue index :)

[fs, 2015/02/26 16:09:34]

I suspect 2^32-2 is a difficult enough limit to reach...

+    m_firstInvalidIndex = m_list.size();
 }
 
 DEFINE_TRACE(TextTrackCueList)