Bug 107808

Summary: [Soup] Streamline cancellation and client checks
Product: WebKit Reporter: Gustavo Noronha (kov) <gustavo>
Component: New BugsAssignee: Gustavo Noronha (kov) <gustavo>
Severity: Normal CC: danw, mrobinson, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 105552    
Description Flags
Patch mrobinson: review+

Description Gustavo Noronha (kov) 2013-01-24 03:44:45 PST
[Soup] Streamline cancellation and client checks
Comment 1 Gustavo Noronha (kov) 2013-01-24 03:46:14 PST
Created attachment 184459 [details]
Comment 2 Martin Robinson 2013-01-24 17:59:01 PST
Comment on attachment 184459 [details]

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.
Comment 3 Gustavo Noronha (kov) 2013-01-25 09:07:54 PST
Committed r140836: <http://trac.webkit.org/changeset/140836>