Bug 240671 – Introduce WKHTTPCookieStore.deleteAllCookies

• Component: WebKit API
• Status: RESOLVED WONTFIX

[Yury Semikhatsky]

There is currently no easy and efficient way to delete all cookie without iterating them one by one.

[Yury Semikhatsky]

Pull request: https://github.com/WebKit/WebKit/pull/798

[Radar WebKit Bug Importer]

<rdar://problem/93655655>

[Alex Christensen]

What about this?

[WKWebsiteDataStore.defaultDataStore removeDataOfTypes:[NSSet setWithObject:WKWebsiteDataTypeCookies] modifiedSince:[NSDate distantPast] completionHandler:^{ }];

[Yury Semikhatsky]

(In reply to Alex Christensen from comment #3)
> What about this?
> 
> [WKWebsiteDataStore.defaultDataStore removeDataOfTypes:[NSSet
> setWithObject:WKWebsiteDataTypeCookies] modifiedSince:[NSDate distantPast]
> completionHandler:^{ }];

I may well be missing something but the only place where I see Cookie data type handled is https://github.com/WebKit/WebKit/blob/452ee7982845475da6114437f62e77ef79e8cf64/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp#L661 and it seems to me that the code doesn't go to the network process and does something different.

I can move the logic over there. It'd be quite non-intuitive place to look for deleteAllCookie functionality given that there is already dedicated class for cookie handling but would work for us.

[Darin Adler]

Alex isn’t asking you to move logic. He’s asking you to ensure there isn’t already API that works before adding anything new.

[Alex Christensen]

Replacing "[cookieStore deleteAllCookies:^{" in your API test WKHTTPCookieStore.DeleteAllCookies with this:

[webView.get().configuration.websiteDataStore removeDataOfTypes:[NSSet setWithObject:WKWebsiteDataTypeCookies] modifiedSince:[NSDate distantPast] completionHandler:^{

Makes the test still work, which indicates to me that this functionality already does exist.

[Darin Adler]

So if "us" is, say, the Chrome app on iOS, then please don’t add more API; use what’s already there!

[Yury Semikhatsky]

> So if "us" is, say, the Chrome app on iOS,
I guess the chromium email that I use for WebKit contributions makes people
think that I work on Chrome, but I'm currently not affiliated with Google/Chrome.
To clarify, by "us" I mean Playwright[1] team. Some more context on what
prompted this work: I'm upstreaming parts of
our fork that other clients of WebKit would benefit from (like better
insepctor instrumentation) and also trying to switch more of our code to the
public WebKit APIs. The latter sometimes requires improving existing APIs
and sometimes adding new ones (e.g. time zone override).

The ultimate goal is to minimize our fork and ideally get rid of
the downstream changes to WebKit by moving more of the functionality to
our embedder layer. Currently, many automation features that we have
are implemented inside WebKit by adding new instrumentation and new inspector
agents in UIProcess. One option to improve this would be to just upstream
that logic and expose it via the Web Inspector protocol. This was proposed in
https://bugs.webkit.org/show_bug.cgi?id=203870 and was not welcomed. The new
approach we have in mind is to move all of the instrumenting agents
from the UI process layer to the embedder and implement it there using piblic
WebKit APIs. I don't think this bug is the best place for an in-depth discussion
of that, but if you are interested in more details I could do a write-up of
what we currenly have and the ways that we see it could be cotributed to WebKit.
That way we could discuss the details of the approach in general.

[1] https://playwright.dev/


> then please don’t add more API; use what’s already there!
My bad, I was not aware of the alternative API. Thanks for pointing that out,
we'll use it instead (we'll need to f.

In my defense, I started with WebCookieManagerProxy::deleteAllCookies (that
we currently call in the fork) which was already exposed in public C API
(WKHTTPCookieStoreDeleteAllCookies) but not in Cocoa and also lacked proper
handling of the completion handler (that we had to fix in our fork). Moreover,
existing code in WKHTTPCookieStore test simply deletes cookies one by one
when it needs to clear the store. So it seemed natural to fix the completion
hadler problem and add WKHTTPCookieStore.deleteAllCookies.

I believe fixing WebCookieManagerProxy::deleteAllCookies completion handler logic would be still useful for those who call WKHTTPCookieStoreDeleteAllCookies. Will do that in a separate PR as we'll need that in our Windows port (there is no counterpart of removeDataOfTypes on WKWebsiteDataStoreRef).

[Alex Christensen]

Fixing WebCookieManagerProxy::deleteAllCookies is definitely something we should do.  If you put that in its own PR I'm pretty sure I'd approve that.  This is definitely the kind of contribution I want to encourage.

As for https://bugs.webkit.org/show_bug.cgi?id=203870 I wouldn't say it wasn't welcomed since I don't see any negative commentary.  I just think it didn't get the attention of the proper reviewers, which is a slightly different problem.

[Yury Semikhatsky]

(In reply to Alex Christensen from comment #9)
> Fixing WebCookieManagerProxy::deleteAllCookies is definitely something we
> should do.  If you put that in its own PR I'm pretty sure I'd approve that. 
> This is definitely the kind of contribution I want to encourage.
>

OK, will send it shortly.

> As for https://bugs.webkit.org/show_bug.cgi?id=203870 I wouldn't say it
> wasn't welcomed since I don't see any negative commentary.  I just think it
> didn't get the attention of the proper reviewers, which is a slightly
> different problem.

The discussion happened primarily on #web-inspector-dev channel in slack with Devin Rousso so it's unfortunately not reflected in the bug (and I can't seem to dig that far into slack history). I also talked in person with him and Joseph Pecoraro about the direction in general during the WebKit summit in 2019. In any case, at this point it's easier to just start the design discussion over as we now have a WebKit-based browser that provides all the automation features that we need and we have better understanding of the alternative ways our fork could be up-streamed.