WKWebsiteDataStore remove methods don't properly delete cookies
Created attachment 257488 [details] Patch
Created attachment 257489 [details] Patch
Committed r187375: <http://trac.webkit.org/changeset/187375>
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.