corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1938424 - metrics event when tabs added to tab group r=dao,jswinarton,tabbrowser-reviewers,extension-reviewers,robwu

Browse Source 
When using the tab context menu or drag-dropping tabs to put them into a group, record a metric for the number of tabs added to the group.

Data Science asked us to launch with this metric disabled so that they could control it using server knobs. They expect a high volume of events, so they expect to only enable this metric for some proportion of users.

I converted the existing `TabMove` event derived from `UIEvent` being fired when tabs change their tab index in the tab strip. `UIEvent` doesn't allow for attaching additional context/detail to the event. `TabMove` is now a `CustomEvent` that provides more context about the moved tab and it fires in more cases -- it's possible for the tab index not to change despite the tab having "moved" into/out of a tab group.

This approach would not capture tab movements that occur across multiple frames/event loop iterations.

Differential Revision: https://phabricator.services.mozilla.com/D244616
This commit is contained in:
Stephen Thompson
2025-04-11 13:52:22 +00:00
parent cd6729c28d
commit f2e07044ae
17 changed files with 488 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files

217
browser/components/tabbrowser/content/tabbrowser.js
View File

@@ -110,8 +110,8 @@
PictureInPicture: "resource://gre/modules/PictureInPicture.sys.mjs",
SmartTabGroupingManager:
"moz-src:///browser/components/tabbrowser/SmartTabGrouping.sys.mjs",
TabGroupMetrics:
"moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs",
TabMetrics:
"moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs",
TabStateFlusher:
"resource:///modules/sessionstore/TabStateFlusher.sys.mjs",
UrlbarProviderOpenTabs:
@@ -2958,7 +2958,7 @@
* Causes the group create UI to be displayed and telemetry events to be fired.
* @param {string} [options.telemetryUserCreateSource]
* The means by which the tab group was created.
* @see TabGroupMetrics.METRIC_SOURCE for possible values.
* @see TabMetrics.METRIC_SOURCE for possible values.
* Defaults to "unknown".
*/
addTabGroup(
@@ -3035,14 +3035,14 @@
* switches windows). This causes telemetry events to fire.
* @param {string} [options.telemetrySource="unknown"]
* The means by which the tab group was removed.
* @see TabGroupMetrics.METRIC_SOURCE for possible values.
* @see TabMetrics.METRIC_SOURCE for possible values.
* Defaults to "unknown".
*/
async removeTabGroup(
group,
options = {
isUserTriggered: false,
telemetrySource: this.TabGroupMetrics.METRIC_SOURCE.UNKNOWN,
telemetrySource: this.TabMetrics.METRIC_SOURCE.UNKNOWN,
}
) {
if (this.tabGroupMenu.panel.state != "closed") {
@@ -3943,7 +3943,7 @@
// Place tab at the end of the contextual tab group because one of:
// 1) no `itemAfter` so `tab` should be the last tab in the tab strip
// 2) `itemAfter` is in a different tab group
this.moveTabToGroup(tab, tabGroup);
tabGroup.appendChild(tab);
}
} else if (
(this.isTab(itemAfter) && itemAfter.group?.tabs[0] == itemAfter) ||
@@ -5925,8 +5925,26 @@
* any possibility of entering a tab group. For example, setting `true`
* ensures that a pinned tab will not accidentally be placed inside of
* a tab group, since pinned tabs are presently not allowed in tab groups.
* @property {boolean} [options.isUserTriggered=false]
* Should be true if there was an explicit action/request from the user
* (as opposed to some action being taken internally or for technical
* bookkeeping reasons alone) to move the tab. This causes telemetry
* events to fire.
* @property {string} [options.telemetrySource="unknown"]
* The system, surface, or control the user used to move the tab.
* @see TabMetrics.METRIC_SOURCE for possible values.
* Defaults to "unknown".
*/
moveTabTo(aTab, { elementIndex, tabIndex, forceUngrouped = false } = {}) {
moveTabTo(
aTab,
{
elementIndex,
tabIndex,
forceUngrouped = false,
isUserTriggered = false,
telemetrySource = this.TabMetrics.METRIC_SOURCE.UNKNOWN,
} = {}
) {
if (typeof elementIndex == "number") {
tabIndex = this.#elementIndexToTabIndex(elementIndex);
}
@@ -5954,49 +5972,57 @@
aTab = aTab.group;
}
this.#handleTabMove(aTab, () => {
let neighbor = this.tabs[tabIndex];
if (forceUngrouped && neighbor.group) {
neighbor = neighbor.group;
}
if (neighbor && tabIndex > aTab._tPos) {
neighbor.after(aTab);
} else {
this.tabContainer.insertBefore(aTab, neighbor);
}
});
this.#handleTabMove(
aTab,
() => {
let neighbor = this.tabs[tabIndex];
if (forceUngrouped && neighbor.group) {
neighbor = neighbor.group;
}
if (neighbor && tabIndex > aTab._tPos) {
neighbor.after(aTab);
} else {
this.tabContainer.insertBefore(aTab, neighbor);
}
},
{ isUserTriggered, telemetrySource }
);
}
/**
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} tab
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement
* @param {TabMetricsContext} [metricsContext]
*/
moveTabBefore(tab, targetElement) {
this.#moveTabNextTo(tab, targetElement, true);
moveTabBefore(tab, targetElement, metricsContext) {
this.#moveTabNextTo(tab, targetElement, true, metricsContext);
}
/**
* @param {MozTabbrowserTab|MozTabbrowserTabGroup[]} tabs
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement
* @param {TabMetricsContext} [metricsContext]
*/
moveTabsBefore(tabs, targetElement) {
this.#moveTabsNextTo(tabs, targetElement, true);
moveTabsBefore(tabs, targetElement, metricsContext) {
this.#moveTabsNextTo(tabs, targetElement, true, metricsContext);
}
/**
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} tab
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement
* @param {TabMetricsContext} [metricsContext]
*/
moveTabAfter(tab, targetElement) {
this.#moveTabNextTo(tab, targetElement, false);
moveTabAfter(tab, targetElement, metricsContext) {
this.#moveTabNextTo(tab, targetElement, false, metricsContext);
}
/**
* @param {MozTabbrowserTab|MozTabbrowserTabGroup[]} tabs
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement
* @param {TabMetricsContext} [metricsContext]
*/
moveTabsAfter(tabs, targetElement) {
this.#moveTabsNextTo(tabs, targetElement, false);
moveTabsAfter(tabs, targetElement, metricsContext) {
this.#moveTabsNextTo(tabs, targetElement, false, metricsContext);
}
/**
@@ -6004,9 +6030,10 @@
* The tab or tab group to move. Also accepts a tab group label as a
* stand-in for its group.
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement
* @param {boolean} moveBefore
* @param {boolean} [moveBefore=false]
* @param {TabMetricsContext} [metricsContext]
*/
#moveTabNextTo(tab, targetElement, moveBefore = false) {
#moveTabNextTo(tab, targetElement, moveBefore = false, metricsContext) {
if (this.isTabGroupLabel(targetElement)) {
targetElement = targetElement.group;
if (!moveBefore) {
@@ -6037,30 +6064,41 @@
return this.tabContainer;
};
this.#handleTabMove(tab, () => {
if (moveBefore) {
getContainer().insertBefore(tab, targetElement);
} else if (targetElement) {
targetElement.after(tab);
} else {
getContainer().appendChild(tab);
}
});
this.#handleTabMove(
tab,
() => {
if (moveBefore) {
getContainer().insertBefore(tab, targetElement);
} else if (targetElement) {
targetElement.after(tab);
} else {
getContainer().appendChild(tab);
}
},
metricsContext
);
}
/**
* @param {MozTabbrowserTab[]} tabs
* @param {MozTabbrowserTab|MozTabbrowserTabGroup} targetElement
* @param {boolean} moveBefore
* @param {boolean} [moveBefore=false]
* @param {TabMetricsContext} [metricsContext]
*/
#moveTabsNextTo(tabs, targetElement, moveBefore = false) {
this.#moveTabNextTo(tabs[0], targetElement, moveBefore);
#moveTabsNextTo(tabs, targetElement, moveBefore = false, metricsContext) {
this.#moveTabNextTo(tabs[0], targetElement, moveBefore, metricsContext);
for (let i = 1; i < tabs.length; i++) {
this.#moveTabNextTo(tabs[i], tabs[i - 1]);
this.#moveTabNextTo(tabs[i], tabs[i - 1], false, metricsContext);
}
}
moveTabToGroup(aTab, aGroup) {
/**
*
* @param {MozTabbrowserTab} aTab
* @param {MozTabbrowserTabGroup} aGroup
* @param {TabMetricsContext} [metricsContext]
*/
moveTabToGroup(aTab, aGroup, metricsContext) {
if (aTab.pinned) {
return;
}
@@ -6069,19 +6107,80 @@
}
aGroup.collapsed = false;
this.#handleTabMove(aTab, () => aGroup.appendChild(aTab));
this.#handleTabMove(aTab, () => aGroup.appendChild(aTab), metricsContext);
this.removeFromMultiSelectedTabs(aTab);
this.tabContainer._notifyBackgroundTab(aTab);
}
/**
* @typedef {object} TabMoveState
* @property {number} tabIndex
* @property {number} [elementIndex]
* @property {string} [tabGroupId]
*/
/**
* @param {MozTabbrowserTab} tab
* @returns {TabMoveState|undefined}
*/
#getTabMoveState(tab) {
if (!this.isTab(tab)) {
return undefined;
}
let state = {
tabIndex: tab._tPos,
};
if (tab.visible) {
state.elementIndex = tab.elementIndex;
}
if (tab.group) {
state.tabGroupId = tab.group.id;
}
return state;
}
/**
* @param {MozTabbrowserTab} tab
* @param {TabMoveState} [previousTabState]
* @param {TabMoveState} [currentTabState]
* @param {TabMetricsContext} [metricsContext]
*/
#notifyOnTabMove(tab, previousTabState, currentTabState, metricsContext) {
if (!this.isTab(tab) || !previousTabState || !currentTabState) {
return;
}
let changedPosition =
previousTabState.tabIndex != currentTabState.tabIndex;
let changedTabGroup =
previousTabState.tabGroupId != currentTabState.tabGroupId;
if (changedPosition || changedTabGroup) {
tab.dispatchEvent(
new CustomEvent("TabMove", {
bubbles: true,
detail: {
previousTabState,
currentTabState,
isUserTriggered: metricsContext?.isUserTriggered ?? false,
telemetrySource:
metricsContext?.telemetrySource ??
this.TabMetrics.METRIC_SOURCE.UNKNOWN,
},
})
);
}
}
/**
* @param {MozTabbrowserTab} aTab
* @param {function():void} moveActionCallback
* @returns
* @param {TabMetricsContext} [metricsContext]
*/
#handleTabMove(aTab, moveActionCallback) {
#handleTabMove(aTab, moveActionCallback, metricsContext) {
let wasFocused = document.activeElement == this.selectedTab;
let oldPosition = this.isTab(aTab) && aTab._tPos;
let previousTabState = this.#getTabMoveState(aTab);
moveActionCallback();
@@ -6104,13 +6203,15 @@
if (aTab.pinned) {
this.tabContainer._positionPinnedTabs();
}
// Pinning/unpinning vertical tabs, and moving tabs into tab groups, both bypass moveTabTo.
// We still want to check whether its worth dispatching an event.
if (this.isTab(aTab) && oldPosition != aTab._tPos) {
let evt = document.createEvent("UIEvents");
evt.initUIEvent("TabMove", true, false, window, oldPosition);
aTab.dispatchEvent(evt);
}
let currentTabState = this.#getTabMoveState(aTab);
this.#notifyOnTabMove(
aTab,
previousTabState,
currentTabState,
metricsContext
);
}
/**
@@ -9056,8 +9157,16 @@ var TabContextMenu = {
gTabsPanel.hideAllTabsPanel();
},
/**
* @param {MozTabbrowserTabGroup} group
*/
moveTabsToGroup(group) {
group.addTabs(this.contextTabs);
group.addTabs(
this.contextTabs,
gBrowser.TabMetrics.userTriggeredContext(
gBrowser.TabMetrics.METRIC_SOURCE.TAB_MENU
)
);
group.ownerGlobal.focus();
},

14
browser/components/tabbrowser/content/tabgroup-menu.js
View File

@@ -7,8 +7,8 @@
// This is loaded into chrome windows with the subscript loader. Wrap in
// a block to prevent accidentally leaking globals onto `window`.
{
const { TabGroupMetrics } = ChromeUtils.importESModule(
"moz-src:///browser/components/tabbrowser/TabGroupMetrics.sys.mjs"
const { TabMetrics } = ChromeUtils.importESModule(
"moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs"
);
const { TabStateFlusher } = ChromeUtils.importESModule(
"resource:///modules/sessionstore/TabStateFlusher.sys.mjs"
@@ -448,10 +448,12 @@
document
.getElementById("tabGroupEditor_deleteGroup")
.addEventListener("command", () => {
gBrowser.removeTabGroup(this.activeGroup, {
isUserTriggered: true,
telemetrySource: TabGroupMetrics.METRIC_SOURCE.TAB_GROUP_MENU,
});
gBrowser.removeTabGroup(
this.activeGroup,
TabMetrics.userTriggeredContext(
TabMetrics.METRIC_SOURCE.TAB_GROUP_MENU
)
);
});
this.panel.addEventListener("popupshown", this);

8
browser/components/tabbrowser/content/tabgroup.js
View File

@@ -234,9 +234,11 @@
/**
* add tabs to the group
*
* @param tabs array of tabs to add
* @param {MozTabbrowserTab[]} tabs
* @param {TabMetricsContext} [metricsContext]
* Optional context to record for metrics purposes.
*/
addTabs(tabs) {
addTabs(tabs, metricsContext) {
for (let tab of tabs) {
let tabToMove =
this.ownerGlobal === tab.ownerGlobal
@@ -245,7 +247,7 @@
tabIndex: gBrowser.tabs.at(-1)._tPos + 1,
selectTab: tab.selected,
});
gBrowser.moveTabToGroup(tabToMove, this);
gBrowser.moveTabToGroup(tabToMove, this, metricsContext);
}
this.#lastAddedTo = Date.now();
}

25
browser/components/tabbrowser/content/tabs.js
View File

@@ -9,6 +9,11 @@
// This is loaded into all browser windows. Wrap in a block to prevent
// leaking to window scope.
{
const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
TabMetrics: "moz-src:///browser/components/tabbrowser/TabMetrics.sys.mjs",
});
const TAB_PREVIEW_PREF = "browser.tabs.hoverPreview.enabled";
const DIRECTION_BACKWARD = -1;
@@ -1060,6 +1065,10 @@
var dropEffect = dt.dropEffect;
var draggedTab;
let movingTabs;
/** @type {TabMetricsContext} */
const dropMetricsContext = lazy.TabMetrics.userTriggeredContext(
lazy.TabMetrics.METRIC_SOURCE.DRAG_AND_DROP
);
if (dt.mozTypesAt(0)[0] == TAB_DROP_TYPE) {
// tab copy or move
draggedTab = dt.mozGetDataAt(TAB_DROP_TYPE, 0);
@@ -1084,7 +1093,7 @@
duplicatedDraggedTab = duplicatedTab;
}
}
gBrowser.moveTabsBefore(duplicatedTabs, dropTarget);
gBrowser.moveTabsBefore(duplicatedTabs, dropTarget, dropMetricsContext);
if (draggedTab.container != this || event.shiftKey) {
this.selectedItem = duplicatedDraggedTab;
}
@@ -1172,15 +1181,23 @@
let moveTabs = () => {
if (dropIndex !== undefined) {
for (let tab of movingTabs) {
gBrowser.moveTabTo(tab, { elementIndex: dropIndex });
gBrowser.moveTabTo(
tab,
{ elementIndex: dropIndex },
dropMetricsContext
);
if (!directionForward) {
dropIndex++;
}
}
} else if (dropBefore) {
gBrowser.moveTabsBefore(movingTabs, dropElement);
gBrowser.moveTabsBefore(
movingTabs,
dropElement,
dropMetricsContext
);
} else {
gBrowser.moveTabsAfter(movingTabs, dropElement);
gBrowser.moveTabsAfter(movingTabs, dropElement, dropMetricsContext);
}
this.#expandGroupOnDrop(draggedTab);
};