[webkit-reviews] review granted: [Bug 55435] WebKit2: Use CFNetwork Sessions API : [Attachment 84878] Patch (Part 4) Take 2

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Jessie Berlin <jberlin at webkit.org>'s
request for review:
Bug 55435: WebKit2: Use CFNetwork Sessions API
https://bugs.webkit.org/show_bug.cgi?id=55435

Attachment 84878: Patch (Part 4) Take 2
https://bugs.webkit.org/attachment.cgi?id=84878&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=84878&action=review

> Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:74
> +#if USE(CFURLSTORAGESESSIONS)
> +    privateBrowsingCookieStorage().adoptCF(enabled ?
wkCreatePrivateInMemoryHTTPCookieStorage(ResourceHandle::privateBrowsingStorage
Session()) : 0);
> +#else
> +    privateBrowsingCookieStorage().adoptCF(enabled ?
wkCreatePrivateInMemoryHTTPCookieStorage(0) : 0);
>  

I think the use of the ternary operator here makes things less clear and a
simple if statement would work better.

> Source/WebCore/platform/network/mac/CookieStorageMac.mm:103
> +    privateBrowsingCookieStorage().adoptCF(0);

This would read clearer if you just used .clear() instead of adoptCF(0).

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:542
> +    if (shouldRelaxThirdPartyCookiePolicy(firstRequest URL])) {

This probably doesn't compile. It is missing a [, or maybe the ] is wrong.