[webkit-reviews] review denied: [Bug 33186] Track state for whether a ResourceRequest is fetched via SPDY : [Attachment 46001] Update with Darin's renaming suggestions.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Mike Belshe <mbelshe at chromium.org>'s
request for review:
Bug 33186: Track state for whether a ResourceRequest is fetched via SPDY
https://bugs.webkit.org/show_bug.cgi?id=33186

Attachment 46001: Update with Darin's renaming suggestions.
https://bugs.webkit.org/attachment.cgi?id=46001&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
If the name is SPDY, the we would prefer to have that in the identifiers, not
Spdy.

> +	   Enabled only in Chromium.

No longer true.

> +	   * platform/network/chromium/ResourceResponse.h:
> +	   (WebCore::ResourceResponse::isSpdy):
> +	   (WebCore::ResourceResponse::setIsSpdy):

Names here out of date.

Where is m_fetchedViaSpdy initialized to false? Normally constructors should
initialize all members.

> +	   void setWasFetchedViaSpdy(const bool value)

The const here isn't needed or helpful.

> +	   bool m_fetchedViaSpdy;

Even data boolean members work well with the "class-name <xxx>" naming scheme,
so I would call this m_wasFetchedViaSpdy.

> +	   Enabled only in Chromium.

Not true any more. And not really important to add in a Chromium directory
ChangeLog.

> +	   * src/WebURLResponse.cpp:
> +	   (WebKit::WebURLResponse::isSpdy):
> +	   (WebKit::WebURLResponse::setIsSpdy):

Names here out of date.

review- because of this round of nitpicks -- seems generally good