Create separate Garbage Collect Cookies algorithm to run after Store A Cookie #13
Conversation
| then return null. | ||
|
|
||
| 1. If _cookie_'s secure flag is equal to _oldCookie_'s secure flag, _cookie_'s same-site is equal to _oldCookie_'s | ||
| same-site, and _cookie_'s expiry-time is equal to _oldCookie_'s expiry-time, then return null. |
There was a problem hiding this comment.
We should probably write a test that if the expiry-time differs but everything else is the same you do get the event. Unless this is tested already?
There was a problem hiding this comment.
I could not find anything in https://github.com/web-platform-tests/wpt/tree/master/cookie-store, I think it is worth adding something for that and also testing that setting an already-expired cookie only triggers a change if it overwrites an existing one.
There was a problem hiding this comment.
This looks good to me, modulo the test which I think we should have before we land this.
I also spotted some necessary follow-up to fully complete this.
Run Garbage Collect Cookies handles expired cookies, but expired cookies are also still separately handled at the top of #cookie-store-eviction. That's problematic. Instead we should let Fetch et al determine when expired cookies are handled so we can account for any mutation events they might need to make happen.
So I think we need a separate "Remove Expired Cookies" that's also a host hook, but it's probably okay if Garbage Collect Cookies calls it as well.
And then the SHOULD/MUST requirements in #cookie-store-eviction should be made to only apply to non-browser user agents as browsers will have their own requirements. (Although we can suggest they should have requirements along the same lines, as we did with the other hooks I think.)
|
It also seems we need to figure out what's wrong with CI. |
This change is to help facilitate support of cookie change events in the CookieStore API. It makes the following changes to the spec:
Move removing expired cookies, Remove Excess Cookies for a Host, and Remove Global Excess Cookies from inside the Store A Cookie algorithm into a new algorithm, Garbage Collect Cookies, which should be invoked by the caller of Store A Cookie.
Have Store A Cookie return the resulting cookie if it is stored, null otherwise. This is to support calling Remove Excess Cookies for a Host outside Store A Cookie, and returning this value helps the Fetch spec support cookie change events.
Have Garbage Collect Cookies return a list of cookies it removes. This is to support cookie change events, since if the cookie returned by Store A Cookie is present in this list, we should not trigger a change event.