Backed out changeset b832377ab7a5 (bug 1502857) for dt failures on browser_inspector_extension_sidebar.js and browser_fontinspector.js CLOSED TREE
authorarthur.iakab <aiakab@mozilla.com>
Mon, 05 Nov 2018 16:47:45 +0200
changeset 444446 50eb173f1bc2b8e8ce2921fcfd53a714e40dcab1
parent 444445 9bc3707f8ae4b19d4f63f689924893608fa7f75c
child 444447 ffb2442651963d1e043176caa3866e8b80cf7379
push id109593
push usernbeleuzu@mozilla.com
push dateMon, 05 Nov 2018 21:54:22 +0000
treeherdermozilla-inbound@c58b8835f297 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1502857
milestone65.0a1
backs outb832377ab7a5642158f81e34c74f1f079c8bb3b3
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
Backed out changeset b832377ab7a5 (bug 1502857) for dt failures on browser_inspector_extension_sidebar.js and browser_fontinspector.js CLOSED TREE
devtools/client/inspector/animation/test/head.js
devtools/client/inspector/inspector.js
devtools/client/inspector/test/shared-head.js
--- a/devtools/client/inspector/animation/test/head.js
+++ b/devtools/client/inspector/animation/test/head.js
@@ -26,17 +26,17 @@ registerCleanupFunction(() => {
  * Open the toolbox, with the inspector tool visible and the animationinspector
  * sidebar selected.
  *
  * @return {Promise} that resolves when the inspector is ready.
  */
 const openAnimationInspector = async function() {
   const { inspector, toolbox } = await openInspectorSidebarTab(TAB_NAME);
   await inspector.once("inspector-updated");
-  const animationInspector = inspector.getPanel("animationinspector");
+  const { animationinspector: animationInspector } = inspector;
   await waitForRendering(animationInspector);
   const panel = inspector.panelWin.document.getElementById("animation-container");
   return { animationInspector, toolbox, inspector, panel };
 };
 
 /**
  * Close the toolbox.
  *
@@ -400,17 +400,17 @@ const mouseOutOnTargetNode = function(an
  *
  * @param {InspectorPanel} inspector
  */
 const selectAnimationInspector = async function(inspector) {
   await inspector.toolbox.selectTool("inspector");
   const onUpdated = inspector.once("inspector-updated");
   inspector.sidebar.select("animationinspector");
   await onUpdated;
-  await waitForRendering(inspector.getPanel("animationinspector"));
+  await waitForRendering(inspector.animationinspector);
 };
 
 /**
  * Set the inspector's current selection to a node or to the first match of the
  * given css selector and wait for the animations to be displayed
  *
  * @param {String|NodeFront}
  *        data The node to select
@@ -423,17 +423,17 @@ const selectAnimationInspector = async f
  *                   and animations of its subtree are properly displayed.
  */
 const selectNodeAndWaitForAnimations = async function(data, inspector, reason = "test") {
   // We want to make sure the rest of the test waits for the animations to
   // be properly displayed (wait for all target DOM nodes to be previewed).
   const onUpdated = inspector.once("inspector-updated");
   await selectNode(data, inspector, reason);
   await onUpdated;
-  await waitForRendering(inspector.getPanel("animationinspector"));
+  await waitForRendering(inspector.animationinspector);
 };
 
 /**
  * Send keyboard event of space to given panel.
  *
  * @param {AnimationInspector} animationInspector
  * @param {DOMElement} target element.
  */
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -830,63 +830,38 @@ Inspector.prototype = {
 
   /**
    * Lazily get and create panel instances displayed in the sidebar
    */
   getPanel: function(id) {
     if (this._panels.has(id)) {
       return this._panels.get(id);
     }
-
     let panel;
     switch (id) {
-      case "animationinspector":
-        const AnimationInspector =
-          this.browserRequire("devtools/client/inspector/animation/animation");
-        panel = new AnimationInspector(this, this.panelWin);
+      case "computedview":
+        const {ComputedViewTool} =
+          this.browserRequire("devtools/client/inspector/computed/computed");
+        panel = new ComputedViewTool(this, this.panelWin);
+        break;
+      case "ruleview":
+        const {RuleViewTool} = require("devtools/client/inspector/rules/rules");
+        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
         // the layout view which retrieves an instance via getPanel.
         const BoxModel = require("devtools/client/inspector/boxmodel/box-model");
         panel = new BoxModel(this, this.panelWin);
         break;
-      case "changesview":
-        const ChangesView =
-          this.browserRequire("devtools/client/inspector/changes/ChangesView");
-        panel = new ChangesView(this, this.panelWin);
-        break;
-      case "computedview":
-        const {ComputedViewTool} =
-          this.browserRequire("devtools/client/inspector/computed/computed");
-        panel = new ComputedViewTool(this, this.panelWin);
-        break;
-      case "fontinspector":
-        const FontInspector =
-          this.browserRequire("devtools/client/inspector/fonts/fonts");
-        panel = new FontInspector(this, this.panelWin);
-        break;
-      case "layoutview":
-        const LayoutView =
-          this.browserRequire("devtools/client/inspector/layout/layout");
-        panel = new LayoutView(this, this.panelWin);
-        break;
-      case "ruleview":
-        const {RuleViewTool} = require("devtools/client/inspector/rules/rules");
-        panel = new RuleViewTool(this, this.panelWin);
-        break;
       default:
         // This is a custom panel or a non lazy-loaded one.
         return null;
     }
-
-    if (panel) {
-      this._panels.set(id, panel);
-    }
-
+    this._panels.set(id, panel);
     return panel;
   },
 
   /**
    * Build the sidebar.
    */
   async setupSidebar() {
     const sidebar = this.panelDoc.getElementById("inspector-sidebar");
@@ -915,60 +890,113 @@ Inspector.prototype = {
     if (this.is3PaneModeEnabled && defaultTab === "ruleview") {
       defaultTab = "layoutview";
     }
 
     // Append all side panels
 
     await this.addRuleView({ defaultTab });
 
-    // Inspector sidebar panels in order of appearance.
-    const sidebarPanels = [
-      {
-        id: "layoutview",
-        title: INSPECTOR_L10N.getStr("inspector.sidebar.layoutViewTitle2"),
-      },
+    // Inject a lazy loaded react tab by exposing a fake React object
+    // with a lazy defined Tab thanks to `panel` being a function
+    const layoutId = "layoutview";
+    const layoutTitle = INSPECTOR_L10N.getStr("inspector.sidebar.layoutViewTitle2");
+    this.sidebar.queueTab(
+      layoutId,
+      layoutTitle,
       {
-        id: "computedview",
-        title: INSPECTOR_L10N.getStr("inspector.sidebar.computedViewTitle"),
+        props: {
+          id: layoutId,
+          title: layoutTitle,
+        },
+        panel: () => {
+          if (!this.layoutview) {
+            const LayoutView =
+              this.browserRequire("devtools/client/inspector/layout/layout");
+            this.layoutview = new LayoutView(this, this.panelWin);
+          }
+
+          return this.layoutview.provider;
+        },
       },
-      {
-        id: "animationinspector",
-        title: INSPECTOR_L10N.getStr("inspector.sidebar.animationInspectorTitle"),
-      },
-      {
-        id: "fontinspector",
-        title: INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle"),
-      },
+      defaultTab == layoutId);
+
+    this.sidebar.queueExistingTab(
+      "computedview",
+      INSPECTOR_L10N.getStr("inspector.sidebar.computedViewTitle"),
+      defaultTab == "computedview");
+
+    const animationId = "animationinspector";
+    const animationTitle =
+      INSPECTOR_L10N.getStr("inspector.sidebar.animationInspectorTitle");
+    this.sidebar.queueTab(
+      animationId,
+      animationTitle,
       {
-        id: "changesview",
-        title: INSPECTOR_L10N.getStr("inspector.sidebar.changesViewTitle"),
+        props: {
+          id: animationId,
+          title: animationTitle,
+        },
+        panel: () => {
+          const AnimationInspector =
+            this.browserRequire("devtools/client/inspector/animation/animation");
+          this.animationinspector = new AnimationInspector(this, this.panelWin);
+          return this.animationinspector.provider;
+        },
       },
-    ];
+      defaultTab == animationId);
 
-    for (const { id, title } of sidebarPanels) {
-      // The Computed panel is not a React-based panel. We pick its element container from
-      // the DOM and wrap it in a React component (InspectorTabPanel) so it behaves like
-      // other panels when using the Inspector's tool sidebar.
-      if (id === "computedview") {
-        this.sidebar.queueExistingTab(id, title, defaultTab === id);
-      } else {
-        // When `panel` is a function, it is called when the tab should render. It is
-        // expected to return a React component to populate the tab's content area.
-        // Calling this method on-demand allows us to lazy-load the requested panel.
-        this.sidebar.queueTab(id, title, {
+    // Inject a lazy loaded react tab by exposing a fake React object
+    // with a lazy defined Tab thanks to `panel` being a function
+    const fontId = "fontinspector";
+    const fontTitle = INSPECTOR_L10N.getStr("inspector.sidebar.fontInspectorTitle");
+    this.sidebar.queueTab(
+      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);
+
+    if (Services.prefs.getBoolPref(TRACK_CHANGES_PREF)) {
+      // Inject a lazy loaded react tab by exposing a fake React object
+      // with a lazy defined Tab thanks to `panel` being a function
+      const changesId = "changesview";
+      const changesTitle = INSPECTOR_L10N.getStr("inspector.sidebar.changesViewTitle");
+      this.sidebar.queueTab(
+        changesId,
+        changesTitle,
+        {
           props: {
-            id,
-            title,
+            id: changesId,
+            title: changesTitle,
           },
           panel: () => {
-            return this.getPanel(id).provider;
+            if (!this.changesView) {
+              const ChangesView =
+                this.browserRequire("devtools/client/inspector/changes/ChangesView");
+              this.changesView = new ChangesView(this, this.panelWin);
+            }
+
+            return this.changesView.provider;
           },
-        }, defaultTab === id);
-      }
+        },
+        defaultTab == changesId);
     }
 
     this.sidebar.addAllQueuedTabs();
 
     // Persist splitter state in preferences.
     this.sidebar.on("show", this.onSidebarShown);
     this.sidebar.on("hide", this.onSidebarHidden);
     this.sidebar.on("destroy", this.onSidebarHidden);
@@ -1407,16 +1435,32 @@ Inspector.prototype = {
     this.target.off("thread-resumed", this._updateDebuggerPausedWarning);
     this._toolbox.off("select", this._updateDebuggerPausedWarning);
 
     for (const [, panel] of this._panels) {
       panel.destroy();
     }
     this._panels.clear();
 
+    if (this.layoutview) {
+      this.layoutview.destroy();
+    }
+
+    if (this.changesView) {
+      this.changesView.destroy();
+    }
+
+    if (this.fontinspector) {
+      this.fontinspector.destroy();
+    }
+
+    if (this.animationinspector) {
+      this.animationinspector.destroy();
+    }
+
     if (this._highlighters) {
       this._highlighters.destroy();
       this._highlighters = null;
     }
 
     if (this._markupFrame) {
       this._markupFrame.removeEventListener("load", this._onMarkupFrameLoad, true);
       this._markupFrame.removeEventListener("contextmenu", this._onContextMenu);
--- a/devtools/client/inspector/test/shared-head.js
+++ b/devtools/client/inspector/test/shared-head.js
@@ -128,17 +128,17 @@ function openComputedView() {
  * view is visible and ready
  */
 function openChangesView() {
   return openInspectorSidebarTab("changesview").then(data => {
     return {
       toolbox: data.toolbox,
       inspector: data.inspector,
       testActor: data.testActor,
-      view: data.inspector.getPanel("changesview"),
+      view: data.inspector.changesView,
     };
   });
 }
 
 /**
  * Open the toolbox, with the inspector tool visible, and the layout view
  * sidebar tab selected to display the box model view with properties.
  *
@@ -158,19 +158,19 @@ function openLayoutView() {
       };
     }
     mockHighlighter(data.toolbox);
 
     return {
       toolbox: data.toolbox,
       inspector: data.inspector,
       boxmodel: data.inspector.getPanel("boxmodel"),
-      gridInspector: data.inspector.getPanel("layoutview").gridInspector,
-      flexboxInspector: data.inspector.getPanel("layoutview").flexboxInspector,
-      layoutView: data.inspector.getPanel("layoutview"),
+      gridInspector: data.inspector.layoutview.gridInspector,
+      flexboxInspector: data.inspector.layoutview.flexboxInspector,
+      layoutView: data.inspector.layoutview,
       testActor: data.testActor,
     };
   });
 }
 
 /**
  * Select the rule view sidebar tab on an already opened inspector panel.
  *
@@ -198,17 +198,17 @@ function selectComputedView(inspector) {
  * Select the changes view sidebar tab on an already opened inspector panel.
  *
  * @param {InspectorPanel} inspector
  *        The opened inspector panel
  * @return {ChangesView} the changes view
  */
 function selectChangesView(inspector) {
   inspector.sidebar.select("changesview");
-  return inspector.getPanel("changesview");
+  return inspector.changesView;
 }
 
 /**
  * Select the layout view sidebar tab on an already opened inspector panel.
  *
  * @param  {InspectorPanel} inspector
  * @return {BoxModel} the box model
  */