Closed Bug 1373728 Opened 3 years ago Closed 3 years ago

Record Telemetry about the number of Pinned Tabs

Categories

(Firefox :: Tabbed Browser, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: chutten|PTO, Assigned: chutten|PTO)

References

Details

Attachments

(1 file)

My quick look around Histograms.json and Scalars.yaml suggests we don't actually record how many pinned tabs a user has.

Since the number of pinned tabs drastically changes the session restore picture (and possibly the allocation of content processes for e10s-multi), I think it'd be a good idea to record how many a user has.

Perhaps count them at session restore?
Probably into a uint scalar.
Probably opt-out.
Probably under browser.engagement.
See Also: → 1361855
Given the impact this has on how a user experiences startup performance, I think this is very important. In particular, if we can identify the number of heavy users who have pinned tabs and that correlation is high, I would greatly value optimizing Firefox's startup behaviour around pinned tabs.
Severity: normal → major
Priority: -- → P1
Assignee: nobody → chutten
Blocks: 1357749
Status: NEW → ASSIGNED
According to :Yoric, at any point after [1] the initial state (documentation[2]) is set and can be read.

I'm thinking an idleDispatch and a scalar would get the job done from there.

[1]: http://searchfox.org/mozilla-central/source/browser/components/sessionstore/nsSessionStartup.js#150
[2]: https://wiki.mozilla.org/Firefox/session_restore#The_structure_of_sessionstore.js
Comment on attachment 8882313 [details]
bug 1373728 - Count how many pinned tabs are restored in a session

https://reviewboard.mozilla.org/r/153406/#review158602

data-r=me
Attachment #8882313 - Flags: review?(benjamin) → review+
Comment on attachment 8882313 [details]
bug 1373728 - Count how many pinned tabs are restored in a session

https://reviewboard.mozilla.org/r/153406/#review158608

This looks good with the fix below, thanks Chris! Since this can have startup impact, do you need to make Florian/people working on Quantum aware?

::: browser/components/sessionstore/nsSessionStartup.js:167
(Diff revision 1)
> +        return winAcc + win.tabs.reduce((tabAcc, tab) => {
> +          return tabAcc + (tab.pinned ? 1 : 0);
> +        }, 0);
> +      }, 0);
> +      Services.telemetry.scalarSet("browser.engagement.restored_pinned_tabs_count", pinnedTabCount);
> +    });

Mh, I think you might want to specify a timeout for *idleDispatchToMainThread* (see [here](http://searchfox.org/mozilla-central/rev/152c0296f8a10f81185ee88dfb4114ec3882b4c6/xpcom/threads/nsIThreadManager.idl#108)), otherwise you risk never executing this code. I'm not sure what a reasonable timeout would be here though.
Attachment #8882313 - Flags: review?(alessio.placitelli) → review+
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e6b552124c9c
Count how many pinned tabs are restored in a session r=bsmedberg,Dexter
https://hg.mozilla.org/mozilla-central/rev/e6b552124c9c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
This probe is about to expire in a couple of weeks, so we need to decide if we want to keep it or if we can drop it.

So what does the data show? Mostly zero:

Two nines (99%) of sessions on beta restore with 0 pinned tabs: https://mzl.la/2pFS62j
It's only one nine (93%) on Nightly: https://mzl.la/2pG74W9
A quick look at main_summary on sql.tmo shows a similar ratio (98%) for release: https://sql.telemetry.mozilla.org/queries/52238/source

So, :canuckistani, is there value in renewing this probe, or should we remove it?
Flags: needinfo?(jgriffiths)
(In reply to Chris H-C :chutten from comment #9)
> This probe is about to expire in a couple of weeks, so we need to decide if
> we want to keep it or if we can drop it.
> 
> So what does the data show? Mostly zero:
> 
> Two nines (99%) of sessions on beta restore with 0 pinned tabs:
> https://mzl.la/2pFS62j
> It's only one nine (93%) on Nightly: https://mzl.la/2pG74W9
> A quick look at main_summary on sql.tmo shows a similar ratio (98%) for
> release: https://sql.telemetry.mozilla.org/queries/52238/source
> 
> So, :canuckistani, is there value in renewing this probe, or should we
> remove it?

There might be value in renewing the probe. Your analysis is a little naive, we might care a lot about 2% of users if they load a lot of uris and/or make a lot of searches. Is there a quick way to find that out?
Flags: needinfo?(jgriffiths) → needinfo?(chutten)
If you're willing to define the terms "a lot of uris" and "a lot of searches" then yes. sql.tmo can get you the main pings of clients restoring >0 pinned tabs, and from that we can sum their uri loads and search counts.

I expect them to load more uris than the 0-pinned-tab cohort (and not just because restoring pinned tabs loads uris) because knowing that pinned tabs even exist is likely an indicator that they are heavier users. The question then becomes whether they are "more heavy than expected" given that they've already shown a command of the browser UI beyond that of large subpopulations of our users.

...and that doesn't seem like an easy thing to determine.

I'm in favour of removing the probe. We've learned what we can from it. We've also learned what it can't answer.

But renewing it is easy, so I'm not against renewal. I leave it to you.
Flags: needinfo?(chutten) → needinfo?(jgriffiths)
(In reply to Chris H-C :chutten from comment #11)
> If you're willing to define the terms "a lot of uris" and "a lot of
> searches" then yes. sql.tmo can get you the main pings of clients restoring
> >0 pinned tabs, and from that we can sum their uri loads and search counts.
> 
> I expect them to load more uris than the 0-pinned-tab cohort (and not just
> because restoring pinned tabs loads uris) because knowing that pinned tabs
> even exist is likely an indicator that they are heavier users. The question
> then becomes whether they are "more heavy than expected" given that they've
> already shown a command of the browser UI beyond that of large
> subpopulations of our users.
> 
> ...and that doesn't seem like an easy thing to determine.
> 
> I'm in favour of removing the probe. We've learned what we can from it.
> We've also learned what it can't answer.
> 
> But renewing it is easy, so I'm not against renewal. I leave it to you.

Let's renew then.
Flags: needinfo?(jgriffiths)
You need to log in before you can comment on or make changes to this bug.