| Summary: | [Soup] Streamline cancellation and client checks | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> | ||||
| Component: | New Bugs | Assignee: | Gustavo Noronha (kov) <gustavo> | ||||
| Status: | RESOLVED FIXED | ||||||
| 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 | ||||||
| Attachments: |
|
||||||
|
Description
Gustavo Noronha (kov)
2013-01-24 03:44:45 PST
Created attachment 184459 [details]
Patch
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. Committed r140836: <http://trac.webkit.org/changeset/140836> |