Bug 1750182 - [devtools] Clear FontHighlighter reference on will-navigate. r=jdescottes.

This will cause a new highlighter to be created when onToggleFontHighlight
is called next.
Ideally, we wouldn't have a reference to the highlighter as this is probably
not working for Fission, and we would use the HighlightersOverlay like the
other inspector panel, but that probably requires its own bug.
A test is added to ensure this works as expected.

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

devtools/client/inspector/fonts/fonts.js

@@ -110,6 +110,7 @@ class FontInspector {
     this.updateFontVariationSettings = this.updateFontVariationSettings.bind(
       this
     );
+    this.onResourceAvailable = this.onResourceAvailable.bind(this);
 
     this.init();
   }
@@ -176,6 +177,11 @@ class FontInspector {
     // @see ToolSidebar.onSidebarTabSelected()
     this.inspector.sidebar.on("fontinspector-selected", this.onNewNode);
 
+    this.inspector.toolbox.resourceCommand.watchResources(
+      [this.inspector.toolbox.resourceCommand.TYPES.DOCUMENT_EVENT],
+      { onAvailable: this.onResourceAvailable }
+    );
+
     // Listen for theme changes as the color of the previews depend on the theme
     gDevTools.on("theme-switched", this.onThemeChanged);
   }
@@ -316,6 +322,12 @@ class FontInspector {
     this.ruleView.off("property-value-updated", this.onRulePropertyUpdated);
     gDevTools.off("theme-switched", this.onThemeChanged);
 
+    this.inspector.toolbox.resourceCommand.unwatchResources(
+      [this.inspector.toolbox.resourceCommand.TYPES.DOCUMENT_EVENT],
+      { onAvailable: this.onResourceAvailable }
+    );
+
+    this.fontsHighlighter = null;
     this.document = null;
     this.inspector = null;
     this.node = null;
@@ -328,6 +340,21 @@ class FontInspector {
     this.writers = null;
   }
 
+  onResourceAvailable(resources) {
+    for (const resource of resources) {
+      if (
+        resource.resourceType ===
+          this.inspector.commands.resourceCommand.TYPES.DOCUMENT_EVENT &&
+        resource.name === "will-navigate" &&
+        resource.targetFront.isTopLevel
+      ) {
+        // Reset the fontsHighlighter so the next call to `onToggleFontHighlight` will
+        // re-create it from the inspector front tied to the new document.
+        this.fontsHighlighter = null;
+      }
+    }
+  }
+
   /**
    * Get all expected CSS font properties and values from the node's matching rules and
    * fallback to computed style. Skip CSS Custom Properties, `calc()` and keyword values.
@@ -867,9 +894,8 @@ class FontInspector {
           "FontsHighlighter"
         );
       } catch (e) {
-        // When connecting to an older server or when debugging a XUL document, the
-        // FontsHighlighter won't be available. Silently fail here and prevent any future
-        // calls to the function.
+        // the FontsHighlighter won't be available when debugging a XUL document.
+        // Silently fail here and prevent any future calls to the function.
         this.onToggleFontHighlight = () => {};
         return;
       }

devtools/client/inspector/fonts/test/browser_fontinspector_reveal-in-page.js

@@ -11,11 +11,15 @@ add_task(async function() {
   // events simulation will fail.
   await pushPref("devtools.toolbox.footer.height", 500);
 
-  const { tab, view } = await openFontInspectorForURL(TEST_URI);
-  const viewDoc = view.document;
+  const { view } = await openFontInspectorForURL(TEST_URI);
+  await testFontHighlighting(view);
 
-  const fontEls = getUsedFontsEls(viewDoc);
+  info("Check that highlighting still works after reloading the page");
+  await reloadBrowser();
+  await testFontHighlighting(view);
+});
 
+async function testFontHighlighting(view) {
   // The number of window selection change events we expect to get as we hover over each
   // font in the list. Waiting for those events is how we know that text-runs were
   // highlighted in the page.
@@ -23,15 +27,26 @@ add_task(async function() {
   // 1 selection range object, depending on the number of text-runs found.
   const expectedSelectionChangeEvents = [2, 2, 2, 1, 1];
 
+  const viewDoc = view.document;
+
+  // Wait for the view to have all the expected used fonts.
+  const fontEls = await waitFor(() => {
+    const els = getUsedFontsEls(viewDoc);
+    if (els.length !== expectedSelectionChangeEvents.length) {
+      return false;
+    }
+
+    return els;
+  });
+
   for (let i = 0; i < fontEls.length; i++) {
-    info(`Mousing over and out of font number ${i} in the list`);
+    info(
+      `Mousing over and out of font number ${i} ("${fontEls[i].textContent}") in the list`
+    );
 
     // Simulating a mouse over event on the font name and expecting a selectionchange.
     const nameEl = fontEls[i];
-    let onEvents = waitForNSelectionEvents(
-      tab,
-      expectedSelectionChangeEvents[i]
-    );
+    let onEvents = waitForNSelectionEvents(expectedSelectionChangeEvents[i]);
     EventUtils.synthesizeMouse(
       nameEl,
       2,
@@ -48,7 +63,7 @@ add_task(async function() {
 
     // Simulating a mouse out event on the font name and expecting a selectionchange.
     const otherEl = viewDoc.querySelector("body");
-    onEvents = waitForNSelectionEvents(tab, 1);
+    onEvents = waitForNSelectionEvents(1);
     EventUtils.synthesizeMouse(
       otherEl,
       2,
@@ -60,24 +75,26 @@ add_task(async function() {
 
     ok(true, "1 selectionchange events detected on mouseout");
   }
-});
-
-async function waitForNSelectionEvents(tab, numberOfTimes) {
-  await SpecialPowers.spawn(tab.linkedBrowser, [numberOfTimes], async function(
-    n
-  ) {
-    const win = content.wrappedJSObject;
-
-    await new Promise(resolve => {
-      let received = 0;
-      win.document.addEventListener("selectionchange", function listen() {
-        received++;
-
-        if (received === n) {
-          win.document.removeEventListener("selectionchange", listen);
-          resolve();
-        }
-      });
-    });
-  });
+}
+
+async function waitForNSelectionEvents(numberOfTimes) {
+  await SpecialPowers.spawn(
+    gBrowser.selectedBrowser,
+    [numberOfTimes],
+    async function(n) {
+      const win = content.wrappedJSObject;
+
+      await new Promise(resolve => {
+        let received = 0;
+        win.document.addEventListener("selectionchange", function listen() {
+          received++;
+
+          if (received === n) {
+            win.document.removeEventListener("selectionchange", listen);
+            resolve();
+          }
+        });
+      });
+    }
+  );
 }