From c7935445d501c48b14bd5cf2aff3bce873191c06 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Mon, 16 Jul 2018 14:50:00 +0100 Subject: [PATCH] Bug 1474356 - Most other panel openings without panel_name r=nchevobbe Changes: Pretty much all files contain changes to the order of events and properties... these are to stop subtle issues with events not being logged because not all properties were sent and some events were sent in the wrong order. -devtools/client/framework/test/browser_devtools_api.js - Renamed test-tool-1 and 2 because we need to include the panel names in Events.yaml and "-" is not allowed. - devtools/client/framework/toolbox.js - Removed unneeded const REGEX_PANEL. - Generally shifted things around to ensure we send events from all codepaths. - toolkit/components/telemetry/Events.yaml - Changed telemetry event `"devtools.main", "enter"` to include all built-in panel names including "other." MozReview-Commit-ID: 2G3Z9pzj1QC --HG-- extra : rebase_source : bc691ae42b521e3a58c1a4aa9dbd455a3aa258a0 --- .../framework/test/browser_devtools_api.js | 4 +-- .../browser_toolbox_dynamic_registration.js | 8 ++--- .../framework/test/browser_toolbox_options.js | 8 ++--- ...owser_toolbox_selected_tool_unavailable.js | 4 +-- .../browser_toolbox_textbox_context_menu.js | 2 +- devtools/client/framework/toolbox.js | 30 +++++++++++-------- .../webconsole/webconsole-output-wrapper.js | 9 ++++-- toolkit/components/telemetry/Events.yaml | 10 +++---- 8 files changed, 43 insertions(+), 32 deletions(-) diff --git a/devtools/client/framework/test/browser_devtools_api.js b/devtools/client/framework/test/browser_devtools_api.js index fc720a87bf20..56588af9747e 100644 --- a/devtools/client/framework/test/browser_devtools_api.js +++ b/devtools/client/framework/test/browser_devtools_api.js @@ -7,8 +7,8 @@ "use strict"; -const toolId1 = "test-tool-1"; -const toolId2 = "test-tool-2"; +const toolId1 = "testtool1"; +const toolId2 = "testtool2"; function test() { addTab("about:blank").then(runTests1); diff --git a/devtools/client/framework/test/browser_toolbox_dynamic_registration.js b/devtools/client/framework/test/browser_toolbox_dynamic_registration.js index ffc8f4aa5ee5..8be72b6a5afa 100644 --- a/devtools/client/framework/test/browser_toolbox_dynamic_registration.js +++ b/devtools/client/framework/test/browser_toolbox_dynamic_registration.js @@ -19,7 +19,7 @@ function testRegister(aToolbox) { gDevTools.once("tool-registered", toolRegistered); gDevTools.registerTool({ - id: "test-tool", + id: "testTool", label: "Test Tool", inMenu: true, isTargetSupported: () => true, @@ -28,7 +28,7 @@ function testRegister(aToolbox) { } function toolRegistered(toolId) { - is(toolId, "test-tool", "tool-registered event handler sent tool id"); + is(toolId, "testTool", "tool-registered event handler sent tool id"); ok(gDevTools.getToolDefinitionMap().has(toolId), "tool added to map"); @@ -61,11 +61,11 @@ function getAllBrowserWindows() { function testUnregister() { gDevTools.once("tool-unregistered", toolUnregistered); - gDevTools.unregisterTool("test-tool"); + gDevTools.unregisterTool("testTool"); } function toolUnregistered(toolId) { - is(toolId, "test-tool", "tool-unregistered event handler sent tool id"); + is(toolId, "testTool", "tool-unregistered event handler sent tool id"); ok(!gDevTools.getToolDefinitionMap().has(toolId), "tool removed from map"); diff --git a/devtools/client/framework/test/browser_toolbox_options.js b/devtools/client/framework/test/browser_toolbox_options.js index 98cdb984b429..2690551d1c6c 100644 --- a/devtools/client/framework/test/browser_toolbox_options.js +++ b/devtools/client/framework/test/browser_toolbox_options.js @@ -39,7 +39,7 @@ add_task(async function() { function registerNewTool() { const toolDefinition = { - id: "test-tool", + id: "testTool", isTargetSupported: () => true, visibilityswitch: "devtools.test-tool.enabled", url: "about:blank", @@ -47,11 +47,11 @@ function registerNewTool() { }; ok(gDevTools, "gDevTools exists"); - ok(!gDevTools.getToolDefinitionMap().has("test-tool"), + ok(!gDevTools.getToolDefinitionMap().has("testTool"), "The tool is not registered"); gDevTools.registerTool(toolDefinition); - ok(gDevTools.getToolDefinitionMap().has("test-tool"), + ok(gDevTools.getToolDefinitionMap().has("testTool"), "The tool is registered"); } @@ -476,7 +476,7 @@ async function lookupButtonForToolId(toolId) { } async function cleanup() { - gDevTools.unregisterTool("test-tool"); + gDevTools.unregisterTool("testTool"); await toolbox.destroy(); gBrowser.removeCurrentTab(); for (const pref of modifiedPrefs) { diff --git a/devtools/client/framework/test/browser_toolbox_selected_tool_unavailable.js b/devtools/client/framework/test/browser_toolbox_selected_tool_unavailable.js index 5cd2dd50ba8b..92c9af57bf39 100644 --- a/devtools/client/framework/test/browser_toolbox_selected_tool_unavailable.js +++ b/devtools/client/framework/test/browser_toolbox_selected_tool_unavailable.js @@ -9,7 +9,7 @@ // tool is not supported. const testToolDefinition = { - id: "test-tool", + id: "testTool", isTargetSupported: () => true, visibilityswitch: "devtools.test-tool.enabled", url: "about:blank", @@ -31,7 +31,7 @@ add_task(async function() { let target = TargetFactory.forTab(tab); let toolbox = await gDevTools.showToolbox(target, testToolDefinition.id); - is(toolbox.currentToolId, "test-tool", "test-tool was selected"); + is(toolbox.currentToolId, "testTool", "test-tool was selected"); await gDevTools.closeToolbox(target); // Make the previously selected tool unavailable. diff --git a/devtools/client/framework/test/browser_toolbox_textbox_context_menu.js b/devtools/client/framework/test/browser_toolbox_textbox_context_menu.js index cdf10bbf3451..bfdd5c4e9323 100644 --- a/devtools/client/framework/test/browser_toolbox_textbox_context_menu.js +++ b/devtools/client/framework/test/browser_toolbox_textbox_context_menu.js @@ -10,7 +10,7 @@ // right state, and that it works for an input inside of a panel. const URL = "data:text/html;charset=utf8,test for textbox context menu"; -const textboxToolId = "test-tool-1"; +const textboxToolId = "testtool1"; registerCleanupFunction(() => { gDevTools.unregisterTool(textboxToolId); diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index 39e818da0df8..83f958b12967 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -13,8 +13,6 @@ const DISABLE_AUTOHIDE_PREF = "ui.popup.disable_autohide"; const HOST_HISTOGRAM = "DEVTOOLS_TOOLBOX_HOST"; const CURRENT_THEME_SCALAR = "devtools.current_theme"; const HTML_NS = "http://www.w3.org/1999/xhtml"; -const REGEX_PANEL = - /^(?:webconsole|inspector|jsdebugger|styleeditor|netmonitor|storage)$/; var {Ci, Cc} = require("chrome"); var promise = require("promise"); @@ -1929,16 +1927,17 @@ Toolbox.prototype = { const pending = ["host", "width", "start_state", "panel_name", "cold", "session_id"]; if (id === "webconsole") { pending.push("message_count"); - - // Cold webconsole event message_count is handled in - // devtools/client/webconsole/webconsole-output-wrapper.js - if (!cold) { - this.telemetry.addEventProperty( - "devtools.main", "enter", "webconsole", null, "message_count", 0); - } } - this.telemetry.preparePendingEvent( - "devtools.main", "enter", panelName, null, pending); + + this.telemetry.preparePendingEvent("devtools.main", "enter", panelName, null, pending); + + // 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 ); @@ -2910,6 +2909,7 @@ Toolbox.prototype = { } this.browserRequire = null; + this._toolNames = null; // Now that we are closing the toolbox we can re-enable the cache settings // and disable the service workers testing settings for the current tab. @@ -3378,7 +3378,13 @@ Toolbox.prototype = { }, getTelemetryPanelNameOrOther: function(id) { - if (!REGEX_PANEL.test(id)) { + if (!this._toolNames) { + const definitions = gDevTools.getToolDefinitionArray(); + const definitionIds = definitions.map(definition => definition.id); + + this._toolNames = new Set(definitionIds); + } + if (!this._toolNames.has(id)) { return "other"; } return id; diff --git a/devtools/client/webconsole/webconsole-output-wrapper.js b/devtools/client/webconsole/webconsole-output-wrapper.js index 935e828d2460..9323ebd3f83a 100644 --- a/devtools/client/webconsole/webconsole-output-wrapper.js +++ b/devtools/client/webconsole/webconsole-output-wrapper.js @@ -429,8 +429,13 @@ WebConsoleOutputWrapper.prototype = { store.dispatch(actions.messagesAdd(this.queuedMessageAdds)); const length = this.queuedMessageAdds.length; - this.telemetry.addEventProperty( - "devtools.main", "enter", "webconsole", null, "message_count", length); + + // This telemetry event is only useful when we have a toolbox so only + // send it when we have one. + if (this.toolbox) { + this.telemetry.addEventProperty( + "devtools.main", "enter", "webconsole", null, "message_count", length); + } this.queuedMessageAdds = []; diff --git a/toolkit/components/telemetry/Events.yaml b/toolkit/components/telemetry/Events.yaml index 322ae4bde562..b1995837a377 100644 --- a/toolkit/components/telemetry/Events.yaml +++ b/toolkit/components/telemetry/Events.yaml @@ -447,7 +447,7 @@ devtools.main: width: Toolbox width rounded up to the nearest 50px. session_id: The start time of the session in milliseconds since epoch (Unix Timestamp) e.g. 1396381378123. enter: - objects: ["webconsole", "inspector", "jsdebugger", "styleeditor", "netmonitor", "storage", "other"] + objects: ["accessibility", "application", "canvasdebugger", "dom", "inspector", "jsdebugger", "memory", "netmonitor", "options", "performance", "scratchpad", "shadereditor", "storage", "styleeditor", "webaudioeditor", "webconsole", "other", "fakeTool4242", "testTool", "testtool1", "testTool1072208", "testtool2"] bug_numbers: [1441070] notification_emails: ["dev-developer-tools@lists.mozilla.org", "hkirschner@mozilla.com"] record_in_processes: ["main"] @@ -459,11 +459,11 @@ devtools.main: width: Toolbox width rounded up to the nearest 50px. message_count: The number of cached console messages. start_state: debuggerStatement, breakpoint, exception, tab_switch, toolbox_show, initial_panel, toggle_settings_off, toggle_settings_on, key_shortcut, select_next_key, select_prev_key, tool_unloaded, inspect_dom, unknown etc. - panel_name: The name of the panel opened, webconsole, inspector, jsdebugger, styleeditor, netmonitor, storage or other + panel_name: The name of the panel opened or other cold: Is this the first time the current panel has been opened in this toolbox? session_id: The start time of the session in milliseconds since epoch (Unix Timestamp) e.g. 1396381378123. exit: - objects: ["webconsole", "inspector", "jsdebugger", "styleeditor", "netmonitor", "storage", "other"] + objects: ["accessibility", "application", "canvasdebugger", "dom", "inspector", "jsdebugger", "memory", "netmonitor", "options", "performance", "scratchpad", "shadereditor", "storage", "styleeditor", "webaudioeditor", "webconsole", "other", "fakeTool4242", "testTool", "testtool1", "testTool1072208", "testtool2"] bug_numbers: [1455270] notification_emails: ["dev-developer-tools@lists.mozilla.org", "hkirschner@mozilla.com"] record_in_processes: ["main"] @@ -473,8 +473,8 @@ devtools.main: extra_keys: host: "Toolbox host (positioning): bottom, side, window or other." width: Toolbox width rounded up to the nearest 50px. - next_panel: The name of the panel closed, webconsole, inspector, jsdebugger, styleeditor, netmonitor, storage or other. - panel_name: The name of the panel opened, webconsole, inspector, jsdebugger, styleeditor, netmonitor, storage or other + next_panel: The name of the panel closed or other. + panel_name: The name of the panel opened or other reason: debuggerStatement, breakpoint, exception, tab_switch, toolbox_show, initial_panel, toggle_settings_off, toggle_settings_on, key_shortcut, select_next_key, select_prev_key, tool_unloaded, inspect_dom, toolbox_closed, unknown etc. session_id: The start time of the session in milliseconds since epoch (Unix Timestamp) e.g. 1396381378123. activate: