576347 - Periodically purge expired cookies and vacuum db

Status: NEW

(Core :: Networking: Cookies, defect, P5)

Description

[[e-mail]]

In bug 572223 I removed the startup purge of expired cookies from the db. We should add this back, but in a smarter way. I don't think we want to leave it up to PurgeCookies(): this guarantees that deletion of expired cookies will not occur for a long time. The metric for purging, in all cases, should be when there is a significant amount of data (> 10% of 3000 cookies) to cull. For live cookies, that's handled fine by PurgeCookies(), but it is not invoked based on how many expired cookies exist.

I think we want to do this as follows. Each time we do a table enumeration (i.e. during startup and PurgeCookies()), make a measurement of the expiration time of the 300th cookie when sorted by ascending expiry time. (Same idea as measuring the oldest lastAccessed time for purging.) When that equals PR_Now(), call up PurgeCookies(). This means we won't necessarily purge every startup, which is far superior.

We could also vacuum at that point but we might want to do it less frequently. Maybe by storing an integer in the table that counts how many cookies have been removed. When that hits 3000 (i.e. as many removed as the nominal table size) we vacuum.

Comment 1

[Jo Hermans]

Just add it PurgeCookies to the idle-daily routine. And the vacuuming can be done together with he places vacuum (also idle-daily, but at most once per month).

Comment 2

[[e-mail]]

We could do that, yeah. Taras mentioned that vacuuming doesn't help much with on-disk file fragmentation, so rather than that we might want to actually rewrite the entire db to a new file ourselves.

Comment 3

[[e-mail]]

Reassigning to nobody. If anyone wants to work on this, feel free!

Comment 4

[[e-mail]]

When *do* expired cookies actually get purged currently?  I was thinking of opening a bug and found this one while searching.  I have loads of cookies that are way (months) past their expiry date that are sitting around not being deleted.  I have the same problem if I create a new profile, and copy my cookies.sqllite over to it - the expired cookies are not purged.  What do I have to do to get them all cleaned up?!  There doesn't seem to be a problem with the sqllite file, I did a cookie trace and that worked fine, I could see cookies being accepted/rejected/sent etc.

This is on Mozilla/5.0 (Windows NT 5.1; rv:7.0a2) Gecko/20110730 Firefox/7.0a2, on winXP.

Comment 5

[Jo Hermans]

Currently when the cookie-database is 10% over the size-limit (3000) AND the age-limit (oldest cookie must be at least be 30 days old). See the call to PurgeCookies at <http://dxr.mozilla.org/mozilla/mozilla-central/netwerk/cookie/nsCookieService.cpp.html#l2732>

Also note that not all expired cookies are purged, only the oldest ones until the number of cookies is below 3000 again (see <http://dxr.mozilla.org/mozilla/mozilla-central/netwerk/cookie/nsCookieService.cpp.html#l3423>). So, normally when you reach 3300 cookies, the oldest 300 ones are removed.

If you're constantly deleting cookies manually, you'll never reach the 3000 cookies-limit, and this code doesn't get triggered. That's why this bug is needed.

Comment 8

[David E. Ross]

Users currently have the ability to manually remove cookies.  If users are not going to get confused by the proliferation of cookies that are already expired while reviewing their total set of cookies, then ALL expired cookies should be purged.  

Since some users leave their computers running for long periods (days or even weeks) and leave their browsers running the entire time, purging only at startup is unacceptable.  Purging only at some startups or only monthly is worse, as is purging only cookies that expired more than 30 days ago.  Expired cookies should be purged at least daily.  Better yet, expired cookies should be purged whenever a new cookie is added to the database.  This should not create significant overhead since very few expired cookies -- those expiring today -- or none at all will remain present for purging.  

This is not an issue of the size of cookies.sqlite.  This is an issue of end-users wanting to see what cookies are currently active without having to wade through expired cookies.  When a cookie has expired, I do not want it to still exist on my computer.

Comment 9

[Ian Nartowicz]

At the very least, if the cookies are not going to be physically purged very regularly, they shouldn't be shown by casual end-user tools such as the options cookie list.  Currently I have cookies that expired a year ago showing up when I press the "Show cookies" button.

And absolutely shouldn't be provided on HTTP exchanges.  I think that is the case already, although I didn't test.

Comment 10

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 20 votes.
:kershaw, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 12

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 13

[Oliver Kästner]

It is still relevant in the sense that the expired cookies are still visible in the Storage tab. However, they do not seem to be sent in requests.

For me as user, it is confusing to see cookies in the dev tools that are expired 6 months ago.

Comment 14

[08xjcec48]

Paul, I've seen your recent comments on Bug 1767271. Please look into this one too. Other browsers, like Edge, delete expired cookies immediately, while Firefox keeps all expired cookies indefinitely, including cookies with your Youtube searches: https://github.com/webcompat/web-bugs/issues/121615 .