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

RESOLVED FIXED in mozilla28

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Irving, Assigned: Irving)

Tracking

(Blocks: 1 bug)

unspecified
mozilla28
All
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Created attachment 8340546 [details] [diff] [review]
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.
Attachment #8340546 - Flags: review?(bmcbride)
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?
Attachment #8340546 - Flags: review?(bmcbride) → review-
Created attachment 8341417 [details] [diff] [review]
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)
Attachment #8340546 - Attachment is obsolete: true
Attachment #8341417 - Flags: review?(bmcbride)
(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 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)
Attachment #8341417 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/mozilla-central/rev/bf901d895201
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.