1462516 - Move still more XPI database code out of XPIProvider

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[Kris Maglione [:kmag]]

Most of it is already gone, but we still have a bunch of non-trivial code for querying the add-ons DB which is never needed before the DB is loaded.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1462516: Move more XPI database code out of XPIProvider.jsm.

Review commit: https://reviewboard.mozilla.org/r/244862/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244862/

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 8976758 [details]
Bug 1462516: Move more XPI database code out of XPIProvider.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/244862/diff/1-2/

Comment 3

[Andrew Swan [:aswan]]

Comment on attachment 8976758 [details]
Bug 1462516: Move more XPI database code out of XPIProvider.jsm.

https://reviewboard.mozilla.org/r/244862/#review251574

::: toolkit/mozapps/extensions/internal/XPIDatabase.jsm:1907
(Diff revision 2)
>     *
> -   * @param {Array<string>?} aTypes
> +   * @param {Set<string>?} aTypes
>     *        The types of add-ons to retrieve or null to get all types
>     * @returns {Promise<Array<AddonInternal>>}
>     */
>    getVisibleAddonsWithPendingOperations(aTypes) {

this function can just be removed now

::: toolkit/mozapps/extensions/internal/XPIDatabase.jsm:2424
(Diff revision 2)
>  
>    /**
> +   * Update the appDisabled property for all add-ons.
> +   */
> +  updateAddonAppDisabledStates() {
> +    dump(`UPDATE ADS\n`);

remove (also 2 lines down)

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm
(Diff revision 2)
> -    // If the database doesn't exist and there are add-ons installed then we
> -    // must update the database however if there are no add-ons then there is
> -    // no need to update the database.
> -    let dbFile = FileUtils.getFile(KEY_PROFILEDIR, [FILE_DATABASE], true);
> -    if (!dbFile.exists() && haveAnyAddons) {
> -      updateReasons.push("needNewDatabase");
> -    }

just to make sure I understand, this isn't necessary since it will just get handled in loadDB() ?

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8976758 [details]
Bug 1462516: Move more XPI database code out of XPIProvider.jsm.

https://reviewboard.mozilla.org/r/244862/#review251574

> this function can just be removed now

Already did this after we discussed it on IRC. I guess pushing the updated patch failed and I didn't notice.

> remove (also 2 lines down)

Yeah, fixed that just after I pushed.

> just to make sure I understand, this isn't necessary since it will just get handled in loadDB() ?

Yup. There's no reason for us to force a DB rebuild at startup, in this (unlikely) case. It'll happen automatically when we load the DB after startup. Which means there's no need for the extra stat and code complexity that this adds.

Comment 5

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b9b19b3f3b37464c8b5d453abb003b23602e13b
Bug 1462516: Move more XPI database code out of XPIProvider.jsm. r=aswan

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4b9b19b3f3b3

Comment 7

[CosminB]

Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!