Backed out changeset 830b3b513bf1 (bug 1373483) for build failures in TelemetryHistogram.o a=backout

MozReview-Commit-ID: I5RRKMjXWKM

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

@@ -347,9 +347,10 @@ BoxModel.prototype = {
   },
 
   /**
-   * 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.
+   * 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.
    */
   onSidebarSelect() {
     if (!this.isPanelVisible()) {

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

@@ -20,8 +20,6 @@ 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";
@@ -45,7 +43,6 @@ 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);
@@ -94,7 +91,6 @@ 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();
   }),
@@ -107,7 +103,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.inspector.target.off("navigate", this.onGridLayoutChange);
+    this.layoutInspector.off("grid-layout-changed", this.onGridLayoutChange);
 
     this.inspector.reflowTracker.untrackReflows(this, this.onReflow);
 
@@ -244,34 +240,31 @@ GridInspector.prototype = {
     this.lastHighlighterNode = node;
     this.lastHighlighterState = node !== this.highlighters.gridHighlighterShown;
 
-    this.highlighters.toggleGridHighlighter(node, settings, "grid");
+    this.highlighters.toggleGridHighlighter(node, settings);
   },
 
   /**
    * 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* () {
+  updateGridPanel: Task.async(function* (gridFronts) {
     // 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.
-    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;
+    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 grids = [];
@@ -303,11 +296,14 @@ GridInspector.prototype = {
   }),
 
   /**
-   * Handler for "navigate" event fired by the tab target. Updates grid panel contents.
+   * Handler for "grid-layout-changed" events emitted from the LayoutActor.
+   *
+   * @param  {Array} grids
+   *         Array of all GridFront in the current page.
    */
-  onGridLayoutChange() {
+  onGridLayoutChange(grids) {
     if (this.isPanelVisible()) {
-      this.updateGridPanel();
+      this.updateGridPanel(grids);
     }
   },
 
@@ -465,12 +461,14 @@ GridInspector.prototype = {
   },
 
   /**
-   * 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.
+   * 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.
    */
   onSidebarSelect() {
     if (!this.isPanelVisible()) {
+      this.layoutInspector.off("grid-layout-changed", this.onGridLayoutChange);
       this.inspector.reflowTracker.untrackReflows(this, this.onReflow);
       return;
     }
@@ -479,6 +477,7 @@ 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();
   },
 
@@ -510,10 +509,6 @@ 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) {
@@ -537,10 +532,6 @@ 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) {
@@ -564,10 +555,6 @@ 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,4 +29,3 @@ 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

@@ -1,45 +0,0 @@
-/* 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,10 +101,6 @@ 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,7 +6,6 @@
 
 "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");
@@ -108,18 +107,14 @@ 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 = {}, trigger) {
+  toggleGridHighlighter: Task.async(function* (node, options = {}) {
     if (node == this.gridHighlighterShown) {
       yield this.hideGridHighlighter(node);
       return;
     }
 
-    yield this.showGridHighlighter(node, options, trigger);
+    yield this.showGridHighlighter(node, options);
   }),
 
   /**
@@ -130,7 +125,7 @@ HighlightersOverlay.prototype = {
    * @param  {Object} options
    *         Object used for passing options to the grid highlighter.
    */
-  showGridHighlighter: Task.async(function* (node, options, trigger) {
+  showGridHighlighter: Task.async(function* (node, options) {
     let highlighter = yield this._getHighlighter("CssGridHighlighter");
     if (!highlighter) {
       return;
@@ -143,12 +138,6 @@ 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;
@@ -402,8 +391,7 @@ HighlightersOverlay.prototype = {
 
     highlighterSettings.color = grid ? grid.color : DEFAULT_GRID_COLOR;
 
-    this.toggleGridHighlighter(this.inspector.selection.nodeFront, highlighterSettings,
-      "rule");
+    this.toggleGridHighlighter(this.inspector.selection.nodeFront, highlighterSettings);
   },
 
   onMouseMove: function (event) {
@@ -528,7 +516,6 @@ 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,15 +177,6 @@ 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,6 +4,7 @@
 
 "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");
@@ -71,11 +72,17 @@ 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;
   },
@@ -135,6 +142,11 @@ 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,6 +16,13 @@ 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,17 +10097,6 @@
     "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": 32,
-    "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,76 +476,6 @@ 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: