Bug 107808 - [Soup] Streamline cancellation and client checks
Summary: [Soup] Streamline cancellation and client checks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gustavo Noronha (kov)
URL:
Keywords:
Depends on:
Blocks: 105552
  Show dependency treegraph
 
Reported: 2013-01-24 03:44 PST by Gustavo Noronha (kov)
Modified: 2013-01-25 09:07 PST (History)
4 users (show)

See Also:


Attachments
Patch (9.62 KB, patch)
2013-01-24 03:46 PST, Gustavo Noronha (kov)
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
Patch
Comment 2 Martin Robinson 2013-01-24 17:59:01 PST
Comment on attachment 184459 [details]
Patch

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>