<!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>[173959] releases/WebKitGTK/webkit-2.4/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/173959">173959</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2014-09-25 06:46:47 -0700 (Thu, 25 Sep 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/172928">r172928</a> - [GStreamer] ASSERT failure in WebKitWebSource in StreamingClient
https://bugs.webkit.org/show_bug.cgi?id=136132
adoptGRef() has an ASSERT failure if it's used on a floating pointer. For some reason,
WebKitWebSrc* src in StreamingClient's constructor is floating. Since we
don't construct this ourselves, I assume this is happening in Playbin.
If we remove the ref and adopt, GRefPtr's constructor calls gst_object_ref_sink,
which removes the floating reference and doesn't increment the reference count.
This should work, but actually causes the page to either lock up or crash (different
results for different testers).
In this case, it seems like the adoptGRef / gst_object_ref was the correct thing to do,
but adoptGRef won't actually let us do. Removing the ASSERT is a bad idea, because
usually we don't want to adopt floating pointers.
This is all a long way of saying that making m_src a raw pointer and manually
calling gst_object_ref(), and calling gst_object_unref in the destructor is the
best solution in this case, since it fixes the problem while leaving the ASSERT
to protect us in the much more common case where adopting a floating reference is bad.
Reviewed by Philippe Normand.
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(StreamingClient::StreamingClient): Make m_src a raw pointer instead of a GRefPtr.
(StreamingClient::~StreamingClient): Unref m_src.
(StreamingClient::createReadBuffer): Replace m_src.get() with m_src, since it's a raw pointer now.
(StreamingClient::handleResponseReceived): Same.
(StreamingClient::handleDataReceived): Same.
(StreamingClient::handleNotifyFinished): Same.
(CachedResourceStreamingClient::notifyFinished): Same.
(ResourceHandleStreamingClient::didFail): Same.
(ResourceHandleStreamingClient::wasBlocked): Same.
(ResourceHandleStreamingClient::cannotShowURL): Same.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit24SourceWebCoreChangeLog">releases/WebKitGTK/webkit-2.4/Source/WebCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit24SourceWebCoreplatformgraphicsgstreamerWebKitWebSourceGStreamercpp">releases/WebKitGTK/webkit-2.4/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit24SourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.4/Source/WebCore/ChangeLog (173958 => 173959)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.4/Source/WebCore/ChangeLog 2014-09-25 13:39:11 UTC (rev 173958)
+++ releases/WebKitGTK/webkit-2.4/Source/WebCore/ChangeLog 2014-09-25 13:46:47 UTC (rev 173959)
</span><span class="lines">@@ -1,3 +1,40 @@
</span><ins>+2014-08-25 Brendan Long <b.long@cablelabs.com>
+
+ [GStreamer] ASSERT failure in WebKitWebSource in StreamingClient
+ https://bugs.webkit.org/show_bug.cgi?id=136132
+
+ adoptGRef() has an ASSERT failure if it's used on a floating pointer. For some reason,
+ WebKitWebSrc* src in StreamingClient's constructor is floating. Since we
+ don't construct this ourselves, I assume this is happening in Playbin.
+
+ If we remove the ref and adopt, GRefPtr's constructor calls gst_object_ref_sink,
+ which removes the floating reference and doesn't increment the reference count.
+ This should work, but actually causes the page to either lock up or crash (different
+ results for different testers).
+
+ In this case, it seems like the adoptGRef / gst_object_ref was the correct thing to do,
+ but adoptGRef won't actually let us do. Removing the ASSERT is a bad idea, because
+ usually we don't want to adopt floating pointers.
+
+ This is all a long way of saying that making m_src a raw pointer and manually
+ calling gst_object_ref(), and calling gst_object_unref in the destructor is the
+ best solution in this case, since it fixes the problem while leaving the ASSERT
+ to protect us in the much more common case where adopting a floating reference is bad.
+
+ Reviewed by Philippe Normand.
+
+ * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+ (StreamingClient::StreamingClient): Make m_src a raw pointer instead of a GRefPtr.
+ (StreamingClient::~StreamingClient): Unref m_src.
+ (StreamingClient::createReadBuffer): Replace m_src.get() with m_src, since it's a raw pointer now.
+ (StreamingClient::handleResponseReceived): Same.
+ (StreamingClient::handleDataReceived): Same.
+ (StreamingClient::handleNotifyFinished): Same.
+ (CachedResourceStreamingClient::notifyFinished): Same.
+ (ResourceHandleStreamingClient::didFail): Same.
+ (ResourceHandleStreamingClient::wasBlocked): Same.
+ (ResourceHandleStreamingClient::cannotShowURL): Same.
+
</ins><span class="cx"> 2014-08-24 Michael Catanzaro <mcatanzaro@igalia.com>
</span><span class="cx">
</span><span class="cx"> [GTK] Toggle buttons visually broken with GTK+ 3.13.7
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit24SourceWebCoreplatformgraphicsgstreamerWebKitWebSourceGStreamercpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.4/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp (173958 => 173959)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.4/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2014-09-25 13:39:11 UTC (rev 173958)
+++ releases/WebKitGTK/webkit-2.4/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2014-09-25 13:46:47 UTC (rev 173959)
</span><span class="lines">@@ -69,7 +69,7 @@
</span><span class="cx"> void handleDataReceived(const char*, int);
</span><span class="cx"> void handleNotifyFinished();
</span><span class="cx">
</span><del>- GRefPtr<GstElement> m_src;
</del><ins>+ GstElement* m_src;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> class CachedResourceStreamingClient : public CachedRawResourceClient, public StreamingClient {
</span><span class="lines">@@ -821,17 +821,18 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> StreamingClient::StreamingClient(WebKitWebSrc* src)
</span><del>- : m_src(adoptGRef(static_cast<GstElement*>(gst_object_ref(src))))
</del><ins>+ : m_src(static_cast<GstElement*>(gst_object_ref(src)))
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> StreamingClient::~StreamingClient()
</span><span class="cx"> {
</span><ins>+ gst_object_unref(m_src);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> char* StreamingClient::createReadBuffer(size_t requestedSize, size_t& actualSize)
</span><span class="cx"> {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx"> WebKitWebSrcPrivate* priv = src->priv;
</span><span class="cx">
</span><span class="cx"> ASSERT(!priv->buffer);
</span><span class="lines">@@ -850,7 +851,7 @@
</span><span class="cx">
</span><span class="cx"> void StreamingClient::handleResponseReceived(const ResourceResponse& response, CORSAccessCheckResult corsAccessCheck)
</span><span class="cx"> {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx"> WebKitWebSrcPrivate* priv = src->priv;
</span><span class="cx">
</span><span class="cx"> GST_DEBUG_OBJECT(src, "Received response: %d", response.httpStatusCode());
</span><span class="lines">@@ -962,7 +963,7 @@
</span><span class="cx">
</span><span class="cx"> void StreamingClient::handleDataReceived(const char* data, int length)
</span><span class="cx"> {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx"> WebKitWebSrcPrivate* priv = src->priv;
</span><span class="cx">
</span><span class="cx"> GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
</span><span class="lines">@@ -1029,7 +1030,7 @@
</span><span class="cx">
</span><span class="cx"> void StreamingClient::handleNotifyFinished()
</span><span class="cx"> {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx"> WebKitWebSrcPrivate* priv = src->priv;
</span><span class="cx">
</span><span class="cx"> GST_DEBUG_OBJECT(src, "Have EOS");
</span><span class="lines">@@ -1102,7 +1103,7 @@
</span><span class="cx"> void CachedResourceStreamingClient::notifyFinished(CachedResource* resource)
</span><span class="cx"> {
</span><span class="cx"> if (resource->loadFailedOrCanceled()) {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx">
</span><span class="cx"> if (!resource->wasCanceled()) {
</span><span class="cx"> const ResourceError& error = resource->resourceError();
</span><span class="lines">@@ -1179,7 +1180,7 @@
</span><span class="cx">
</span><span class="cx"> void ResourceHandleStreamingClient::didFail(ResourceHandle*, const ResourceError& error)
</span><span class="cx"> {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx">
</span><span class="cx"> GST_ERROR_OBJECT(src, "Have failure: %s", error.localizedDescription().utf8().data());
</span><span class="cx"> GST_ELEMENT_ERROR(src, RESOURCE, FAILED, ("%s", error.localizedDescription().utf8().data()), (0));
</span><span class="lines">@@ -1188,7 +1189,7 @@
</span><span class="cx">
</span><span class="cx"> void ResourceHandleStreamingClient::wasBlocked(ResourceHandle*)
</span><span class="cx"> {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx"> GUniquePtr<gchar> uri;
</span><span class="cx">
</span><span class="cx"> GST_ERROR_OBJECT(src, "Request was blocked");
</span><span class="lines">@@ -1202,7 +1203,7 @@
</span><span class="cx">
</span><span class="cx"> void ResourceHandleStreamingClient::cannotShowURL(ResourceHandle*)
</span><span class="cx"> {
</span><del>- WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
</del><ins>+ WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
</ins><span class="cx"> GUniquePtr<gchar> uri;
</span><span class="cx">
</span><span class="cx"> GST_ERROR_OBJECT(src, "Cannot show URL");
</span></span></pre>
</div>
</div>
</body>
</html>