[webkit-reviews] review denied: [Bug 57488] Cached Resource Overhead Space Usage and Accounting Inaccurate : [Attachment 87725] Patch with update in response to reviewer's comments

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 57488: Cached Resource Overhead Space Usage and Accounting Inaccurate
https://bugs.webkit.org/show_bug.cgi?id=57488

Attachment 87725: Patch with update in response to reviewer's comments
https://bugs.webkit.org/attachment.cgi?id=87725&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=87725&action=review

review- since this doesn’t compile on Windows.

Otherwise, the only place with some room for improvement is with the comments
and logic in the function to fetch header fields.

> Source/WebCore/platform/network/ResourceResponseBase.cpp:253
> +    // Optimistic check first

You can probably omit this comment. If you did want to leave a comment you
could have it address why we think this is a good idea. I think the rationale
is that this prevents fetching a common field with a non-empty value from
causing initialization of all fields. Putting that in writing could make it
clear that we also want to optimize fetching a common field with an empty
value.

> Source/WebCore/platform/network/cf/ResourceResponseCFNet.cpp:88
> +    if ((m_initLevel < CommonFieldsOnly) && (initLevel >= CommonFieldsOnly))
{

I think this reads better without the extra parentheses.

> Source/WebCore/platform/network/mac/ResourceResponseMac.mm:49
> +static const NSString* commonHeaderFields[] = {

The const here is in the wrong place. It needs to be after the "*".