Bug 1373483 - Add telemetry for the grid inspector. r=pbro. data-r=bsmedberg

devtools/client/inspector/boxmodel/box-model.js

@@ -347,10 +347,9 @@ BoxModel.prototype = {
   },
 
   /**
-   * Handler for the inspector sidebar select event. Starts listening for
-   * "grid-layout-changed" if the layout panel is visible. Otherwise, stop
-   * listening for grid layout changes. Finally, refresh the layout view if
-   * it is visible.
+   * Handler for the inspector sidebar select event. Starts tracking reflows if the
+   * layout panel is visible. Otherwise, stop tracking reflows. Finally, refresh the box
+   * model view if it is visible.
    */
   onSidebarSelect() {
     if (!this.isPanelVisible()) {

devtools/client/inspector/grids/grid-inspector.js

@@ -20,6 +20,8 @@ const {
   updateShowInfiniteLines,
 } = require("./actions/highlighter-settings");
 
+const CSS_GRID_COUNT_HISTOGRAM_ID = "DEVTOOLS_NUMBER_OF_CSS_GRIDS_IN_A_PAGE";
+
 const SHOW_GRID_AREAS = "devtools.gridinspector.showGridAreas";
 const SHOW_GRID_LINE_NUMBERS = "devtools.gridinspector.showGridLineNumbers";
 const SHOW_INFINITE_LINES_PREF = "devtools.gridinspector.showInfiniteLines";
@@ -43,6 +45,7 @@ function GridInspector(inspector, window) {
   this.highlighters = inspector.highlighters;
   this.inspector = inspector;
   this.store = inspector.store;
+  this.telemetry = inspector.telemetry;
   this.walker = this.inspector.walker;
 
   this.getSwatchColorPickerTooltip = this.getSwatchColorPickerTooltip.bind(this);
@@ -91,6 +94,7 @@ GridInspector.prototype = {
     this.highlighters.on("grid-highlighter-hidden", this.onHighlighterChange);
     this.highlighters.on("grid-highlighter-shown", this.onHighlighterChange);
     this.inspector.sidebar.on("select", this.onSidebarSelect);
+    this.inspector.target.on("navigate", this.onGridLayoutChange);
 
     this.onSidebarSelect();
   }),
@@ -103,7 +107,7 @@ GridInspector.prototype = {
     this.highlighters.off("grid-highlighter-hidden", this.onHighlighterChange);
     this.highlighters.off("grid-highlighter-shown", this.onHighlighterChange);
     this.inspector.sidebar.off("select", this.onSidebarSelect);
-    this.layoutInspector.off("grid-layout-changed", this.onGridLayoutChange);
+    this.inspector.target.off("navigate", this.onGridLayoutChange);
 
     this.inspector.reflowTracker.untrackReflows(this, this.onReflow);
 
@@ -240,31 +244,34 @@ GridInspector.prototype = {
     this.lastHighlighterNode = node;
     this.lastHighlighterState = node !== this.highlighters.gridHighlighterShown;
 
-    this.highlighters.toggleGridHighlighter(node, settings);
+    this.highlighters.toggleGridHighlighter(node, settings, "grid");
   },
 
   /**
    * Updates the grid panel by dispatching the new grid data. This is called when the
    * layout view becomes visible or the view needs to be updated with new grid data.
-   *
-   * @param  {Array|null} gridFronts
-   *         Optional array of all GridFront in the current page.
    */
-  updateGridPanel: Task.async(function* (gridFronts) {
+  updateGridPanel: Task.async(function* () {
     // Stop refreshing if the inspector or store is already destroyed.
     if (!this.inspector || !this.store) {
       return;
     }
 
     // Get all the GridFront from the server if no gridFronts were provided.
-    if (!gridFronts) {
-      try {
-        gridFronts = yield this.layoutInspector.getAllGrids(this.walker.rootNode);
-      } catch (e) {
-        // This call might fail if called asynchrously after the toolbox is finished
-        // closing.
-        return;
-      }
+    let gridFronts;
+    try {
+      gridFronts = yield this.layoutInspector.getAllGrids(this.walker.rootNode);
+    } catch (e) {
+      // This call might fail if called asynchrously after the toolbox is finished
+      // closing.
+      return;
+    }
+
+    // Log how many CSS Grid elements DevTools sees.
+    if (gridFronts.length > 0 &&
+        this.inspector.target.url != this.inspector.previousURL) {
+      this.telemetry.log(CSS_GRID_COUNT_HISTOGRAM_ID, gridFronts.length);
+      this.inspector.previousURL = this.inspector.target.url;
     }
 
     let grids = [];
@@ -296,14 +303,11 @@ GridInspector.prototype = {
   }),
 
   /**
-   * Handler for "grid-layout-changed" events emitted from the LayoutActor.
-   *
-   * @param  {Array} grids
-   *         Array of all GridFront in the current page.
+   * Handler for "navigate" event fired by the tab target. Updates grid panel contents.
    */
-  onGridLayoutChange(grids) {
+  onGridLayoutChange() {
     if (this.isPanelVisible()) {
-      this.updateGridPanel(grids);
+      this.updateGridPanel();
     }
   },
 
@@ -461,14 +465,12 @@ GridInspector.prototype = {
   },
 
   /**
-   * Handler for the inspector sidebar select event. Starts listening for
-   * "grid-layout-changed" if the layout panel is visible. Otherwise, stop
-   * listening for grid layout changes. Finally, refresh the layout view if
-   * it is visible.
+   * Handler for the inspector sidebar "select" event. Starts tracking reflows
+   * if the layout panel is visible. Otherwise, stop tracking reflows.
+   * Finally, refresh the layout view if it is visible.
    */
   onSidebarSelect() {
     if (!this.isPanelVisible()) {
-      this.layoutInspector.off("grid-layout-changed", this.onGridLayoutChange);
       this.inspector.reflowTracker.untrackReflows(this, this.onReflow);
       return;
     }
@@ -477,7 +479,6 @@ GridInspector.prototype = {
     Services.prefs.setIntPref(PROMOTE_COUNT_PREF, 0);
 
     this.inspector.reflowTracker.trackReflows(this, this.onReflow);
-    this.layoutInspector.on("grid-layout-changed", this.onGridLayoutChange);
     this.updateGridPanel();
   },
 
@@ -509,6 +510,10 @@ GridInspector.prototype = {
     this.store.dispatch(updateShowGridAreas(enabled));
     Services.prefs.setBoolPref(SHOW_GRID_AREAS, enabled);
 
+    if (enabled) {
+      this.telemetry.toolOpened("gridInspectorShowGridAreasOverlayChecked");
+    }
+
     let { grids } = this.store.getState();
 
     for (let grid of grids) {
@@ -532,6 +537,10 @@ GridInspector.prototype = {
     this.store.dispatch(updateShowGridLineNumbers(enabled));
     Services.prefs.setBoolPref(SHOW_GRID_LINE_NUMBERS, enabled);
 
+    if (enabled) {
+      this.telemetry.toolOpened("gridInspectorShowGridLineNumbersChecked");
+    }
+
     let { grids } = this.store.getState();
 
     for (let grid of grids) {
@@ -555,6 +564,10 @@ GridInspector.prototype = {
     this.store.dispatch(updateShowInfiniteLines(enabled));
     Services.prefs.setBoolPref(SHOW_INFINITE_LINES_PREF, enabled);
 
+    if (enabled) {
+      this.telemetry.toolOpened("gridInspectorShowInfiniteLinesChecked");
+    }
+
     let { grids } = this.store.getState();
 
     for (let grid of grids) {

devtools/client/inspector/grids/test/browser.ini

@@ -29,3 +29,4 @@ support-files =
 [browser_grids_grid-outline-highlight-cell.js]
 [browser_grids_grid-outline-selected-grid.js]
 [browser_grids_highlighter-setting-rules-grid-toggle.js]
+[browser_grids_number-of-css-grids-telemetry.js]

devtools/client/inspector/grids/test/browser_grids_number-of-css-grids-telemetry.js

@@ -0,0 +1,45 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that the telemetry count for the number of CSS Grid Elements on a page navigation
+// is correct when the toolbox is opened.
+
+const TEST_URI1 = `
+  <div></div>
+`;
+
+const TEST_URI2 = `
+  <style type='text/css'>
+    #grid {
+      display: grid;
+    }
+  </style>
+  <div id="grid">
+    <div id="cell1">cell1</div>
+    <div id="cell2">cell2</div>
+  </div>
+`;
+
+const CSS_GRID_COUNT_HISTOGRAM_ID = "DEVTOOLS_NUMBER_OF_CSS_GRIDS_IN_A_PAGE";
+
+add_task(function* () {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI1));
+
+  let { inspector } = yield openLayoutView();
+  let { store } = inspector;
+
+  info("Navigate to TEST_URI2");
+
+  let onGridListUpdate = waitUntilState(store, state => state.grids.length == 1);
+  yield navigateTo(inspector,
+    "data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI2));
+  yield onGridListUpdate;
+
+  let histogram = Services.telemetry.getHistogramById(CSS_GRID_COUNT_HISTOGRAM_ID);
+  let snapshot = histogram.snapshot();
+
+  is(snapshot.counts[1], 1, "Got a count of 1 for 1 CSS Grid element seen.");
+  is(snapshot.sum, 1, "Got the correct sum.");
+});

devtools/client/inspector/inspector.js

@@ -101,6 +101,10 @@ function Inspector(toolbox) {
   this.store = Store();
   this.telemetry = new Telemetry();
 
+  // 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);

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

@@ -6,6 +6,7 @@
 
 "use strict";
 
+const Services = require("Services");
 const {Task} = require("devtools/shared/task");
 const EventEmitter = require("devtools/shared/event-emitter");
 const { VIEW_NODE_VALUE_TYPE } = require("devtools/client/inspector/shared/node-types");
@@ -107,14 +108,18 @@ HighlightersOverlay.prototype = {
    *         The NodeFront of the grid container element to highlight.
    * @param  {Object} options
    *         Object used for passing options to the grid highlighter.
+   * @param. {String|null} trigger
+   *         String name matching "grid" or "rule" to indicate where the
+   *         grid highlighter was toggled on from. "grid" represents the grid view
+   *         "rule" represents the rule view.
    */
-  toggleGridHighlighter: Task.async(function* (node, options = {}) {
+  toggleGridHighlighter: Task.async(function* (node, options = {}, trigger) {
     if (node == this.gridHighlighterShown) {
       yield this.hideGridHighlighter(node);
       return;
     }
 
-    yield this.showGridHighlighter(node, options);
+    yield this.showGridHighlighter(node, options, trigger);
   }),
 
   /**
@@ -125,7 +130,7 @@ HighlightersOverlay.prototype = {
    * @param  {Object} options
    *         Object used for passing options to the grid highlighter.
    */
-  showGridHighlighter: Task.async(function* (node, options) {
+  showGridHighlighter: Task.async(function* (node, options, trigger) {
     let highlighter = yield this._getHighlighter("CssGridHighlighter");
     if (!highlighter) {
       return;
@@ -138,6 +143,12 @@ HighlightersOverlay.prototype = {
 
     this._toggleRuleViewGridIcon(node, true);
 
+    if (trigger == "grid") {
+      Services.telemetry.scalarAdd("devtools.grid.gridinspector.opened", 1);
+    } else if (trigger == "rule") {
+      Services.telemetry.scalarAdd("devtools.rules.gridinspector.opened", 1);
+    }
+
     try {
       // Save grid highlighter state.
       let { url } = this.inspector.target;
@@ -391,7 +402,8 @@ HighlightersOverlay.prototype = {
 
     highlighterSettings.color = grid ? grid.color : DEFAULT_GRID_COLOR;
 
-    this.toggleGridHighlighter(this.inspector.selection.nodeFront, highlighterSettings);
+    this.toggleGridHighlighter(this.inspector.selection.nodeFront, highlighterSettings,
+      "rule");
   },
 
   onMouseMove: function (event) {
@@ -516,6 +528,7 @@ HighlightersOverlay.prototype = {
     this.highlighters = null;
     this.highlighterUtils = null;
     this.supportsHighlighters = null;
+    this.state = null;
 
     this.geometryEditorHighlighterShown = null;
     this.gridHighlighterShown = null;

devtools/client/shared/telemetry.js

@@ -177,6 +177,15 @@ Telemetry.prototype = {
     reloadAddonReload: {
       histogram: "DEVTOOLS_RELOAD_ADDON_RELOAD_COUNT",
     },
+    gridInspectorShowGridAreasOverlayChecked: {
+      scalar: "devtools.grid.showGridAreasOverlay.checked",
+    },
+    gridInspectorShowGridLineNumbersChecked: {
+      scalar: "devtools.grid.showGridLineNumbers.checked",
+    },
+    gridInspectorShowInfiniteLinesChecked: {
+      scalar: "devtools.grid.showInfiniteLines.checked",
+    },
   },
 
   /**

devtools/server/actors/layout.js

@@ -4,7 +4,6 @@
 
 "use strict";
 
-const events = require("sdk/event/core");
 const { Actor, ActorClassWithSpec } = require("devtools/shared/protocol");
 const { getStringifiableFragments } =
   require("devtools/server/actors/utils/css-grid-utils");
@@ -72,17 +71,11 @@ var LayoutActor = ActorClassWithSpec(layoutSpec, {
 
     this.tabActor = tabActor;
     this.walker = walker;
-
-    this.onNavigate = this.onNavigate.bind(this);
-
-    events.on(this.tabActor, "navigate", this.onNavigate);
   },
 
   destroy: function () {
     Actor.prototype.destroy.call(this);
 
-    events.off(this.tabActor, "navigate", this.onNavigate);
-
     this.tabActor = null;
     this.walker = null;
   },
@@ -142,11 +135,6 @@ var LayoutActor = ActorClassWithSpec(layoutSpec, {
     return grids;
   },
 
-  onNavigate: function () {
-    let grids = this.getAllGrids(this.walker.rootNode);
-    events.emit(this, "grid-layout-changed", grids);
-  },
-
 });
 
 exports.GridActor = GridActor;

devtools/shared/specs/layout.js

@@ -16,13 +16,6 @@ const gridSpec = generateActorSpec({
 const layoutSpec = generateActorSpec({
   typeName: "layout",
 
-  events: {
-    "grid-layout-changed": {
-      type: "grid-layout-changed",
-      grids: Arg(0, "array:grid")
-    }
-  },
-
   methods: {
     getAllGrids: {
       request: {

toolkit/components/telemetry/Histograms.json

@@ -10097,6 +10097,17 @@
     "kind": "count",
     "description": "Reports the command name used in GCLI e.g. 'screenshot'"
   },
+  "DEVTOOLS_NUMBER_OF_CSS_GRIDS_IN_A_PAGE": {
+    "record_in_processes": ["main", "content"],
+    "alert_emails": ["dev-developer-tools@lists.mozilla.org"],
+    "expires_in_version": "never",
+    "kind": "linear",
+    "high": 30,
+    "n_buckets": 29,
+    "bug_numbers": [1373483],
+    "description": "On page load, record the number of CSS Grid elements present on a page when the DevTools is open",
+    "releaseChannelCollection": "opt-out"
+  },
   "VIEW_SOURCE_IN_BROWSER_OPENED_BOOLEAN": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["mozilla-dev-developer-tools@lists.mozilla.org", "jryans@mozilla.com"],

toolkit/components/telemetry/Scalars.yaml

@@ -476,6 +476,76 @@ devtools.copy.xpath:
     record_in_processes:
       - 'main'
 
+devtools.rules.gridinspector:
+  opened:
+    bug_numbers:
+      - 1373483
+    description: >
+      Number of times the DevTools grid inspector was opened from the rules view.
+    expires: never
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+devtools.grid.gridinspector:
+  opened:
+    bug_numbers:
+      - 1373483
+    description: >
+      Number of times the DevTools grid inspector was opened from the grid view.
+    expires: never
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+devtools.grid.showGridAreasOverlay:
+  checked:
+    bug_numbers:
+      - 1373483
+    description: >
+      Number of times the DevTools grid inspector's "Display grid areas" was checked.
+    expires: never
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+devtools.grid.showGridLineNumbers:
+  checked:
+    bug_numbers:
+      - 1373483
+    description: >
+      Number of times the DevTools grid inspector's "Display grid numbers" was checked.
+    expires: never
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
+devtools.grid.showInfiniteLines:
+  checked:
+    bug_numbers:
+      - 1373483
+    description: >
+      Number of times the DevTools grid inspector's "Extend grid lines infinitely" was checked.
+    expires: never
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
 navigator.storage:
   estimate_count:
     bug_numbers: