Bug 1358479 - Check if a grid fragment is valid before highlighting it; r=jdescottes

Grid fragments returned by the chrome-only el.getGridFragments() API
may sometimes in fact be empty. When they are, the returned object still
looks like a fragment, but has either no rows or no columns.
So this commit simply adds a quick check on this in the css grid highlighter
before attempting to highlight a grid fragment.

MozReview-Commit-ID: GjsGu9hpU5G

--HG--
extra : rebase_source : 355073b26035ce02db11f51497c74f7746d35a77

devtools/client/inspector/test/browser.ini

@@ -72,6 +72,7 @@ skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keybo
 [browser_inspector_highlighter-cancel.js]
 [browser_inspector_highlighter-comments.js]
 [browser_inspector_highlighter-cssgrid_01.js]
+[browser_inspector_highlighter-cssgrid_02.js]
 [browser_inspector_highlighter-csstransform_01.js]
 [browser_inspector_highlighter-csstransform_02.js]
 [browser_inspector_highlighter-embed.js]

devtools/client/inspector/test/browser_inspector_highlighter-cssgrid_02.js

@@ -0,0 +1,44 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+// Test that grid layouts without items don't cause grid highlighter errors.
+
+const TEST_URL = `
+  <style type='text/css'>
+    .grid {
+      display: grid;
+      grid-template-columns: 20px 20px;
+      grid-gap: 15px;
+    }
+  </style>
+  <div class="grid"></div>
+`;
+
+const HIGHLIGHTER_TYPE = "CssGridHighlighter";
+
+add_task(function* () {
+  let {inspector, testActor} = yield openInspectorForURL(
+    "data:text/html;charset=utf-8," + encodeURIComponent(TEST_URL));
+  let front = inspector.inspector;
+  let highlighter = yield front.getHighlighterByType(HIGHLIGHTER_TYPE);
+
+  info("Try to show the highlighter on the grid container");
+  let node = yield getNodeFront(".grid", inspector);
+  yield highlighter.show(node);
+
+  let hidden = yield testActor.getHighlighterNodeAttribute(
+    "css-grid-canvas", "hidden", highlighter);
+  ok(!hidden, "The highlighter is visible");
+
+  info("Hiding the highlighter");
+  yield highlighter.hide();
+
+  hidden = yield testActor.getHighlighterNodeAttribute(
+    "css-grid-canvas", "hidden", highlighter);
+  ok(hidden, "The highlighter is hidden");
+
+  yield highlighter.finalize();
+});

devtools/server/actors/highlighters/css-grid.js

@@ -695,6 +695,18 @@ CssGridHighlighter.prototype = extend(AutoRefreshHighlighter.prototype, {
     return this.currentNode.getGridFragments().length > 0;
   },
 
+  /**
+   * Is a given grid fragment valid? i.e. does it actually have tracks? In some cases, we
+   * may have a fragment that defines column tracks but doesn't have any rows (or vice
+   * versa). In which case we do not want to draw anything for that fragment.
+   *
+   * @param {Object} fragment
+   * @return {Boolean}
+   */
+  isValidFragment(fragment) {
+    return fragment.cols.tracks.length && fragment.rows.tracks.length;
+  },
+
   /**
    * The AutoRefreshHighlighter's _hasMoved method returns true only if the
    * element's quads have changed. Override it so it also returns true if the
@@ -740,8 +752,7 @@ CssGridHighlighter.prototype = extend(AutoRefreshHighlighter.prototype, {
 
     // Start drawing the grid fragments.
     for (let i = 0; i < this.gridData.length; i++) {
-      let fragment = this.gridData[i];
+      this.renderFragment(this.gridData[i]);
-      this.renderFragment(fragment);
     }
 
     // Display the grid area highlights if needed.
@@ -1030,6 +1041,10 @@ CssGridHighlighter.prototype = extend(AutoRefreshHighlighter.prototype, {
   },
 
   renderFragment(fragment) {
+    if (!this.isValidFragment(fragment)) {
+      return;
+    }
+
     this.renderLines(fragment.cols, COLUMNS, "left", "top", "height",
                      this.getFirstRowLinePos(fragment),
                      this.getLastRowLinePos(fragment));