Collect Telemetry on when in a tabs lifetime a tab switch spinner is displayed

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
2 months ago
a month ago

People

(Reporter: mconley, Unassigned)

Tracking

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

Tab switch spinners can occur anytime that the content process hasn't given us a layer tree in enough time to composite them in response to a user switching tabs.

I want to know when, in a tabs lifetime, those spinners are likely to occur. I'm going to break a tabs lifetime down into three discrete chunks:

1) Tab is new* and has never been seen
2) Tab is not new* and has never been seen
3) Tab is one we've seen before, and we're switching back to it.

My hypothesis is that we tend to show a lot of spinners in case (1). If so, I suspect there are ways we can address those.

* newness is the fuzzy part of this. I need to settle on a solid definition of newness - perhaps as a measure of time since the tab was created.
Duplicate of this bug: 1338564
Comment hidden (mozreview-request)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=94aa630f53e4
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8843334 - Flags: review?(liuche)
Hey liuche, got time for a quick data-review?
Flags: needinfo?(liuche)

Comment 8

2 months ago
mozreview-review
Comment on attachment 8844118 [details]
Bug 1342464 - Add regression tests for the FX_TAB_SWITCH_SPINNER_TYPE probe.

https://reviewboard.mozilla.org/r/117652/#review119648

::: browser/base/content/test/tabs/browser_tabSpinnerTypeProbe.js:156
(Diff revision 1)
> + * probe works. This means that we show a spinner for a tab that we've never
> + * seen before, and not enough time has passed since its creation that we consider
> + * it "old" (See the NEWNESS_THRESHOLD constant in the async tabswitcher for
> + * the exact definition).
> + */
> +add_task(function* test_unseenOld_spinner_type_probe() {

This function name seems wrong.
Attachment #8844118 - Flags: review?(dtownsend) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8843334 - Flags: review?(liuche)

Comment 12

2 months ago
mozreview-review
Comment on attachment 8843334 [details]
Bug 1342464 - Collect Telemetry on when a tab switch spinner is shown. , data-review=liuche

https://reviewboard.mozilla.org/r/117100/#review119780

::: dom/interfaces/base/nsITabParent.idl:59
(Diff revision 4)
>     * If aForward is false, navigate to the last focusable element or document.
>     */
>    void navigateByKey(in bool aForward, in bool aForDocumentNavigation);
>  
>    readonly attribute boolean hasContentOpener;
> +  readonly attribute boolean hasPresented;

Can you comment that this means we've previously received layers for this tab when switching to it.

::: toolkit/components/telemetry/Histograms.json:5101
(Diff revision 4)
> +    "expires_in_version": "60",
> +    "kind": "categorical",
> +    "bug_numbers": [1301104],
> +    "alert_emails": ["mconley@mozilla.com"],
> +    "releaseChannelCollection": "opt-out",
> +    "description": "Firefox: If the spinner interstitial displays during tab switching, record if the tab being switched to has been seen before. If not, record if the tab is 'old' (> 1000ms since creation) or 'new'.",

Might want to make clear that "seen" means that it's content was visible.
Attachment #8843334 - Flags: review?(wmccloskey) → review+

Comment 13

2 months ago
mozreview-review
Comment on attachment 8843334 [details]
Bug 1342464 - Collect Telemetry on when a tab switch spinner is shown. , data-review=liuche

https://reviewboard.mozilla.org/r/117100/#review120188

Clear description (and +1 to billm's suggestion), has expiration - r+ (data-review only)
Attachment #8843334 - Flags: review?(liuche) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

2 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aae7b99fbe2f
Collect Telemetry on when a tab switch spinner is shown. r=billm,liuche, data-review=liuche
https://hg.mozilla.org/integration/autoland/rev/3313b3efefea
Add regression tests for the FX_TAB_SWITCH_SPINNER_TYPE probe. r=mossop

Comment 17

2 months ago
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d1c9574f8a26
Convert line endings to satisfy eslint

Comment 18

2 months ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/f8d82fa1218c
More lint for the lint gods. r=Aryx

Comment 19

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/aae7b99fbe2f
https://hg.mozilla.org/mozilla-central/rev/3313b3efefea
https://hg.mozilla.org/mozilla-central/rev/d1c9574f8a26
https://hg.mozilla.org/mozilla-central/rev/f8d82fa1218c
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Updated

a month ago
Flags: needinfo?(liuche)
You need to log in before you can comment on or make changes to this bug.