[webkit-reviews] review granted: [Bug 112279] Small ResourceLoader cleanups : [Attachment 192972] Patch v1

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 112279: Small ResourceLoader cleanups
https://bugs.webkit.org/show_bug.cgi?id=112279

Attachment 192972: Patch v1
https://bugs.webkit.org/attachment.cgi?id=192972&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=192972&action=review


r=me

> Source/WebCore/loader/ResourceLoaderTypes.h:27
> +#ifndef ResourceLoaderTypes_h
> +#define ResourceLoaderTypes_h

Are more types going in this header? If not, it should be named
"DataPayloadType.h".

> Source/WebCore/loader/ResourceLoaderTypes.h:32
> +//  - DataPayloadWholeResource is expected when the buffer points to a whole
resource. There will only be one such didReceiveData callback for the load.

"is expected" is a little strong in this context. DataPayloadWholeResource is
an optimization that happens sometimes. We don't have a specific expectation
about how often. I would say, "DataPayloadWholeResource indicates that the
buffer points...".

> Source/WebCore/loader/ResourceLoaderTypes.h:33
> +//  - DataPayloadBytes is expected when the buffer points to a range of
bytes, which may or may not be a whole resource.

I would use "indicates" here as well.