Backed out changeset 3f1747588a69 (bug 1664949) for xpcshell failure on test_update_theme.js CLOSED TREE

toolkit/mozapps/extensions/internal/XPIInstall.jsm

@@ -113,7 +113,7 @@ const PREF_XPI_DIRECT_WHITELISTED = "xpinstall.whitelist.directRequest";
 const PREF_XPI_FILE_WHITELISTED = "xpinstall.whitelist.fileRequest";
 const PREF_XPI_WHITELIST_REQUIRED = "xpinstall.whitelist.required";
 
-const PREF_SELECTED_THEME = "extensions.activeThemeID";
+const PREF_SELECTED_LWT = "extensions.activeThemeID";
 
 const TOOLKIT_ID = "toolkit@mozilla.org";
 
@@ -210,10 +210,10 @@ const LOGGER_ID = "addons.xpi";
 // (Requires AddonManager.jsm)
 var logger = Log.repository.getLogger(LOGGER_ID);
 
-// Stores the ID of the theme which was selected during the last session,
-// if any. When installing a new built-in theme with this ID, it will be
-// automatically enabled.
-let lastSelectedTheme = null;
+// Stores the ID of the lightweight theme which was selected during the
+// last session, if any. When installing a new built-in theme with this
+// ID, it will be automatically enabled.
+let lastLightweightTheme = null;
 
 function getJarURI(file, path = "") {
   if (file instanceof Ci.nsIFile) {
@@ -4378,11 +4378,9 @@ var XPIInstall = {
    *          installed.
    */
   async installBuiltinAddon(base) {
-    // We have to get this before the install, as the install will overwrite
-    // the pref. We then keep the value for this run, so we can restore
-    // the selected theme once it becomes available.
-    if (lastSelectedTheme === null) {
-      lastSelectedTheme = Services.prefs.getCharPref(PREF_SELECTED_THEME, "");
+    if (lastLightweightTheme === null) {
+      lastLightweightTheme = Services.prefs.getCharPref(PREF_SELECTED_LWT, "");
+      Services.prefs.clearUserPref(PREF_SELECTED_LWT);
     }
 
     let baseURL = Services.io.newURI(base);
@@ -4402,14 +4400,20 @@ var XPIInstall = {
     // If this is a theme, decide whether to enable it. Themes are
     // disabled by default. However:
     //
+    // If a lightweight theme was selected in the last session, and this
+    // theme has the same ID, then we clearly want to enable it.
+    //
+    // If it is the default theme, more specialized behavior applies:
+    //
     // We always want one theme to be active, falling back to the
-    // default theme when the active theme is disabled.
-    // During a theme migration, such as a change in the path to the addon, we
-    // will need to ensure a correct theme is enabled.
+    // default theme when the active theme is disabled. The first time
+    // we install the default theme, though, there likely aren't any
+    // other theme add-ons installed yet, in which case we want to
+    // enable it immediately.
     if (addon.type === "theme") {
       if (
-        addon.id === lastSelectedTheme ||
-        (!lastSelectedTheme.endsWith("@mozilla.org") &&
+        addon.id === lastLightweightTheme ||
+        (!lastLightweightTheme.endsWith("@mozilla.org") &&
           addon.id === AddonSettings.DEFAULT_THEME_ID &&
           !XPIDatabase.getAddonsByType("theme").some(theme => !theme.disabled))
       ) {

toolkit/mozapps/extensions/test/xpcshell/head_addons.js

@@ -1324,7 +1324,7 @@ async function setInitialState(addon, initialState) {
   }
 }
 
-async function setupBuiltinExtension(extensionData, location = "ext-test") {
+async function setupBuiltinExtension(extensionData) {
   let xpi = await AddonTestUtils.createTempWebExtensionFile(extensionData);
 
   // The built-in location requires a resource: URL that maps to a
@@ -1334,7 +1334,7 @@ async function setupBuiltinExtension(extensionData, location = "ext-test") {
   let resProto = Services.io
     .getProtocolHandler("resource")
     .QueryInterface(Ci.nsIResProtocolHandler);
-  resProto.setSubstitution(location, base);
+  resProto.setSubstitution("ext-test", base);
 }
 
 async function installBuiltinExtension(extensionData, waitForStartup = true) {

toolkit/mozapps/extensions/test/xpcshell/test_update_theme.js

@@ -8,8 +8,6 @@ XPCOMUtils.defineLazyGetter(this, "Management", () => {
 });
 
 add_task(async function setup() {
-  let scopes = AddonManager.SCOPE_PROFILE | AddonManager.SCOPE_APPLICATION;
-  Services.prefs.setIntPref("extensions.enabledScopes", scopes);
   createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "42.0", "42.0");
 
   await promiseStartupManager();
@@ -51,75 +49,3 @@ add_task(async function test_update_of_disabled_theme() {
   ok(addon.userDisabled, "Theme is still disabled after an update");
   await addon.uninstall();
 });
-
-add_task(async function test_builtin_location_migration() {
-  const ADDON_ID = "mytheme@mozilla.org";
-
-  let themeDef = {
-    manifest: {
-      applications: { gecko: { id: ADDON_ID } },
-      version: "1.0",
-      theme: {},
-    },
-  };
-
-  await setupBuiltinExtension(themeDef, "first-loc", false);
-  let themeInstalled = AddonTestUtils.promiseAddonEvent("onInstalled");
-  await AddonManager.maybeInstallBuiltinAddon(
-    ADDON_ID,
-    "1.0",
-    "resource://first-loc/"
-  );
-  await themeInstalled;
-
-  let addon = await AddonManager.getAddonByID(ADDON_ID);
-  await addon.enable();
-  Assert.ok(!addon.userDisabled, "Add-on should be enabled.");
-
-  Assert.equal(
-    Services.prefs.getCharPref("extensions.activeThemeID", ""),
-    ADDON_ID,
-    "Pref should be set."
-  );
-
-  let { addons: activeThemes } = await AddonManager.getActiveAddons(["theme"]);
-  Assert.equal(activeThemes.length, 1, "Should have 1 theme.");
-  Assert.equal(activeThemes[0].id, ADDON_ID, "Should have enabled the theme.");
-
-  // If we restart and update, and install a newer version of the theme,
-  // it should be activated.
-  await promiseShutdownManager();
-
-  // Force schema change and restart
-  Services.prefs.setIntPref("extensions.databaseSchema", 0);
-  await promiseStartupManager();
-
-  // Set up a new version of the builtin add-on.
-  let newDef = { manifest: Object.assign({}, themeDef.manifest) };
-  newDef.manifest.version = "1.1";
-  await setupBuiltinExtension(newDef, "second-loc");
-  themeInstalled = AddonTestUtils.promiseAddonEvent("onInstalled");
-  await AddonManager.maybeInstallBuiltinAddon(
-    ADDON_ID,
-    "1.1",
-    "resource://second-loc/"
-  );
-  await themeInstalled;
-
-  let newAddon = await AddonManager.getAddonByID(ADDON_ID);
-  Assert.ok(!newAddon.userDisabled, "Add-on should be enabled.");
-
-  ({ addons: activeThemes } = await AddonManager.getActiveAddons(["theme"]));
-  Assert.equal(activeThemes.length, 1, "Should still have 1 theme.");
-  Assert.equal(
-    activeThemes[0].id,
-    ADDON_ID,
-    "Should still have the theme enabled."
-  );
-  Assert.equal(
-    Services.prefs.getCharPref("extensions.activeThemeID", ""),
-    ADDON_ID,
-    "Pref should still be set."
-  );
-  await promiseShutdownManager();
-});