944006 - Add-on manager doesn't set schema version preference if there are no add-ons

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[:Irving Reid (No longer working on Firefox)]

Looking at the telemetry data tracking why the add-on XPI database is loaded at startup time, I noticed that on Android the database is loaded unusually often because the XPI provider believes the schema has changed. This has startup time cost because XPIProvider loads the XPIProviderUtils.js module and does additional work.

The current implementation of the XPI database only sets the extensions.databaseSchema preference after it successfully writes out a copy of the database; this is to protect against failures where the provider loads an old DB, upgrades the in-memory copy but doesn't update the on-disk version of the DB.

Because the default Android browser has no add-ons (unlike desktop, which always has at least the built in default theme), we never write the database, so the schema version preference is never set.

Comment 1

[:Irving Reid (No longer working on Firefox)]

Attachment: Update databaseSchema at startup when no extensions are installed

Well that turned into a bit of a marathon. Found a few things that misbehaved in subtle ways when I tried proceeding without creating an empty database, so the changes spread around a bit.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8340546 [details] [diff] [review]
Update databaseSchema at startup when no extensions are installed

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

All good, aside from the missing file.

::: toolkit/mozapps/extensions/XPIProviderUtils.js
@@ +800,5 @@
>                                         true);
>  
>      if (!addonsList.exists())
> +      // XXX Irving believes this is broken in the case where there is no
> +      // extensions.ini but there are bootstrap extensions (e.g. Android)

Yep :\ IIRC, we have a bug somewhere for the other case where this method can return null but shouldn't, but not this.

::: toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
@@ +204,5 @@
>  [test_migrate4.js]
>  [test_migrate5.js]
>  [test_migrateAddonRepository.js]
>  [test_migrate_max_version.js]
> +[test_no_addons.js]

Forgot to add this file?

Comment 3

[:Irving Reid (No longer working on Firefox)]

Attachment: Update databaseSchema at startup when no extensions are installed, with test file

Oops. Test file added...

My preferred fix for the extensions.ini issue would be to get rid of it completely, and probably end up with something that's async saved and contains extensions.ini + pref(extensions.installCache) + pref(extensions.bootstrapAddons)

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to :Irving Reid from comment #3)
> My preferred fix for the extensions.ini issue would be to get rid of it
> completely, and probably end up with something that's async saved and
> contains extensions.ini + pref(extensions.installCache) +
> pref(extensions.bootstrapAddons)

I'd be open to something like this. At the very least, we do need to fix bug 731489 somehow.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8341417 [details] [diff] [review]
Update databaseSchema at startup when no extensions are installed, with test file

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

(Just blindly assumed the only change was the added test file)

Comment 6

[:Irving Reid (No longer working on Firefox)]

https://hg.mozilla.org/integration/fx-team/rev/bf901d895201

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bf901d895201