Closed Bug 1342464 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: mconley, Unassigned)

References

Details

Attachments

(2 files)

Bug 1342464 - Collect Telemetry on when a tab switch spinner is shown. , data-review=liuche
59 bytes, text/x-review-board-request
billm
: review+
Details
Bug 1342464 - Add regression tests for the FX_TAB_SWITCH_SPINNER_TYPE probe.
59 bytes, text/x-review-board-request
mossop
: review+
Details 
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.
Attachment #8843334 - Flags: review?(liuche)
Hey liuche, got time for a quick data-review?
Flags: needinfo?(liuche)
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+
Attachment #8843334 - Flags: review?(liuche)
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 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+
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
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d1c9574f8a26
Convert line endings to satisfy eslint
Flags: needinfo?(liuche)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: