Bug 1094821 - Store currently selected lightweight theme in the selectedThemeID pref instead figuring it out based on the order of the usedThemes pref;r=Gijs
authorBrian Grinstead <bgrinstead@mozilla.com>
Mon, 23 Mar 2015 15:32:41 -0700
changeset 235047 a12de055e090438061e91c3731041de672be0fe6
parent 235046 06f1b43f5a7b8f5cba9bb4ae18cdc488946154ff
child 235048 8129ad6db86a37c4efcefb66c77b776c96307093
push id11920
push userbgrinstead@mozilla.com
push dateMon, 23 Mar 2015 22:33:08 +0000
treeherderfx-team@8129ad6db86a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1094821
milestone39.0a1
Bug 1094821 - Store currently selected lightweight theme in the selectedThemeID pref instead figuring it out based on the order of the usedThemes pref;r=Gijs
browser/app/profile/firefox.js
browser/base/content/browser-devedition.js
browser/base/content/test/general/browser_devedition.js
browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js
browser/metro/profile/metro.js
mobile/android/chrome/content/browser.js
services/sync/modules/engines/prefs.js
services/sync/tests/unit/test_prefs_store.js
toolkit/mozapps/extensions/LightweightThemeManager.jsm
toolkit/mozapps/extensions/test/xpcshell/test_migrate3.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1293,17 +1293,17 @@ pref("services.sync.prefs.sync.dom.disab
 pref("services.sync.prefs.sync.dom.disable_window_flip", true);
 pref("services.sync.prefs.sync.dom.disable_window_move_resize", true);
 pref("services.sync.prefs.sync.dom.event.contextmenu.enabled", true);
 pref("services.sync.prefs.sync.extensions.personas.current", true);
 pref("services.sync.prefs.sync.extensions.update.enabled", true);
 pref("services.sync.prefs.sync.intl.accept_languages", true);
 pref("services.sync.prefs.sync.javascript.enabled", true);
 pref("services.sync.prefs.sync.layout.spellcheckDefault", true);
-pref("services.sync.prefs.sync.lightweightThemes.isThemeSelected", true);
+pref("services.sync.prefs.sync.lightweightThemes.selectedThemeID", true);
 pref("services.sync.prefs.sync.lightweightThemes.usedThemes", true);
 pref("services.sync.prefs.sync.network.cookie.cookieBehavior", true);
 pref("services.sync.prefs.sync.network.cookie.lifetimePolicy", true);
 pref("services.sync.prefs.sync.permissions.default.image", true);
 pref("services.sync.prefs.sync.pref.advanced.images.disable_button.view_image", true);
 pref("services.sync.prefs.sync.pref.advanced.javascript.disable_button.advanced", true);
 pref("services.sync.prefs.sync.pref.downloads.disable_button.edit_actions", true);
 pref("services.sync.prefs.sync.pref.privacy.disable_button.cookie_exceptions", true);
--- a/browser/base/content/browser-devedition.js
+++ b/browser/base/content/browser-devedition.js
@@ -4,17 +4,17 @@
 
 /**
  * Listeners for the DevEdition theme.  This adds an extra stylesheet
  * to browser.xul if a pref is set and no other themes are applied.
  */
 let DevEdition = {
   _prefName: "browser.devedition.theme.enabled",
   _themePrefName: "general.skins.selectedSkin",
-  _lwThemePrefName: "lightweightThemes.isThemeSelected",
+  _lwThemePrefName: "lightweightThemes.selectedThemeID",
   _devtoolsThemePrefName: "devtools.theme",
 
   styleSheetLocation: "chrome://browser/skin/devedition.css",
   styleSheet: null,
 
   init: function () {
     this._updateDevtoolsThemeAttribute();
     this._updateStyleSheetFromPrefs();
@@ -71,17 +71,17 @@ let DevEdition = {
     document.documentElement.setAttribute("devtoolstheme", devtoolsTheme);
     this._inferBrightness();
     this._updateStyleSheetFromPrefs();
   },
 
   _updateStyleSheetFromPrefs: function() {
     let lightweightThemeSelected = false;
     try {
-      lightweightThemeSelected = Services.prefs.getBoolPref(this._lwThemePrefName);
+      lightweightThemeSelected = !!Services.prefs.getCharPref(this._lwThemePrefName);
     } catch(e) {}
 
     let defaultThemeSelected = false;
     try {
        defaultThemeSelected = Services.prefs.getCharPref(this._themePrefName) == "classic/1.0";
     } catch(e) {}
 
     let deveditionThemeEnabled = Services.prefs.getBoolPref(this._prefName) &&
--- a/browser/base/content/test/general/browser_devedition.js
+++ b/browser/base/content/test/general/browser_devedition.js
@@ -1,44 +1,48 @@
 /*
  * Testing changes for Developer Edition theme.
  * A special stylesheet should be added to the browser.xul document
  * when browser.devedition.theme.enabled is set to true and no themes
  * are applied.
  */
 
 const PREF_DEVEDITION_THEME = "browser.devedition.theme.enabled";
-const PREF_LWTHEME = "lightweightThemes.isThemeSelected";
+const PREF_LWTHEME = "lightweightThemes.selectedThemeID";
+const PREF_LWTHEME_USED_THEMES = "lightweightThemes.usedThemes";
 const PREF_DEVTOOLS_THEME = "devtools.theme";
+const {LightweightThemeManager} = Components.utils.import("resource://gre/modules/LightweightThemeManager.jsm", {});
 
 registerCleanupFunction(() => {
   // Set preferences back to their original values
+  LightweightThemeManager.currentTheme = null;
   Services.prefs.clearUserPref(PREF_DEVEDITION_THEME);
   Services.prefs.clearUserPref(PREF_LWTHEME);
   Services.prefs.clearUserPref(PREF_DEVTOOLS_THEME);
+  Services.prefs.clearUserPref(PREF_LWTHEME_USED_THEMES);
 });
 
 add_task(function* startTests() {
   Services.prefs.setCharPref(PREF_DEVTOOLS_THEME, "dark");
 
   info ("Setting browser.devedition.theme.enabled to false.");
   Services.prefs.setBoolPref(PREF_DEVEDITION_THEME, false);
   ok (!DevEdition.styleSheet, "There is no devedition style sheet when the pref is false.");
 
   info ("Setting browser.devedition.theme.enabled to true.");
   Services.prefs.setBoolPref(PREF_DEVEDITION_THEME, true);
   ok (DevEdition.styleSheet, "There is a devedition stylesheet when no themes are applied and pref is set.");
 
   info ("Adding a lightweight theme.");
-  Services.prefs.setBoolPref(PREF_LWTHEME, true);
+  LightweightThemeManager.currentTheme = dummyLightweightTheme("preview0");
   ok (!DevEdition.styleSheet, "The devedition stylesheet has been removed when a lightweight theme is applied.");
 
   info ("Removing a lightweight theme.");
   let onAttributeAdded = waitForBrightTitlebarAttribute();
-  Services.prefs.setBoolPref(PREF_LWTHEME, false);
+  LightweightThemeManager.currentTheme = null;
   ok (DevEdition.styleSheet, "The devedition stylesheet has been added when a lightweight theme is removed.");
   yield onAttributeAdded;
 
   is (document.documentElement.getAttribute("brighttitlebarforeground"), "true",
      "The brighttitlebarforeground attribute is set on the window.");
 
   info ("Setting browser.devedition.theme.enabled to false.");
   Services.prefs.setBoolPref(PREF_DEVEDITION_THEME, false);
@@ -80,26 +84,24 @@ add_task(function* testDevtoolsTheme() {
   ok (!document.documentElement.hasAttribute("brighttitlebarforeground"),
      "The brighttitlebarforeground attribute is not set on the window with light devtools theme.");
 });
 
 function dummyLightweightTheme(id) {
   return {
     id: id,
     name: id,
-    headerURL: "http://lwttest.invalid/a.png",
-    footerURL: "http://lwttest.invalid/b.png",
+    headerURL: "resource:///chrome/browser/content/browser/defaultthemes/1.header.jpg",
+    iconURL: "resource:///chrome/browser/content/browser/defaultthemes/1.icon.jpg",
     textcolor: "red",
     accentcolor: "blue"
   };
 }
 
 add_task(function* testLightweightThemePreview() {
-  let {LightweightThemeManager} = Components.utils.import("resource://gre/modules/LightweightThemeManager.jsm", {});
-
   info ("Turning the pref on, then previewing lightweight themes");
   Services.prefs.setBoolPref(PREF_DEVEDITION_THEME, true);
   ok (DevEdition.styleSheet, "The devedition stylesheet is enabled.");
   LightweightThemeManager.previewTheme(dummyLightweightTheme("preview0"));
   ok (!DevEdition.styleSheet, "The devedition stylesheet is not enabled after a lightweight theme preview.");
   LightweightThemeManager.resetPreview();
   LightweightThemeManager.previewTheme(dummyLightweightTheme("preview1"));
   ok (!DevEdition.styleSheet, "The devedition stylesheet is not enabled after a second lightweight theme preview.");
--- a/browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js
+++ b/browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js
@@ -44,17 +44,17 @@ add_task(function () {
   ok(installedThemeId.startsWith(firstLWThemeId),
      "The second theme in the 'My Themes' section should be the newly installed theme: " +
      "Installed theme id: " + installedThemeId + "; First theme ID: " + firstLWThemeId);
   is(header.nextSibling.nextSibling.nextSibling, recommendedHeader,
      "There should be two themes in the 'My Themes' section");
 
   let defaultTheme = header.nextSibling;
   defaultTheme.doCommand();
-  is(Services.prefs.getBoolPref("lightweightThemes.isThemeSelected"), false, "No lwtheme should be selected");
+  is(Services.prefs.prefHasUserValue("lightweightThemes.selectedThemeID"), false, "No lwtheme should be selected");
 });
 
 add_task(function asyncCleanup() {
   yield endCustomizing();
 
   Services.prefs.clearUserPref("lightweightThemes.usedThemes");
   Services.prefs.clearUserPref("lightweightThemes.recommendedThemes");
 })
\ No newline at end of file
--- a/browser/metro/profile/metro.js
+++ b/browser/metro/profile/metro.js
@@ -539,17 +539,17 @@ pref("editor.singleLine.pasteNewlines", 
 
 #ifdef MOZ_SERVICES_SYNC
 // sync service
 pref("services.sync.registerEngines", "Tab,Bookmarks,Form,History,Password,Prefs");
 
 // prefs to sync by default
 pref("services.sync.prefs.sync.browser.tabs.warnOnClose", true);
 pref("services.sync.prefs.sync.devtools.errorconsole.enabled", true);
-pref("services.sync.prefs.sync.lightweightThemes.isThemeSelected", true);
+pref("services.sync.prefs.sync.lightweightThemes.selectedThemeID", true);
 pref("services.sync.prefs.sync.lightweightThemes.usedThemes", true);
 pref("services.sync.prefs.sync.privacy.donottrackheader.enabled", true);
 pref("services.sync.prefs.sync.privacy.donottrackheader.value", true);
 pref("services.sync.prefs.sync.signon.rememberSignons", true);
 #endif
 
 // threshold where a tap becomes a drag, in 1/240" reference pixels
 // The names of the preferences are to be in sync with EventStateManager.cpp
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -7743,17 +7743,17 @@ var Distribution = {
           case "undefined":
             defaults.setCharPref(key, value);
             break;
         }
       } catch (e) { /* ignore bad prefs and move on */ }
     }
 
     // Apply a lightweight theme if necessary
-    if (prefs && prefs["lightweightThemes.isThemeSelected"]) {
+    if (prefs && prefs["lightweightThemes.selectedThemeID"]) {
       Services.obs.notifyObservers(null, "lightweight-theme-apply", "");
     }
 
     let localizedString = Cc["@mozilla.org/pref-localizedstring;1"].createInstance(Ci.nsIPrefLocalizedString);
     let localizeablePrefs = aData["LocalizablePreferences"];
     for (let key in localizeablePrefs) {
       try {
         let value = localizeablePrefs[key];
--- a/services/sync/modules/engines/prefs.js
+++ b/services/sync/modules/engines/prefs.js
@@ -105,19 +105,18 @@ PrefStore.prototype = {
         // Missing prefs get the null value.
         values[pref] = this._prefs.get(pref, null);
       }
     }
     return values;
   },
 
   _setAllPrefs: function PrefStore__setAllPrefs(values) {
-    let enabledPref = "lightweightThemes.isThemeSelected";
-    let enabledBefore = this._prefs.get(enabledPref, false);
-    let prevTheme = LightweightThemeManager.currentTheme;
+    let selectedThemeIDPref = "lightweightThemes.selectedThemeID";
+    let selectedThemeIDBefore = this._prefs.get(selectedThemeIDPref, null);
 
     for (let [pref, value] in Iterator(values)) {
       if (!this._isSynced(pref))
         continue;
 
       // Pref has gone missing, best we can do is reset it.
       if (value == null) {
         this._prefs.reset(pref);
@@ -126,23 +125,24 @@ PrefStore.prototype = {
 
       try {
         this._prefs.set(pref, value);
       } catch(ex) {
         this._log.trace("Failed to set pref: " + pref + ": " + ex);
       } 
     }
 
-    // Notify the lightweight theme manager of all the new values
-    let enabledNow = this._prefs.get(enabledPref, false);
-    if (enabledBefore && !enabledNow) {
+    // Notify the lightweight theme manager if the selected theme has changed.
+    let selectedThemeIDAfter = this._prefs.get(selectedThemeIDPref, null);
+    if (selectedThemeIDBefore != selectedThemeIDAfter) {
+      // The currentTheme getter will reflect the theme with the new
+      // selectedThemeID (if there is one).  Just reset it to itself
+      let currentTheme = LightweightThemeManager.currentTheme;
       LightweightThemeManager.currentTheme = null;
-    } else if (enabledNow && LightweightThemeManager.usedThemes[0] != prevTheme) {
-      LightweightThemeManager.currentTheme = null;
-      LightweightThemeManager.currentTheme = LightweightThemeManager.usedThemes[0];
+      LightweightThemeManager.currentTheme = currentTheme;
     }
   },
 
   getAllIDs: function PrefStore_getAllIDs() {
     /* We store all prefs in just one WBO, with just one GUID */
     let allprefs = {};
     allprefs[PREFS_GUID] = true;
     return allprefs;
--- a/services/sync/tests/unit/test_prefs_store.js
+++ b/services/sync/tests/unit/test_prefs_store.js
@@ -90,38 +90,38 @@ function run_test() {
     do_check_eq(prefs.get("testing.deleteme"), undefined);
     do_check_eq(prefs.get("testing.dont.change"), "Please don't change me.");
     do_check_eq(Svc.Prefs.get("prefs.sync.testing.somepref"), true);
 
     _("Enable persona");
     // Ensure we don't go to the network to fetch personas and end up leaking
     // stuff.
     Services.io.offline = true;
-    do_check_false(!!prefs.get("lightweightThemes.isThemeSelected"));
+    do_check_false(!!prefs.get("lightweightThemes.selectedThemeID"));
     do_check_eq(LightweightThemeManager.currentTheme, null);
 
     let persona1 = makePersona();
     let persona2 = makePersona();
     let usedThemes = JSON.stringify([persona1, persona2]);
     record.value = {
-      "lightweightThemes.isThemeSelected": true,
+      "lightweightThemes.selectedThemeID": persona1.id,
       "lightweightThemes.usedThemes": usedThemes
     };
     store.update(record);
-    do_check_true(prefs.get("lightweightThemes.isThemeSelected"));
+    do_check_eq(prefs.get("lightweightThemes.selectedThemeID"), persona1.id);
     do_check_true(Utils.deepEquals(LightweightThemeManager.currentTheme,
                   persona1));
 
     _("Disable persona");
     record.value = {
-      "lightweightThemes.isThemeSelected": false,
+      "lightweightThemes.selectedThemeID": null,
       "lightweightThemes.usedThemes": usedThemes
     };
     store.update(record);
-    do_check_false(prefs.get("lightweightThemes.isThemeSelected"));
+    do_check_false(!!prefs.get("lightweightThemes.selectedThemeID"));
     do_check_eq(LightweightThemeManager.currentTheme, null);
 
     _("Only the current app's preferences are applied.");
     record = new PrefRec("prefs", "some-fake-app");
     record.value = {
       "testing.int": 98
     };
     store.update(record);
--- a/toolkit/mozapps/extensions/LightweightThemeManager.jsm
+++ b/toolkit/mozapps/extensions/LightweightThemeManager.jsm
@@ -64,35 +64,62 @@ this.__defineSetter__("_maxUsedThemes", 
 });
 
 // Holds the ID of the theme being enabled or disabled while sending out the
 // events so cached AddonWrapper instances can return correct values for
 // permissions and pendingOperations
 var _themeIDBeingEnabled = null;
 var _themeIDBeingDisabled = null;
 
+// Convert from the old storage format (in which the order of usedThemes
+// was combined with isThemeSelected to determine which theme was selected)
+// to the new one (where a selectedThemeID determines which theme is selected).
+(function migrateToNewStorageFormat() {
+  let wasThemeSelected = false;
+  try {
+    wasThemeSelected = _prefs.getBoolPref("isThemeSelected");
+  } catch(e) { }
+
+  if (wasThemeSelected) {
+    _prefs.clearUserPref("isThemeSelected");
+    let themes = [];
+    try {
+      themes = JSON.parse(_prefs.getComplexValue("usedThemes",
+                                                 Ci.nsISupportsString).data);
+    } catch (e) { }
+
+    if (Array.isArray(themes) && themes[0]) {
+      _prefs.setCharPref("selectedThemeID", themes[0].id);
+    }
+  }
+})();
+
 this.LightweightThemeManager = {
   get name() "LightweightThemeManager",
 
   get usedThemes () {
     try {
       return JSON.parse(_prefs.getComplexValue("usedThemes",
                                                Ci.nsISupportsString).data);
     } catch (e) {
       return [];
     }
   },
 
   get currentTheme () {
+    let selectedThemeID = null;
     try {
-      if (_prefs.getBoolPref("isThemeSelected"))
-        var data = this.usedThemes[0];
+      selectedThemeID = _prefs.getCharPref("selectedThemeID");
     } catch (e) {}
 
-    return data || null;
+    let data = null;
+    if (selectedThemeID) {
+      data = this.getUsedTheme(selectedThemeID);
+    }
+    return data;
   },
 
   get currentThemeForDisplay () {
     var data = this.currentTheme;
 
     if (data && PERSIST_ENABLED) {
       for (let key in PERSIST_FILES) {
         try {
@@ -237,17 +264,21 @@ this.LightweightThemeManager = {
       if (PERSIST_ENABLED) {
         LightweightThemeImageOptimizer.purge();
         _persistImages(aData, function themeChanged_persistImages() {
           _notifyWindows(this.currentThemeForDisplay);
         }.bind(this));
       }
     }
 
-    _prefs.setBoolPref("isThemeSelected", aData != null);
+    if (aData)
+      _prefs.setCharPref("selectedThemeID", aData.id);
+    else
+      _prefs.clearUserPref("selectedThemeID");
+
     _notifyWindows(aData);
     Services.obs.notifyObservers(null, "lightweight-theme-changed", null);
   },
 
   /**
    * Starts the Addons provider and enables the new lightweight theme if
    * necessary.
    */
--- a/toolkit/mozapps/extensions/test/xpcshell/test_migrate3.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_migrate3.js
@@ -113,17 +113,17 @@ function run_test() {
     description: "A test theme",
     author: "Mozilla",
     homepageURL: "http://localhost/data/index.html",
     headerURL: "http://localhost/data/header.png",
     footerURL: "http://localhost/data/footer.png",
     previewURL: "http://localhost/data/preview.png",
     iconURL: "http://localhost/data/icon.png"
   }]));
-  Services.prefs.setBoolPref("lightweightThemes.isThemeSelected", true);
+  Services.prefs.setCharPref("lightweightThemes.selectedThemeID", "1");
 
   let stagedXPIs = profileDir.clone();
   stagedXPIs.append("staged-xpis");
   stagedXPIs.append("addon6@tests.mozilla.org");
   stagedXPIs.create(AM_Ci.nsIFile.DIRECTORY_TYPE, 0755);
 
   let addon6 = do_get_addon("test_migrate6");
   addon6.copyTo(stagedXPIs, "tmp.xpi");