Bug 1302148 - Update devtools theme on devtools.theme pref change. r=bgrins
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 14 Sep 2016 06:33:55 -0700
changeset 314418 d0b6ed087223c1be7d9ce55bfc4dbd663551dd32
parent 314417 d772aeb63c38a896a00453229e309601dae88c35
child 314419 1fdbce2821b589bff8e03407712646f7c6716756
push id20574
push usercbook@mozilla.com
push dateTue, 20 Sep 2016 10:05:16 +0000
treeherderfx-team@14705f779a46 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1302148
milestone51.0a1
Bug 1302148 - Update devtools theme on devtools.theme pref change. r=bgrins MozReview-Commit-ID: Bu9AhcV8NLX
devtools/client/framework/test/browser_toolbox_theme_registration.js
devtools/client/framework/toolbox-options.js
devtools/client/shared/theme-switching.js
--- a/devtools/client/framework/test/browser_toolbox_theme_registration.js
+++ b/devtools/client/framework/test/browser_toolbox_theme_registration.js
@@ -73,24 +73,26 @@ add_task(function* themeInOptionsPanel()
 
   onThemeSwithComplete = once(panelWin, "theme-switch-complete");
   // Select test theme again.
   testThemeOption.click();
   yield onThemeSwithComplete;
 });
 
 add_task(function* themeUnregistration() {
+  let panelWin = toolbox.getCurrentPanel().panelWin;
   let onUnRegisteredTheme = once(gDevTools, "theme-unregistered");
+  let onThemeSwitchComplete = once(panelWin, "theme-switch-complete");
   gDevTools.unregisterTheme("test-theme");
   yield onUnRegisteredTheme;
+  yield onThemeSwitchComplete;
 
   ok(!gDevTools.getThemeDefinitionMap().has("test-theme"),
     "theme removed from map");
 
-  let panelWin = toolbox.getCurrentPanel().panelWin;
   let doc = panelWin.frameElement.contentDocument;
   let themeBox = doc.getElementById("devtools-theme-box");
 
   // The default light theme must be selected now.
   is(themeBox.querySelector("#devtools-theme-box [value=light]").checked, true,
     "light theme must be selected");
 });
 
--- a/devtools/client/framework/toolbox-options.js
+++ b/devtools/client/framework/toolbox-options.js
@@ -93,34 +93,36 @@ OptionsPanel.prototype = {
     this.setupThemeList();
     yield this.populatePreferences();
     this.isReady = true;
     this.emit("ready");
     return this;
   }),
 
   _addListeners: function () {
-    gDevTools.on("pref-changed", this._prefChanged);
+    Services.prefs.addObserver("devtools.cache.disabled", this._prefChanged, false);
+    Services.prefs.addObserver("devtools.theme", this._prefChanged, false);
     gDevTools.on("theme-registered", this._themeRegistered);
     gDevTools.on("theme-unregistered", this._themeUnregistered);
   },
 
   _removeListeners: function () {
-    gDevTools.off("pref-changed", this._prefChanged);
+    Services.prefs.removeObserver("devtools.cache.disabled", this._prefChanged);
+    Services.prefs.removeObserver("devtools.theme", this._prefChanged);
     gDevTools.off("theme-registered", this._themeRegistered);
     gDevTools.off("theme-unregistered", this._themeUnregistered);
   },
 
-  _prefChanged: function (event, data) {
-    if (data.pref === "devtools.cache.disabled") {
+  _prefChanged: function (subject, topic, prefName) {
+    if (prefName === "devtools.cache.disabled") {
       let cacheDisabled = data.newValue;
       let cbx = this.panelDoc.getElementById("devtools-disable-cache");
 
       cbx.checked = cacheDisabled;
-    } else if (data.pref === "devtools.theme") {
+    } else if (prefName === "devtools.theme") {
       this.updateCurrentTheme();
     }
   },
 
   _themeRegistered: function (event, themeId) {
     this.setupThemeList();
   },
 
@@ -249,16 +251,17 @@ OptionsPanel.prototype = {
       toolsNotSupportedLabel.style.display = "none";
     }
 
     this.panelWin.focus();
   },
 
   setupThemeList: function () {
     let themeBox = this.panelDoc.getElementById("devtools-theme-box");
+    themeBox.innerHTML = "";
 
     let createThemeOption = theme => {
       let inputLabel = this.panelDoc.createElement("label");
       let inputRadio = this.panelDoc.createElement("input");
       inputRadio.setAttribute("type", "radio");
       inputRadio.setAttribute("value", theme.id);
       inputRadio.setAttribute("name", "devtools-theme-item");
       inputRadio.addEventListener("change", function (e) {
@@ -345,21 +348,21 @@ OptionsPanel.prototype = {
   },
 
   updateCurrentTheme: function () {
     let currentTheme = GetPref("devtools.theme");
     let themeBox = this.panelDoc.getElementById("devtools-theme-box");
     let themeRadioInput = themeBox.querySelector(`[value=${currentTheme}]`);
 
     if (themeRadioInput) {
-      themeRadioInput.click();
+      themeRadioInput.checked = true;
     } else {
       // If the current theme does not exist anymore, switch to light theme
       let lightThemeInputRadio = themeBox.querySelector("[value=light]");
-      lightThemeInputRadio.click();
+      lightThemeInputRadio.checked = true;
     }
   },
 
   /**
    * Disables JavaScript for the currently loaded tab. We force a page refresh
    * here because setting docShell.allowJavascript to true fails to block JS
    * execution from event listeners added using addEventListener(), AJAX calls
    * and timers. The page refresh prevents these things from being added in the
--- a/devtools/client/shared/theme-switching.js
+++ b/devtools/client/shared/theme-switching.js
@@ -22,16 +22,17 @@
 
   // no-theme attributes allows to just est the platform attribute
   // to have per-platform CSS working correctly.
   if (documentElement.getAttribute("no-theme") === "true") {
     return;
   }
 
   let devtoolsStyleSheets = new WeakMap();
+  let gOldTheme = "";
 
   function forceStyle() {
     let computedStyle = window.getComputedStyle(documentElement);
     if (!computedStyle) {
       // Null when documentElement is not ready. This method is anyways not
       // required then as scrollbars would be in their state without flushing.
       return;
     }
@@ -78,20 +79,22 @@
   function notifyWindow() {
     window.dispatchEvent(new CustomEvent("theme-switch-complete", {}));
   }
 
   /*
    * Apply all the sheets from `newTheme` and remove all of the sheets
    * from `oldTheme`
    */
-  function switchTheme(newTheme, oldTheme) {
-    if (newTheme === oldTheme) {
+  function switchTheme(newTheme) {
+    if (newTheme === gOldTheme) {
       return;
     }
+    let oldTheme = gOldTheme;
+    gOldTheme = newTheme;
 
     let oldThemeDef = gDevTools.getThemeDefinition(oldTheme);
     let newThemeDef = gDevTools.getThemeDefinition(newTheme);
 
     // The theme might not be available anymore (e.g. uninstalled)
     // Use the default one.
     if (!newThemeDef) {
       newThemeDef = gDevTools.getThemeDefinition("light");
@@ -151,34 +154,32 @@
       }
 
       // Final notification for further theme-switching related logic.
       gDevTools.emit("theme-switched", window, newTheme, oldTheme);
       notifyWindow();
     }, console.error.bind(console));
   }
 
-  function handlePrefChange(event, data) {
-    if (data.pref == "devtools.theme") {
-      switchTheme(data.newValue, data.oldValue);
-    }
+  function handlePrefChange() {
+    switchTheme(Services.prefs.getCharPref("devtools.theme"));
   }
 
   const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
   const { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
   const Services = require("Services");
   const { gDevTools } = require("devtools/client/framework/devtools");
   const StylesheetUtils = require("sdk/stylesheet/utils");
   const { watchCSS } = require("devtools/client/shared/css-reload");
 
   if (documentElement.hasAttribute("force-theme")) {
     switchTheme(documentElement.getAttribute("force-theme"));
   } else {
     switchTheme(Services.prefs.getCharPref("devtools.theme"));
 
-    gDevTools.on("pref-changed", handlePrefChange);
+    Services.prefs.addObserver("devtools.theme", handlePrefChange, false);
     window.addEventListener("unload", function () {
-      gDevTools.off("pref-changed", handlePrefChange);
-    });
+      Services.prefs.removeObserver("devtools.theme", handlePrefChange);
+    }, { once: true });
   }
 
   watchCSS(window);
 })();