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 265602 a12de055e090438061e91c3731041de672be0fe6
parent 265601 06f1b43f5a7b8f5cba9bb4ae18cdc488946154ff
child 265603 8129ad6db86a37c4efcefb66c77b776c96307093
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1094821
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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");