[webkit-reviews] review granted: [Bug 55427] WebKit2: Need a way to send notifications to client when cookies change : [Attachment 84483] [PATCH] Fix Part 2

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 55427: WebKit2: Need a way to send notifications to client when cookies
change
https://bugs.webkit.org/show_bug.cgi?id=55427

Attachment 84483: [PATCH] Fix Part 2
https://bugs.webkit.org/attachment.cgi?id=84483&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=84483&action=review

> Source/WebKit2/UIProcess/WebCookieManagerProxyClient.h:30
> +#include "WKCookieManager.h"

I don't think this #include is needed.

> Source/WebKit2/UIProcess/API/C/WKCookieManager.h:41
> +    WKCookieManagerDidModifyCookiesCallback				  
didModifyCookies;

CookiesDidChange seems more accurate than DidModifyCookies. The latter makes it
sound like the manager itself is modifying cookies.

> Source/WebKit2/WebProcess/Cookies/WebCookieManager.cpp:89
> +void WebCookieManager::startObservingCookieChanges()
> +{
> +#if USE(CFNETWORK) || PLATFORM(MAC)
> +    WebCore::startObservingCookieChanges();
> +#endif
> +}
> +
> +void WebCookieManager::stopObservingCookieChanges()
> +{
> +#if USE(CFNETWORK) || PLATFORM(MAC)
> +    WebCore::stopObservingCookieChanges();
> +#endif
> +}

It would be nice if WebKit didn't have to know which ports have implemented
this feature. If all ports implemented this function (but some ports just had
stub implementations), WebKit wouldn't have to care.