[webkit-reviews] review denied: [Bug 87743] Crash in WebCore::SubresourceLoader::releaseResources when connection fails : [Attachment 149309] Fix chromium compile

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 20 03:05:15 PST 2012


Antti Koivisto <koivisto at iki.fi> has denied Nate Chapin <japhet at chromium.org>'s
request for review:
Bug 87743: Crash in WebCore::SubresourceLoader::releaseResources when
connection fails
https://bugs.webkit.org/show_bug.cgi?id=87743

Attachment 149309: Fix chromium compile
https://bugs.webkit.org/attachment.cgi?id=149309&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=149309&action=review


Seems like a sensible change. Naming should be improved though.

> Source/WebCore/loader/cache/CachedResource.h:205
> +    void cancelled();

Loader code has hots of bad legacy naming but we shouldn't add more. Please
follow the usual naming rules. This should be something like cancelLoad().

> Source/WebCore/loader/ResourceLoader.h:160
> +	   bool cancelled() const { return m_cancellationStatus >= Cancelled; }


Here is another cancelled() now with bool return. It should be along the lines
of wasCanceled().

> Source/WebCore/loader/SubresourceLoader.h:78
> +    void finish();

These need more descriptive name as well. Finish what? Finish how?


More information about the webkit-reviews mailing list