Bug 1262439 - 8 - Re-implement telemetry in the eye-dropper; r=miker draft
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 09 Jun 2016 16:02:08 +0200
changeset 385014 1aa0d4db036a145c7304505f7d58b0ece3b3f5a1
parent 385013 e770b7c1203c6488cc8db56b55f350ea9890085a
child 524813 0f610e7d96df220c66746fdc2ae7ca13b05ce2ba
push id22388
push userpbrosset@mozilla.com
push dateThu, 07 Jul 2016 13:03:20 +0000
reviewersmiker
bugs1262439
milestone50.0a1
Bug 1262439 - 8 - Re-implement telemetry in the eye-dropper; r=miker MozReview-Commit-ID: 2XDhAb3iXdu
devtools/client/inspector/inspector-commands.js
devtools/client/inspector/inspector-panel.js
devtools/client/menus.js
devtools/client/shared/telemetry.js
devtools/client/shared/test/browser.ini
devtools/client/shared/test/browser_telemetry_button_eyedropper.js
devtools/client/shared/widgets/Tooltip.js
--- a/devtools/client/inspector/inspector-commands.js
+++ b/devtools/client/inspector/inspector-commands.js
@@ -2,16 +2,17 @@
  * 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";
 
 const l10n = require("gcli/l10n");
 loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools", true);
 const {EyeDropper, HighlighterEnvironment} = require("devtools/server/actors/highlighters");
+const Telemetry = require("devtools/client/shared/telemetry");
 
 exports.items = [{
   item: "command",
   runAt: "server",
   name: "inspect",
   description: l10n.lookup("inspectDesc"),
   manual: l10n.lookup("inspectManual"),
   params: [
@@ -25,20 +26,41 @@ exports.items = [{
   exec: function (args, context) {
     let target = context.environment.target;
     return gDevTools.showToolbox(target, "inspector").then(toolbox => {
       toolbox.getCurrentPanel().selection.setNode(args.selector, "gcli");
     });
   }
 }, {
   item: "command",
-  runAt: "server",
+  runAt: "client",
   name: "eyedropper",
   description: l10n.lookup("eyedropperDesc"),
   manual: l10n.lookup("eyedropperManual"),
+  params: [{
+    // This hidden parameter is only set to true when the eyedropper browser menu item is
+    // used. It is useful to log a different telemetry event whether the tool was used
+    // from the menu, or from the gcli command line.
+    group: "hiddengroup",
+    params: [{
+      name: "frommenu",
+      type: "boolean",
+      hidden: true
+    }]
+  }],
+  exec: function (args, context) {
+    let telemetry = new Telemetry();
+    telemetry.toolOpened(args.frommenu ? "menueyedropper" : "eyedropper");
+    context.updateExec("eyedropper_server").catch(e => console.error(e));
+  }
+}, {
+  item: "command",
+  runAt: "server",
+  name: "eyedropper_server",
+  hidden: true,
   exec: function (args, {environment}) {
     let env = new HighlighterEnvironment();
     env.initFromWindow(environment.window);
     let eyeDropper = new EyeDropper(env);
 
     eyeDropper.show(environment.document.documentElement, {copyOnSelect: true});
 
     eyeDropper.once("hidden", () => {
--- a/devtools/client/inspector/inspector-panel.js
+++ b/devtools/client/inspector/inspector-panel.js
@@ -15,16 +15,17 @@ var promise = require("promise");
 var defer = require("devtools/shared/defer");
 var EventEmitter = require("devtools/shared/event-emitter");
 var clipboard = require("sdk/clipboard");
 const {executeSoon} = require("devtools/shared/DevToolsUtils");
 var {KeyShortcuts} = require("devtools/client/shared/key-shortcuts");
 var {Task} = require("devtools/shared/task");
 const {initCssProperties} = require("devtools/shared/fronts/css-properties");
 const nodeConstants = require("devtools/shared/dom-node-constants");
+const Telemetry = require("devtools/client/shared/telemetry");
 
 const Menu = require("devtools/client/framework/menu");
 const MenuItem = require("devtools/client/framework/menu-item");
 
 loader.lazyRequireGetter(this, "CSS", "CSS");
 
 loader.lazyRequireGetter(this, "CommandUtils", "devtools/client/shared/developer-toolbar", true);
 loader.lazyRequireGetter(this, "ComputedViewTool", "devtools/client/inspector/computed/computed", true);
@@ -85,16 +86,18 @@ loader.lazyGetter(this, "clipboardHelper
  */
 function InspectorPanel(iframeWindow, toolbox) {
   this._toolbox = toolbox;
   this._target = toolbox._target;
   this.panelDoc = iframeWindow.document;
   this.panelWin = iframeWindow;
   this.panelWin.inspector = this;
 
+  this.telemetry = new Telemetry();
+
   this.nodeMenuTriggerInfo = null;
 
   this._onBeforeNavigate = this._onBeforeNavigate.bind(this);
   this.onNewRoot = this.onNewRoot.bind(this);
   this._onContextMenu = this._onContextMenu.bind(this);
   this._updateSearchResultsLabel = this._updateSearchResultsLabel.bind(this);
   this.onNewSelection = this.onNewSelection.bind(this);
   this.onBeforeNewSelection = this.onBeforeNewSelection.bind(this);
@@ -1231,26 +1234,37 @@ InspectorPanel.prototype = {
     this.walker.off("new-root", this.onEyeDropperDone);
   },
 
   onEyeDropperDone: function () {
     this.eyeDropperButton.removeAttribute("checked");
     this.stopEyeDropperListeners();
   },
 
+  /**
+   * Show the eyedropper on the page.
+   * @return {Promise} resolves when the eyedropper is visible.
+   */
   showEyeDropper: function () {
+    this.telemetry.toolOpened("toolbareyedropper");
     this.eyeDropperButton.setAttribute("checked", "true");
-    this.inspector.pickColorFromPage({copyOnSelect: true}).catch(e => console.error(e));
     this.startEyeDropperListeners();
+    return this.inspector.pickColorFromPage({copyOnSelect: true})
+                         .catch(e => console.error(e));
   },
 
+  /**
+   * Hide the eyedropper.
+   * @return {Promise} resolves when the eyedropper is hidden.
+   */
   hideEyeDropper: function () {
     this.eyeDropperButton.removeAttribute("checked");
-    this.inspector.cancelPickColorFromPage().catch(e => console.error(e));
     this.stopEyeDropperListeners();
+    return this.inspector.cancelPickColorFromPage()
+                         .catch(e => console.error(e));
   },
 
   /**
    * Create a new node as the last child of the current selection, expand the
    * parent and select the new node.
    */
   addNode: Task.async(function* () {
     if (!this.canAddHTMLChild()) {
--- a/devtools/client/menus.js
+++ b/devtools/client/menus.js
@@ -145,17 +145,17 @@ exports.menuitems = [
     l10nKey: "eyedropper",
     oncommand(event) {
       let window = event.target.ownerDocument.defaultView;
       let target = TargetFactory.forTab(window.gBrowser.selectedTab);
 
       CommandUtils.createRequisition(target, {
         environment: CommandUtils.createEnvironment({target})
       }).then(requisition => {
-        requisition.updateExec("eyedropper");
+        requisition.updateExec("eyedropper --frommenu");
       }, e => console.error(e));
     },
     checkbox: true
   },
   { id: "menu_scratchpad",
     l10nKey: "scratchpad",
     oncommand() {
       ScratchpadManager.openScratchpad();
--- a/devtools/client/shared/telemetry.js
+++ b/devtools/client/shared/telemetry.js
@@ -189,16 +189,20 @@ Telemetry.prototype = {
     menueyedropper: {
       histogram: "DEVTOOLS_MENU_EYEDROPPER_OPENED_COUNT",
       userHistogram: "DEVTOOLS_MENU_EYEDROPPER_OPENED_PER_USER_FLAG",
     },
     pickereyedropper: {
       histogram: "DEVTOOLS_PICKER_EYEDROPPER_OPENED_COUNT",
       userHistogram: "DEVTOOLS_PICKER_EYEDROPPER_OPENED_PER_USER_FLAG",
     },
+    toolbareyedropper: {
+      histogram: "DEVTOOLS_TOOLBAR_EYEDROPPER_OPENED_COUNT",
+      userHistogram: "DEVTOOLS_TOOLBAR_EYEDROPPER_OPENED_PER_USER_FLAG",
+    },
     developertoolbar: {
       histogram: "DEVTOOLS_DEVELOPERTOOLBAR_OPENED_COUNT",
       userHistogram: "DEVTOOLS_DEVELOPERTOOLBAR_OPENED_PER_USER_FLAG",
       timerHistogram: "DEVTOOLS_DEVELOPERTOOLBAR_TIME_ACTIVE_SECONDS"
     },
     aboutdebugging: {
       histogram: "DEVTOOLS_ABOUTDEBUGGING_OPENED_COUNT",
       userHistogram: "DEVTOOLS_ABOUTDEBUGGING_OPENED_PER_USER_FLAG",
--- a/devtools/client/shared/test/browser.ini
+++ b/devtools/client/shared/test/browser.ini
@@ -146,16 +146,17 @@ skip-if = e10s # Test intermittently fai
 [browser_poller.js]
 [browser_prefs-01.js]
 [browser_prefs-02.js]
 [browser_spectrum.js]
 [browser_theme.js]
 [browser_tableWidget_basic.js]
 [browser_tableWidget_keyboard_interaction.js]
 [browser_tableWidget_mouse_interaction.js]
+[browser_telemetry_button_eyedropper.js]
 [browser_telemetry_button_paintflashing.js]
 skip-if = e10s # Bug 937167 - e10s paintflashing
 [browser_telemetry_button_responsive.js]
 skip-if = e10s # Bug 1067145 - e10s responsiveview
 [browser_telemetry_button_scratchpad.js]
 [browser_telemetry_sidebar.js]
 [browser_telemetry_toolbox.js]
 [browser_telemetry_toolboxtabs_canvasdebugger.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/shared/test/browser_telemetry_button_eyedropper.js
@@ -0,0 +1,55 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+const TEST_URI = "data:text/html;charset=utf-8," +
+  "<p>browser_telemetry_button_eyedropper.js</p><div>test</div>";
+
+add_task(function* () {
+  yield addTab(TEST_URI);
+  let Telemetry = loadTelemetryAndRecordLogs();
+
+  let target = TargetFactory.forTab(gBrowser.selectedTab);
+  let toolbox = yield gDevTools.showToolbox(target, "inspector");
+  info("inspector opened");
+
+  info("testing the eyedropper button");
+  yield testButton(toolbox, Telemetry);
+
+  stopRecordingTelemetryLogs(Telemetry);
+  yield gDevTools.closeToolbox(target);
+  gBrowser.removeCurrentTab();
+});
+
+function* testButton(toolbox, Telemetry) {
+  info("Calling the eyedropper button's callback");
+  // We call the button callback directly because we don't need to test the UI here, we're
+  // only concerned about testing the telemetry probe.
+  yield toolbox.getPanel("inspector").showEyeDropper();
+
+  checkResults("_EYEDROPPER_", Telemetry);
+}
+
+function checkResults(histIdFocus, Telemetry) {
+  let result = Telemetry.prototype.telemetryInfo;
+
+  for (let [histId, value] of Iterator(result)) {
+    if (histId.startsWith("DEVTOOLS_INSPECTOR_") ||
+        !histId.includes(histIdFocus)) {
+      // Inspector stats are tested in
+      // browser_telemetry_toolboxtabs_{toolname}.js so we skip them here
+      // because we only open the inspector once for this test.
+      continue;
+    }
+
+    if (histId.endsWith("OPENED_PER_USER_FLAG")) {
+      ok(value.length === 1 && value[0] === true,
+         "Per user value " + histId + " has a single value of true");
+    } else if (histId.endsWith("OPENED_COUNT")) {
+      is(value.length, 1, histId + " has one entry");
+
+      let okay = value.every(element => element === true);
+      ok(okay, "All " + histId + " entries are === true");
+    }
+  }
+}
--- a/devtools/client/shared/widgets/Tooltip.js
+++ b/devtools/client/shared/widgets/Tooltip.js
@@ -920,17 +920,18 @@ Heritage.extend(SwatchBasedEditorTooltip
 
       if (this.eyedropperOpen) {
         this.commit();
       }
     }
   },
 
   _openEyeDropper: function () {
-    let {inspector, toolbox} = this.inspector;
+    let {inspector, toolbox, telemetry} = this.inspector;
+    telemetry.toolOpened("pickereyedropper");
     inspector.pickColorFromPage({copyOnSelect: false}).catch(e => console.error(e));
 
     inspector.once("color-picked", color => {
       toolbox.win.focus();
       this._selectColor(color);
     });
 
     inspector.once("color-pick-canceled", () => {