[Webkit-unassigned] [Bug 159024] [Soup] Clean up SocketStreamHandle soup implementation

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 281842 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=281842&action=review
> 
> 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.

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

-- 
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/20160623/80439bcc/attachment.html>