From 89c6584074760b9470f9c56e0e5d3aa0493096e4 Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Tue, 20 Jun 2017 15:03:04 -0700 Subject: [PATCH] Backed out changeset 830b3b513bf1 (bug 1373483) for build failures in TelemetryHistogram.o a=backout MozReview-Commit-ID: I5RRKMjXWKM --- .../client/inspector/boxmodel/box-model.js | 7 +- .../client/inspector/grids/grid-inspector.js | 65 +++++++---------- .../client/inspector/grids/test/browser.ini | 1 - ...ser_grids_number-of-css-grids-telemetry.js | 45 ------------ devtools/client/inspector/inspector.js | 4 -- .../inspector/shared/highlighters-overlay.js | 21 ++---- devtools/client/shared/telemetry.js | 9 --- devtools/server/actors/layout.js | 12 ++++ devtools/shared/specs/layout.js | 7 ++ toolkit/components/telemetry/Histograms.json | 11 --- toolkit/components/telemetry/Scalars.yaml | 70 ------------------- 11 files changed, 53 insertions(+), 199 deletions(-) delete mode 100644 devtools/client/inspector/grids/test/browser_grids_number-of-css-grids-telemetry.js diff --git a/devtools/client/inspector/boxmodel/box-model.js b/devtools/client/inspector/boxmodel/box-model.js index 01ed5e579ac6..cc3f1e435838 100644 --- a/devtools/client/inspector/boxmodel/box-model.js +++ b/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()) { diff --git a/devtools/client/inspector/grids/grid-inspector.js b/devtools/client/inspector/grids/grid-inspector.js index 8efa464c1695..b879e7ce1a64 100644 --- a/devtools/client/inspector/grids/grid-inspector.js +++ b/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) { diff --git a/devtools/client/inspector/grids/test/browser.ini b/devtools/client/inspector/grids/test/browser.ini index fc23ef1e5ca7..f3aad88eb655 100644 --- a/devtools/client/inspector/grids/test/browser.ini +++ b/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] diff --git a/devtools/client/inspector/grids/test/browser_grids_number-of-css-grids-telemetry.js b/devtools/client/inspector/grids/test/browser_grids_number-of-css-grids-telemetry.js deleted file mode 100644 index ef901d302e5a..000000000000 --- a/devtools/client/inspector/grids/test/browser_grids_number-of-css-grids-telemetry.js +++ /dev/null @@ -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 = ` -
-`; - -const TEST_URI2 = ` - -
-
cell1
-
cell2
-
-`; - -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."); -}); diff --git a/devtools/client/inspector/inspector.js b/devtools/client/inspector/inspector.js index 37f835d31a8b..d1fa320a6b5a 100644 --- a/devtools/client/inspector/inspector.js +++ b/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); diff --git a/devtools/client/inspector/shared/highlighters-overlay.js b/devtools/client/inspector/shared/highlighters-overlay.js index 809b9fd28780..5ba230fcdb01 100644 --- a/devtools/client/inspector/shared/highlighters-overlay.js +++ b/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; diff --git a/devtools/client/shared/telemetry.js b/devtools/client/shared/telemetry.js index 3cbb8f80c1c4..775604877c29 100644 --- a/devtools/client/shared/telemetry.js +++ b/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", - }, }, /** diff --git a/devtools/server/actors/layout.js b/devtools/server/actors/layout.js index 1650eef0bc30..2edbd70b59a2 100644 --- a/devtools/server/actors/layout.js +++ b/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; diff --git a/devtools/shared/specs/layout.js b/devtools/shared/specs/layout.js index 1a9bf5ea2e56..d6976258334d 100644 --- a/devtools/shared/specs/layout.js +++ b/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: { diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 07a7d254640c..0b9f20d19444 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/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"], diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index 6db736b591b8..4a00cc61a024 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/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: