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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 22 23:34:55 PDT 2016


--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(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.

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/96f5191e/attachment.html>

More information about the webkit-unassigned mailing list