Bug 1340842 - Add FX_TAB_CLOSE_TIME_ probes to measure how long it takes for tabs to close. r?Mossop, data-review=liuche
We're adding histograms for both animated tab closing and non-animated tab closing to
avoid a needlessly bi-modal histogram.
MozReview-Commit-ID: J4MzsiwaLcT
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2550,16 +2550,33 @@
[]
</field>
<method name="removeTab">
<parameter name="aTab"/>
<parameter name="aParams"/>
<body>
<![CDATA[
+ // We collect Telemetry on how long it takes to close a tab.
+ // Sometimes, tabs animate closed, and sometimes they don't, and
+ // it doesn't really make sense to put timings for both cases in the
+ // same bucket.
+ //
+ // We decide later on whether or not we're actually going to animate
+ // the tab closed. In order to capture as much of the time we care
+ // about with these stopwatches, but also in order to avoid any
+ // unnecessary work (since calculating whether or not we're going
+ // to do the animation might involved a style flush), we start
+ // stopwatches for both the animating and non-animating case. When
+ // we decide that we're animating, we cancel the non-animating case,
+ // and vice-versa. Then, in _endRemoveTab, we "finish" both
+ // stopwatches, which is a no-op for cancelled stopwatches.
+ TelemetryStopwatch.start("FX_TAB_CLOSE_TIME_ANIM_MS", aTab);
+ TelemetryStopwatch.start("FX_TAB_CLOSE_TIME_NO_ANIM_MS", aTab);
+
if (aParams) {
var animate = aParams.animate;
var byMouse = aParams.byMouse;
var skipPermitUnload = aParams.skipPermitUnload;
}
window.maybeRecordAbandonmentTelemetry(aTab, "tabClosed");
@@ -2568,36 +2585,44 @@
if (!animate &&
aTab.closing) {
this._endRemoveTab(aTab);
return;
}
var isLastTab = (this.tabs.length - this._removingTabs.length == 1);
- if (!this._beginRemoveTab(aTab, null, null, true, skipPermitUnload))
+ if (!this._beginRemoveTab(aTab, null, null, true, skipPermitUnload)) {
+ TelemetryStopwatch.cancel("FX_TAB_CLOSE_TIME_ANIM_MS", aTab);
+ TelemetryStopwatch.cancel("FX_TAB_CLOSE_TIME_NO_ANIM_MS", aTab);
return;
+ }
if (!aTab.pinned && !aTab.hidden && aTab._fullyOpen && byMouse)
this.tabContainer._lockTabSizing(aTab);
else
this.tabContainer._unlockTabSizing();
if (!animate /* the caller didn't opt in */ ||
isLastTab ||
aTab.pinned ||
aTab.hidden ||
this._removingTabs.length > 3 /* don't want lots of concurrent animations */ ||
aTab.getAttribute("fadein") != "true" /* fade-in transition hasn't been triggered yet */ ||
window.getComputedStyle(aTab).maxWidth == "0.1px" /* fade-in transition hasn't moved yet */ ||
!Services.prefs.getBoolPref("browser.tabs.animate")) {
+ // We're not animating, so we can cancel the animation stopwatch.
+ TelemetryStopwatch.cancel("FX_TAB_CLOSE_TIME_ANIM_MS", aTab);
this._endRemoveTab(aTab);
return;
}
+ // We're animating, so we can cancel the non-animation stopwatch.
+ TelemetryStopwatch.cancel("FX_TAB_CLOSE_TIME_NO_ANIM_MS", aTab);
+
this.tabContainer._handleTabTelemetryStart(aTab);
this._blurTab(aTab);
aTab.style.maxWidth = ""; // ensure that fade-out transition happens
aTab.removeAttribute("fadein");
setTimeout(function(tab, tabbrowser) {
if (tab.parentNode &&
@@ -2844,16 +2869,24 @@
// Release the browser in case something is erroneously holding a
// reference to the tab after its removal.
this._tabForBrowser.delete(aTab.linkedBrowser);
aTab.linkedBrowser = null;
panel.remove();
+ // closeWindow might wait an arbitrary length of time if we're supposed
+ // to warn about closing the window, so we'll just stop the tab close
+ // stopwatches here instead.
+ TelemetryStopwatch.finish("FX_TAB_CLOSE_TIME_ANIM_MS", aTab,
+ true /* aCanceledOkay */);
+ TelemetryStopwatch.finish("FX_TAB_CLOSE_TIME_NO_ANIM_MS", aTab,
+ true /* aCanceledOkay */);
+
if (aCloseWindow)
this._windowIsClosing = closeWindow(true, window.warnAboutClosingWindow);
]]>
</body>
</method>
<method name="_blurTab">
<parameter name="aTab"/>
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -5025,16 +5025,34 @@
},
"FX_GESTURE_COMPRESS_SNAPSHOT_OF_PAGE": {
"expires_in_version": "50",
"kind": "exponential",
"high": 1000,
"n_buckets": 30,
"description": "Firefox: Time taken to kick off image compression of the canvas that will be used during swiping through history (ms)."
},
+ "FX_TAB_CLOSE_TIME_ANIM_MS": {
+ "alert_emails": ["mconley@mozilla.com", "hkirschner@mozilla.com"],
+ "bug_numbers": [1340842],
+ "expires_in_version": "60",
+ "kind": "exponential",
+ "high": 10000,
+ "n_buckets": 50,
+ "description": "Firefox: Time taken from the point of closing a tab (with animation), to the browser element being removed from the DOM. (ms)."
+ },
+ "FX_TAB_CLOSE_TIME_NO_ANIM_MS": {
+ "alert_emails": ["mconley@mozilla.com", "hkirschner@mozilla.com"],
+ "bug_numbers": [1340842],
+ "expires_in_version": "60",
+ "kind": "exponential",
+ "high": 10000,
+ "n_buckets": 50,
+ "description": "Firefox: Time taken from the point of closing a tab (without animation) to the browser element being removed from the DOM. (ms)."
+ },
"FX_TAB_ANIM_OPEN_PREVIEW_FRAME_INTERVAL_MS": {
"expires_in_version": "never",
"kind": "exponential",
"low": 7,
"high": 500,
"n_buckets": 50,
"description": "Average frame interval during tab open animation of about:newtab (preview=on), when other tabs are unaffected"
},