1022247 - [Vertical] Icons fetched by shared/gaia_grid/js/icon_retriever.js are cached in idb forever

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Description

[James Lal [:lightsofapollo]]

Any consumer of gaia_grid will grow its async_storage database indefinitely as we add icons to the cache whenever it is displayed but we never remove them from the cache.

This effects not only the system app but any other apps which use the gaia_grid webcomponent.

Comment 1

[James Lal [:lightsofapollo]]

The logic in the icon_retriever (the one used on the vertical home) could use some reworking... We always hit http first before checking the cache. This is the correct behavior for app:// protocol fetches (fetching the blob via xhr takes 1-2s vs 5-6s in the idb async storage cache) but is incorrect for hosted apps which take an extra 1s prior to hitting the cache.

We can improve this logic by:

  - just hitting packaged apps directly via xhr rather then going through idb (never cache)
  - try http and the cache in parallel for hosted apps

Comment 2

[James Lal [:lightsofapollo]]

The sensible thing here is to probably store the blob of the icon (when needed for caching) along with .detail in the grid_item classes. Unless I am mistaken it's safe to hold a reference to the blob (without keeping its memory alive).

Comment 3

[James Lal [:lightsofapollo]]

Meaning we can ditch async_storage which has it's own secondary idb database with its own overhead... loading the first icon out of the cache should then be faster since we initialize the "item" store anyway to figure out where stuff is on the homescreen.

Comment 4

[James Lal [:lightsofapollo]]

^ the above makes perfect sense for the homescreen but not for anywhere else (since we don't have these centralized store things across all the apps) which makes a LRU cache (based on when the item was last accessed) make more sense. Maybe there is something in the platform which can/does cache this for us so we don't need to ourselves.

Comment 5

[James Lal [:lightsofapollo]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/20199

Detailed notes in github about what this does... This also should fix the following bugs:

https://bugzilla.mozilla.org/show_bug.cgi?id=1022098
https://bugzilla.mozilla.org/show_bug.cgi?id=1022497

In theory performance should be the same or better but I have not done a solid perf run yet on verticalhome (I will this week once I have my symbols and a linux box?)

Comment 6

[James Lal [:lightsofapollo]]

Comment on attachment 8437517 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20199

Moving both to f? This is not quite ready to land yet but is good enough for a first pass for approach... I need an additional test to verify the use of the cached blob (which I know is not working correct as it is not passed by all items from the db right now).

Comment 7

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8437517 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20199

I am very happy with this great job (congrats!!!) and also we are in the same page because if you see my pr to cancel and resume downloads you can see how we have taken the same solution :)

https://github.com/mozilla-b2g/gaia/pull/20234

Just a comment but this is a question of tastes thought. I would like that icons were clever and they would know to retry after fetching an icon and it fails instead of app.js. But it is just my idea and I could be completely wrong in my assumption.

Comment 8

[James Lal [:lightsofapollo]]

> would know to retry after fetching an icon and it fails instead of app.js. But it is just my idea and I could be completely wrong in my assumption.

Hrm- Is this just a design decision or do we want to issue icon retries on search/collections app?

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 8437517 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20199

I like it. Seems very clean and easy to understand. Nice work.

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8437517 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20199

Continuing to look over the pull request, let's just make sure we get the icon rounding issue solved before landing.

Comment 11

[James Lal [:lightsofapollo]]

https://github.com/mozilla-b2g/gaia/commit/5438f4773400a77f4d04f9e120a3c488c699ef7b

Comment 12

[Jason Smith [:jsmith]]

Blocks two blockers - blocking+

Comment 13

[Kevin Grandon :kgrandon]

I believe this was uplifted as: https://github.com/mozilla-b2g/gaia/commit/d016f4aa9add440c244ce3c3f615cac82101031a