Prevent 'removetrack' events from firing when all inband text tracks are removed.

Source/core/html/HTMLMediaElement.cpp

Index: Source/core/html/HTMLMediaElement.cpp
diff --git a/Source/core/html/HTMLMediaElement.cpp b/Source/core/html/HTMLMediaElement.cpp
index 8c2c9e1f16f4f925e784ef572e87e70ee7b1423b..1796b7588c23bc34bdb6d3fbe7048e458978f247 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -658,14 +658,40 @@ void HTMLMediaElement::prepareForLoad()
 
     // 4 - If the media element's networkState is not set to NETWORK_EMPTY, then run these substeps
     if (m_networkState != NETWORK_EMPTY) {
+        // 4.1 - Queue a task to fire a simple event named emptied at the media element.
+        scheduleEvent(EventTypeNames::emptied);
+
+        // 4.2 - If a fetching process is in progress for the media element, the user agent should stop it.
         m_networkState = NETWORK_EMPTY;
+
+        // 4.3 - Forget the media element's media-resource-specific tracks.
+        forgetResourceSpecificTracks();
+
+        // 4.4 - If readyState is not set to HAVE_NOTHING, then set it to that state.
         m_readyState = HAVE_NOTHING;
         m_readyStateMaximum = HAVE_NOTHING;
-        refreshCachedTime();
+
+        // 4.5 - If the paused attribute is false, then set it to true.
         m_paused = true;
+
+        // 4.6 - If seeking is true, set it to false.
         m_seeking = false;
+
+        // 4.7 - Set the current playback position to 0.
+        //       Set the official playback position to 0.
+        //       If this changed the official playback position, then queue a task to fire a simple event named timeupdate at the media element.
+        // FIXME: Add support for firing this event.
+
+        // 4.8 - Set the initial playback position to 0.
+        // FIXME: Make this less subtle. The position only becomes 0 because of the createMediaPlayer() call
+        // above.
+        refreshCachedTime();
         invalidateCachedTime();
-        scheduleEvent(EventTypeNames::emptied);
+
+        // 4.9 - Set the timeline offset to Not-a-Number (NaN).
+        // 4.10 - Update the duration attribute to Not-a-Number (NaN).
+
+
         updateMediaController();
         if (RuntimeEnabledFeatures::videoTrackEnabled())
             updateActiveTextTrackCues(0);
@@ -689,6 +715,9 @@ void HTMLMediaElement::prepareForLoad()
     // 2 - Asynchronously await a stable state.
 
     m_playedTimeRanges = TimeRanges::create();
+
+    // FIXME: Investigate whether these can be moved into m_networkState != NETWORK_EMPTY block above
+    // so they are closer to the relevant spec steps.
     m_lastSeekTime = 0;
     m_duration = numeric_limits<double>::quiet_NaN();
 
@@ -1359,6 +1388,7 @@ void HTMLMediaElement::noneSupported()
     m_error = MediaError::create(MediaError::MEDIA_ERR_SRC_NOT_SUPPORTED);
 
     // 6.2 - Forget the media element's media-resource-specific text tracks.
+    forgetResourceSpecificTracks();
 
     // 6.3 - Set the element's networkState attribute to the NETWORK_NO_SOURCE value.
     m_networkState = NETWORK_NO_SOURCE;
@@ -1433,11 +1463,18 @@ void HTMLMediaElement::mediaLoadingFailed(MediaPlayer::NetworkState error)
     // <source> children, schedule the next one
     if (m_readyState < HAVE_METADATA && m_loadState == LoadingFromSourceElement) {
 
+        // resource selection algorithm
+        // Step 9.Otherwise.9 - Failed with elements: Queue a task, using the DOM manipulation task source, to fire a simple event named error at the candidate element.
         if (m_currentSourceNode)
             m_currentSourceNode->scheduleErrorEvent();
         else
             WTF_LOG(Media, "HTMLMediaElement::setNetworkState - error event not sent, <source> was removed");
 
+        // 9.Otherwise.10 - Asynchronously await a stable state. The synchronous section consists of all the remaining steps of this algorithm until the algorithm says the synchronous section has ended.
+
+        // 9.Otherwise.11 - Forget the media element's media-resource-specific tracks.
+        forgetResourceSpecificTracks();
+
         if (havePotentialSourceChild()) {
             WTF_LOG(Media, "HTMLMediaElement::setNetworkState - scheduling next <source>");
             scheduleNextSourceChild();
@@ -2496,7 +2533,7 @@ void HTMLMediaElement::mediaPlayerDidRemoveTextTrack(WebInbandTextTrack* webTrac
     if (!textTrack)
         return;
 
-    removeTextTrack(textTrack.get());
+    removeTextTrack(textTrack.get(), true);
 }
 
 void HTMLMediaElement::closeCaptionTracksChanged()
@@ -2512,28 +2549,27 @@ void HTMLMediaElement::addTextTrack(TextTrack* track)
     closeCaptionTracksChanged();
 }
 
-void HTMLMediaElement::removeTextTrack(TextTrack* track)
+void HTMLMediaElement::removeTextTrack(TextTrack* track, bool scheduleEvent)
 {
     TrackDisplayUpdateScope scope(this);
     TextTrackCueList* cues = track->cues();
     if (cues)
         textTrackRemoveCues(track, cues);
-    m_textTracks->remove(track);
+    m_textTracks->remove(track, scheduleEvent);
 
     closeCaptionTracksChanged();
 }
 
-void HTMLMediaElement::removeAllInbandTracks()
+void HTMLMediaElement::forgetResourceSpecificTracks()
 {
-    if (!m_textTracks)
-        return;
+    if (m_textTracks) {
+        TrackDisplayUpdateScope scope(this);
+        for (int i = m_textTracks->length() - 1; i >= 0; --i) {

[adamk, 2014/02/26 00:45:32]

length() - 1 is scary since that's an unsigned. Are we sure that
m_textTracks.length() > 0 here? I guess that hasn't changed, but it'd be good to
at least add an ASSERT before this loop.

+            TextTrack* track = m_textTracks->item(i);
 
-    TrackDisplayUpdateScope scope(this);
-    for (int i = m_textTracks->length() - 1; i >= 0; --i) {
-        TextTrack* track = m_textTracks->item(i);
-
-        if (track->trackType() == TextTrack::InBand)
-            removeTextTrack(track);
+            if (track->trackType() == TextTrack::InBand)
+                removeTextTrack(track, false);
+        }
     }
 }
 
@@ -2636,7 +2672,7 @@ void HTMLMediaElement::didRemoveTrackElement(HTMLTrackElement* trackElement)
     // When a track element's parent element changes and the old parent was a media element,
     // then the user agent must remove the track element's corresponding text track from the
     // media element's list of text tracks.
-    removeTextTrack(textTrack.get());
+    removeTextTrack(textTrack.get(), true);
 
     size_t index = m_textTracksWhenResourceSelectionBegan.find(textTrack.get());
     if (index != kNotFound)
@@ -3389,7 +3425,7 @@ void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClient()
 
 void HTMLMediaElement::clearMediaPlayer(int flags)
 {
-    removeAllInbandTracks();
+    forgetResourceSpecificTracks();
 
     closeMediaSource();