[webkit-reviews] review granted: [Bug 107808] [Soup] Streamline cancellation and client checks : [Attachment 184459] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 107808: [Soup] Streamline cancellation and client checks
https://bugs.webkit.org/show_bug.cgi?id=107808

Attachment 184459: Patch
https://bugs.webkit.org/attachment.cgi?id=184459&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=184459&action=review


Looks good other than the following naming nit:

> Source/WebCore/platform/network/ResourceHandle.h:169
> +    bool shouldBail();

Okay. I'm torn. I like the name shouldBail(), but something more descriptive
might make it clearer why you can use the client safely after this returns
false. Maybe cancelledOrClientless?

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:624
> +    ResourceHandleInternal* d = handle->getInternal();
> +    ResourceHandleClient* client = handle->client();
> +
>      ASSERT(!d->m_inputStream);

This is a bit of a micronit, but in many places in the patch, the declarations
look lonely. Maybe you can staple them to the lines that actually use them
before landing.