[webkit-reviews] review granted: [Bug 180883] Add optional logging of per-resource cookie information : [Attachment 329865] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Dec 20 16:52:41 PST 2017
Brent Fulgham <bfulgham at webkit.org> has granted Keith Rollin
<krollin at apple.com>'s request for review:
Bug 180883: Add optional logging of per-resource cookie information
https://bugs.webkit.org/show_bug.cgi?id=180883
Attachment 329865: Patch
https://bugs.webkit.org/attachment.cgi?id=329865&action=review
--- Comment #20 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 329865
--> https://bugs.webkit.org/attachment.cgi?id=329865
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=329865&action=review
Looks (and works!) well. r=me, but please consider not dumping cookie logging
when there are no cookies.
> Source/WebCore/platform/network/cocoa/CookieCocoa.mm:60
> + id value = props[@"Created"];
Maybe:
id value = cookie.props[@"Created"];
> Source/WebCore/platform/network/cocoa/CookieCocoa.mm:67
> + return toCanonicalFormat(((NSNumber*) value).doubleValue);
Please use ugly Objective-C syntax: "((NSNumber *)value)" and elsewhere.
> Source/WebCore/platform/network/cocoa/CookieCocoa.mm:108
> + // FIXME: Set Created property
Do we need to do this in this patch?
> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:743
> + LOCAL_LOG(R"( "cookies": [)");
Do we want to output this information when the cookie.size() == 0? Maybe that's
just noise in the log?
More information about the webkit-reviews
mailing list