[webkit-reviews] review denied: [Bug 99340] Remove cookie-related functions from PlatformSupport : [Attachment 174206] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Mark Pilgrim (Google)
<pilgrim at chromium.org>'s request for review:
Bug 99340: Remove cookie-related functions from PlatformSupport
https://bugs.webkit.org/show_bug.cgi?id=99340

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=174206&action=review


This looks great.  One nit and one question.

> Source/WebCore/loader/CookieJar.cpp:68
> +#if PLATFORM(CHROMIUM)
> +    return cookiesEnabled(networkingContext(document),
document->cookieURL(), document->firstPartyForCookies());
> +#else
>      return cookiesEnabled(networkingContext(document));
> +#endif

You should change PlatformCookieJar.h to take these extra arguments and make
all the other implementations ignore them.

> Source/WebCore/loader/CookieJar.cpp:73
> +#if PLATFORM(CHROMIUM)

ditto

> Source/WebCore/loader/CookieJar.cpp:82
> +#if PLATFORM(CHROMIUM)

ditto

> Source/WebCore/platform/network/chromium/CookieJarChromium.cpp:44
> +void setCookiesFromDOM(NetworkingContext* context, const KURL&
firstPartyForCookies, const KURL& url, const String& cookieStr)

cookieStr -> cookieString

> Source/WebCore/platform/network/chromium/CookieJarChromium.cpp:47
> +    if (!context)
> +	   return;

Is this right, or should we use the default cookie jar from WebKit::Platform?