Bug 1376774 - Restore the highlighter states on markup loaded prior to emitting the "new-root" event. r=jdescottes
authorGabriel Luong <gabriel.luong@gmail.com>
Wed, 26 Jul 2017 16:26:09 -0400
changeset 420048 aa4bd3b77a1739f00a00043ee5e90245031a5ef0
parent 420047 898cac60f7c3e2a698b789615adb68106124363b
child 420049 0a667bf8c1a72692512122bedb41be805a4a19f1
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1376774
milestone56.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 1376774 - Restore the highlighter states on markup loaded prior to emitting the "new-root" event. r=jdescottes
devtools/client/inspector/grids/test/browser.ini
devtools/client/inspector/grids/test/browser_grids_restored-after-reload.js
devtools/client/inspector/inspector.js
devtools/client/inspector/shared/highlighters-overlay.js
devtools/server/actors/highlighters/css-grid.js
--- a/devtools/client/inspector/grids/test/browser.ini
+++ b/devtools/client/inspector/grids/test/browser.ini
@@ -28,8 +28,9 @@ support-files =
 [browser_grids_grid-list-toggle-single-grid.js]
 [browser_grids_grid-outline-cannot-show-outline.js]
 [browser_grids_grid-outline-highlight-area.js]
 [browser_grids_grid-outline-highlight-cell.js]
 [browser_grids_grid-outline-selected-grid.js]
 [browser_grids_grid-outline-updates-on-grid-change.js]
 [browser_grids_highlighter-setting-rules-grid-toggle.js]
 [browser_grids_number-of-css-grids-telemetry.js]
+[browser_grids_restored-after-reload.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/grids/test/browser_grids_restored-after-reload.js
@@ -0,0 +1,87 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that the grid highlighter is re-displayed after reloading a page and the grid
+// item is highlighted.
+
+const TEST_URI = `
+  <style type='text/css'>
+    #grid {
+      display: grid;
+    }
+  </style>
+  <div id="grid">
+    <div id="cell1">cell1</div>
+    <div id="cell2">cell2</div>
+  </div>
+`;
+
+const OTHER_URI = `
+  <style type='text/css'>
+    #grid {
+      display: grid;
+    }
+  </style>
+  <div id="grid">
+    <div id="cell1">cell1</div>
+    <div id="cell2">cell2</div>
+    <div id="cell3">cell3</div>
+  </div>
+`;
+
+add_task(function* () {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let { gridInspector, inspector } = yield openLayoutView();
+  let { document: doc } = gridInspector;
+  let { highlighters, store } = inspector;
+
+  yield selectNode("#grid", inspector);
+  let gridList = doc.getElementById("grid-list");
+  let checkbox = gridList.children[0].querySelector("input");
+
+  info("Toggling ON the CSS grid highlighter from the layout panel.");
+  let onHighlighterShown = highlighters.once("grid-highlighter-shown");
+  let onCheckboxChange = waitUntilState(store, state =>
+    state.grids.length == 1 &&
+    state.grids[0].highlighted);
+  checkbox.click();
+  yield onHighlighterShown;
+  yield onCheckboxChange;
+
+  info("Checking the CSS grid highlighter is created.");
+  ok(highlighters.highlighters[HIGHLIGHTER_TYPE],
+    "CSS grid highlighter is created in the highlighters overlay.");
+  ok(highlighters.gridHighlighterShown, "CSS grid highlighter is shown.");
+
+  info("Reload the page, expect the highlighter to be displayed once again and " +
+    "grid is checked");
+  let onStateRestored = highlighters.once("grid-state-restored");
+  let onGridListRestored = waitUntilState(store, state =>
+    state.grids.length == 1 &&
+    state.grids[0].highlighted);
+  yield refreshTab(gBrowser.selectedTab);
+  let { restored } = yield onStateRestored;
+  yield onGridListRestored;
+
+  info("Check that the grid highlighter can be displayed after reloading the page");
+  ok(restored, "The highlighter state was restored");
+  ok(highlighters.gridHighlighterShown, "CSS grid highlighter is shown.");
+
+  info("Navigate to another URL, and check that the highlighter is hidden and " +
+    "grid is unchecked");
+  let otherUri = "data:text/html;charset=utf-8," + encodeURIComponent(OTHER_URI);
+  onStateRestored = highlighters.once("grid-state-restored");
+  onGridListRestored = waitUntilState(store, state =>
+    state.grids.length == 1 &&
+    !state.grids[0].highlighted);
+  yield navigateTo(inspector, otherUri);
+  ({ restored } = yield onStateRestored);
+  yield onGridListRestored;
+
+  info("Check that the grid highlighter is hidden after navigating to a different page");
+  ok(!restored, "The highlighter state was not restored");
+  ok(!highlighters.gridHighlighterShown, "CSS grid highlighter is hidden.");
+});
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -103,31 +103,33 @@ function Inspector(toolbox) {
 
   // Store the URL of the target page prior to navigation in order to ensure
   // telemetry counts in the Grid Inspector are not double counted on reload.
   this.previousURL = this.target.url;
 
   this.nodeMenuTriggerInfo = null;
 
   this._handleRejectionIfNotDestroyed = this._handleRejectionIfNotDestroyed.bind(this);
+  this._onContextMenu = this._onContextMenu.bind(this);
   this._onBeforeNavigate = this._onBeforeNavigate.bind(this);
-  this.onNewRoot = this.onNewRoot.bind(this);
-  this._onContextMenu = this._onContextMenu.bind(this);
-  this.onTextBoxContextMenu = this.onTextBoxContextMenu.bind(this);
+  this._onMarkupFrameLoad = this._onMarkupFrameLoad.bind(this);
   this._updateSearchResultsLabel = this._updateSearchResultsLabel.bind(this);
+
+  this.onDetached = this.onDetached.bind(this);
+  this.onMarkupLoaded = this.onMarkupLoaded.bind(this);
   this.onNewSelection = this.onNewSelection.bind(this);
-  this.onDetached = this.onDetached.bind(this);
+  this.onNewRoot = this.onNewRoot.bind(this);
   this.onPaneToggleButtonClicked = this.onPaneToggleButtonClicked.bind(this);
-  this._onMarkupFrameLoad = this._onMarkupFrameLoad.bind(this);
   this.onPanelWindowResize = this.onPanelWindowResize.bind(this);
-  this.onSidebarShown = this.onSidebarShown.bind(this);
+  this.onShowBoxModelHighlighterForNode =
+    this.onShowBoxModelHighlighterForNode.bind(this);
   this.onSidebarHidden = this.onSidebarHidden.bind(this);
   this.onSidebarSelect = this.onSidebarSelect.bind(this);
-  this.onShowBoxModelHighlighterForNode =
-    this.onShowBoxModelHighlighterForNode.bind(this);
+  this.onSidebarShown = this.onSidebarShown.bind(this);
+  this.onTextBoxContextMenu = this.onTextBoxContextMenu.bind(this);
 
   this._target.on("will-navigate", this._onBeforeNavigate);
   this._detectingActorFeatures = this._detectActorFeatures();
 }
 
 Inspector.prototype = {
   /**
    * open is effectively an asynchronous constructor
@@ -774,32 +776,47 @@ Inspector.prototype = {
       // been queued up.
       if (this._pendingSelection != onNodeSelected) {
         return;
       }
       this._pendingSelection = null;
       this.selection.setNodeFront(defaultNode, "navigateaway");
 
       this._initMarkup();
-      this.once("markuploaded", () => {
-        if (!this.markup) {
-          return;
-        }
-        this.markup.expandNode(this.selection.nodeFront);
-        this.emit("new-root");
-      });
+      this.once("markuploaded", this.onMarkupLoaded);
 
       // Setup the toolbar again, since its content may depend on the current document.
       this.setupToolbar();
     };
     this._pendingSelection = onNodeSelected;
     this._getDefaultNodeForSelection()
         .then(onNodeSelected, this._handleRejectionIfNotDestroyed);
   },
 
+  /**
+   * Handler for "markuploaded" event fired on a new root mutation and after the markup
+   * view is initialized. Expands the current selected node and restores the saved
+   * highlighter state.
+   */
+  onMarkupLoaded: Task.async(function* () {
+    if (!this.markup) {
+      return;
+    }
+
+    this.markup.expandNode(this.selection.nodeFront);
+
+    // Restore the highlighter states prior to emitting "new-root".
+    yield Promise.all([
+      this.highlighters.restoreGridState(),
+      this.highlighters.restoreShapeState()
+    ]);
+
+    this.emit("new-root");
+  }),
+
   _selectionCssSelector: null,
 
   /**
    * Set the currently selected node unique css selector.
    * Will store the current target url along with it to allow pre-selection at
    * reload
    */
   set selectionCssSelector(cssSelector = null) {
--- a/devtools/client/inspector/shared/highlighters-overlay.js
+++ b/devtools/client/inspector/shared/highlighters-overlay.js
@@ -47,25 +47,23 @@ function HighlightersOverlay(inspector) 
     shapes: {}
   };
 
   this.onClick = this.onClick.bind(this);
   this.onMarkupMutation = this.onMarkupMutation.bind(this);
   this.onMouseMove = this.onMouseMove.bind(this);
   this.onMouseOut = this.onMouseOut.bind(this);
   this.onWillNavigate = this.onWillNavigate.bind(this);
-  this.onNavigate = this.onNavigate.bind(this);
   this.showGridHighlighter = this.showGridHighlighter.bind(this);
   this.showShapesHighlighter = this.showShapesHighlighter.bind(this);
   this._handleRejection = this._handleRejection.bind(this);
   this._onHighlighterEvent = this._onHighlighterEvent.bind(this);
 
   // Add inspector events, not specific to a given view.
   this.inspector.on("markupmutation", this.onMarkupMutation);
-  this.inspector.target.on("navigate", this.onNavigate);
   this.inspector.target.on("will-navigate", this.onWillNavigate);
 
   EventEmitter.decorate(this);
 }
 
 HighlightersOverlay.prototype = {
   get isRuleView() {
     return this.inspector.sidebar.getCurrentTabID() == "ruleview";
@@ -382,37 +380,59 @@ HighlightersOverlay.prototype = {
     } else if (data.type === "shape-hover-off") {
       this.state.shapes.hoverPoint = null;
       this.emit("hover-shape-point", null);
     }
     this.emit("highlighter-event-handled");
   },
 
   /**
-   * Restore the saved highlighter states.
+   * Restores the saved grid highlighter state.
+   */
+  restoreGridState: Task.async(function* () {
+    try {
+      yield this.restoreState("grid", this.state.grid, this.showGridHighlighter);
+    } catch (e) {
+      this._handleRejection(e);
+    }
+  }),
+
+  /**
+   * Restores the saved shape highlighter state.
+   */
+  restoreShapeState: Task.async(function* () {
+    try {
+      yield this.restoreState("shapes", this.state.shapes, this.showShapesHighlighter);
+    } catch (e) {
+      this._handleRejection(e);
+    }
+  }),
+
+  /**
+   * Helper function called by restoreGridState and restoreShapeState.
+   * Restores the saved highlighter state for the given highlighter and their state.
+   *
    * @param {String} name
    *        The name of the highlighter to be restored
    * @param {Object} state
    *        The state of the highlighter to be restored
    * @param {Function} showFunction
    *        The function that shows the highlighter
    * @return {Promise} that resolves when the highlighter state was restored, and the
    *         expected highlighters are displayed.
    */
   restoreState: Task.async(function* (name, state, showFunction) {
     let { selector, options, url } = state;
+
     if (!selector || url !== this.inspector.target.url) {
       // Bail out if no selector was saved, or if we are on a different page.
       this.emit(`${name}-state-restored`, { restored: false });
       return;
     }
 
-    // Wait for the new root to be ready in the inspector.
-    yield this.onInspectorNewRoot;
-
     let walker = this.inspector.walker;
     let rootNode = yield walker.getRootNode();
     let nodeFront = yield walker.querySelector(rootNode, selector);
 
     if (nodeFront) {
       if (options.hoverPoint) {
         options.hoverPoint = null;
       }
@@ -709,39 +729,24 @@ HighlightersOverlay.prototype = {
         }
       } catch (e) {
         console.error(e);
       }
     }
   }),
 
   /**
-   * Restore saved highlighter state after navigate.
-   */
-  onNavigate: Task.async(function* () {
-    try {
-      yield this.restoreState("grid", this.state.grid, this.showGridHighlighter);
-      yield this.restoreState("shapes", this.state.shapes, this.showShapesHighlighter);
-    } catch (e) {
-      this._handleRejection(e);
-    }
-  }),
-
-  /**
    * Clear saved highlighter shown properties on will-navigate.
    */
   onWillNavigate: function () {
     this.geometryEditorHighlighterShown = null;
     this.gridHighlighterShown = null;
     this.hoveredHighlighterShown = null;
     this.selectorHighlighterShown = null;
     this.shapesHighlighterShown = null;
-
-    // The inspector panel should emit the new-root event when it is ready after navigate.
-    this.onInspectorNewRoot = this.inspector.once("new-root");
   },
 
   /**
    * Destroy this overlay instance, removing it from the view and destroying
    * all initialized highlighters.
    */
   destroy: function () {
     for (let type in this.highlighters) {
@@ -751,17 +756,16 @@ HighlightersOverlay.prototype = {
         }
         this.highlighters[type].finalize();
         this.highlighters[type] = null;
       }
     }
 
     // Remove inspector events.
     this.inspector.off("markupmutation", this.onMarkupMutation);
-    this.inspector.target.off("navigate", this.onNavigate);
     this.inspector.target.off("will-navigate", this.onWillNavigate);
 
     this._lastHovered = null;
 
     this.inspector = null;
     this.highlighters = null;
     this.highlighterUtils = null;
     this.supportsHighlighters = null;
--- a/devtools/server/actors/highlighters/css-grid.js
+++ b/devtools/server/actors/highlighters/css-grid.js
@@ -347,21 +347,19 @@ class CssGridHighlighter extends AutoRef
   constructor(highlighterEnv) {
     super(highlighterEnv);
 
     this.ID_CLASS_PREFIX = "css-grid-";
 
     this.markup = new CanvasFrameAnonymousContentHelper(this.highlighterEnv,
       this._buildMarkup.bind(this));
 
-    this.onNavigate = this.onNavigate.bind(this);
     this.onPageHide = this.onPageHide.bind(this);
     this.onWillNavigate = this.onWillNavigate.bind(this);
 
-    this.highlighterEnv.on("navigate", this.onNavigate);
     this.highlighterEnv.on("will-navigate", this.onWillNavigate);
 
     let { pageListenerTarget } = highlighterEnv;
     pageListenerTarget.addEventListener("pagehide", this.onPageHide);
 
     // Initialize the <canvas> position to the top left corner of the page
     this._canvasPosition = {
       x: 0,
@@ -585,17 +583,16 @@ class CssGridHighlighter extends AutoRef
       prefix: this.ID_CLASS_PREFIX
     });
 
     return container;
   }
 
   destroy() {
     let { highlighterEnv } = this;
-    highlighterEnv.off("navigate", this.onNavigate);
     highlighterEnv.off("will-navigate", this.onWillNavigate);
 
     let { pageListenerTarget } = highlighterEnv;
     if (pageListenerTarget) {
       pageListenerTarget.removeEventListener("pagehide", this.onPageHide);
     }
 
     this.markup.destroy();
@@ -672,33 +669,32 @@ class CssGridHighlighter extends AutoRef
     let pattern = ctx.createPattern(canvas, "repeat");
 
     gridPatternMap.set(dimension, pattern);
     gCachedGridPattern.set(devicePixelRatio, gridPatternMap);
 
     return pattern;
   }
 
-  /**
-   * Called when the page navigates. Used to clear the cached gap patterns and avoid
-   * using DeadWrapper objects as gap patterns the next time.
-   */
-  onNavigate() {
-    this._clearCache();
-  }
-
   onPageHide({ target }) {
     // If a page hide event is triggered for current window's highlighter, hide the
     // highlighter.
     if (target.defaultView === this.win) {
       this.hide();
     }
   }
 
+  /**
+   * Called when the page will-navigate. Used to hide the grid highlighter and clear
+   * the cached gap patterns and avoid using DeadWrapper obejcts as gap patterns the
+   * next time.
+   */
   onWillNavigate({ isTopLevel }) {
+    this._clearCache();
+
     if (isTopLevel) {
       this.hide();
     }
   }
 
   _show() {
     if (Services.prefs.getBoolPref(CSS_GRID_ENABLED_PREF) && !this.isGrid()) {
       this.hide();