Cleanup HTMLMediaElement track lists.
authoreric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Aug 2016 23:03:30 +0000 (23:03 +0000)
committereric.carlson@apple.com <eric.carlson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Aug 2016 23:03:30 +0000 (23:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160470

Reviewed by David Kilzer.

Source/WebCore:

Test: media/range-extract-contents-crash.html

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::~HTMLMediaElement):

* html/track/AudioTrack.cpp:
(WebCore::AudioTrack::willRemove): ASSERT if media element is NULL.

* html/track/TextTrackList.cpp:
(TextTrackList::clearElement): Clear track media element pointers and client.
* html/track/TextTrackList.h:

* html/track/TrackListBase.cpp:
(TrackListBase::~TrackListBase): Call clearElement.
(TrackListBase::clearElement): Clear track media element pointers and client.
* html/track/TrackListBase.h:

LayoutTests:

* media/range-extract-contents-crash-expected.txt: Added.
* media/range-extract-contents-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204050 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/range-extract-contents-crash-expected.txt [new file with mode: 0644]
LayoutTests/media/range-extract-contents-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/track/AudioTrack.cpp
Source/WebCore/html/track/TextTrackList.cpp
Source/WebCore/html/track/TextTrackList.h
Source/WebCore/html/track/TrackListBase.cpp
Source/WebCore/html/track/TrackListBase.h

index 9c3a7b2..d9b286e 100644 (file)
@@ -1,3 +1,13 @@
+2016-08-02  Eric Carlson  <eric.carlson@apple.com>
+
+        Cleanup HTMLMediaElement track lists.
+        https://bugs.webkit.org/show_bug.cgi?id=160470
+
+        Reviewed by David Kilzer.
+
+        * media/range-extract-contents-crash-expected.txt: Added.
+        * media/range-extract-contents-crash.html: Added.
+
 2016-08-02  Chris Dumez  <cdumez@apple.com>
 
         Named / Indexed properties should be configurable
diff --git a/LayoutTests/media/range-extract-contents-crash-expected.txt b/LayoutTests/media/range-extract-contents-crash-expected.txt
new file mode 100644 (file)
index 0000000..3d00292
--- /dev/null
@@ -0,0 +1,4 @@
+
+Tests that moving a video element from the document tree into a DocumentFragment does not crash.
+
+If this test does not crash, it passes.
diff --git a/LayoutTests/media/range-extract-contents-crash.html b/LayoutTests/media/range-extract-contents-crash.html
new file mode 100644 (file)
index 0000000..4679262
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src="../resources/gc.js"></script>
+<script>
+    if (window.testRunner) {
+        window.testRunner.dumpAsText();
+        window.testRunner.waitUntilDone();
+    }
+
+    let tracks = [];
+
+    function runTest()
+    {
+        div = document.getElementsByTagName('div')[0];
+
+        audio = document.createElement("audio");
+        div.appendChild(audio);
+
+        video = document.createElement("video");
+        div.appendChild(video);
+
+        for (let i = 0; i < 10; i++) {
+            let track = document.createElement("track");
+            track.src = 'data:text/vtt,'+encodeURIComponent('WEBVTT\n\n00:00:00.000 --> 00:00:01.000\ntest\n');
+            track.kind = "caption";
+            track.srclang = "fr";
+            video.appendChild(track);
+            tracks.push(track);
+        }
+
+        object = document.createElement("object");
+        video.appendChild(object);
+
+        range = document.createRange();
+        range.selectNodeContents(audio);
+        range.setEndBefore(object);
+        setTimeout(step1, 0);
+        gc();
+    }
+
+    function step1()
+    {
+        range.extractContents();
+        gc();
+        setTimeout(step2, 0);
+    }
+
+    function step2()
+    {
+        for (let i = 0; i < 10; i++)
+            tracks[i].srclang = "en";
+
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    }
+</script>
+</head>
+<body onload="runTest()">
+    <div></div>
+    <p>Tests that moving a video element from the document tree into a DocumentFragment does not crash.</p>
+    <p>If this test does not crash, it passes.</p>
+</body>
+</html>
index 088cd5e..d7fe402 100644 (file)
@@ -1,3 +1,27 @@
+2016-08-02  Eric Carlson  <eric.carlson@apple.com>
+
+        Cleanup HTMLMediaElement track lists.
+        https://bugs.webkit.org/show_bug.cgi?id=160470
+
+        Reviewed by David Kilzer.
+
+        Test: media/range-extract-contents-crash.html
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::~HTMLMediaElement): 
+
+        * html/track/AudioTrack.cpp:
+        (WebCore::AudioTrack::willRemove): ASSERT if media element is NULL.
+
+        * html/track/TextTrackList.cpp:
+        (TextTrackList::clearElement): Clear track media element pointers and client.
+        * html/track/TextTrackList.h:
+
+        * html/track/TrackListBase.cpp:
+        (TrackListBase::~TrackListBase): Call clearElement.
+        (TrackListBase::clearElement): Clear track media element pointers and client.
+        * html/track/TrackListBase.h:
+
 2016-08-02  Anders Carlsson  <andersca@apple.com>
 
         Remove more Objective-C bindings that are not used
index 7ab3f5d..ae78778 100644 (file)
@@ -509,22 +509,12 @@ HTMLMediaElement::~HTMLMediaElement()
     unregisterWithDocument(document());
 
 #if ENABLE(VIDEO_TRACK)
-    if (m_audioTracks) {
+    if (m_audioTracks)
         m_audioTracks->clearElement();
-        for (unsigned i = 0; i < m_audioTracks->length(); ++i)
-            m_audioTracks->item(i)->clearClient();
-    }
     if (m_textTracks)
         m_textTracks->clearElement();
-    if (m_textTracks) {
-        for (unsigned i = 0; i < m_textTracks->length(); ++i)
-            m_textTracks->item(i)->clearClient();
-    }
-    if (m_videoTracks) {
+    if (m_videoTracks)
         m_videoTracks->clearElement();
-        for (unsigned i = 0; i < m_videoTracks->length(); ++i)
-            m_videoTracks->item(i)->clearClient();
-    }
 #endif
 
 #if ENABLE(WIRELESS_PLAYBACK_TARGET)
index 8c356f7..23921af 100644 (file)
@@ -167,7 +167,9 @@ void AudioTrack::languageChanged(TrackPrivateBase* trackPrivate, const AtomicStr
 void AudioTrack::willRemove(TrackPrivateBase* trackPrivate)
 {
     ASSERT_UNUSED(trackPrivate, trackPrivate == m_private);
-    mediaElement()->removeAudioTrack(*this);
+    ASSERT(mediaElement());
+    if (mediaElement())
+        mediaElement()->removeAudioTrack(*this);
 }
 
 void AudioTrack::updateKindFromPrivate()
index 9d78705..d684189 100644 (file)
@@ -46,6 +46,19 @@ TextTrackList::~TextTrackList()
 {
 }
 
+void TextTrackList::clearElement()
+{
+    TrackListBase::clearElement();
+    for (auto& track : m_elementTracks) {
+        track->setMediaElement(nullptr);
+        track->clearClient();
+    }
+    for (auto& track : m_addTrackTracks) {
+        track->setMediaElement(nullptr);
+        track->clearClient();
+    }
+}
+
 unsigned TextTrackList::length() const
 {
     return m_addTrackTracks.size() + m_elementTracks.size() + m_inbandTracks.size();
index 9297855..baa7c96 100644 (file)
@@ -42,6 +42,8 @@ public:
     }
     virtual ~TextTrackList();
 
+    void clearElement() override;
+
     unsigned length() const override;
     int getTrackIndex(TextTrack&);
     int getTrackIndexRelativeToRenderedTracks(TextTrack&);
index dbed3aa..c9372bf 100644 (file)
@@ -46,6 +46,16 @@ TrackListBase::TrackListBase(HTMLMediaElement* element, ScriptExecutionContext*
 
 TrackListBase::~TrackListBase()
 {
+    clearElement();
+}
+
+void TrackListBase::clearElement()
+{
+    m_element = nullptr;
+    for (auto& track : m_inbandTracks) {
+        track->setMediaElement(nullptr);
+        track->clearClient();
+    }
 }
 
 Element* TrackListBase::element() const
index e2e18d0..feff13e 100644 (file)
@@ -56,7 +56,7 @@ public:
     using RefCounted<TrackListBase>::deref;
     ScriptExecutionContext* scriptExecutionContext() const final { return m_context; }
 
-    void clearElement() { m_element = 0; }
+    virtual void clearElement();
     Element* element() const;
     HTMLMediaElement* mediaElement() const { return m_element; }