Title: [138224] trunk
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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to