<!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>[200455] trunk/Source/WebCore</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/200455">200455</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-05-05 06:03:46 -0700 (Thu, 05 May 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[GStreamer] Adaptive streaming issues
https://bugs.webkit.org/show_bug.cgi?id=144040
Reviewed by Philippe Normand.
In the case of adaptive streaming, the GST URI downloader object is creating the source object, in our case
WebKitWebSrc, without taking its ownership. This is breaking the lifetime of the WebKitWebSrc element. We are
using GRefPtr in WebKitWebSrc to ref/unref the object when sending notifications to the main thread, ensuring
that the object is not destroyed before the main thread dispatches the message. But our smart pointers are so
smart that in case of receiving a floating reference, it's converted to a full reference, so that the first time
we try to take a ref of a WebKitWebSrc having a floating reference we are actually taking the ownership
instead. When we try to release the reference, we are actuallty destroying the object, something that the actual
owner is not expecting and causing runtime critical warnings and very often web process crashes.
(WebKitWebProcess:6863): GStreamer-CRITICAL **:
Trying to dispose element appsrc1, but it is in READY instead of the NULL state.
You need to explicitly set elements to the NULL state before
dropping the final reference, to allow them to clean up.
This problem may also be caused by a refcounting bug in the
application or some element.
(WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_handler_get_uri: assertion 'GST_IS_URI_HANDLER(handler)' failed
(WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_get_protocol: assertion 'uri != NULL' failed
This should be fixed in GST, but we can workaround it in WebKit while it's fixed in GST or to prevent this from
happening if other users make the same mistake. The idea is to add a ensureGRef() only available for GRefPtr
when using WebKitWebSrc objects that consumes the floating reference if needed before taking the actual reference.
* platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
(WTF::ensureGRef): Consume the floating ref if needed.
* platform/graphics/gstreamer/GRefPtrGStreamer.h:
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(webKitWebSrcChangeState): Use ensureGRef().</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsgstreamerGRefPtrGStreamercpp">trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsgstreamerGRefPtrGStreamerh">trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsgstreamerWebKitWebSourceGStreamercpp">trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (200454 => 200455)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-05-05 08:40:21 UTC (rev 200454)
+++ trunk/Source/WebCore/ChangeLog 2016-05-05 13:03:46 UTC (rev 200455)
</span><span class="lines">@@ -1,3 +1,40 @@
</span><ins>+2016-05-05 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [GStreamer] Adaptive streaming issues
+ https://bugs.webkit.org/show_bug.cgi?id=144040
+
+ Reviewed by Philippe Normand.
+
+ In the case of adaptive streaming, the GST URI downloader object is creating the source object, in our case
+ WebKitWebSrc, without taking its ownership. This is breaking the lifetime of the WebKitWebSrc element. We are
+ using GRefPtr in WebKitWebSrc to ref/unref the object when sending notifications to the main thread, ensuring
+ that the object is not destroyed before the main thread dispatches the message. But our smart pointers are so
+ smart that in case of receiving a floating reference, it's converted to a full reference, so that the first time
+ we try to take a ref of a WebKitWebSrc having a floating reference we are actually taking the ownership
+ instead. When we try to release the reference, we are actuallty destroying the object, something that the actual
+ owner is not expecting and causing runtime critical warnings and very often web process crashes.
+
+ (WebKitWebProcess:6863): GStreamer-CRITICAL **:
+ Trying to dispose element appsrc1, but it is in READY instead of the NULL state.
+ You need to explicitly set elements to the NULL state before
+ dropping the final reference, to allow them to clean up.
+ This problem may also be caused by a refcounting bug in the
+ application or some element.
+
+ (WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_handler_get_uri: assertion 'GST_IS_URI_HANDLER(handler)' failed
+
+ (WebKitWebProcess:6863): GStreamer-CRITICAL **: gst_uri_get_protocol: assertion 'uri != NULL' failed
+
+ This should be fixed in GST, but we can workaround it in WebKit while it's fixed in GST or to prevent this from
+ happening if other users make the same mistake. The idea is to add a ensureGRef() only available for GRefPtr
+ when using WebKitWebSrc objects that consumes the floating reference if needed before taking the actual reference.
+
+ * platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
+ (WTF::ensureGRef): Consume the floating ref if needed.
+ * platform/graphics/gstreamer/GRefPtrGStreamer.h:
+ * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+ (webKitWebSrcChangeState): Use ensureGRef().
+
</ins><span class="cx"> 2016-05-05 Csaba Osztrogonác <ossy@webkit.org>
</span><span class="cx">
</span><span class="cx"> [Mac][cmake] Unreviewed speculative buildfix after r200433, just for fun.
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsgstreamerGRefPtrGStreamercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp (200454 => 200455)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp 2016-05-05 08:40:21 UTC (rev 200454)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp 2016-05-05 13:03:46 UTC (rev 200455)
</span><span class="lines">@@ -342,6 +342,16 @@
</span><span class="cx"> return GRefPtr<WebKitWebSrc>(ptr, GRefPtrAdopt);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+// This method is only available for WebKitWebSrc and should not be used for any other type.
+// This is only to work around a bug in GST where the URI downloader is not taking the ownership of WebKitWebSrc.
+// See https://bugs.webkit.org/show_bug.cgi?id=144040.
+GRefPtr<WebKitWebSrc> ensureGRef(WebKitWebSrc* ptr)
+{
+ if (ptr && g_object_is_floating(ptr))
+ gst_object_ref_sink(GST_OBJECT(ptr));
+ return GRefPtr<WebKitWebSrc>(ptr);
+}
+
</ins><span class="cx"> template <> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr)
</span><span class="cx"> {
</span><span class="cx"> if (ptr)
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsgstreamerGRefPtrGStreamerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h (200454 => 200455)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h 2016-05-05 08:40:21 UTC (rev 200454)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h 2016-05-05 13:03:46 UTC (rev 200455)
</span><span class="lines">@@ -108,6 +108,7 @@
</span><span class="cx"> template<> void derefGPtr<WebKitVideoSink>(WebKitVideoSink* ptr);
</span><span class="cx">
</span><span class="cx"> template<> GRefPtr<WebKitWebSrc> adoptGRef(WebKitWebSrc* ptr);
</span><ins>+GRefPtr<WebKitWebSrc> ensureGRef(WebKitWebSrc* ptr);
</ins><span class="cx"> template<> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
</span><span class="cx"> template<> void derefGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsgstreamerWebKitWebSourceGStreamercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp (200454 => 200455)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2016-05-05 08:40:21 UTC (rev 200454)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2016-05-05 13:03:46 UTC (rev 200455)
</span><span class="lines">@@ -188,7 +188,7 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- GRefPtr<WebKitWebSrc> protector(src);
</del><ins>+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
</ins><span class="cx"> priv->notifier.notify(MainThreadSourceNotification::NeedData, [protector] { webKitWebSrcNeedData(protector.get()); });
</span><span class="cx"> },
</span><span class="cx"> // enough_data
</span><span class="lines">@@ -202,7 +202,7 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- GRefPtr<WebKitWebSrc> protector(src);
</del><ins>+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
</ins><span class="cx"> priv->notifier.notify(MainThreadSourceNotification::EnoughData, [protector] { webKitWebSrcEnoughData(protector.get()); });
</span><span class="cx"> },
</span><span class="cx"> // seek_data
</span><span class="lines">@@ -222,7 +222,7 @@
</span><span class="cx"> priv->requestedOffset = offset;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- GRefPtr<WebKitWebSrc> protector(src);
</del><ins>+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
</ins><span class="cx"> priv->notifier.notify(MainThreadSourceNotification::Seek, [protector] { webKitWebSrcSeek(protector.get()); });
</span><span class="cx"> return TRUE;
</span><span class="cx"> },
</span><span class="lines">@@ -640,7 +640,7 @@
</span><span class="cx"> case GST_STATE_CHANGE_READY_TO_PAUSED:
</span><span class="cx"> {
</span><span class="cx"> GST_DEBUG_OBJECT(src, "READY->PAUSED");
</span><del>- GRefPtr<WebKitWebSrc> protector(src);
</del><ins>+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
</ins><span class="cx"> priv->notifier.notify(MainThreadSourceNotification::Start, [protector] { webKitWebSrcStart(protector.get()); });
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="lines">@@ -648,7 +648,7 @@
</span><span class="cx"> {
</span><span class="cx"> GST_DEBUG_OBJECT(src, "PAUSED->READY");
</span><span class="cx"> priv->notifier.cancelPendingNotifications();
</span><del>- GRefPtr<WebKitWebSrc> protector(src);
</del><ins>+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
</ins><span class="cx"> priv->notifier.notify(MainThreadSourceNotification::Stop, [protector] { webKitWebSrcStop(protector.get()); });
</span><span class="cx"> break;
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>