1867344 - Make JumpListBuilder smarter about clearing old favicons from the disk

Status: NEW

(Firefox :: Shell Integration, task, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

mhowell pointed this out in this review: https://phabricator.services.mozilla.com/D181675?id=777266#inline-1063600

Basically, every time we repopulate the jump list, we run an async job to clear the favicons from the disk before potentially repopulating them. This could probably be smarter, because these icons are not super likely to change over time, especially for users that have fairly repetitive browsing habits where the "frequent" list of sites does not change.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Ideally, whatever solution we provide here would also take care of bug 1503809, and move all of this work off of the main thread.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

we run an async job to clear the favicons from the disk before potentially repopulating them.

So, it turns out there's more nuance here than that. Despite the name, AsyncDeleteAllFaviconsFromDisk doesn't always... delete all favicons from the disk. It takes an optional boolean argument for whether or not recently modified favicons should be skipped when clearing. Both the old and new Jump List backends pass that argument so that recently created favicons are not removed.

It looks like we will clear out any favicons that are over 24 hours + 10 minutes old (as in, since being modified on the disk).

And it also looks like we won't update a favicon until it's at least 24 hours old.

Is this an elegant solution to maintaining the icon files? Not really. But it's fairly simple, so that's kind of nice. I don't want to accidentally make things more complicated by adding a more sophisticated icon management system here.

mhowell, did you have any suggestions on how we might make this icon maintenance stuff smarter?

Comment 3

[Molly Howell (she/her) (mostly inactive)]

Hmm. Yeah, everything else I can think of is indeed more complicated, I don't have any brilliant solution for you. But dang, that is so much unnecessary disk output happening.