[Webkit-unassigned] [Bug 154285] [GStreamer] clean-up various leaks

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=154285

--- Comment #5 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #4)
> Comment on attachment 271425 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271425&action=review
> 
> >>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:196
> >>> +    priv->task = adoptGRef(gst_task_new(reinterpret_cast<GstTaskFunction>(webKitWebAudioSrcLoop), src, 0));
> >> 
> >> GstTask is GInitiallyUnowned, so this returns a floating reference that we should consume here, so adoptGRef is not correct. Aren't you hitting the assert in GRefPtr<GstTask> adoptGRef(GstTask* ptr) ?
> > 
> > Heh. No :)
> > But I'll double-check.
> 
> This should be adoptGRef(g_object_ref_sink(gst_task_new(...))), correct?

No.

> I think floating references are not useful in combination with GRefPtr, so
> why don't we take it always in adoptGRef() (e.g. by calling
> g_object_is_floating() and then g_object_ref_sink() if that return TRUE)?

Because in that case you are not adopting the ref, but consuming the floating reference.

> What is the value in asserting that the ptr is not floating? (Why do we do
> that only in GRefPtrGStreamer.cpp and not in GRefPtr.cpp?) Am I missing
> something here?

Because we want to prevent misuses of GRefPtr with floating references. You should never adopt a floating reference. I think we only do this in GRefPtrGStreamer because GstObject is GInitiallyUnowned, so we know all gst classes are using floating references. Since this seems to be a bit confusing, I've tried to clarify it and written this http://trac.webkit.org/wiki/WebKitGTK/GRefPtrAndFloatingRefs. I hope it helps.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160219/c22efb11/attachment.html>