git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 1f64465)
CRASH at WebCore::SourceBuffer::removeCodedFrames + 37
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Oct 2016 18:13:19 +0000 (18:13 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Oct 2016 18:13:19 +0000 (18:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163336

Reviewed by Alex Christensen.

Source/WebCore:

Test: media/media-source/media-source-remove-crash.html

A null-deref crash can occur if a SourceBuffer is removed from a MediaSource after
SourceBuffer.remove() is called, but before the removeTimer is fired.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::removeTimerFired):

LayoutTests:

* media/media-source/media-source-remove-crash-expected.txt: Added.
* media/media-source/media-source-remove-crash.html: Added.

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

LayoutTests/ChangeLog patch | blob | history
LayoutTests/media/media-source/media-source-remove-crash-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/media/media-source/media-source-remove-crash.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/Modules/mediasource/SourceBuffer.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 0d0f753..cc0e3f1 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2016-10-13  Jer Noble  <jer.noble@apple.com>
+
+        CRASH at WebCore::SourceBuffer::removeCodedFrames + 37
+        https://bugs.webkit.org/show_bug.cgi?id=163336
+
+        Reviewed by Alex Christensen.
+
+        * media/media-source/media-source-remove-crash-expected.txt: Added.
+        * media/media-source/media-source-remove-crash.html: Added.
+
 2016-10-13  Sergio Villar Senin  <svillar@igalia.com>
 
         [css-grid] Use min-size instead of min-content contribution for intrinsic maximums resolution
diff --git a/LayoutTests/media/media-source/media-source-remove-crash-expected.txt b/LayoutTests/media/media-source/media-source-remove-crash-expected.txt
new file mode 100644 (file)
index 0000000..3248ab8
--- /dev/null
+++ b/LayoutTests/media/media-source/media-source-remove-crash-expected.txt
@@ -0,0 +1,15 @@
+
+RUN(video.src = URL.createObjectURL(source))
+EVENT(sourceopen)
+RUN(source.duration = loader.duration())
+RUN(sourceBuffer = source.addSourceBuffer(loader.type()))
+RUN(sourceBuffer.appendBuffer(loader.initSegment()))
+EVENT(update)
+Append a media segment.
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(0)))
+EVENT(update)
+Remove a range, then remove SourceBuffer from its MediaSource. Should not crash.
+RUN(sourceBuffer.remove(0, source.duration))
+RUN(source.removeSourceBuffer(sourceBuffer))
+END OF TEST
+
diff --git a/LayoutTests/media/media-source/media-source-remove-crash.html b/LayoutTests/media/media-source/media-source-remove-crash.html
new file mode 100644 (file)
index 0000000..dca16c3
--- /dev/null
+++ b/LayoutTests/media/media-source/media-source-remove-crash.html
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>media-source-abort-resets-parser</title>
+    <script src="media-source-loader.js"></script>
+    <script src="../video-test.js"></script>
+    <script>
+    var loader;
+    var source;
+    var sourceBuffer;
+
+    function runTest() {
+        findMediaElement();
+
+        loader = new MediaSourceLoader('content/test-fragmented-manifest.json');
+        loader.onload = mediaDataLoaded;
+        loader.onerror = mediaDataLoadingFailed;
+    }
+
+    function mediaDataLoadingFailed() {
+        failTest('Media data loading failed');
+    }
+
+    function mediaDataLoaded() {
+        source = new MediaSource();
+        waitForEvent('sourceopen', sourceOpen, false, false, source);
+        waitForEventAndFail('error');
+        run('video.src = URL.createObjectURL(source)');
+    }
+
+    function sourceOpen() {
+        run('source.duration = loader.duration()');
+        run('sourceBuffer = source.addSourceBuffer(loader.type())');
+        waitForEventOn(sourceBuffer, 'update', sourceInitialized, false, true);
+        run('sourceBuffer.appendBuffer(loader.initSegment())');
+    }
+
+    function sourceInitialized() {
+        consoleWrite('Append a media segment.')
+        waitForEventOn(sourceBuffer, 'update', mediaSegmentAppended, false, true);
+        run('sourceBuffer.appendBuffer(loader.mediaSegment(0))');
+    }
+
+    function mediaSegmentAppended() {
+        consoleWrite('Remove a range, then remove SourceBuffer from its MediaSource. Should not crash.')
+        run('sourceBuffer.remove(0, source.duration)');
+        run('source.removeSourceBuffer(sourceBuffer)');
+        setTimeout(endTest, 100);
+    }
+    </script>
+</head>
+<body onload="runTest()">
+    <video controls></video>
+</body>
+</html>
\ No newline at end of file
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index f768cf4..dfca420 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2016-10-13  Jer Noble  <jer.noble@apple.com>
+
+        CRASH at WebCore::SourceBuffer::removeCodedFrames + 37
+        https://bugs.webkit.org/show_bug.cgi?id=163336
+
+        Reviewed by Alex Christensen.
+
+        Test: media/media-source/media-source-remove-crash.html
+
+        A null-deref crash can occur if a SourceBuffer is removed from a MediaSource after
+        SourceBuffer.remove() is called, but before the removeTimer is fired.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::removeTimerFired):
+
 2016-10-13  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [SOUP] SHOULD NEVER BE REACHED ../../Source/WebCore/platform/URL.cpp(1291) : void WebCore::URL::parse(const WTF::String&)
diff --git a/Source/WebCore/Modules/mediasource/SourceBuffer.cpp b/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
index eb445e3..0bd1d5d 100644 (file)
--- a/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
+++ b/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
@@ -829,6 +829,9 @@ void SourceBuffer::removeCodedFrames(const MediaTime& start, const MediaTime& en
 
 void SourceBuffer::removeTimerFired()
 {
+    if (isRemoved())
+        return;
+
     ASSERT(m_updating);
     ASSERT(m_pendingRemoveStart.isValid());
     ASSERT(m_pendingRemoveStart < m_pendingRemoveEnd);
Mirror of the WebKit open source project from svn.webkit.org.