Title: [135906] trunk
- Revision
- 135906
- Author
- [email protected]
- Date
- 2012-11-27 13:00:23 -0800 (Tue, 27 Nov 2012)
Log Message
Use-after-free in media player handling
https://bugs.webkit.org/show_bug.cgi?id=103426
Patch by Aaron Colwell <[email protected]> on 2012-11-27
Reviewed by Eric Carlson.
Source/WebCore:
Fixed use-after-free bugs caused by the MediaSource not being closed before the HTMLMediaElement or the MediaPlayer
is destroyed. Closing the MediaSource causes it to clear its reference to the MediaPlayer which prevents
the use-after-free problems from happening.
Test: http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction.html
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::~HTMLMediaElement):
(WebCore::HTMLMediaElement::clearMediaPlayer):
LayoutTests:
- Added a test to verify that the MediaSource gets closed when the HTMLMediaElement is destroyed
after it is removed from the DOM.
- Updated video-media-source-state-changes-expected.txt to reflect a slight change in event dispatch ordering.
* http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction-expected.txt: Added.
* http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction.html: Added.
* http/tests/media/media-source/video-media-source-state-changes-expected.txt:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (135905 => 135906)
--- trunk/LayoutTests/ChangeLog 2012-11-27 20:47:57 UTC (rev 135905)
+++ trunk/LayoutTests/ChangeLog 2012-11-27 21:00:23 UTC (rev 135906)
@@ -1,3 +1,19 @@
+2012-11-27 Aaron Colwell <[email protected]>
+
+ Use-after-free in media player handling
+ https://bugs.webkit.org/show_bug.cgi?id=103426
+
+ Reviewed by Eric Carlson.
+
+ - Added a test to verify that the MediaSource gets closed when the HTMLMediaElement is destroyed
+ after it is removed from the DOM.
+ - Updated video-media-source-state-changes-expected.txt to reflect a slight change in event dispatch ordering.
+
+
+ * http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction-expected.txt: Added.
+ * http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction.html: Added.
+ * http/tests/media/media-source/video-media-source-state-changes-expected.txt:
+
2012-11-27 David Grogan <[email protected]>
IndexedDB: Remove IDBDatabase.setVersion API
Added: trunk/LayoutTests/http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction-expected.txt (0 => 135906)
--- trunk/LayoutTests/http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction-expected.txt 2012-11-27 21:00:23 UTC (rev 135906)
@@ -0,0 +1,9 @@
+Tests that the MediaSource is closed when the HTMLMediaElement is destroyed.
+
+sourceOpened called.
+Removing video element from DOM.
+Running the garbage collector.
+Setting timestampOffset failed as expected: InvalidStateError
+sourceClosed called.
+END OF TEST
+
Added: trunk/LayoutTests/http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction.html (0 => 135906)
--- trunk/LayoutTests/http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction.html (rev 0)
+++ trunk/LayoutTests/http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction.html 2012-11-27 21:00:23 UTC (rev 135906)
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <script src=""
+ <script src=""
+
+ <script>
+ var ms = new WebKitMediaSource();
+
+ function sourceOpened()
+ {
+ consoleWrite("sourceOpened called.");
+ var vid = document.getElementById('vid');
+ var buffer = ms.addSourceBuffer('video/webm; codecs="vorbis,vp8"');
+
+ MediaSourceTest.expectSourceState(ms, "open");
+
+ consoleWrite("Removing video element from DOM.");
+ vid.parentNode.removeChild(vid);
+ vid = null;
+
+ consoleWrite("Running the garbage collector.");
+ gc();
+
+ MediaSourceTest.expectSourceState(ms, "closed");
+
+ try {
+ buffer.timestampOffset = 42;
+ failTest("Expected an exception");
+ } catch (e) {
+ consoleWrite("Setting timestampOffset failed as expected: " + e.name);
+ }
+ }
+
+ function sourceClosed()
+ {
+ consoleWrite("sourceClosed called.");
+ endTest();
+ }
+
+ function onLoad()
+ {
+ waitForEventAndFail('error');
+
+ ms.addEventListener('webkitsourceopen', sourceOpened);
+ ms.addEventListener('webkitsourceclose', sourceClosed);
+
+ document.getElementById('vid').src = ""
+ }
+ </script>
+ </head>
+ <body _onload_="onLoad()">
+ <video id="vid"></video>
+ <p>Tests that the MediaSource is closed when the HTMLMediaElement is destroyed.</p>
+ </body>
+</html>
Modified: trunk/LayoutTests/http/tests/media/media-source/video-media-source-state-changes-expected.txt (135905 => 135906)
--- trunk/LayoutTests/http/tests/media/media-source/video-media-source-state-changes-expected.txt 2012-11-27 20:47:57 UTC (rev 135905)
+++ trunk/LayoutTests/http/tests/media/media-source/video-media-source-state-changes-expected.txt 2012-11-27 21:00:23 UTC (rev 135906)
@@ -16,18 +16,18 @@
onSecondSeeking
EVENT(seeked)
onSecondSeeked
-EVENT(emptied)
EVENT(webkitsourceclose) : closed
onFirstSourceClose
+EVENT(emptied)
EVENT(webkitsourceopen) : open
onSecondSourceOpen
EVENT(webkitsourceended) : ended
onSecondSourceEnded
EVENT(playing)
triggerSecondSourceClose
-EVENT(emptied)
EVENT(webkitsourceclose) : closed
onSecondSourceClose
+EVENT(emptied)
EVENT(webkitsourceopen) : open
onThirdSourceOpen
END OF TEST
Modified: trunk/Source/WebCore/ChangeLog (135905 => 135906)
--- trunk/Source/WebCore/ChangeLog 2012-11-27 20:47:57 UTC (rev 135905)
+++ trunk/Source/WebCore/ChangeLog 2012-11-27 21:00:23 UTC (rev 135906)
@@ -1,3 +1,20 @@
+2012-11-27 Aaron Colwell <[email protected]>
+
+ Use-after-free in media player handling
+ https://bugs.webkit.org/show_bug.cgi?id=103426
+
+ Reviewed by Eric Carlson.
+
+ Fixed use-after-free bugs caused by the MediaSource not being closed before the HTMLMediaElement or the MediaPlayer
+ is destroyed. Closing the MediaSource causes it to clear its reference to the MediaPlayer which prevents
+ the use-after-free problems from happening.
+
+ Test: http/tests/media/media-source/video-media-source-closed-on-htmlmediaelement-destruction.html
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::~HTMLMediaElement):
+ (WebCore::HTMLMediaElement::clearMediaPlayer):
+
2012-11-27 David Grogan <[email protected]>
IndexedDB: Remove IDBDatabase.setVersion API
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (135905 => 135906)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2012-11-27 20:47:57 UTC (rev 135905)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2012-11-27 21:00:23 UTC (rev 135906)
@@ -316,6 +316,10 @@
if (m_mediaController)
m_mediaController->removeMediaElement(this);
+#if ENABLE(MEDIA_SOURCE)
+ setSourceState(MediaSource::closedKeyword());
+#endif
+
removeElementFromDocumentMap(this, document());
}
@@ -3750,6 +3754,11 @@
void HTMLMediaElement::clearMediaPlayer(int flags)
{
#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO)
+
+#if ENABLE(MEDIA_SOURCE)
+ setSourceState(MediaSource::closedKeyword());
+#endif
+
m_player.clear();
#endif
stopPeriodicTimers();
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
