Bug 1349896 - Part 1: Expose devtools theme via gDevTools, r?rpl draft
authorBob Silverberg <bsilverberg@mozilla.com>
Tue, 09 May 2017 13:03:19 -0400
changeset 583033 a692c944a95092f74e7bb9f38f3e630dcb720bf0
parent 582730 5bc1c758ab57c1885dceab4e7837e58af27b998c
child 583034 69ad2716e7ad487ac66d568da91eff99abe0ccc9
push id60278
push userbmo:bob.silverberg@gmail.com
push dateTue, 23 May 2017 16:25:10 +0000
reviewersrpl
bugs1349896
milestone55.0a1
Bug 1349896 - Part 1: Expose devtools theme via gDevTools, r?rpl Added gDevTools.getTheme() to provide the theme name, and a gDevTools "theme-changed" event to provide a notification when the theme is changed. MozReview-Commit-ID: EeUAmtyPpUy
devtools/client/framework/devtools.js
devtools/client/framework/gDevTools.jsm
devtools/client/framework/test/browser_toolbox_theme_registration.js
devtools/client/shared/theme.js
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -16,16 +16,18 @@ loader.lazyRequireGetter(this, "ToolboxH
 loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true);
 
 const {defaultTools: DefaultTools, defaultThemes: DefaultThemes} =
   require("devtools/client/definitions");
 const EventEmitter = require("devtools/shared/event-emitter");
 const {JsonView} = require("devtools/client/jsonview/main");
 const AboutDevTools = require("devtools/client/framework/about-devtools-toolbox");
 const {Task} = require("devtools/shared/task");
+const {getTheme, setTheme, addThemeObserver, removeThemeObserver} =
+  require("devtools/client/shared/theme");
 
 const FORBIDDEN_IDS = new Set(["toolbox", ""]);
 const MAX_ORDINAL = 99;
 
 /**
  * DevTools is a class that represents a set of developer tools, it holds a
  * set of tools and keeps track of open toolboxes in the browser.
  */
@@ -38,16 +40,20 @@ function DevTools() {
 
   // JSON Viewer for 'application/json' documents.
   JsonView.initialize();
 
   AboutDevTools.register();
 
   EventEmitter.decorate(this);
 
+  // Listen for changes to the theme pref.
+  this._onThemeChanged = this._onThemeChanged.bind(this);
+  addThemeObserver(this._onThemeChanged);
+
   // This is important step in initialization codepath where we are going to
   // start registering all default tools and themes: create menuitems, keys, emit
   // related events.
   this.registerDefaults();
 
   // Register this new DevTools instance to Firefox. DevToolsShim is part of Firefox,
   // integrating with all Firefox codebase and making the glue between code from
   // mozilla-central and DevTools add-on on github
@@ -239,16 +245,33 @@ DevTools.prototype = {
         definitions.push(definition);
       }
     }
 
     return definitions.sort(this.ordinalSort);
   },
 
   /**
+   * Returns the name of the current theme for devtools.
+   *
+   * @return {string} theme
+   *         The name of the current devtools theme.
+   */
+  getTheme() {
+    return getTheme();
+  },
+
+  /**
+   * Called when the developer tools theme changes.
+   */
+  _onThemeChanged() {
+    this.emit("theme-changed", getTheme());
+  },
+
+  /**
    * Register a new theme for developer tools toolbox.
    *
    * A definition is a light object that holds various information about a
    * theme.
    *
    * Each themeDefinition has the following properties:
    * - id: Unique identifier for this theme (string|required)
    * - label: Localized name for the theme to be displayed to the user
@@ -292,30 +315,30 @@ DevTools.prototype = {
     let themeId = null;
     if (typeof theme == "string") {
       themeId = theme;
       theme = this._themes.get(theme);
     } else {
       themeId = theme.id;
     }
 
-    let currTheme = Services.prefs.getCharPref("devtools.theme");
+    let currTheme = getTheme();
 
     // Note that we can't check if `theme` is an item
     // of `DefaultThemes` as we end up reloading definitions
     // module and end up with different theme objects
     let isCoreTheme = DefaultThemes.some(t => t.id === themeId);
 
     // Reset the theme if an extension theme that's currently applied
     // is being removed.
     // Ignore shutdown since addons get disabled during that time.
     if (!Services.startup.shuttingDown &&
         !isCoreTheme &&
         theme.id == currTheme) {
-      Services.prefs.setCharPref("devtools.theme", "light");
+      setTheme("light");
 
       this.emit("theme-unregistered", theme);
     }
 
     this._themes.delete(themeId);
   },
 
   /**
@@ -510,16 +533,18 @@ DevTools.prototype = {
     for (let [key, ] of this.getToolDefinitionMap()) {
       this.unregisterTool(key, true);
     }
 
     JsonView.destroy();
 
     gDevTools.unregisterDefaults();
 
+    removeThemeObserver(this._onThemeChanged);
+
     // Notify the DevToolsShim that DevTools are no longer available, particularly if the
     // destroy was caused by disabling/removing the DevTools add-on.
     DevToolsShim.unregister();
 
     // Cleaning down the toolboxes: i.e.
     //   for (let [target, toolbox] of this._toolboxes) toolbox.destroy();
     // Is taken care of by the gDevToolsBrowser.forgetBrowserWindow
   },
--- a/devtools/client/framework/gDevTools.jsm
+++ b/devtools/client/framework/gDevTools.jsm
@@ -67,16 +67,19 @@ let gDevToolsMethods = [
   "registerTheme",
   "unregisterTool",
   "unregisterTheme",
 
   // Used by main.js and test
   "getToolDefinitionArray",
   "getThemeDefinitionArray",
 
+  // Used by WebExtensions devtools API
+  "getTheme",
+
   // Used by theme-switching.js
   "getThemeDefinition",
   "emit",
 
   // Used by /devtools
   "on",
   "off",
   "once",
--- a/devtools/client/framework/test/browser_toolbox_theme_registration.js
+++ b/devtools/client/framework/test/browser_toolbox_theme_registration.js
@@ -3,100 +3,129 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /* import-globals-from shared-head.js */
 "use strict";
 
 // Test for dynamically registering and unregistering themes
 const CHROME_URL = "chrome://mochitests/content/browser/devtools/client/framework/test/";
+const TEST_THEME_NAME = "test-theme";
+const LIGHT_THEME_NAME = "light";
 
 var toolbox;
 
 add_task(function* themeRegistration() {
   let tab = yield addTab("data:text/html,test");
   let target = TargetFactory.forTab(tab);
   toolbox = yield gDevTools.showToolbox(target, "options");
 
   let themeId = yield new Promise(resolve => {
     gDevTools.once("theme-registered", (e, registeredThemeId) => {
       resolve(registeredThemeId);
     });
 
     gDevTools.registerTheme({
-      id: "test-theme",
+      id: TEST_THEME_NAME,
       label: "Test theme",
       stylesheets: [CHROME_URL + "doc_theme.css"],
       classList: ["theme-test"],
     });
   });
 
-  is(themeId, "test-theme", "theme-registered event handler sent theme id");
+  is(themeId, TEST_THEME_NAME, "theme-registered event handler sent theme id");
 
   ok(gDevTools.getThemeDefinitionMap().has(themeId), "theme added to map");
 });
 
 add_task(function* themeInOptionsPanel() {
   let panelWin = toolbox.getCurrentPanel().panelWin;
   let doc = panelWin.frameElement.contentDocument;
   let themeBox = doc.getElementById("devtools-theme-box");
   let testThemeOption = themeBox.querySelector(
-    "input[type=radio][value=test-theme]");
+    `input[type=radio][value=${TEST_THEME_NAME}]`);
+  let eventsRecorded = [];
+
+  function onThemeChanged(event, theme) {
+    eventsRecorded.push(theme);
+  }
+  gDevTools.on("theme-changed", onThemeChanged);
 
   ok(testThemeOption, "new theme exists in the Options panel");
 
   let lightThemeOption = themeBox.querySelector(
-    "input[type=radio][value=light]");
+    `input[type=radio][value=${LIGHT_THEME_NAME}]`);
 
   let color = panelWin.getComputedStyle(themeBox).color;
   isnot(color, "rgb(255, 0, 0)", "style unapplied");
 
   let onThemeSwithComplete = once(panelWin, "theme-switch-complete");
 
   // Select test theme.
   testThemeOption.click();
 
   info("Waiting for theme to finish loading");
   yield onThemeSwithComplete;
 
+  is(gDevTools.getTheme(), TEST_THEME_NAME, "getTheme returns the expected theme");
+  is(eventsRecorded.pop(), TEST_THEME_NAME, "theme-changed fired with the expected theme");
+
   color = panelWin.getComputedStyle(themeBox).color;
   is(color, "rgb(255, 0, 0)", "style applied");
 
   onThemeSwithComplete = once(panelWin, "theme-switch-complete");
 
   // Select light theme
   lightThemeOption.click();
 
   info("Waiting for theme to finish loading");
   yield onThemeSwithComplete;
 
+  is(gDevTools.getTheme(), LIGHT_THEME_NAME, "getTheme returns the expected theme");
+  is(eventsRecorded.pop(), LIGHT_THEME_NAME, "theme-changed fired with the expected theme");
+
   color = panelWin.getComputedStyle(themeBox).color;
   isnot(color, "rgb(255, 0, 0)", "style unapplied");
 
   onThemeSwithComplete = once(panelWin, "theme-switch-complete");
   // Select test theme again.
   testThemeOption.click();
   yield onThemeSwithComplete;
+  is(gDevTools.getTheme(), TEST_THEME_NAME, "getTheme returns the expected theme");
+  is(eventsRecorded.pop(), TEST_THEME_NAME, "theme-changed fired with the expected theme");
+
+  gDevTools.off("theme-changed", onThemeChanged);
 });
 
 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");
+  let eventsRecorded = [];
+
+  function onThemeChanged(event, theme) {
+    eventsRecorded.push(theme);
+  }
+  gDevTools.on("theme-changed", onThemeChanged);
+
+  gDevTools.unregisterTheme(TEST_THEME_NAME);
   yield onUnRegisteredTheme;
   yield onThemeSwitchComplete;
 
-  ok(!gDevTools.getThemeDefinitionMap().has("test-theme"),
+  is(gDevTools.getTheme(), LIGHT_THEME_NAME, "getTheme returns the expected theme");
+  is(eventsRecorded.pop(), LIGHT_THEME_NAME, "theme-changed fired with the expected theme");
+  ok(!gDevTools.getThemeDefinitionMap().has(TEST_THEME_NAME),
     "theme removed from map");
 
   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");
+  is(themeBox.querySelector(`#devtools-theme-box [value=${LIGHT_THEME_NAME}]`).checked, true,
+    `${LIGHT_THEME_NAME} theme must be selected`);
+
+  gDevTools.off("theme-changed", onThemeChanged);
 });
 
 add_task(function* cleanup() {
   yield toolbox.destroy();
   toolbox = null;
 });
--- a/devtools/client/shared/theme.js
+++ b/devtools/client/shared/theme.js
@@ -13,16 +13,17 @@ const Services = require("Services");
 
 const variableFileContents = require("raw!devtools/client/themes/variables.css");
 
 const THEME_SELECTOR_STRINGS = {
   light: ":root.theme-light {",
   dark: ":root.theme-dark {",
   firebug: ":root.theme-firebug {"
 };
+const THEME_PREF = "devtools.theme";
 
 /**
  * Takes a theme name and returns the contents of its variable rule block.
  * The first time this runs fetches the variables CSS file and caches it.
  */
 function getThemeFile(name) {
   // If there's no theme expected for this name, use `light` as default.
   let selector = THEME_SELECTOR_STRINGS[name] ||
@@ -41,17 +42,17 @@ function getThemeFile(name) {
   return theme;
 }
 
 /**
  * Returns the string value of the current theme,
  * like "dark" or "light".
  */
 const getTheme = exports.getTheme = () => {
-  return Services.prefs.getCharPref("devtools.theme");
+  return Services.prefs.getCharPref(THEME_PREF);
 };
 
 /**
  * Returns a color indicated by `type` (like "toolbar-background", or
  * "highlight-red"), with the ability to specify a theme, or use whatever the
  * current theme is if left unset. If theme not found, falls back to "light"
  * theme. Returns null if the type cannot be found for the theme given.
  */
@@ -64,11 +65,25 @@ const getColor = exports.getColor = (typ
   // Return the appropriate variable in the theme, or otherwise, null.
   return match ? match[1] : null;
 };
 
 /**
  * Set the theme preference.
  */
 const setTheme = exports.setTheme = (newTheme) => {
-  Services.prefs.setCharPref("devtools.theme", newTheme);
+  Services.prefs.setCharPref(THEME_PREF, newTheme);
+};
+
+/**
+ * Add an observer for theme changes.
+ */
+const addThemeObserver = exports.addThemeObserver = observer => {
+  Services.prefs.addObserver(THEME_PREF, observer);
+};
+
+/**
+ * Remove an observer for theme changes.
+ */
+const removeThemeObserver = exports.removeThemeObserver = observer => {
+  Services.prefs.removeObserver(THEME_PREF, observer);
 };
 /* eslint-enable */