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
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-browser-content-toolbox.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-browser-content-toolbox.js
@@ -44,18 +44,18 @@ add_task(async function() {
clearDebuggerPreferences();
info("Open a tab pointing to doc-scripts.html");
await addTab(EXAMPLE_URL + "doc-scripts.html");
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");
info("Create a breakpoint");
await selectSource(dbg, "simple2");
await addBreakpoint(dbg, "simple2", 3);
--- a/devtools/client/framework/browser-menus.js
+++ b/devtools/client/framework/browser-menus.js
@@ -13,16 +13,19 @@
*/
const {Cu} = require("chrome");
const {LocalizationHelper} = require("devtools/shared/l10n");
const MENUS_L10N = new LocalizationHelper("devtools/client/locales/menus.properties");
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
const FragmentsCache = new Map();
function l10n(key) {
return MENUS_L10N.getStr(key);
}
@@ -73,16 +76,17 @@ function createToolMenuElements(toolDefi
// Prevent multiple entries for the same tool.
if (doc.getElementById(menuId)) {
return;
}
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({
doc,
id: "menuitem_" + id,
label: toolDefinition.menuLabel || toolDefinition.label,
accesskey: toolDefinition.accesskey
});
@@ -92,16 +96,36 @@ function createToolMenuElements(toolDefi
menuitem.addEventListener("command", oncommand);
return {
menuitem
};
}
/**
+ * 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.
*
* @param {XULDocument} doc
* The document to which the tool is to be registered.
* @param {Object} toolDefinition
* Tool definition of the tool to register.
* @param {Object} prevDef
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -448,16 +448,17 @@ DevTools.prototype = {
* Optional, indicates the time at which the user event related to this toolbox
* opening started. This is a `Cu.now()` timing.
*
* @return {Toolbox} toolbox
* The toolbox that was opened
*/
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);
}
if (toolId != null && toolbox.currentToolId != toolId) {
await toolbox.selectTool(toolId, "toolbox_show");
}
@@ -472,51 +473,57 @@ DevTools.prototype = {
return promise;
}
const toolboxPromise = this.createToolbox(target, toolId, hostType, hostOptions);
this._creatingToolboxes.set(target, toolboxPromise);
toolbox = await toolboxPromise;
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;
},
/**
* Log telemetry related to toolbox opening.
* Two distinct probes are logged. One for cold startup, when we open the very first
* toolbox. This one includes devtools framework loading. And a second one for all
* 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) {
if (/^[0-9a-fA-F]{40}_temporary-addon/.test(toolId)) {
return "temporary-addon";
}
let matches = toolId.match(
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -239,16 +239,20 @@ Toolbox.prototype = {
return this._currentToolId;
},
set currentToolId(id) {
this._currentToolId = id;
this.component.setCurrentToolId(id);
},
+ get defaultToolId() {
+ return this._defaultToolId;
+ },
+
get panelDefinitions() {
return this._panelDefinitions;
},
set panelDefinitions(definitions) {
this._panelDefinitions = definitions;
this._combineAndSortPanelDefinitions();
},
@@ -1896,62 +1900,64 @@ Toolbox.prototype = {
});
},
_pingTelemetrySelectTool(id, reason) {
const width = Math.ceil(this.win.outerWidth / 50) * 50;
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.
if (this.currentToolId) {
this.telemetry.recordEvent("devtools.main", "exit", prevPanelName, null, {
"host": this._hostType,
"width": width,
"panel_name": prevPanelName,
"next_panel": panelName,
"reason": reason,
"session_id": this.sessionId
});
}
- 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") {
this.telemetry.addEventProperty(
"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);
},
/**
* Focus a tool's panel by id
* @param {string} id
* The id of tool to focus
*/
@@ -2958,29 +2964,29 @@ Toolbox.prototype = {
// We need to grab a reference to win before this._host is destroyed.
const win = this.win;
const host = this._getTelemetryHostString();
const width = Math.ceil(win.outerWidth / 50) * 50;
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,
"panel_name": this.getTelemetryPanelNameOrOther(this.currentToolId),
"next_panel": "none",
"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
// target.
this._destroyer = new Promise(resolve => {
resolve(settleAll(outstanding)
.catch(console.error)
.then(() => {
--- a/devtools/startup/DevToolsShim.jsm
+++ b/devtools/startup/DevToolsShim.jsm
@@ -259,16 +259,19 @@ this.DevToolsShim = {
*/
initDevTools: function(reason) {
if (!this.isEnabled()) {
throw new Error("DevTools are not enabled and can not be initialized.");
}
if (reason) {
this.telemetry.addEventProperty(
+ "devtools.main", "open", "tools", null, "shortcut", ""
+ );
+ this.telemetry.addEventProperty(
"devtools.main", "open", "tools", null, "entrypoint", reason
);
}
if (!this.isInitialized()) {
DevtoolsStartup.initDevTools(reason);
}
},
--- a/devtools/startup/devtools-startup.js
+++ b/devtools/startup/devtools-startup.js
@@ -836,16 +836,28 @@ DevToolsStartup.prototype = {
dump("Unable to start debugger server on " + portOrPath + ": " + e);
}
if (cmdLine.state == Ci.nsICommandLine.STATE_REMOTE_AUTO) {
cmdLine.preventDefault = true;
}
},
+ /**
+ * 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;
}
let keys = "";
if (reason === "KeyShortcut") {