[webkit-reviews] review denied: [Bug 187379] Add support for fetching and remove type _WKWebsiteDataTypeHSTSCache : [Attachment 344673] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Sihui Liu
<sihui_liu at apple.com>'s request for review:
Bug 187379: Add support for fetching and remove type 
_WKWebsiteDataTypeHSTSCache
https://bugs.webkit.org/show_bug.cgi?id=187379

Attachment 344673: Patch

https://bugs.webkit.org/attachment.cgi?id=344673&action=review




--- Comment #7 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 344673
  --> https://bugs.webkit.org/attachment.cgi?id=344673
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344673&action=review

I think this looks very close, but I believe your change to
WebsiteDataRecord::displayNameForCookieHostName will break some ITP
functionality. r- to revise that section, but otherwise I think this looks
good.

> Source/WebCore/PAL/ChangeLog:3
> +	   Add support for fetching and remove type 
_WKWebsiteDataTypeHSTSCache

You have an extra space between 'type' and '_WKWebsite..'

> Source/WebKit/ChangeLog:3
> +	   Add support for fetching and remove type 
_WKWebsiteDataTypeHSTSCache

Ditto.

> Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm:155
> +    HashSet<String>* hostnames = (HashSet<String>*)context;

You should use the proper C++ casting here (probably a reinterpret_cast).

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataRecord.cpp:51
> +    return displayNameForHostName(hostName);

I don't think this change is correct, since you are losing the special handling
of localhost and removal of the leading period.

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1008
> +

Please remove this whitespace change.