- Revision
- 138224
- Author
- eric.carl...@apple.com
- Date
- 2012-12-19 21:50:34 -0800 (Wed, 19 Dec 2012)
Log Message
Crash in TextTrack::trackIndexRelativeToRenderedTracks()
https://bugs.webkit.org/show_bug.cgi?id=105371
Reviewed by Simon Fraser.
Source/WebCore:
Add an RAII object to manage text track update blocking, use it to always process the
current cues to ensure that cues from a track that is deleted are removed from the
shadow DOM before the next layout.
No new tests, this fixes a crash in media/video-controls-captions-trackmenu.html.
* html/HTMLMediaElement.cpp:
(WebCore::TrackDisplayUpdateScope::TrackDisplayUpdateScope): New, call beginIgnoringTrackDisplayUpdateRequests.
(WebCore::TrackDisplayUpdateScope::~TrackDisplayUpdateScope): New, call endIgnoringTrackDisplayUpdateRequests.
(WebCore::HTMLMediaElement::beginIgnoringTrackDisplayUpdateRequests):
(WebCore::HTMLMediaElement::endIgnoringTrackDisplayUpdateRequests): Call updateActiveTextTrackCues
when the ignore count reaches zero.
(WebCore::HTMLMediaElement::textTrackAddCues): Use TrackDisplayUpdateScope instead of calling
beginIgnoringTrackDisplayUpdateRequests and endIgnoringTrackDisplayUpdateRequests directly.
(WebCore::HTMLMediaElement::textTrackRemoveCues): Ditto.
(WebCore::HTMLMediaElement::removeTrack): Ditto.
(WebCore::HTMLMediaElement::removeAllInbandTracks): Ditto.
(WebCore::HTMLMediaElement::didRemoveTrack): Ditto. Call removeTrack.
* html/HTMLMediaElement.h: Declare TrackDisplayUpdateScope as a friend of HTMLMediaElement so it
can call protected methods.
LayoutTests:
* platform/mac/TestExpectations: Unskip video-controls-captions-trackmenu.html.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (138223 => 138224)
--- trunk/LayoutTests/ChangeLog 2012-12-20 05:50:30 UTC (rev 138223)
+++ trunk/LayoutTests/ChangeLog 2012-12-20 05:50:34 UTC (rev 138224)
@@ -1,5 +1,14 @@
2012-12-19 Eric Carlson <eric.carl...@apple.com>
+ Crash in TextTrack::trackIndexRelativeToRenderedTracks()
+ https://bugs.webkit.org/show_bug.cgi?id=105371
+
+ Reviewed by Simon Fraser.
+
+ * platform/mac/TestExpectations: Unskip video-controls-captions-trackmenu.html.
+
+2012-12-19 Eric Carlson <eric.carl...@apple.com>
+
Update video-controls-captions-trackmenu.html
https://bugs.webkit.org/show_bug.cgi?id=105455
Modified: trunk/LayoutTests/platform/mac/TestExpectations (138223 => 138224)
--- trunk/LayoutTests/platform/mac/TestExpectations 2012-12-20 05:50:30 UTC (rev 138223)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2012-12-20 05:50:34 UTC (rev 138224)
@@ -473,9 +473,6 @@
media/track/track-cue-rendering-vertical.html
media/track/track-webvtt-tc028-unsupported-markup.html
-# Asserts: https://bugs.webkit.org/show_bug.cgi?id=105371
-[ Debug ] media/video-controls-captions-trackmenu.html
-
# Opera-submitted tests to W3C for <track>, a lot of failures still.
# Opera-submitted tests to W3C for <track>, a lot of failures still.
webkit.org/b/103926 media/track/opera/idl/media-idl-tests.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (138223 => 138224)
--- trunk/Source/WebCore/ChangeLog 2012-12-20 05:50:30 UTC (rev 138223)
+++ trunk/Source/WebCore/ChangeLog 2012-12-20 05:50:34 UTC (rev 138224)
@@ -1,3 +1,31 @@
+2012-12-19 Eric Carlson <eric.carl...@apple.com>
+
+ Crash in TextTrack::trackIndexRelativeToRenderedTracks()
+ https://bugs.webkit.org/show_bug.cgi?id=105371
+
+ Reviewed by Simon Fraser.
+
+ Add an RAII object to manage text track update blocking, use it to always process the
+ current cues to ensure that cues from a track that is deleted are removed from the
+ shadow DOM before the next layout.
+
+ No new tests, this fixes a crash in media/video-controls-captions-trackmenu.html.
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::TrackDisplayUpdateScope::TrackDisplayUpdateScope): New, call beginIgnoringTrackDisplayUpdateRequests.
+ (WebCore::TrackDisplayUpdateScope::~TrackDisplayUpdateScope): New, call endIgnoringTrackDisplayUpdateRequests.
+ (WebCore::HTMLMediaElement::beginIgnoringTrackDisplayUpdateRequests):
+ (WebCore::HTMLMediaElement::endIgnoringTrackDisplayUpdateRequests): Call updateActiveTextTrackCues
+ when the ignore count reaches zero.
+ (WebCore::HTMLMediaElement::textTrackAddCues): Use TrackDisplayUpdateScope instead of calling
+ beginIgnoringTrackDisplayUpdateRequests and endIgnoringTrackDisplayUpdateRequests directly.
+ (WebCore::HTMLMediaElement::textTrackRemoveCues): Ditto.
+ (WebCore::HTMLMediaElement::removeTrack): Ditto.
+ (WebCore::HTMLMediaElement::removeAllInbandTracks): Ditto.
+ (WebCore::HTMLMediaElement::didRemoveTrack): Ditto. Call removeTrack.
+ * html/HTMLMediaElement.h: Declare TrackDisplayUpdateScope as a friend of HTMLMediaElement so it
+ can call protected methods.
+
2012-12-19 Nate Chapin <jap...@chromium.org>
REGRESSION(r137607): resource load client callbacks are not called for the main resource when loading HTML string
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (138223 => 138224)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2012-12-20 05:50:30 UTC (rev 138223)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2012-12-20 05:50:34 UTC (rev 138224)
@@ -202,6 +202,25 @@
}
#endif
+#if ENABLE(VIDEO_TRACK)
+class TrackDisplayUpdateScope {
+public:
+ TrackDisplayUpdateScope(HTMLMediaElement* mediaElement)
+ {
+ m_mediaElement = mediaElement;
+ m_mediaElement->beginIgnoringTrackDisplayUpdateRequests();
+ }
+ ~TrackDisplayUpdateScope()
+ {
+ ASSERT(m_mediaElement);
+ m_mediaElement->endIgnoringTrackDisplayUpdateRequests();
+ }
+
+private:
+ HTMLMediaElement* m_mediaElement;
+};
+#endif
+
HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document* document, bool createdByParser)
: HTMLElement(tagName, document)
, ActiveDOMObject(document, this)
@@ -1352,22 +1371,31 @@
track->setMode(TextTrack::hiddenKeyword());
}
+void HTMLMediaElement::beginIgnoringTrackDisplayUpdateRequests()
+{
+ ++m_ignoreTrackDisplayUpdate;
+}
+
+void HTMLMediaElement::endIgnoringTrackDisplayUpdateRequests()
+{
+ ASSERT(m_ignoreTrackDisplayUpdate);
+ --m_ignoreTrackDisplayUpdate;
+ if (!m_ignoreTrackDisplayUpdate)
+ updateActiveTextTrackCues(currentTime());
+}
+
void HTMLMediaElement::textTrackAddCues(TextTrack*, const TextTrackCueList* cues)
{
- beginIgnoringTrackDisplayUpdateRequests();
+ TrackDisplayUpdateScope(this);
for (size_t i = 0; i < cues->length(); ++i)
textTrackAddCue(cues->item(i)->track(), cues->item(i));
- endIgnoringTrackDisplayUpdateRequests();
- updateActiveTextTrackCues(currentTime());
}
void HTMLMediaElement::textTrackRemoveCues(TextTrack*, const TextTrackCueList* cues)
{
- beginIgnoringTrackDisplayUpdateRequests();
+ TrackDisplayUpdateScope(this);
for (size_t i = 0; i < cues->length(); ++i)
textTrackRemoveCue(cues->item(i)->track(), cues->item(i));
- endIgnoringTrackDisplayUpdateRequests();
- updateActiveTextTrackCues(currentTime());
}
void HTMLMediaElement::textTrackAddCue(TextTrack*, PassRefPtr<TextTrackCue> cue)
@@ -2773,12 +2801,11 @@
void HTMLMediaElement::removeTrack(TextTrack* track)
{
- beginIgnoringTrackDisplayUpdateRequests();
- m_textTracks->remove(track);
+ TrackDisplayUpdateScope(this);
TextTrackCueList* cues = track->cues();
if (cues)
textTrackRemoveCues(track, cues);
- endIgnoringTrackDisplayUpdateRequests();
+ m_textTracks->remove(track);
}
void HTMLMediaElement::removeAllInbandTracks()
@@ -2786,14 +2813,13 @@
if (!m_textTracks)
return;
- beginIgnoringTrackDisplayUpdateRequests();
+ TrackDisplayUpdateScope(this);
for (int i = m_textTracks->length() - 1; i >= 0; --i) {
TextTrack* track = m_textTracks->item(i);
if (track->trackType() == TextTrack::InBand)
removeTrack(track);
}
- endIgnoringTrackDisplayUpdateRequests();
}
PassRefPtr<TextTrack> HTMLMediaElement::addTextTrack(const String& kind, const String& label, const String& language, ExceptionCode& ec)
@@ -2897,14 +2923,7 @@
// 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.
- m_textTracks->remove(textTrack.get());
- if (textTrack->cues()) {
- TextTrackCueList* cues = textTrack->cues();
- beginIgnoringTrackDisplayUpdateRequests();
- for (size_t i = 0; i < cues->length(); ++i)
- textTrackRemoveCue(cues->item(i)->track(), cues->item(i));
- endIgnoringTrackDisplayUpdateRequests();
- }
+ removeTrack(textTrack.get());
if (hasMediaControls())
mediaControls()->closedCaptionTracksChanged();
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (138223 => 138224)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2012-12-20 05:50:30 UTC (rev 138223)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2012-12-20 05:50:34 UTC (rev 138224)
@@ -372,7 +372,13 @@
void addBehaviorRestriction(BehaviorRestrictions restriction) { m_restrictions |= restriction; }
void removeBehaviorRestriction(BehaviorRestrictions restriction) { m_restrictions &= ~restriction; }
-
+
+#if ENABLE(VIDEO_TRACK)
+ bool ignoreTrackDisplayUpdateRequests() const { return m_ignoreTrackDisplayUpdate > 0; }
+ void beginIgnoringTrackDisplayUpdateRequests();
+ void endIgnoringTrackDisplayUpdateRequests();
+#endif
+
private:
void createMediaPlayer();
@@ -501,10 +507,6 @@
void updateActiveTextTrackCues(float);
HTMLTrackElement* showingTrackWithSameKind(HTMLTrackElement*) const;
- bool ignoreTrackDisplayUpdateRequests() const { return m_ignoreTrackDisplayUpdate > 0; }
- void beginIgnoringTrackDisplayUpdateRequests() { ++m_ignoreTrackDisplayUpdate; }
- void endIgnoringTrackDisplayUpdateRequests() { ASSERT(m_ignoreTrackDisplayUpdate); --m_ignoreTrackDisplayUpdate; }
-
void markCaptionAndSubtitleTracksAsUnconfigured();
virtual void captionPreferencesChanged() OVERRIDE;
#endif
@@ -701,6 +703,8 @@
#if PLATFORM(MAC)
OwnPtr<DisplaySleepDisabler> m_sleepDisabler;
#endif
+
+ friend class TrackDisplayUpdateScope;
};
#if ENABLE(VIDEO_TRACK)