Diff
Modified: trunk/LayoutTests/ChangeLog (166237 => 166238)
--- trunk/LayoutTests/ChangeLog 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/LayoutTests/ChangeLog 2014-03-25 16:51:40 UTC (rev 166238)
@@ -1,3 +1,16 @@
+2014-03-24 Brent Fulgham <bfulg...@apple.com>
+
+ Prevent 'removetrack' events from firing when all inband text tracks are removed.
+ https://bugs.webkit.org/show_bug.cgi?id=130704
+
+ Reviewed by Eric Carlson.
+
+ Based on the Blink change (patch by acolw...@chromium.org):
+ https://codereview.chromium.org/177243018/
+
+ * media/track/track-remove-track-expected.txt: Added.
+ * media/track/track-remove-track.html: Added.
+
2014-03-25 Alexey Proskuryakov <a...@apple.com>
media/W3C/audio/canPlayType/canPlayType_application_octet_stream.html frequently fails on EWS
Added: trunk/LayoutTests/media/track/track-remove-track-expected.txt (0 => 166238)
--- trunk/LayoutTests/media/track/track-remove-track-expected.txt (rev 0)
+++ trunk/LayoutTests/media/track/track-remove-track-expected.txt 2014-03-25 16:51:40 UTC (rev 166238)
@@ -0,0 +1,4 @@
+
+PASS Tests that the 'removetrack' event is fired when an out-of-band TextTrack is removed.
+PASS Tests that the 'removetrack' event is NOT fired for inband TextTrack on a failed load.
+
Added: trunk/LayoutTests/media/track/track-remove-track.html (0 => 166238)
--- trunk/LayoutTests/media/track/track-remove-track.html (rev 0)
+++ trunk/LayoutTests/media/track/track-remove-track.html 2014-03-25 16:51:40 UTC (rev 166238)
@@ -0,0 +1,107 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+
+ <script src=""
+ <script src=""
+ <script src=""
+ <script src=""
+ </head>
+ <body>
+ <script>
+ async_test(function(test)
+ {
+ var video = document.createElement("video");
+ var track;
+
+ function trackRemoved()
+ {
+ assert_equals(event.target, video.textTracks);
+ assert_equals(event instanceof window.TrackEvent, true);
+ assert_equals(event.track, track);
+ test.done();
+ }
+
+ var trackElement = document.createElement('track');
+ video.appendChild(trackElement);
+
+ trackElement.src = '';
+ trackElement.track.mode = 'hidden';
+
+ assert_equals(video.textTracks.length, 1);
+
+ track = video.textTracks[0];
+ video.removeChild(trackElement);
+ video.textTracks.addEventListener("removetrack", test.step_func(trackRemoved));
+ }, "Tests that the 'removetrack' event is fired when an out-of-band TextTrack is removed.");
+
+ async_test(function(test)
+ {
+ var video = document.createElement("video");
+
+ // Create an out-of-band text track by adding a track element.
+ var trackElement = document.createElement('track');
+
+ trackElement.addEventListener("error", test.step_func(function()
+ {
+ assert_unreached("'error' event on track element should not fire.")
+ }));
+
+ video.appendChild(trackElement);
+ trackElement.src = '';
+ trackElement.track.mode = 'hidden';
+
+ assert_equals(video.textTracks.length, 1);
+ var outOfBandTrack = video.textTracks[0];
+
+ // Load a media file with an inband text track.
+ var inbandTrack = null;
+ var url = ""
+
+ var firstAddTrackHandler = test.step_func(function()
+ {
+ assert_equals(event.target, video.textTracks);
+ assert_equals(event instanceof window.TrackEvent, true);
+ if (event.track == outOfBandTrack) {
+ return;
+ }
+
+ assert_equals(inbandTrack, null);
+ assert_equals(video.textTracks.length, 2);
+ assert_equals(event.track, video.textTracks[1]);
+ inbandTrack = event.track;
+
+ video.textTracks.removeEventListener("addtrack", firstAddTrackHandler);
+
+ // Clear .src to force the inband track to get destroyed.
+ video.src = ""
+
+ // Verify that the inband track was removed.
+ assert_not_equals(inbandTrack, null);
+ assert_equals(video.textTracks.length, 1);
+ assert_equals(video.textTracks[0], outOfBandTrack);
+
+ // Load the URL again to trigger another 'addtrack' event to make sure
+ // no 'removetrack' event was queued.
+ video.src = ""
+ video.textTracks.addEventListener("addtrack", test.step_func(function()
+ {
+ assert_equals(video.textTracks.length, 2);
+ test.done();
+ }));
+ });
+ video.textTracks.addEventListener("addtrack", firstAddTrackHandler);
+
+ video.textTracks.addEventListener("removetrack", test.step_func(function()
+ {
+ assert_unreached("'removetrack' event should not fire.")
+ }));
+
+ video.src = ""
+ }, "Tests that the 'removetrack' event is NOT fired for inband TextTrack on a failed load.");
+
+ </script>
+ <div id="log"></div>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (166237 => 166238)
--- trunk/Source/WebCore/ChangeLog 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/Source/WebCore/ChangeLog 2014-03-25 16:51:40 UTC (rev 166238)
@@ -1,3 +1,34 @@
+2014-03-24 Brent Fulgham <bfulg...@apple.com>
+
+ Prevent 'removetrack' events from firing when all inband text tracks are removed.
+ https://bugs.webkit.org/show_bug.cgi?id=130704
+
+ Reviewed by Eric Carlson.
+
+ Test: media/track/track-remove-track.html
+
+ Based on the Blink change (patch by acolw...@chromium.org):
+ https://codereview.chromium.org/177243018/
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::prepareForLoad): Reorder steps to match W3C specification.
+ (WebCore::HTMLMediaElement::noneSupported): Forget tracks as required by specification.
+ (WebCore::HTMLMediaElement::mediaLoadingFailed): Forget tracks as required by specification.
+ (WebCore::HTMLMediaElement::removeTextTrack): Only request the 'removetracks' event if
+ requested by caller.
+ (WebCore::HTMLMediaElement::removeAllInbandTracks): Renamed to 'forgetResourceSpecificTracks'
+ (WebCore::HTMLMediaElement::noneSupported): Specify that we want the 'removetracks' event
+ fired for this use case.
+ (WebCore::HTMLMediaElement::prepareForLoad): Switch to new 'forgetResourceSpecificTracks' name.
+ * html/HTMLMediaElement.h:
+ * html/track/TextTrackList.cpp:
+ (TextTrackList::remove): Only fire the 'removetrack' event if the caller requests it.
+ * html/track/TextTrackList.h: Add default argument to fire the 'removetrack' event
+ when removing a track.
+ * html/track/TrackListBase.cpp:
+ (TrackListBase::remove): Only fire the 'removetrack' event if the caller requests it.
+ * html/track/TrackListBase.h: Add default argument to fire the 'removetrack' event.
+
2014-03-25 David Kilzer <ddkil...@apple.com>
Hold a reference to firstSuccessfulSubmitButton in HTMLFormElement::submit
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (166237 => 166238)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2014-03-25 16:51:40 UTC (rev 166238)
@@ -978,14 +978,40 @@
// 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(eventNames().emptiedEvent);
+
+ // 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. e.g., scheduleEvent(eventNames().timeUpdateEvent);
+
+ // 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(eventNames().emptiedEvent);
+
+ // 4.9 - Set the timeline offset to Not-a-Number (NaN).
+ // 4.10 - Update the duration attribute to Not-a-Number (NaN).
+
updateMediaController();
#if ENABLE(VIDEO_TRACK)
if (RuntimeEnabledFeatures::sharedFeatures().webkitVideoTrackEnabled())
@@ -1011,6 +1037,9 @@
// 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;
// The spec doesn't say to block the load event until we actually run the asynchronous section
@@ -1792,6 +1821,7 @@
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;
@@ -1917,11 +1947,18 @@
// <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
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()) {
LOG(Media, "HTMLMediaElement::setNetworkState - scheduling next <source>");
scheduleNextSourceChild();
@@ -3365,7 +3402,7 @@
m_audioTracks->remove(track);
}
-void HTMLMediaElement::removeTextTrack(TextTrack* track)
+void HTMLMediaElement::removeTextTrack(TextTrack* track, bool scheduleEvent)
{
if (!RuntimeEnabledFeatures::sharedFeatures().webkitVideoTrackEnabled())
return;
@@ -3375,7 +3412,7 @@
if (cues)
textTrackRemoveCues(track, cues);
track->clearClient();
- m_textTracks->remove(track);
+ m_textTracks->remove(track, scheduleEvent);
closeCaptionTracksChanged();
}
@@ -3388,7 +3425,7 @@
m_videoTracks->remove(track);
}
-void HTMLMediaElement::removeAllInbandTracks()
+void HTMLMediaElement::forgetResourceSpecificTracks()
{
while (m_audioTracks && m_audioTracks->length())
removeAudioTrack(m_audioTracks->lastItem());
@@ -3399,7 +3436,7 @@
TextTrack* track = m_textTracks->item(i);
if (track->trackType() == TextTrack::InBand)
- removeTextTrack(track);
+ removeTextTrack(track, false);
}
}
@@ -4519,7 +4556,7 @@
#endif
#if ENABLE(VIDEO_TRACK)
- removeAllInbandTracks();
+ forgetResourceSpecificTracks();
#endif
#if ENABLE(MEDIA_SOURCE)
@@ -5424,7 +5461,7 @@
#endif
#if ENABLE(VIDEO_TRACK)
- removeAllInbandTracks();
+ forgetResourceSpecificTracks();
#endif
m_player = MediaPlayer::create(this);
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (166237 => 166238)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2014-03-25 16:51:40 UTC (rev 166238)
@@ -290,9 +290,9 @@
void addTextTrack(PassRefPtr<TextTrack>);
void addVideoTrack(PassRefPtr<VideoTrack>);
void removeAudioTrack(AudioTrack*);
- void removeTextTrack(TextTrack*);
+ void removeTextTrack(TextTrack*, bool scheduleEvent = true);
void removeVideoTrack(VideoTrack*);
- void removeAllInbandTracks();
+ void forgetResourceSpecificTracks();
void closeCaptionTracksChanged();
void notifyMediaPlayerOfTextTrackChanges();
Modified: trunk/Source/WebCore/html/track/TextTrackList.cpp (166237 => 166238)
--- trunk/Source/WebCore/html/track/TextTrackList.cpp 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/Source/WebCore/html/track/TextTrackList.cpp 2014-03-25 16:51:40 UTC (rev 166238)
@@ -195,7 +195,7 @@
scheduleAddTrackEvent(track.release());
}
-void TextTrackList::remove(TrackBase* track)
+void TextTrackList::remove(TrackBase* track, bool scheduleEvent)
{
TextTrack* textTrack = toTextTrack(track);
Vector<RefPtr<TrackBase>>* tracks = 0;
@@ -219,7 +219,9 @@
RefPtr<TrackBase> trackRef = (*tracks)[index];
tracks->remove(index);
- scheduleRemoveTrackEvent(trackRef.release());
+
+ if (scheduleEvent)
+ scheduleRemoveTrackEvent(trackRef.release());
}
bool TextTrackList::contains(TrackBase* track) const
Modified: trunk/Source/WebCore/html/track/TextTrackList.h (166237 => 166238)
--- trunk/Source/WebCore/html/track/TextTrackList.h 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/Source/WebCore/html/track/TextTrackList.h 2014-03-25 16:51:40 UTC (rev 166238)
@@ -52,7 +52,7 @@
TextTrack* lastItem() const { return item(length() - 1); }
void append(PassRefPtr<TextTrack>);
- virtual void remove(TrackBase*) override;
+ virtual void remove(TrackBase*, bool scheduleEvent = true) override;
// EventTarget
virtual EventTargetInterface eventTargetInterface() const override;
Modified: trunk/Source/WebCore/html/track/TrackListBase.cpp (166237 => 166238)
--- trunk/Source/WebCore/html/track/TrackListBase.cpp 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/Source/WebCore/html/track/TrackListBase.cpp 2014-03-25 16:51:40 UTC (rev 166238)
@@ -58,7 +58,7 @@
return m_inbandTracks.size();
}
-void TrackListBase::remove(TrackBase* track)
+void TrackListBase::remove(TrackBase* track, bool scheduleEvent)
{
size_t index = m_inbandTracks.find(track);
ASSERT(index != notFound);
@@ -70,7 +70,8 @@
m_inbandTracks.remove(index);
- scheduleRemoveTrackEvent(trackRef.release());
+ if (scheduleEvent)
+ scheduleRemoveTrackEvent(trackRef.release());
}
bool TrackListBase::contains(TrackBase* track) const
Modified: trunk/Source/WebCore/html/track/TrackListBase.h (166237 => 166238)
--- trunk/Source/WebCore/html/track/TrackListBase.h 2014-03-25 16:44:33 UTC (rev 166237)
+++ trunk/Source/WebCore/html/track/TrackListBase.h 2014-03-25 16:51:40 UTC (rev 166238)
@@ -48,7 +48,7 @@
virtual unsigned length() const;
virtual bool contains(TrackBase*) const;
- virtual void remove(TrackBase*);
+ virtual void remove(TrackBase*, bool scheduleEvent = true);
// EventTarget
virtual EventTargetInterface eventTargetInterface() const = 0;
