Bug 1406778 - Lazily load the font inspector. r=ochameau
authorGabriel Luong <gabriel.luong@gmail.com>
Thu, 12 Oct 2017 23:19:29 -0400
changeset 386056 193014cd4fd4afc31bc634c4b590d2898682b054
parent 386055 2559f86f67f6bfc1aad8b413a77a26e92bfdfb3f
child 386057 2f76e40db328035331dd6aa79d0e5fbf04db7c96
push id32673
push userarchaeopteryx@coole-files.de
push dateFri, 13 Oct 2017 09:13:17 +0000
treeherdermozilla-central@196dadb2fe50 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1406778
milestone58.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 1406778 - Lazily load the font inspector. r=ochameau
devtools/client/inspector/fonts/fonts.js
devtools/client/inspector/fonts/test/head.js
devtools/client/inspector/inspector.js
devtools/client/preferences/devtools.js
--- a/devtools/client/inspector/fonts/fonts.js
+++ b/devtools/client/inspector/fonts/fonts.js
@@ -1,17 +1,16 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-const Services = require("Services");
 const { Task } = require("devtools/shared/task");
 const { getColor } = require("devtools/client/shared/theme");
 
 const { createFactory, createElement } = require("devtools/client/shared/vendor/react");
 const { Provider } = require("devtools/client/shared/vendor/react-redux");
 
 const { gDevTools } = require("devtools/client/framework/devtools");
 
@@ -23,52 +22,48 @@ const INSPECTOR_L10N =
 
 const { updateFonts } = require("./actions/fonts");
 const { updatePreviewText, updateShowAllFonts } = require("./actions/font-options");
 
 function FontInspector(inspector, window) {
   this.document = window.document;
   this.inspector = inspector;
   this.pageStyle = this.inspector.pageStyle;
-  this.store = inspector.store;
+  this.store = this.inspector.store;
 
   this.update = this.update.bind(this);
 
   this.onNewNode = this.onNewNode.bind(this);
   this.onPreviewFonts = this.onPreviewFonts.bind(this);
   this.onShowAllFont = this.onShowAllFont.bind(this);
   this.onThemeChanged = this.onThemeChanged.bind(this);
+
+  this.init();
 }
 
 FontInspector.prototype = {
   init() {
     if (!this.inspector) {
       return;
     }
 
     let app = App({
       onPreviewFonts: this.onPreviewFonts,
       onShowAllFont: this.onShowAllFont,
     });
 
     let provider = createElement(Provider, {
+      id: "fontinspector",
+      key: "fontinspector",
       store: this.store,
-      id: "fontinspector",
-      title: INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle"),
-      key: "fontinspector",
+      title: INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle")
     }, app);
 
-    let defaultTab = Services.prefs.getCharPref("devtools.inspector.activeSidebar");
-
-    this.inspector.addSidebarTab(
-      "fontinspector",
-      INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle"),
-      provider,
-      defaultTab == "fontinspector"
-    );
+    // Expose the provider to let inspector.js use it in setupSidebar.
+    this.provider = provider;
 
     this.inspector.selection.on("new-node-front", this.onNewNode);
     this.inspector.sidebar.on("fontinspector-selected", this.onNewNode);
 
     // Listen for theme changes as the color of the previews depend on the theme
     gDevTools.on("theme-switched", this.onThemeChanged);
 
     this.store.dispatch(updatePreviewText(""));
@@ -130,16 +125,21 @@ FontInspector.prototype = {
    * Handler for click on show all fonts button.
    */
   onShowAllFont() {
     this.store.dispatch(updateShowAllFonts(true));
     this.update();
   },
 
   update: Task.async(function* () {
+    // Stop refreshing if the inspector or store is already destroyed.
+    if (!this.inspector || !this.store) {
+      return;
+    }
+
     let node = this.inspector.selection.nodeFront;
     let fonts = [];
     let { fontOptions } = this.store.getState();
     let { showAllFonts, previewText } = fontOptions;
 
     // Clear the list of fonts if the currently selected node is not connected or an
     // element node unless all fonts are supposed to be shown.
     if (!showAllFonts &&
--- a/devtools/client/inspector/fonts/test/head.js
+++ b/devtools/client/inspector/fonts/test/head.js
@@ -6,20 +6,19 @@
 /* import-globals-from ../../test/head.js */
 "use strict";
 
 // Import the inspector's head.js first (which itself imports shared-head.js).
 Services.scriptloader.loadSubScript(
   "chrome://mochitests/content/browser/devtools/client/inspector/test/head.js",
   this);
 
-Services.prefs.setBoolPref("devtools.fontinspector.enabled", true);
 Services.prefs.setCharPref("devtools.inspector.activeSidebar", "fontinspector");
 registerCleanupFunction(() => {
-  Services.prefs.clearUserPref("devtools.fontinspector.enabled");
+  Services.prefs.clearUserPref("devtools.inspector.activeSidebar");
 });
 
 /**
  * The font-inspector doesn't participate in the inspector's update mechanism
  * (i.e. it doesn't call inspector.updating() when updating), so simply calling
  * the default selectNode isn't enough to guaranty that the panel has finished
  * updating. We also need to wait for the fontinspector-updated event.
  */
@@ -42,17 +41,17 @@ var openFontInspectorForURL = Task.async
   // Call selectNode again here to force a fontinspector update since we don't
   // know if the fontinspector-updated event has been sent while the inspector
   // was being opened or not.
   yield selectNode("body", inspector);
 
   return {
     toolbox,
     inspector,
-    view: inspector.getPanel("fontinspector")
+    view: inspector.fontinspector
   };
 });
 
 /**
  * Clears the preview input field, types new text into it and waits for the
  * preview images to be updated.
  *
  * @param {FontInspector} view - The FontInspector instance.
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -592,20 +592,16 @@ Inspector.prototype = {
         panel = new RuleViewTool(this, this.panelWin);
         break;
       case "boxmodel":
         // box-model isn't a panel on its own, it used to, now it is being used by
         // computed view and layout which retrieves an instance via getPanel.
         const BoxModel = require("devtools/client/inspector/boxmodel/box-model");
         panel = new BoxModel(this, this.panelWin);
         break;
-      case "fontinspector":
-        const FontInspector = require("devtools/client/inspector/fonts/fonts");
-        panel = new FontInspector(this, this.panelWin);
-        break;
       default:
         // This is a custom panel or a non lazy-loaded one.
         return null;
     }
     this._panels.set(id, panel);
     return panel;
   },
 
@@ -616,21 +612,16 @@ Inspector.prototype = {
     let tabbox = this.panelDoc.querySelector("#inspector-sidebar");
     this.sidebar = new ToolSidebar(tabbox, this, "inspector", {
       showAllTabsMenu: true
     });
     this.sidebar.on("select", this.onSidebarSelect);
 
     let defaultTab = Services.prefs.getCharPref("devtools.inspector.activeSidebar");
 
-    if (!Services.prefs.getBoolPref("devtools.fontinspector.enabled") &&
-       defaultTab == "fontinspector") {
-      defaultTab = "ruleview";
-    }
-
     // Append all side panels
     this.sidebar.addExistingTab(
       "ruleview",
       INSPECTOR_L10N.getStr("inspector.sidebar.ruleViewTitle"),
       defaultTab == "ruleview");
 
     this.sidebar.addExistingTab(
       "computedview",
@@ -663,23 +654,39 @@ Inspector.prototype = {
     if (this.target.form.animationsActor) {
       this.sidebar.addFrameTab(
         "animationinspector",
         INSPECTOR_L10N.getStr("inspector.sidebar.animationInspectorTitle"),
         "chrome://devtools/content/animationinspector/animation-inspector.xhtml",
         defaultTab == "animationinspector");
     }
 
-    if (Services.prefs.getBoolPref("devtools.fontinspector.enabled") &&
-        this.canGetUsedFontFaces) {
-      const FontInspector = this.browserRequire("devtools/client/inspector/fonts/fonts");
-      this.fontinspector = new FontInspector(this, this.panelWin);
-      this.fontinspector.init();
-
-      this.sidebar.toggleTab(true, "fontinspector");
+    if (this.canGetUsedFontFaces) {
+      // Inject a lazy loaded react tab by exposing a fake React object
+      // with a lazy defined Tab thanks to `panel` being a function
+      let fontId = "fontinspector";
+      let fontTitle = INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle");
+      this.sidebar.addTab(
+        fontId,
+        fontTitle,
+        {
+          props: {
+            id: fontId,
+            title: fontTitle
+          },
+          panel: () => {
+            if (!this.fontinspector) {
+              const FontInspector =
+                this.browserRequire("devtools/client/inspector/fonts/fonts");
+              this.fontinspector = new FontInspector(this, this.panelWin);
+            }
+            return this.fontinspector.provider;
+          }
+        },
+        defaultTab == fontId);
     }
 
     // Persist splitter state in preferences.
     this.sidebar.on("show", this.onSidebarShown);
     this.sidebar.on("hide", this.onSidebarHidden);
     this.sidebar.on("destroy", this.onSidebarHidden);
 
     this.sidebar.show(defaultTab);
--- a/devtools/client/preferences/devtools.js
+++ b/devtools/client/preferences/devtools.js
@@ -56,19 +56,16 @@ pref("devtools.inspector.imagePreviewToo
 pref("devtools.inspector.showUserAgentStyles", false);
 // Show all native anonymous content (like controls in <video> tags)
 pref("devtools.inspector.showAllAnonymousContent", false);
 // Enable the new color widget
 pref("devtools.inspector.colorWidget.enabled", false);
 // Enable the CSS shapes highlighter
 pref("devtools.inspector.shapesHighlighter.enabled", true);
 
-// Enable the Font Inspector
-pref("devtools.fontinspector.enabled", true);
-
 // Counter to promote the inspector layout view.
 // @remove after release 56 (See Bug 1355747)
 pref("devtools.promote.layoutview", 1);
 // Whether or not to show the promote bar in the layout view
 // @remove after release 56 (See Bug 1355747)
 pref("devtools.promote.layoutview.showPromoteBar", true);
 
 // Grid highlighter preferences