Bug 1466881 - Missing open events for some users r=nchevobbe

Changes:

- All files
  - Generally shuffled around telemetry events to fix issues with properties not been sent or being sent in the wrong order.
- devtools/client/framework/browser-menus.js
  - Added telemetry open event to missed codepath.
- devtools/startup/DevToolsShim.jsm
  - Added missing telemetry "shortcut" property.
- devtools/startup/devtools-startup.js
  - Added comment to sendEntryPointTelemetry().

MozReview-Commit-ID: ELldeyF32rG

--HG--
extra : rebase_source : bbe9a9bf279e6a6c1324a162885cc35b04cf27ce

devtools/client/debugger/new/test/mochitest/browser_dbg-browser-content-toolbox.js

@@ -49,8 +49,8 @@ add_task(async function() {
   info("Open the Browser Content Toolbox");
   let toolbox = await gDevToolsBrowser.openContentProcessToolbox(gBrowser);
 
-  info("Wait for the debugger to be ready");
-  await toolbox.getPanelWhenReady("jsdebugger");
+  info("Select the debugger");
+  await toolbox.selectTool("jsdebugger");
 
   let dbg = createDebuggerContext(toolbox);
   ok(dbg, "Debugger context is available");

devtools/client/framework/browser-menus.js

@@ -18,6 +18,9 @@ const MENUS_L10N = new LocalizationHelper("devtools/client/locales/menus.propert
 
 loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools", true);
 loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true);
+loader.lazyRequireGetter(this, "Telemetry", "devtools/client/shared/telemetry");
+
+let telemetry = null;
 
 // Keep list of inserted DOM Elements in order to remove them on unload
 // Maps browser xul document => list of DOM Elements
@@ -78,6 +81,7 @@ function createToolMenuElements(toolDefinition, doc) {
   const oncommand = function(id, event) {
     const window = event.target.ownerDocument.defaultView;
     gDevToolsBrowser.selectToolCommand(window.gBrowser, id, Cu.now());
+    sendEntryPointTelemetry();
   }.bind(null, id);
 
   const menuitem = createMenuItem({
@@ -96,6 +100,26 @@ function createToolMenuElements(toolDefinition, doc) {
   };
 }
 
+/**
+ * Send entry point telemetry explaining how the devtools were launched when
+ * launched from the System Menu.. This functionality also lives inside
+ * `devtools/startup/devtools-startup.js` but that codepath is only used the
+ * first time a toolbox is opened for a tab.
+ */
+function sendEntryPointTelemetry() {
+  if (!telemetry) {
+    telemetry = new Telemetry();
+  }
+
+  telemetry.addEventProperty(
+    "devtools.main", "open", "tools", null, "shortcut", ""
+  );
+
+  telemetry.addEventProperty(
+    "devtools.main", "open", "tools", null, "entrypoint", "SystemMenu"
+  );
+}
+
 /**
  * Create xul menuitem, key elements for a given tool.
  * And then insert them into browser DOM.

devtools/client/framework/devtools.js

@@ -453,6 +453,7 @@ DevTools.prototype = {
    */
   async showToolbox(target, toolId, hostType, hostOptions, startTime) {
     let toolbox = this._toolboxes.get(target);
+
     if (toolbox) {
       if (hostType != null && toolbox.hostType != hostType) {
         await toolbox.switchHost(hostType);
@@ -477,14 +478,18 @@ DevTools.prototype = {
       this._creatingToolboxes.delete(target);
 
       if (startTime) {
-        this.logToolboxOpenTime(toolbox.currentToolId, startTime);
+        this.logToolboxOpenTime(toolbox, startTime);
       }
       this._firstShowToolbox = false;
     }
 
+    // We send the "enter" width here to ensure it is always sent *after*
+    // the "open" event.
     const width = Math.ceil(toolbox.win.outerWidth / 50) * 50;
+    const panelName = this.makeToolIdHumanReadable(toolId || toolbox.defaultToolId);
     this._telemetry.addEventProperty(
-      "devtools.main", "open", "tools", null, "width", width);
+      "devtools.main", "enter", panelName, null, "width", width
+    );
 
     return toolbox;
   },
@@ -496,22 +501,24 @@ DevTools.prototype = {
    * subsequent toolbox opening, which should all be faster.
    * These two probes are indexed by Tool ID.
    *
-   * @param {String} toolId
-   *        The id of the opened tool.
+   * @param {String} toolbox
+   *        Toolbox instance.
    * @param {Number} startTime
    *        Indicates the time at which the user event related to the toolbox
    *        opening started. This is a `Cu.now()` timing.
    */
-  logToolboxOpenTime(toolId, startTime) {
+  logToolboxOpenTime(toolbox, startTime) {
+    const toolId = toolbox.currentToolId || toolbox.defaultToolId;
     const delay = Cu.now() - startTime;
+    const panelName = this.makeToolIdHumanReadable(toolId);
 
     const telemetryKey = this._firstShowToolbox ?
       "DEVTOOLS_COLD_TOOLBOX_OPEN_DELAY_MS" : "DEVTOOLS_WARM_TOOLBOX_OPEN_DELAY_MS";
     this._telemetry.getKeyedHistogramById(telemetryKey).add(toolId, delay);
 
     this._telemetry.addEventProperty(
-      "devtools.main", "open", "tools", null, "first_panel",
-      this.makeToolIdHumanReadable(toolId));
+      "devtools.main", "open", "tools", null, "first_panel", panelName
+    );
   },
 
   makeToolIdHumanReadable(toolId) {

devtools/client/framework/toolbox.js

@@ -244,6 +244,10 @@ Toolbox.prototype = {
     this.component.setCurrentToolId(id);
   },
 
+  get defaultToolId() {
+    return this._defaultToolId;
+  },
+
   get panelDefinitions() {
     return this._panelDefinitions;
   },
@@ -1901,15 +1905,7 @@ Toolbox.prototype = {
     const panelName = this.getTelemetryPanelNameOrOther(id);
     const prevPanelName = this.getTelemetryPanelNameOrOther(this.currentToolId);
     const cold = !this.getPanel(id);
-
-    this.telemetry.addEventProperties("devtools.main", "enter", panelName, null, {
-      "host": this._hostType,
-      "width": width,
-      "start_state": reason,
-      "panel_name": panelName,
-      "cold": cold,
-      // "session_id" is included at the end of this method.
-    });
+    const pending = ["host", "width", "start_state", "panel_name", "cold", "session_id"];
 
     // On first load this.currentToolId === undefined so we need to skip sending
     // a devtools.main.exit telemetry event.
@@ -1924,13 +1920,32 @@ Toolbox.prototype = {
       });
     }
 
-    const pending = ["host", "width", "start_state", "panel_name", "cold", "session_id"];
+    this.telemetry.addEventProperties("devtools.main", "open", "tools", null, {
+      "width": width,
+      "session_id": this.sessionId
+    });
+
     if (id === "webconsole") {
       pending.push("message_count");
     }
 
     this.telemetry.preparePendingEvent("devtools.main", "enter", panelName, null, pending);
 
+    this.telemetry.addEventProperties("devtools.main", "enter", panelName, null, {
+      "host": this._hostType,
+      "start_state": reason,
+      "panel_name": panelName,
+      "cold": cold,
+      "session_id": this.sessionId
+    });
+
+    if (reason !== "initial_panel") {
+      const width = Math.ceil(this.win.outerWidth / 50) * 50;
+      this.telemetry.addEventProperty(
+        "devtools.main", "enter", panelName, null, "width", width
+      );
+    }
+
     // Cold webconsole event message_count is handled in
     // devtools/client/webconsole/webconsole-output-wrapper.js
     if (!cold && id === "webconsole") {
@@ -1938,15 +1953,6 @@ Toolbox.prototype = {
         "devtools.main", "enter", "webconsole", null, "message_count", 0);
     }
 
-    this.telemetry.addEventProperty(
-      "devtools.main", "open", "tools", null, "session_id", this.sessionId
-    );
-    // We send the "enter" session ID here to ensure it is always sent *after*
-    // the "open" session ID.
-    this.telemetry.addEventProperty(
-      "devtools.main", "enter", panelName, null, "session_id", this.sessionId
-    );
-
     this.telemetry.toolOpened(id);
   },
 
@@ -2963,11 +2969,6 @@ Toolbox.prototype = {
     const prevPanelName = this.getTelemetryPanelNameOrOther(this.currentToolId);
 
     this.telemetry.toolClosed("toolbox");
-    this.telemetry.recordEvent("devtools.main", "close", "tools", null, {
-      "host": host,
-      "width": width,
-      "session_id": this.sessionId
-    });
     this.telemetry.recordEvent("devtools.main", "exit", prevPanelName, null, {
       "host": host,
       "width": width,
@@ -2976,6 +2977,11 @@ Toolbox.prototype = {
       "reason": "toolbox_close",
       "session_id": this.sessionId
     });
+    this.telemetry.recordEvent("devtools.main", "close", "tools", null, {
+      "host": host,
+      "width": width,
+      "session_id": this.sessionId
+    });
 
     // Finish all outstanding tasks (which means finish destroying panels and
     // then destroying the host, successfully or not) before destroying the

devtools/startup/DevToolsShim.jsm

@@ -263,6 +263,9 @@ this.DevToolsShim = {
     }
 
     if (reason) {
+      this.telemetry.addEventProperty(
+        "devtools.main", "open", "tools", null, "shortcut", ""
+      );
       this.telemetry.addEventProperty(
         "devtools.main", "open", "tools", null, "entrypoint", reason
       );

devtools/startup/devtools-startup.js

@@ -841,6 +841,18 @@ DevToolsStartup.prototype = {
     }
   },
 
+  /**
+   * Send entry point telemetry explaining how the devtools were launched. This
+   * functionality also lives inside `devtools/client/framework/browser-menus.js`
+   * because this codepath is only used the first time a toolbox is opened for a
+   * tab.
+   *
+   * @param {String} reason
+   *        One of "KeyShortcut", "SystemMenu", "HamburgerMenu", "ContextMenu",
+   *        "CommandLine".
+   * @param {String} key
+   *        The key used by a key shortcut.
+   */
   sendEntryPointTelemetry(reason, key = "") {
     if (!reason) {
       return;