From db47bce4aa77d563e159b1d2a913c9febec99bfc Mon Sep 17 00:00:00 2001
From: Brad Werth <bwerth@mozilla.com>
Date: Thu, 23 Feb 2023 01:07:34 +0000
Subject: [PATCH] Bug 1815140 Part 2: Make grid highlighter hasMoved directly
 compare the grid fragment objects. r=devtools-reviewers,jdescottes

The Grid objects will be regenerated whenever the grid is reflowed, which
is nearly the same to the old check of comparing the contents of the
grid structure. This approach is prone to false positives, which is
explained in a new comment.

Differential Revision: https://phabricator.services.mozilla.com/D169725
---
 .../server/actors/highlighters/css-grid.js    | 19 ++++++++++++-------
 .../server/actors/utils/css-grid-utils.js     | 15 ---------------
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/devtools/server/actors/highlighters/css-grid.js b/devtools/server/actors/highlighters/css-grid.js
index bc332d5887ae..a3b4ebbaf40c 100644
--- a/devtools/server/actors/highlighters/css-grid.js
+++ b/devtools/server/actors/highlighters/css-grid.js
@@ -33,9 +33,6 @@ const {
   getWindowDimensions,
   setIgnoreLayoutChanges,
 } = require("resource://devtools/shared/layout/utils.js");
-const {
-  stringifyGridFragments,
-} = require("resource://devtools/server/actors/utils/css-grid-utils.js");
 loader.lazyGetter(this, "HighlightersBundle", () => {
   return new Localization(["devtools/shared/highlighters.ftl"], true);
 });
@@ -626,16 +623,24 @@ class CssGridHighlighter extends AutoRefreshHighlighter {
    * The AutoRefreshHighlighter's _hasMoved method returns true only if the
    * element's quads have changed. Override it so it also returns true if the
    * element's grid has changed (which can happen when you change the
-   * grid-template-* CSS properties with the highlighter displayed).
+   * grid-template-* CSS properties with the highlighter displayed). This
+   * check is prone to false positives, because it does a direct object
+   * comparison of the first grid fragment structure. This structure is
+   * generated by the first call to getGridFragments, and on any subsequent
+   * calls where a reflow is needed. Since a reflow is needed when the CSS
+   * changes, this will correctly detect that the grid structure has changed.
+   * However, it's possible that the reflow could generate a novel grid
+   * fragment object containing information that is unchanged -- a false
+   * positive.
    */
   _hasMoved() {
     const hasMoved = AutoRefreshHighlighter.prototype._hasMoved.call(this);
 
-    const oldGridData = stringifyGridFragments(this.gridData);
+    const oldFirstGridFragment = this.gridData?.[0];
     this.gridData = this.currentNode.getGridFragments();
-    const newGridData = stringifyGridFragments(this.gridData);
+    const newFirstGridFragment = this.gridData[0];
 
-    return hasMoved || oldGridData !== newGridData;
+    return hasMoved || oldFirstGridFragment !== newFirstGridFragment;
   }
 
   /**
diff --git a/devtools/server/actors/utils/css-grid-utils.js b/devtools/server/actors/utils/css-grid-utils.js
index 59b8898b8cd3..9631dcd80062 100644
--- a/devtools/server/actors/utils/css-grid-utils.js
+++ b/devtools/server/actors/utils/css-grid-utils.js
@@ -19,20 +19,6 @@ function getStringifiableFragments(fragments = []) {
   return fragments.map(getStringifiableFragment);
 }
 
-/**
- * Returns a string representation of the CSS Grid data as returned by
- * node.getGridFragments. This is useful to compare grid state at each update and redraw
- * the highlighter if needed. It also seralizes the grid fragment data so it can be used
- * by protocol.js.
- *
- * @param  {Object} fragments
- *         Grid fragment object.
- * @return {String} representation of the CSS grid fragment data.
- */
-function stringifyGridFragments(fragments) {
-  return JSON.stringify(getStringifiableFragments(fragments));
-}
-
 function getStringifiableFragment(fragment) {
   return {
     areas: getStringifiableAreas(fragment.areas),
@@ -72,4 +58,3 @@ function getStringifiableTrack({ breadth, start, state, type }) {
 }
 
 exports.getStringifiableFragments = getStringifiableFragments;
-exports.stringifyGridFragments = stringifyGridFragments;