Bug 1572654 - [devtools] Restore active flexbox highlighter on page refresh r=jdescottes

Depends on D96730

This patch refactors slightly the way flexbox highlighters are restored when the page is refreshed while a highlighter is active.
To account for Fission, we store the `browsingContextID` of the target in addition to the node's unique selector, in order to match again the node after refresh and show a flexbox highlighter for it.

The events emitted are renamed to more generic versions "highlighter-restored" and "highlighter-discarded". They contain the highlighter type in the event data payload. Tests are updated accordingly.

Also made slight changes to the grid highlighter restore logic and corresponding tests in order to use the new generic events and test helpers. The bulk of the refactoring for grid highlighters will happen in [Bug 1572652](https://bugzilla.mozilla.org/show_bug.cgi?id=1572652)

Differential Revision: https://phabricator.services.mozilla.com/D97660

devtools/client/inspector/grids/test/browser_grids_restored-after-reload.js

@@ -36,6 +36,11 @@ add_task(async function() {
   const { gridInspector, inspector } = await openLayoutView();
   const { document: doc } = gridInspector;
   const { highlighters, store } = inspector;
+  const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.GRID;
+  const {
+    waitForHighlighterTypeRestored,
+    waitForHighlighterTypeDiscarded,
+  } = getHighlighterTestHelpers(inspector);
 
   await selectNode("#grid", inspector);
   const gridList = doc.getElementById("grid-list");
@@ -65,7 +70,7 @@ add_task(async function() {
     "Reload the page, expect the highlighter to be displayed once again and " +
       "grid is checked"
   );
-  let onStateRestored = highlighters.once("grid-state-restored");
+  const onRestored = waitForHighlighterTypeRestored(HIGHLIGHTER_TYPE);
   let onGridListRestored = waitUntilState(
     store,
     state => state.grids.length == 1 && state.grids[0].highlighted
@@ -76,13 +81,12 @@ add_task(async function() {
   info("Wait for inspector to be reloaded after page reload");
   await onReloaded;
 
-  let { restored } = await onStateRestored;
+  await onRestored;
   await onGridListRestored;
 
   info(
     "Check that the grid highlighter can be displayed after reloading the page"
   );
-  ok(restored, "The highlighter state was restored");
   is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown.");
   is(
     highlighters.state.grids.size,
@@ -96,19 +100,18 @@ add_task(async function() {
   );
   const otherUri =
     "data:text/html;charset=utf-8," + encodeURIComponent(OTHER_URI);
-  onStateRestored = highlighters.once("grid-state-restored");
+  const onDiscarded = waitForHighlighterTypeDiscarded(HIGHLIGHTER_TYPE);
   onGridListRestored = waitUntilState(
     store,
     state => state.grids.length == 1 && !state.grids[0].highlighted
   );
   await navigateTo(otherUri);
-  ({ restored } = await onStateRestored);
+  await onDiscarded;
   await 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.gridHighlighters.size, "CSS grid highlighter is hidden.");
   ok(!highlighters.state.grids.size, "No grids to be restored on page reload.");
 });

devtools/client/inspector/grids/test/browser_grids_restored-multiple-grids-after-reload.js

@@ -120,7 +120,7 @@ add_task(async function() {
   );
   const onStateRestored = waitForNEvents(
     highlighters,
-    "grid-state-restored",
+    "highlighter-restored",
     3
   );
   const onGridListRestored = waitUntilState(
@@ -137,13 +137,12 @@ add_task(async function() {
       state.grids[3].disabled
   );
   await refreshTab();
-  const { restored } = await onStateRestored;
+  await onStateRestored;
   await onGridListRestored;
 
   info(
     "Check that the grid highlighters can be displayed after reloading the page"
   );
-  ok(restored, "The highlighter state was restored");
   is(
     highlighters.gridHighlighters.size,
     3,

devtools/client/inspector/rules/test/browser_rules_flexbox-highlighter-restored-after-reload.js

@@ -30,8 +30,10 @@ add_task(async function() {
   const { inspector, view } = await openRuleView();
   const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.FLEXBOX;
   const {
-    getNodeForActiveHighlighter,
+    getActiveHighlighter,
     waitForHighlighterTypeShown,
+    waitForHighlighterTypeRestored,
+    waitForHighlighterTypeDiscarded,
   } = getHighlighterTestHelpers(inspector);
 
   await selectNode("#flex", inspector);
@@ -44,40 +46,24 @@ add_task(async function() {
   const onHighlighterShown = waitForHighlighterTypeShown(HIGHLIGHTER_TYPE);
   flexboxToggle.click();
   await onHighlighterShown;
-
-  ok(
-    getNodeForActiveHighlighter(HIGHLIGHTER_TYPE),
-    "Flexbox highlighter is shown."
-  );
+  ok(getActiveHighlighter(HIGHLIGHTER_TYPE), "Flexbox highlighter is shown.");
 
   info("Reload the page, expect the highlighter to be displayed once again");
-  let onStateRestored = inspector.highlighters.once("flexbox-state-restored");
-
+  const onRestored = waitForHighlighterTypeRestored(HIGHLIGHTER_TYPE);
   const onReloaded = inspector.once("reloaded");
   await refreshTab();
   info("Wait for inspector to be reloaded after page reload");
   await onReloaded;
-
-  let { restored } = await onStateRestored;
-  ok(restored, "The highlighter state was restored");
-
-  info(
-    "Check that the flexbox highlighter can be displayed after reloading the page"
-  );
-  ok(
-    getNodeForActiveHighlighter(HIGHLIGHTER_TYPE),
-    "Flexbox highlighter is shown."
-  );
+  info("Wait for the highlighter to be restored");
+  await onRestored;
+  ok(getActiveHighlighter(HIGHLIGHTER_TYPE), "Flexbox highlighter restored.");
 
   info("Navigate to another URL, and check that the highlighter is hidden");
   const otherUri =
     "data:text/html;charset=utf-8," + encodeURIComponent(OTHER_URI);
-  onStateRestored = inspector.highlighters.once("flexbox-state-restored");
+  const onDiscarded = waitForHighlighterTypeDiscarded(HIGHLIGHTER_TYPE);
   await navigateTo(otherUri);
-  ({ restored } = await onStateRestored);
-  ok(!restored, "The highlighter state was not restored");
-  ok(
-    !getNodeForActiveHighlighter(HIGHLIGHTER_TYPE),
-    "Flexbox highlighter is hidden."
-  );
+  info("Expect the highlighter not to be restored");
+  await onDiscarded;
+  ok(!getActiveHighlighter(HIGHLIGHTER_TYPE), "Flexbox highlighter not shown.");
 });

devtools/client/inspector/rules/test/browser_rules_grid-highlighter-restored-after-reload.js

@@ -36,6 +36,11 @@ add_task(async function() {
   info("Check that the grid highlighter can be displayed");
   const { inspector, view } = await openRuleView();
   const { highlighters } = view;
+  const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.GRID;
+  const {
+    waitForHighlighterTypeRestored,
+    waitForHighlighterTypeDiscarded,
+  } = getHighlighterTestHelpers(inspector);
 
   await selectNode("#grid", inspector);
   const container = getRuleViewProperty(view, "#grid", "display").valueSpan;
@@ -49,27 +54,29 @@ add_task(async function() {
   is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown.");
 
   info("Reload the page, expect the highlighter to be displayed once again");
-  let onStateRestored = highlighters.once("grid-state-restored");
+  const onRestored = waitForHighlighterTypeRestored(HIGHLIGHTER_TYPE);
 
   const onReloaded = inspector.once("reloaded");
   await refreshTab();
   info("Wait for inspector to be reloaded after page reload");
   await onReloaded;
 
-  let { restored } = await onStateRestored;
-  ok(restored, "The highlighter state was restored");
-
-  info(
-    "Check that the grid highlighter can be displayed after reloading the page"
+  await onRestored;
+  is(
+    highlighters.gridHighlighters.size,
+    1,
+    "CSS grid highlighter was restored."
   );
-  is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown.");
 
   info("Navigate to another URL, and check that the highlighter is hidden");
   const otherUri =
     "data:text/html;charset=utf-8," + encodeURIComponent(OTHER_URI);
-  onStateRestored = highlighters.once("grid-state-restored");
+  const onDiscarded = waitForHighlighterTypeDiscarded(HIGHLIGHTER_TYPE);
   await navigateTo(otherUri);
-  ({ restored } = await onStateRestored);
-  ok(!restored, "The highlighter state was not restored");
-  ok(!highlighters.gridHighlighters.size, "CSS grid highlighter is hidden.");
+  await onDiscarded;
+  is(
+    highlighters.gridHighlighters.size,
+    0,
+    "CSS grid highlighter was not restored."
+  );
 });

devtools/client/inspector/shared/highlighters-overlay.js

@@ -114,6 +114,9 @@ class HighlightersOverlay {
     // until it doesn't complete, this map will be populated with the requested type and
     // a unique symbol identifying that request. Once completed, the entry is removed.
     this._pendingHighlighters = new Map();
+    // Map of highlighter types to objects with metadata used to restore active
+    // highlighters after a page reload.
+    this._restorableHighlighters = new Map();
     // Collection of instantiated highlighter actors like FlexboxHighlighter,
     // ShapesHighlighter and GeometryEditorHighlighter.
     this.highlighters = {};
@@ -139,11 +142,16 @@ class HighlightersOverlay {
     // changes to properties in the Rule view.
     this.editors = {};
 
-    // Saved state to be restore on page navigation.
+    // Highlighter state.
     this.state = {
-      flexbox: {},
       // Map of grid container NodeFront to the their stored grid options
+      // Used to restore grid highlighters on reload (should be migrated to
+      // _restorableHighlighters in Bug 1572652).
       grids: new Map(),
+      // Shape Path Editor highlighter options.
+      // Used as a cache for the latest configuration when showing the highlighter.
+      // It is reused and augmented when hovering coordinates in the Rules view which
+      // mark the corresponding points in the highlighter overlay.
       shapes: {},
     };
 
@@ -181,6 +189,10 @@ class HighlightersOverlay {
       this.showHighlighterTypeForNode.bind(this),
       () => this.destroyed
     );
+    this.restoreState = safeAsyncMethod(
+      this.restoreState.bind(this),
+      () => this.destroyed
+    );
 
     // Add inspector events, not specific to a given view.
     this.inspector.on("markupmutation", this.onMarkupMutation);
@@ -214,6 +226,7 @@ class HighlightersOverlay {
    */
   async _afterShowHighlighterTypeForNode(type, nodeFront, options) {
     // Log telemetry for showing the flexbox highlighter.
+    // Set metadata necessary to restore the active highlighter upon page refresh.
     if (type === TYPES.FLEXBOX) {
       this.telemetry.toolOpened(
         "FLEXBOX_HIGHLIGHTER",
@@ -230,6 +243,16 @@ class HighlightersOverlay {
       if (scalars[options.trigger]) {
         this.telemetry.scalarAdd(scalars[options.trigger], 1);
       }
+
+      const { url } = this.target;
+      const selectors = [...this.inspector.selectionCssSelectors];
+
+      this._restorableHighlighters.set(type, {
+        options,
+        selectors,
+        type,
+        url,
+      });
     }
   }
 
@@ -310,6 +333,9 @@ class HighlightersOverlay {
    * @return {Promise}
    */
   async _beforeHideHighlighterType(type) {
+    // Remove any metadata used to restore this highlighter type on page refresh.
+    this._restorableHighlighters.delete(type);
+
     // Log telemetry for hiding the flexbox highlighter.
     if (type === TYPES.FLEXBOX) {
       this.telemetry.toolClosed(
@@ -776,28 +802,13 @@ class HighlightersOverlay {
       trigger,
       color,
     });
-
-    try {
-      // Save flexbox highlighter state.
-      const { url } = this.target;
-      const selector = await node.getUniqueSelector();
-      this.state.flexbox = { selector, options, url };
-    } catch (e) {
-      this._handleRejection(e);
-    }
   }
 
   /**
-   * Hide the flexbox highlighter for the given flexbox container element.
-   *
-   * @param  {NodeFront} node
-   *         The NodeFront of the flexbox container element to unhighlight.
+   * Hide the flexbox highlighter if any instance is visible.
    */
-  async hideFlexboxHighlighter(node) {
+  async hideFlexboxHighlighter() {
     await this.hideHighlighterType(TYPES.FLEXBOX);
-
-    // Erase flexbox highlighter state.
-    this.state.flexbox = null;
   }
 
   /**
@@ -923,8 +934,8 @@ class HighlightersOverlay {
     try {
       // Save grid highlighter state.
       const { url } = this.target;
-      const selector = await node.getUniqueSelector();
-      this.state.grids.set(node, { selector, options, url });
+      const selectors = await node.getAllSelectors();
+      this.state.grids.set(node, { selectors, options, url });
 
       // Emit the NodeFront of the grid container element that the grid highlighter was
       // shown for, and its options for testing the highlighter setting options.
@@ -1119,15 +1130,13 @@ class HighlightersOverlay {
    * Restores the saved flexbox highlighter state.
    */
   async restoreFlexboxState() {
-    try {
-      await this.restoreState(
-        "flexbox",
-        this.state.flexbox,
-        this.showFlexboxHighlighter
-      );
-    } catch (e) {
-      this._handleRejection(e);
+    const state = this._restorableHighlighters.get(TYPES.FLEXBOX);
+    if (!state) {
+      return;
     }
+
+    this._restorableHighlighters.delete(TYPES.FLEXBOX);
+    await this.restoreState(TYPES.FLEXBOX, state, this.showFlexboxHighlighter);
   }
 
   /**
@@ -1142,7 +1151,11 @@ class HighlightersOverlay {
 
     try {
       for (const gridState of values) {
-        await this.restoreState("grid", gridState, this.showGridHighlighter);
+        await this.restoreState(
+          TYPES.GRID,
+          gridState,
+          this.showGridHighlighter
+        );
       }
     } catch (e) {
       this._handleRejection(e);
@@ -1154,36 +1167,42 @@ class HighlightersOverlay {
    * Restores the saved highlighter state for the given highlighter
    * and their state.
    *
-   * @param  {String} name
-   *         The name of the highlighter to be restored
+   * @param  {String} type
+   *         Highlighter type to be restored.
    * @param  {Object} state
-   *         The state of the highlighter to be restored
+   *         Object containing the metadata used to restore the highlighter.
+   *         {Array} state.selectors
+   *         Array of CSS selector which identifies the node to be highlighted.
+   *         If the node is in the top-level document, the array contains just one item.
+   *         Otherwise, if the node is nested within a stack of iframes, each iframe is
+   *         identified by its unique selector; the last item in the array identifies
+   *         the target node within its host iframe document.
+   *         {Object} state.options
+   *         Configuration options to use when showing the highlighter.
+   *         {String} state.url
+   *         URL of the top-level target when the metadata was stored. Used to identify
+   *         if there was a page refresh or a navigation away to a different page.
    * @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.
+   * @return {Promise} that resolves when the highlighter was restored and shown.
    */
-  async restoreState(name, state, showFunction) {
-    const { selector, options, url } = state;
+  async restoreState(type, state, showFunction) {
+    const { selectors = [], options, url } = state;
 
-    if (!selector || url !== this.target.url) {
+    if (!selectors.length || url !== this.target.url) {
       // Bail out if no selector was saved, or if we are on a different page.
-      this.emit(`${name}-state-restored`, { restored: false });
+      this.emit(`highlighter-discarded`, { type });
       return;
     }
 
-    const rootNode = await this.walker.getRootNode();
-    const nodeFront = await this.walker.querySelector(rootNode, selector);
+    const inspectorFront = await this.target.getFront("inspector");
+    const nodeFront = await inspectorFront.walker.findNodeFront(selectors);
 
     if (nodeFront) {
-      if (options.hoverPoint) {
-        options.hoverPoint = null;
-      }
-
       await showFunction(nodeFront, options);
-      this.emit(`${name}-state-restored`, { restored: true });
+      this.emit(`highlighter-restored`, { type });
     } else {
-      this.emit(`${name}-state-restored`, { restored: false });
+      this.emit(`highlighter-discarded`, { type });
     }
   }
 

devtools/client/inspector/test/head.js

@@ -826,6 +826,12 @@ function getHighlighterTestHelpers(inspector) {
     waitForHighlighterTypeHidden(type) {
       return _waitForHighlighterTypeEvent(type, "highlighter-hidden");
     },
+    waitForHighlighterTypeRestored(type) {
+      return _waitForHighlighterTypeEvent(type, "highlighter-restored");
+    },
+    waitForHighlighterTypeDiscarded(type) {
+      return _waitForHighlighterTypeEvent(type, "highlighter-discarded");
+    },
   };
 }