[webkit-reviews] review denied: [Bug 28269] Inspector: Improve Cookie DataGrid to Show Hidden Data : [Attachment 34805] Improved Cookies DataGrid

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Joseph Pecoraro
<joepeck02 at gmail.com>'s request for review:
Bug 28269: Inspector: Improve Cookie DataGrid to Show Hidden Data
https://bugs.webkit.org/show_bug.cgi?id=28269

Attachment 34805: Improved Cookies DataGrid
https://bugs.webkit.org/attachment.cgi?id=34805&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>

> +    this._useFallback = typeof InspectorController.cookies() ===
"undefined";

This should be use typeof. We have had this talk before. :)

This does seem expensive for the case when it is implemented, sicne you get the
cookie array twice. Maybe you can store the cookies here for use later. Or
delate this until the first update is done?
> +	       cookies.push(new WebInspector.Cookie(cookie.name, cookie.value,
cookie.domain, cookie.path, cookie.expires, cookie.size, cookie.httpOnly,
cookie.secure, cookie.session));

We don't need to use WebInspector.Cookie except for the fallback case. You can
just directly use the cookie objects in the aray.

> +		   cookies.push(new WebInspector.Cookie(name, value, null,
null, null, size));

Maybe we can remove WebInspector.Cookie and just make inline objects to fake a
cookie. So:

cookies.push({name: name, value: value});

> +	   if (String([cookie name]) == cookieName) {

This should be written the other way so you don't convert each cookie's name to
C++.

Convert cookieName once to NSString * before the loop. Then do:

if ([[cookie name] isEqualToString:cookieNameString])