944450 - Collect UITelemetry on number of tabs opened

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

I think we need to clear this up a little before this can get written. Do we want to measure the number of tabs a tab is opened, or just count the number of tabs in all windows at the point of the ping?

Comment 1

[Blake Winton (:bwinton) (:☕️)]

We're interested in how many tabs people have, and whether that changes with Australis (because, for instance, background tabs are less intrusive), so the count of number of tabs (perhaps per-window?) at the point of the ping will give us a reasonable answer, I believe.

Comment 2

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

So would collecting an array of tab counts (1 count per window) be sufficient?

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Yes, that sounds good to me.  Thanks!

Comment 4

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Attachment: Patch v1 - for non-Australis

Comment 5

[:Gijs (he/him)]

Comment on attachment 8346059 [details] [diff] [review]
Patch v1 - for non-Australis

Review of attachment 8346059 [details] [diff] [review]:
-----------------------------------------------------------------

r+ whichever way you answer the below question, but I suspect visibleTabs.length is a more sensible number.

::: browser/modules/BrowserUITelemetry.jsm
@@ +517,5 @@
> +    let openTabs = [];
> +    while (winEnumerator.hasMoreElements()) {
> +      let someWin = winEnumerator.getNext();
> +      if (someWin.gBrowser) {
> +        openTabs.push(someWin.gBrowser.tabs.length);

Interesting. Do we want to count hidden panorama groups that even the user might have forgotten about? Maybe we want visibleTabs here? Should we collect both?

Comment 6

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Blake, do you have a position on that? I tend to agree with Gijs that visible tabs is what we want here.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Yeah, visible tabs sounds like what we're interested in.
(having said that, hidden tabs _would_ be another interesting thing to measure. :)

Comment 8

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Attachment: Patch v1.1 - for non-Australis (r+'d by Gijs)

Alright, this records counts for both visible and hidden tabs.

Comment 9

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Landed on Holly as https://hg.mozilla.org/projects/holly/rev/03fdb3303796.

Thanks Gijs!

Comment 10

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Comment on attachment 8346087 [details] [diff] [review]
Patch v1.1 - for non-Australis (r+'d by Gijs)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

None. This gives BrowserUITelemetry the capability of counting the number of visible and hidden tabs per window.


User impact if declined: 

None.


Testing completed (on m-c, etc.): 

Just manual testing at this point.


Risk to taking this patch (and alternatives if risky): 

None.


String or IDL/UUID changes made by this patch:

None.

Comment 11

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Removing Australis:P1 whiteboard tag because these already block an Australis:P1 bug, and being redundant isn't helpful.

Comment 12

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Landed in mozilla-beta as https://hg.mozilla.org/releases/mozilla-beta/rev/73aedac83ac7

Comment 13

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Landed in mozilla-aurora as https://hg.mozilla.org/releases/mozilla-aurora/rev/27909461e31a

Comment 14

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

At least on OS X, with bug 952180 landed, the tabs will always return 1 active tab, and no hidden tabs, making this measurement useless.

So, do we:

1) Not care enough about this probe to change it?
2) Update the probe to count the tabs at some time between browser startup and before browser shutdown? If so, what is that time?

Comment 15

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) - vacationing from Jan 4 - 12 from comment #14)
> At least on OS X, with bug 952180 landed, the tabs will always return 1
> active tab, and no hidden tabs, making this measurement useless.

Why? If you use tab groups, there should be hidden tabs, too... I just ran this:

    let winEnumerator = Services.wm.getEnumerator("navigator:browser");
    let visibleTabs = [];
    let hiddenTabs = [];
    while (winEnumerator.hasMoreElements()) {
      let someWin = winEnumerator.getNext();
      if (someWin.gBrowser) {
        let visibleTabsNum = someWin.gBrowser.visibleTabs.length;
        visibleTabs.push(visibleTabsNum);
        hiddenTabs.push(someWin.gBrowser.tabs.length - visibleTabsNum);
      }
    } console.log(visibleTabs.join(','), hiddenTabs.join(','))

Which produces:

    "47" "6"

Comment 16

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

(In reply to :Gijs Kruitbosch from comment #15)
> (In reply to Mike Conley (:mconley) - vacationing from Jan 4 - 12 from
> comment #14)
> > At least on OS X, with bug 952180 landed, the tabs will always return 1
> > active tab, and no hidden tabs, making this measurement useless.
> 
> Why? If you use tab groups, there should be hidden tabs, too...

Because bug 952180 made it so that we looked for these tabs immediately after first paint, which was too soon - the session hadn't been restored yet. This guaranteed an active tab count of 1 and a hidden tab count of 0.

Bug 956138 landed, however, which fixes this by making it so that we gather measurements after the sessionstore-windows-restored event fires.

Comment 17

[Blake Winton (:bwinton) (:☕️)]

So this is fixed, and I don't need to provide info?  ;)

Comment 18

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Attachment: Patch v1 - for Australis

This is a straight-port from the non-Australis patch.

Comment 19

[:Gijs (he/him)]

Comment on attachment 8359252 [details] [diff] [review]
Patch v1 - for Australis

Review of attachment 8359252 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Thanks for the tireless work on the telemtry bugs, Mike. :-)

Comment 20

[Mike Conley (:mconley) (:⚙️) (Wayyyy behind on needinfos)]

Thanks for the reviews! :D

Landed on fx-team as https://hg.mozilla.org/integration/fx-team/rev/849d9644387c

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/849d9644387c