<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[211495] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/211495">211495</a></dd>
<dt>Author</dt> <dd>jer.noble@apple.com</dd>
<dt>Date</dt> <dd>2017-02-01 10:22:21 -0800 (Wed, 01 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>NULL-deref crash in TextTrack::removeCue()
https://bugs.webkit.org/show_bug.cgi?id=167615
Reviewed by Eric Carlson.
Source/WebCore:
Test: http/tests/media/track-in-band-hls-metadata-crash.html
Follow-up to <a href="http://trac.webkit.org/projects/webkit/changeset/211401">r211401</a>. When passing around a reference to an object, the assumption is that
the caller is retaining the underlying object. This breaks down for
InbandDataTextTrack::removeDataCue(), which releases its own ownership of the cue object,
then passes the reference to that object to its superclass to do further remove steps. The
retain count of the cue can thus drop to zero within the scope of
InbandTextTrack::removeCue(). Use "take" semantics to remove the cue from the
m_incompleteCueMap without releasing ownership, and pass a reference to that retained object
on to removeCue(), guaranteeing that the cue will not be destroyed until after the
romeveDataCue() method returns.
* html/track/InbandDataTextTrack.cpp:
(WebCore::InbandDataTextTrack::removeDataCue):
LayoutTests:
* http/tests/media/track-in-band-hls-metadata-crash-expected.txt: Added.
* http/tests/media/track-in-band-hls-metadata-crash.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorehtmltrackInbandDataTextTrackcpp">trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestshttptestsmediatrackinbandhlsmetadatacrashexpectedtxt">trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestshttptestsmediatrackinbandhlsmetadatacrashhtml">trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (211494 => 211495)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-02-01 18:20:41 UTC (rev 211494)
+++ trunk/LayoutTests/ChangeLog 2017-02-01 18:22:21 UTC (rev 211495)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2017-02-01 Jer Noble <jer.noble@apple.com>
+
+ NULL-deref crash in TextTrack::removeCue()
+ https://bugs.webkit.org/show_bug.cgi?id=167615
+
+ Reviewed by Eric Carlson.
+
+ * http/tests/media/track-in-band-hls-metadata-crash-expected.txt: Added.
+ * http/tests/media/track-in-band-hls-metadata-crash.html: Added.
+
</ins><span class="cx"> 2017-02-01 Nan Wang <n_wang@apple.com>
</span><span class="cx">
</span><span class="cx"> AX: Incorrect range from index and length in text controls when there are newlines
</span></span></pre></div>
<a id="trunkLayoutTestshttptestsmediatrackinbandhlsmetadatacrashexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt (0 => 211495)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash-expected.txt 2017-02-01 18:22:21 UTC (rev 211495)
</span><span class="lines">@@ -0,0 +1,18 @@
</span><ins>+
+Test that seeking HLS streams containing metadata tracks does not crash.
+
+
+** Set video.src, wait for media data to load
+RUN(video.src = 'http://127.0.0.1:8000/media/resources/hls/metadata/prog_index.m3u8')
+
+EVENT(addtrack)
+RUN(track = video.textTracks[0])
+RUN(track.mode = 'hidden')
+RUN(video.play())
+EVENT(cuechange)
+
+** Seek, should not crash.
+RUN(video.currentTime = 5)
+EVENT(seeked)
+END OF TEST
+
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestsmediatrackinbandhlsmetadatacrashhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html (0 => 211495)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html (rev 0)
+++ trunk/LayoutTests/http/tests/media/track-in-band-hls-metadata-crash.html 2017-02-01 18:22:21 UTC (rev 211495)
</span><span class="lines">@@ -0,0 +1,43 @@
</span><ins>+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+
+ <script src=../../media-resources/video-test.js></script>
+ <script src=../../media-resources/media-file.js></script>
+
+ <script>
+ var track;
+
+ function addtrack(event)
+ {
+ tracks = event.target;
+ run("track = video.textTracks[0]");
+ run("track.mode = 'hidden'");
+ run("video.play()");
+ waitForEvent('cuechange', cuechange, false, true, track);
+ }
+
+ function cuechange()
+ {
+ consoleWrite("<br><em>** Seek, should not crash.</em>");
+ run("video.currentTime = 5");
+ waitForEventAndEnd("seeked");
+ }
+
+ function start()
+ {
+ consoleWrite("<br><em>** Set video.src, wait for media data to load</em>");
+ findMediaElement();
+ run("video.src = 'http://127.0.0.1:8000/media/resources/hls/metadata/prog_index.m3u8'");
+
+ consoleWrite("");
+ waitForEvent('addtrack', addtrack, false, true, video.textTracks);
+ }
+ </script>
+ </head>
+ <body onload="start()">
+ <video controls></video>
+ <p>Test that seeking HLS streams containing metadata tracks does not crash.</p>
+ </body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (211494 => 211495)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-02-01 18:20:41 UTC (rev 211494)
+++ trunk/Source/WebCore/ChangeLog 2017-02-01 18:22:21 UTC (rev 211495)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2017-02-01 Jer Noble <jer.noble@apple.com>
+
+ NULL-deref crash in TextTrack::removeCue()
+ https://bugs.webkit.org/show_bug.cgi?id=167615
+
+ Reviewed by Eric Carlson.
+
+ Test: http/tests/media/track-in-band-hls-metadata-crash.html
+
+ Follow-up to r211401. When passing around a reference to an object, the assumption is that
+ the caller is retaining the underlying object. This breaks down for
+ InbandDataTextTrack::removeDataCue(), which releases its own ownership of the cue object,
+ then passes the reference to that object to its superclass to do further remove steps. The
+ retain count of the cue can thus drop to zero within the scope of
+ InbandTextTrack::removeCue(). Use "take" semantics to remove the cue from the
+ m_incompleteCueMap without releasing ownership, and pass a reference to that retained object
+ on to removeCue(), guaranteeing that the cue will not be destroyed until after the
+ romeveDataCue() method returns.
+
+ * html/track/InbandDataTextTrack.cpp:
+ (WebCore::InbandDataTextTrack::removeDataCue):
+
</ins><span class="cx"> 2017-02-01 Nan Wang <n_wang@apple.com>
</span><span class="cx">
</span><span class="cx"> AX: Incorrect range from index and length in text controls when there are newlines
</span></span></pre></div>
<a id="trunkSourceWebCorehtmltrackInbandDataTextTrackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp (211494 => 211495)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp 2017-02-01 18:20:41 UTC (rev 211494)
+++ trunk/Source/WebCore/html/track/InbandDataTextTrack.cpp 2017-02-01 18:22:21 UTC (rev 211495)
</span><span class="lines">@@ -100,9 +100,9 @@
</span><span class="cx">
</span><span class="cx"> void InbandDataTextTrack::removeDataCue(const MediaTime&, const MediaTime&, SerializedPlatformRepresentation& platformValue)
</span><span class="cx"> {
</span><del>- if (auto* cue = m_incompleteCueMap.get(&platformValue)) {
</del><ins>+ if (auto cue = m_incompleteCueMap.take(&platformValue)) {
</ins><span class="cx"> LOG(Media, "InbandDataTextTrack::removeDataCue removing cue: start=%s, end=%s\n", toString(cue->startTime()).utf8().data(), toString(cue->endTime()).utf8().data());
</span><del>- removeCue(*cue);
</del><ins>+ InbandTextTrack::removeCue(*cue);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>