692712 - Pre-filter recommended add-ons JSON instead of filtering at startup

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Filtering at startup, in about:home requires the add-on manager code, and that requires SQLite access.

We could pre-filter the JSON when we save the file and when we add/remove an add-on. Those happen much less often than startup.

(Thanks Dietrich and Mossop for the idea)

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Note to others: We store the recommended add-ons in a JSON file so we can display them in the startup page. We filter the list to remove any add-ons you already have installed.

Comment 2

[Dave Townsend [:mossop]]

You could cache the unfiltered and filtered lists. On startup call AddonManager.getStartupChanges for STARTUP_CHANGE_INSTALLED and STARTUP_CHANGE_UNINSTALLED. If either returns a non-empty array then the set of installed add-ons has changed so you can re-request the list of add-ons and filter again (in this case the sqlite DB will already have been loaded).

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Dave Townsend (:Mossop) from comment #2)
> You could cache the unfiltered and filtered lists. On startup call
> AddonManager.getStartupChanges for STARTUP_CHANGE_INSTALLED and
> STARTUP_CHANGE_UNINSTALLED. If either returns a non-empty array then the set
> of installed add-ons has changed so you can re-request the list of add-ons
> and filter again (in this case the sqlite DB will already have been loaded).

Checking for these doesn't seem to catch restartless addons

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch makes a few changes to the way we use the recommended add-ons cache:

* Avoids using AddonManager to filter the cache, which avoids causing the add-ons SQLite DB from being opened during the start page activity.
* Filters the cache when fetching it
* Reduces the size of the raw cache file by only storing data fields we actuall (or might actually) use. This reduces my cache from 8.2KB to 654B, making it faster to save and load.
* Forces a cache refresh whenever an add-on is installed. We could do it for uninstalling add-ons too, but this seem like less of a priority.

The

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 565561 [details] [diff] [review]
patch

Review of attachment 565561 [details] [diff] [review]:
-----------------------------------------------------------------

Cool. Sorry for the delay. Just had to dig through and remember what's going on here.

Just to note, this will only prevent us from hitting the AddonsManager when we already have a cache. If you install an addon or if its first run, or the cache was deleted for some other reason, we'll still have to initialize the list. I half wonder if we'd be better to avoid it on firstrun as well, when we may have to create databases?

::: mobile/chrome/content/extensions.js
@@ +1066,5 @@
>  
>      ExtensionsView.showAlert(strings.GetStringFromName(stringName), !aNeedsRestart);
>    },
> +
> +  _refeshRecommendedCache: function xpidm_refeshRecommendedCache() {

This doesn't refresh the cache. It deletes it right? I think we should name this something more in line with what it does. clearRecommendedCache maybe? And then maybe quick comment about why we're deleting it I guess.

::: mobile/components/AddonUpdateService.js
@@ +190,5 @@
>  
> +    // Filter addons already installed
> +    AddonManager.getAllAddons(function(aAllAddons) {
> +      let addons = aAddons.filter(function(addon) {
> +        for (let i =0; i < aAllAddons.length; i++)

nit:missing space

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Fixed nits:
https://hg.mozilla.org/mozilla-central/rev/194720ba054f

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 565561 [details] [diff] [review]
patch

This change is not a 1-liner, but it is simple. It moves code to filter a list of add-ons read from disk on the home screen to the place where we save the list of add-ons to disk.

This reduces the amount of work done at startup and avoids opening the AddonManager SQLite DB during startup.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f41ea62c6d4c