Update a cue's insertion order when reinserting it with addCue()

Source/core/html/track/TextTrack.cpp

 /*
  * Copyright (C) 2011 Google Inc.  All rights reserved.
  * Copyright (C) 2011, 2012, 2013 Apple 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 216 matching lines @@
 {
     if (!prpCue)
         return;
 
     RefPtrWillBeRawPtr<TextTrackCue> cue = prpCue;
 
     // TODO(93143): Add spec-compliant behavior for negative time values.
     if (std::isnan(cue->startTime()) || std::isnan(cue->endTime()) || cue->startTime() < 0 || cue->endTime() < 0)
         return;
 
-    // 4.8.10.12.5 Text track API
+    // https://html.spec.whatwg.org/multipage/embedded-content.html#dom-texttrack-addcue
 
     // The addCue(cue) method of TextTrack objects, when invoked, must run the following steps:
 
-    // 1. If the given cue is in a text track list of cues, then remove cue from that text track
-    // list of cues.
-    TextTrack* cueTrack = cue->track();
-    if (cueTrack && cueTrack != this)
+    // (Steps 1 and 2 - pertaining to association of rendering rules - are not implemented.)
+
+    // 3. If the given cue is in a text track list of cues, then remove cue
+    // from that text track list of cues.
+    if (TextTrack* cueTrack = cue->track())
         cueTrack->removeCue(cue.get(), ASSERT_NO_EXCEPTION);
 
-    // 2. Add cue to the method's TextTrack object's text track's text track list of cues.
+    // 4. Add cue to the method's TextTrack object's text track's text track list of cues.
     cue->setTrack(this);
     ensureTextTrackCueList()->add(cue);
 
     if (cueTimeline() && m_mode != disabledKeyword())
         cueTimeline()->addCue(this, cue.get());
 }
 
 void TextTrack::removeCue(TextTrackCue* cue, ExceptionState& exceptionState)
 {
     if (!cue)
         return;
 
-    // 4.8.10.12.5 Text track API
+    // https://html.spec.whatwg.org/multipage/embedded-content.html#dom-texttrack-removecue
 
     // The removeCue(cue) method of TextTrack objects, when invoked, must run the following steps:
 
     // 1. If the given cue is not currently listed in the method's TextTrack
     // object's text track's text track list of cues, then throw a NotFoundError exception.
-    if (cue->track() != this) {
+    // 2. Remove cue from the method's TextTrack object's text track's text track list of cues.
+    if (cue->track() != this || !m_cues || !m_cues->remove(cue)) {
         exceptionState.throwDOMException(NotFoundError, "The specified cue is not listed in the TextTrack's list of cues.");
         return;
     }
 
-    // 2. Remove cue from the method's TextTrack object's text track's text track list of cues.
-    if (!m_cues || !m_cues->remove(cue)) {

[philipj_slow, 2015/02/23 13:32:13]

If this shouldn't be possible, can we assert it instead?

[fs, 2015/02/23 14:04:49]

Yes. There're a few of these invariants - in this case it's:

cue->track() != 0 => track.cues.contains(cue) which in turn implies
TextTrack::m_cues != 0.

This is not the only place where we're being "cautious" though. I'm trying to
clean these up as I go, but I'm sort of aiming at getting the code to a shape
where it's reasonably easy to verify the invariants to being with.

-        exceptionState.throwDOMException(InvalidStateError, "Failed to remove the specified cue.");
-        return;
-    }
-
     cue->setTrack(0);
     if (cueTimeline())
         cueTimeline()->removeCue(this, cue);
 }
 
 VTTRegionList* TextTrack::ensureVTTRegionList()
 {
     if (!m_regions)
         m_regions = VTTRegionList::create();
 
@@ skipping 162 matching lines @@
 DEFINE_TRACE(TextTrack)
 {
     visitor->trace(m_cues);
     visitor->trace(m_regions);
     visitor->trace(m_trackList);
     TrackBase::trace(visitor);
     EventTargetWithInlineData::trace(visitor);
 }
 
 } // namespace blink