Title: [200853] releases/WebKitGTK/webkit-2.12/Source/WebCore
- Revision
- 200853
- Author
- [email protected]
- Date
- 2016-05-13 06:31:20 -0700 (Fri, 13 May 2016)
Log Message
Merge r200455 - [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().
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.12/Source/WebCore/ChangeLog (200852 => 200853)
--- releases/WebKitGTK/webkit-2.12/Source/WebCore/ChangeLog 2016-05-13 13:29:42 UTC (rev 200852)
+++ releases/WebKitGTK/webkit-2.12/Source/WebCore/ChangeLog 2016-05-13 13:31:20 UTC (rev 200853)
@@ -1,3 +1,40 @@
+2016-05-05 Carlos Garcia Campos <[email protected]>
+
+ [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().
+
2016-05-04 Daniel Bates <[email protected]>
CSP: Perform case sensitive match against path portion of source _expression_ URL that ends in '/'
Modified: releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp (200852 => 200853)
--- releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp 2016-05-13 13:29:42 UTC (rev 200852)
+++ releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp 2016-05-13 13:31:20 UTC (rev 200853)
@@ -342,6 +342,16 @@
return GRefPtr<WebKitWebSrc>(ptr, GRefPtrAdopt);
}
+// 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);
+}
+
template <> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr)
{
if (ptr)
Modified: releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h (200852 => 200853)
--- releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h 2016-05-13 13:29:42 UTC (rev 200852)
+++ releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h 2016-05-13 13:31:20 UTC (rev 200853)
@@ -108,6 +108,7 @@
template<> void derefGPtr<WebKitVideoSink>(WebKitVideoSink* ptr);
template<> GRefPtr<WebKitWebSrc> adoptGRef(WebKitWebSrc* ptr);
+GRefPtr<WebKitWebSrc> ensureGRef(WebKitWebSrc* ptr);
template<> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
template<> void derefGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
Modified: releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp (200852 => 200853)
--- releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2016-05-13 13:29:42 UTC (rev 200852)
+++ releases/WebKitGTK/webkit-2.12/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp 2016-05-13 13:31:20 UTC (rev 200853)
@@ -186,7 +186,7 @@
return;
}
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::NeedData, [protector] { webKitWebSrcNeedData(protector.get()); });
},
// enough_data
@@ -200,7 +200,7 @@
return;
}
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::EnoughData, [protector] { webKitWebSrcEnoughData(protector.get()); });
},
// seek_data
@@ -220,7 +220,7 @@
priv->requestedOffset = offset;
}
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::Seek, [protector] { webKitWebSrcSeek(protector.get()); });
return TRUE;
},
@@ -638,7 +638,7 @@
case GST_STATE_CHANGE_READY_TO_PAUSED:
{
GST_DEBUG_OBJECT(src, "READY->PAUSED");
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::Start, [protector] { webKitWebSrcStart(protector.get()); });
break;
}
@@ -646,7 +646,7 @@
{
GST_DEBUG_OBJECT(src, "PAUSED->READY");
priv->notifier.cancelPendingNotifications();
- GRefPtr<WebKitWebSrc> protector(src);
+ GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
priv->notifier.notify(MainThreadSourceNotification::Stop, [protector] { webKitWebSrcStop(protector.get()); });
break;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
