Closed Bug 944450 Opened 6 years ago Closed 6 years ago

Collect UITelemetry on number of tabs opened

Categories

(Firefox :: Tabbed Browser, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 29
Tracking Status
firefox27 --- fixed
firefox28 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

(Whiteboard: [fixed-in-holly][good first verify])

Attachments

(2 files, 1 obsolete file)

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?
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.
So would collecting an array of tab counts (1 count per window) be sufficient?
Flags: needinfo?(bwinton)
Yes, that sounds good to me.  Thanks!
Flags: needinfo?(bwinton)
Attached patch Patch v1 - for non-Australis (obsolete) — Splinter Review
Attachment #8346059 - Flags: review?(gijskruitbosch+bugs)
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?
Attachment #8346059 - Flags: review?(gijskruitbosch+bugs) → review+
Blake, do you have a position on that? I tend to agree with Gijs that visible tabs is what we want here.
Flags: needinfo?(bwinton)
Yeah, visible tabs sounds like what we're interested in.
(having said that, hidden tabs _would_ be another interesting thing to measure. :)
Flags: needinfo?(bwinton)
Alright, this records counts for both visible and hidden tabs.
Attachment #8346059 - Attachment is obsolete: true
Attachment #8346087 - Flags: review+
Landed on Holly as https://hg.mozilla.org/projects/holly/rev/03fdb3303796.

Thanks Gijs!
Whiteboard: [Australis:P1] → [Australis:P1][fixed-in-holly]
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.
Attachment #8346087 - Flags: approval-mozilla-beta?
Attachment #8346087 - Flags: approval-mozilla-aurora?
Removing Australis:P1 whiteboard tag because these already block an Australis:P1 bug, and being redundant isn't helpful.
Whiteboard: [Australis:P1][fixed-in-holly] → [fixed-in-holly]
Attachment #8346087 - Flags: approval-mozilla-beta?
Attachment #8346087 - Flags: approval-mozilla-beta+
Attachment #8346087 - Flags: approval-mozilla-aurora?
Attachment #8346087 - Flags: approval-mozilla-aurora+
Whiteboard: [fixed-in-holly] → [fixed-in-holly][good first verify]
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?
Flags: needinfo?(bwinton)
(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"
(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.
So this is fixed, and I don't need to provide info?  ;)
Flags: needinfo?(bwinton)
This is a straight-port from the non-Australis patch.
Attachment #8359252 - Flags: review?(gijskruitbosch+bugs)
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. :-)
Attachment #8359252 - Flags: review?(gijskruitbosch+bugs) → review+
Thanks for the reviews! :D

Landed on fx-team as https://hg.mozilla.org/integration/fx-team/rev/849d9644387c
Whiteboard: [fixed-in-holly][good first verify] → [fixed-in-holly][good first verify][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/849d9644387c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-holly][good first verify][fixed-in-fx-team] → [fixed-in-holly][good first verify]
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.