[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.

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6bec849..d272276 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,40 @@
+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.
+
 2014-08-25  Zan Dobersek  <zdobersek@igalia.com>
 
         [GTK] Remove PopupMenuGtk, SearchPopupMenuGtk

diff --git a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
index efc2e37..87cb203 100644 (file)
--- a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
+++ b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
@@ -71,7 +71,7 @@ class StreamingClient {
         void handleDataReceived(const char*, int);
         void handleNotifyFinished();
 
-        GRefPtr<GstElement> m_src;
+        GstElement* m_src;
 };
 
 class CachedResourceStreamingClient : public CachedRawResourceClient, public StreamingClient {
@@ -773,17 +773,18 @@ bool webKitSrcPassedCORSAccessCheck(WebKitWebSrc* src)
 }
 
 StreamingClient::StreamingClient(WebKitWebSrc* src)
-    : m_src(adoptGRef(static_cast<GstElement*>(gst_object_ref(src))))
+    : m_src(static_cast<GstElement*>(gst_object_ref(src)))
 {
 }
 
 StreamingClient::~StreamingClient()
 {
+    gst_object_unref(m_src);
 }
 
 char* StreamingClient::createReadBuffer(size_t requestedSize, size_t& actualSize)
 {
-    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
     WebKitWebSrcPrivate* priv = src->priv;
 
     ASSERT(!priv->buffer);
@@ -802,7 +803,7 @@ char* StreamingClient::createReadBuffer(size_t requestedSize, size_t& actualSize
 
 void StreamingClient::handleResponseReceived(const ResourceResponse& response, CORSAccessCheckResult corsAccessCheck)
 {
-    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
     WebKitWebSrcPrivate* priv = src->priv;
 
     GST_DEBUG_OBJECT(src, "Received response: %d", response.httpStatusCode());
@@ -914,7 +915,7 @@ void StreamingClient::handleResponseReceived(const ResourceResponse& response, C
 
 void StreamingClient::handleDataReceived(const char* data, int length)
 {
-    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
     WebKitWebSrcPrivate* priv = src->priv;
 
     GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
@@ -981,7 +982,7 @@ void StreamingClient::handleDataReceived(const char* data, int length)
 
 void StreamingClient::handleNotifyFinished()
 {
-    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
     WebKitWebSrcPrivate* priv = src->priv;
 
     GST_DEBUG_OBJECT(src, "Have EOS");
@@ -1054,7 +1055,7 @@ void CachedResourceStreamingClient::dataReceived(CachedResource*, const char* da
 void CachedResourceStreamingClient::notifyFinished(CachedResource* resource)
 {
     if (resource->loadFailedOrCanceled()) {
-        WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+        WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
 
         if (!resource->wasCanceled()) {
             const ResourceError& error = resource->resourceError();
@@ -1131,7 +1132,7 @@ void ResourceHandleStreamingClient::didFinishLoading(ResourceHandle*, double)
 
 void ResourceHandleStreamingClient::didFail(ResourceHandle*, const ResourceError& error)
 {
-    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
 
     GST_ERROR_OBJECT(src, "Have failure: %s", error.localizedDescription().utf8().data());
     GST_ELEMENT_ERROR(src, RESOURCE, FAILED, ("%s", error.localizedDescription().utf8().data()), (0));
@@ -1140,7 +1141,7 @@ void ResourceHandleStreamingClient::didFail(ResourceHandle*, const ResourceError
 
 void ResourceHandleStreamingClient::wasBlocked(ResourceHandle*)
 {
-    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
     GUniquePtr<gchar> uri;
 
     GST_ERROR_OBJECT(src, "Request was blocked");
@@ -1154,7 +1155,7 @@ void ResourceHandleStreamingClient::wasBlocked(ResourceHandle*)
 
 void ResourceHandleStreamingClient::cannotShowURL(ResourceHandle*)
 {
-    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
+    WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src);
     GUniquePtr<gchar> uri;
 
     GST_ERROR_OBJECT(src, "Cannot show URL");