Bug 147282 – WKWebsiteDataStore remove methods don't properly delete cookies

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

WKWebsiteDataStore remove methods don't properly delete cookies

[Anders Carlsson]

Created attachment 257488 [details]
Patch

[Anders Carlsson]

Created attachment 257489 [details]
Patch

[Anders Carlsson]

Committed r187375: <http://trac.webkit.org/changeset/187375>

[Darin Adler]

Comment on attachment 257489 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257489&action=review

> Source/WebCore/platform/network/cf/CookieJarCFNet.cpp:240
> +void deleteCookiesForHostnames(const NetworkStorageSession& session, const Vector<String>& hostnames)
> +{
> +}
> +
>  void deleteAllCookiesModifiedSince(const NetworkStorageSession&, std::chrono::system_clock::time_point)
>  {
>  }

Seems kind of lame that these don’t even have a comment about the fact that they are not implemented and do nothing! Also would be nice to leave out the unused argument names to make this build with unused argument warnings.

> Source/WebCore/platform/network/mac/CookieJarMac.mm:230
> +    HashMap<String, RetainPtr<NSHTTPCookie>> cookiesByDomain;

Is there a need for RetainPtr here? Doesn’t the cookies array retains all the cookies in that array? Or maybe that array is an internal data structure that changes once you start calling wkDeleteHTTPCookie?

> Source/WebCore/platform/network/mac/CookieJarMac.mm:232
> +        cookiesByDomain.set(cookie.domain, cookie);

Why set instead of add?

> Source/WebCore/platform/network/mac/CookieJarMac.mm:234
> +    for (const auto& hostname : hostnames) {

Not sure that const here is all that useful.

> Source/WebCore/platform/network/mac/CookieJarMac.mm:239
> +    [WebCore::cookieStorage(session) _saveCookies];

Seems to me that the use of CFHTTPCookieStorageRef to get the list of cookies but the use of NSHTTPCookieStorage for the call to _saveCookies is peculiar enough that it needs a motivating comment.