<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [Soup] Clean up SocketStreamHandle soup implementation"
href="https://bugs.webkit.org/show_bug.cgi?id=159024#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [Soup] Clean up SocketStreamHandle soup implementation"
href="https://bugs.webkit.org/show_bug.cgi?id=159024">bug 159024</a>
from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=159024#c3">comment #3</a>)
<span class="quote">> (In reply to <a href="show_bug.cgi?id=159024#c2">comment #2</a>)
> > Comment on <span class=""><a href="attachment.cgi?id=281842&action=diff" name="attach_281842" title="Patch">attachment 281842</a> <a href="attachment.cgi?id=281842&action=edit" title="Patch">[details]</a></span>
> > Patch
> >
> > View in context:
> > <a href="https://bugs.webkit.org/attachment.cgi?id=281842&action=review">https://bugs.webkit.org/attachment.cgi?id=281842&action=review</a>
>
> Thanks for the review!
>
> > > Source/WebCore/platform/network/soup/SocketStreamHandleSoup.cpp:223
> > > + g_source_set_callback(m_writeReadySource.get(), reinterpret_cast<GSourceFunc>(writeReadyCallback), this, [](gpointer handle) {
> > > + static_cast<SocketStreamHandle*>(handle)->deref();
> > > + });
> >
> > IMO the lambda would be more readable if its header was also placed on its
> > own line, and all three lines being indented by 4 spaces compared to the
> > start of this expression.
>
> I agree, I'll do that.
>
> > The rest of this patch adopts the leaked pointer into a RefPtr<> wrapper,
> > this should do the same.
>
> Because in the other cases the callback is always called, even if the
> operation is cancelled, but in this case, if the source is destroyed before
> being dispatched the callback is not called and the handler leaked, that's
> why I'm using the destroy notify that is always called. Also note that the
> callback is called multiple times, because by default the source continues
> until it's cancelled or destroyed.</span >
Sorry, I misunderstood, I thought you meant to adopt the ref in the source callback, not in the destroy notify callback. In this case I used explicit ref/deref because it's clearer than having a function that only adopts a ref and returns</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>