[webkit-reviews] review denied: [Bug 32398] [Chromium] Expose SecurityOrigin::shouldHideReferrer() in chromium port api. : [Attachment 44646] patch

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has denied Nate Chapin
<japhet at chromium.org>'s request for review:
Bug 32398: [Chromium] Expose SecurityOrigin::shouldHideReferrer() in chromium
port api.
https://bugs.webkit.org/show_bug.cgi?id=32398

Attachment 44646: patch
https://bugs.webkit.org/attachment.cgi?id=44646&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
A few style nits:

> Index: WebKit/chromium/src/WebSecurityOrigin.cpp
> +bool WebSecurityOrigin::shouldHideReferrer(const WebURL& url, const
WebString& referrer) {

'}' should go on the next line as bot said ^^^

> +    return SecurityOrigin::shouldHideReferrer(WebCore::KURL(url.spec(),
url.parsed(), url.isValid()), 

You could simply pass WebURL into SecurityOrigin::shouldHideReferer since
WebURL has conversion operator to convert to KURL.

> +	   WebCore::String(referrer));

WebKit code usually does not wrap a long line like this.

> Index: WebKit/chromium/public/WebSecurityOrigin.h

> +    // Returns whether the url should be allowed to see the referrer
> +    // based on their respective protocols.
> +    WEBKIT_API static bool shouldHideReferrer(const WebURL& url, 
> +	   const WebString& referrer);

No need to wrap lines like this.