Bug 107808

Summary: [Soup] Streamline cancellation and client checks
Product: WebKit Reporter: Gustavo Noronha (kov) <gustavo>
Component: New BugsAssignee: 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 Flags
Patch mrobinson: review+

Gustavo Noronha (kov)
Reported 2013-01-24 03:44:45 PST
[Soup] Streamline cancellation and client checks
Attachments
Patch (9.62 KB, patch)
2013-01-24 03:46 PST, Gustavo Noronha (kov) 		mrobinson: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Gustavo Noronha (kov)
Comment 1 2013-01-24 03:46:14 PST
Created attachment 184459 [details] Patch
Martin Robinson
Comment 2 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.
Gustavo Noronha (kov)
Comment 3 2013-01-25 09:07:54 PST
Committed r140836: <http://trac.webkit.org/changeset/140836>
Note You need to log in before you can comment on or make changes to this bug.