The eventual goal of static WebExtension themes is that they will replace the existing lightweight themes. We'll do this by changing AMO so that it accepts WebExtension themes, users who install new themes will get the new theming style. However there will be millions (roughly 6 of users have a lightweight theme) who will be stuck on the legacy lightweight theme. Themes have an update URL in them, but it looks like all it does is update the properties on a theme, ideally we want to turn into a WebExtension static theme. There's probably a few checks we should do on this flow to ensure that something bad doesn't happen. Eg: check its a WebExtension static theme that is signed and maybe coming from AMO? https://dxr.mozilla.org/mozilla-central/rev/d2edd256c3aadb1cea48da0c8f62f725bd53cb76/toolkit/mozapps/extensions/LightweightThemeManager.jsm#253

That should be 6% of users. Or thereabouts. More than 6 anyway.

The plan is for the theme update ping to be changed on AMO for migrated themes, returning the path of the static theme to be used for updating. More details to be sorted out on https://github.com/mozilla/addons-server/issues/7976

Comment on attachment 8984555 [details] Bug 1430276 Provide a method to update LWTs to xpi-packaged themes https://reviewboard.mozilla.org/r/250430/#review256690 ::: toolkit/mozapps/extensions/AddonManager.jsm:1280 (Diff revision 1) > let scope = {}; > ChromeUtils.import("resource://gre/modules/LightweightThemeManager.jsm", scope); Gross. Can we just have a ChromeUtils.defineModuleGetter at the top-level for this? ::: toolkit/mozapps/extensions/LightweightThemeManager.jsm:274 (Diff revision 1) > req.channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE; > // Prevent the request from writing to the cache. > req.channel.loadFlags |= Ci.nsIRequest.INHIBIT_CACHING; > > - req.addEventListener("load", () => { > + let loadPromise = new Promise(resolve => { > + req.addEventListener("load", resolve); `{once: true}` ::: toolkit/mozapps/extensions/LightweightThemeManager.jsm:277 (Diff revision 1) > > - req.addEventListener("load", () => { > + let loadPromise = new Promise(resolve => { > + req.addEventListener("load", resolve); > + }); > + > + req.send(null); : await new Promise(resolve => { ... req.send(); }); ::: toolkit/mozapps/extensions/LightweightThemeManager.jsm:306 (Diff revision 1) > + resolve(addon); > + } > + > + listener = { > + onDownloadEnded() { > + if (install.addon && install.type != "theme") { s/!=/&=/ ::: toolkit/mozapps/extensions/LightweightThemeManager.jsm:308 (Diff revision 1) > + > + listener = { > + onDownloadEnded() { > + if (install.addon && install.type != "theme") { > + Cu.reportError(`Refusing to update lightweight theme to a ${install.type} (from ${url})`); > + install.cancel(); Can just return `false` ::: toolkit/mozapps/extensions/LightweightThemeManager.jsm:318 (Diff revision 1) > + onDownloadFailed() { finish(null); }, > + onInstallFailed() { finish(null); }, > + onInstallEnded(_install, addon) { finish(addon); }, > + }; > + install.addListener(listener); > + install.install(); Probably may as well just `updated = await install.install()` here. No need to even remove the listener, really, or implement the other methods. ::: toolkit/mozapps/extensions/LightweightThemeManager.jsm:349 (Diff revision 1) > + } catch (e) { > + return; > + } Why try-catch? ::: toolkit/mozapps/extensions/LightweightThemeManager.jsm:363 (Diff revision 1) > + } > + > + let selectedID = _prefs.getStringPref("selectedThemeID", DEFAULT_THEME_ID); > + let newThemes = []; > + for (let theme of allThemes) { > + let newTheme = await this._updateOneTheme(theme, theme.id == selectedID); Can we do these in parallel? ::: toolkit/mozapps/extensions/test/xpcshell/test_theme_update.js:1 (Diff revision 1) > + "use strict";

Comment on attachment 8984555 [details] Bug 1430276 Provide a method to update LWTs to xpi-packaged themes https://reviewboard.mozilla.org/r/250430/#review256690 > Gross. Can we just have a ChromeUtils.defineModuleGetter at the top-level for this? of course we already have one in addition to this. i removed the import() and the scope gunk.

Comment on attachment 8984555 [details] Bug 1430276 Provide a method to update LWTs to xpi-packaged themes https://reviewboard.mozilla.org/r/250430/#review256690 > Can just return `false` I believe this just postpones the install, it doesn't actually end it (ie, none of the `On*Failed` listeners are invoked so the install promise never resolve)

Comment on attachment 8984555 [details] Bug 1430276 Provide a method to update LWTs to xpi-packaged themes https://reviewboard.mozilla.org/r/250430/#review256690 > I believe this just postpones the install, it doesn't actually end it (ie, none of the `On*Failed` listeners are invoked so the install promise never resolve) Fair point. Please *also* return false, then.

Comment on attachment 8984555 [details] Bug 1430276 Provide a method to update LWTs to xpi-packaged themes https://reviewboard.mozilla.org/r/250430/#review256690 > Why try-catch? I dunno, I didn't change this code.

Comment on attachment 8984555 [details] Bug 1430276 Provide a method to update LWTs to xpi-packaged themes https://reviewboard.mozilla.org/r/250430/#review256690 > I dunno, I didn't change this code. Eh, blame mozreview for not detecting the move, then. Please get rid of the try catch and pass a default pref value instead.

Pushed by aswan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e2c7f43bb207 Provide a method to update LWTs to xpi-packaged themes r=kmag

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

Its not going to be possible to test this manually until the AMO side is done (https://github.com/mozilla/addons-server/issues/7976)