Closed Bug 1272679 Opened 4 years ago Closed 4 years ago
Clear Recent History, 1 hour, left accessed site in places
58 bytes, text/x-review-board-request
STR: ① Create new profile ② Access http://existentialcomics.com (or any site you wish to test on) and wait for it to finish loading ③ Hit ctrl-shift-del, using the default one hour period, select all checkboxes under details. ④ Close the browser. I tested using both the MATE Window Manager ⓧ button and the Firefox menu exit. ⑤ Search the new firefox profile for the website $ grep -rl existentialcomics.com * | xargs places.sqlite places.sqlite-wal sessionstore.js The sessionstore.js thing is a separate bug that :mak asked me to file. bug #1272675 On restart of the profile, the -wal was merged, but places.sqlite entries remained. They appear to be due to the favicon - favicon entries seem to have no create/modify timestamps which is complicating cleanup? Oddly favicon is preserved despite no other site data in places.
The problem here is that clearing less than Everything doesn't send an onClearHistory notification, and as such Places expiration doesn't start doing its job. The only critical tracking information we don't remove is favicons, the only other textual info we have is in moz_places (removed) and moz_hosts (removed). I think we should remove orphan favicons directly, rather than rely on expiration since they contain an url.
Assignee: nobody → mak77
Priority: -- → P1
This will have multiple parts: 1. since now RemoveAllPages has been removed, I'm moving back orphans expiration from Places Expiration to History.jsm 2. Places expiration shutdown can likely be simplified, maybe places-will-close-connection can go 3. History.remove and History.removeVisitsByFilter should remove orphan favicons directly This should give us some perf advantage on clear history (we remove pages before visits, so the visits trigger has less to do, plus we can use a transaction), a simpler shutdown, and solve this bug.
I moved the second part (places-will-close-connection removal) to bug 1275878, since it's unrelated and could be more regression prone.
Unfortunately I see a leak that I can't explain, that may slowdown things... https://treeherder.mozilla.org/#/jobs?repo=try&revision=60ef2c531ebb6d210015371072e10da9da8d8b1f TEST-UNEXPECTED-FAIL | browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placestitle.js | leaked 2 window(s) until shutdown [url = about:blank] TEST-UNEXPECTED-FAIL | browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placestitle.js | leaked 2 window(s) until shutdown [url = chrome://browser/content/browser.xul] This is at the end of the browser/components/privatebrowsing/test/browser folder. There's something fishy in these tests :(
I splitted all the "unrelated" stuff to bug 1275878, so here we can fix the privacy leak, and we won't be blocked by the memory leak the patches uncovered.
Review commit: https://reviewboard.mozilla.org/r/56276/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56276/
Attachment #8757962 - Flags: review?(adw)
Comment on attachment 8757962 [details] MozReview Request: Bug 1272679 - Expire orphans in removeVisitsByFilter. r=adw https://reviewboard.mozilla.org/r/56276/#review54956
Attachment #8757962 - Flags: review?(adw) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/fx-team/rev/90899efbaf73 Expire orphans in removeVisitsByFilter. r=adw
You need to log in before you can comment on or make changes to this bug.