1292337 - browser.engagement.tab_open_event_count Counts restored tab opening

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[John Dorlus [:Silne30]]

Environment:
Mac OSX 10.11.6
Firefox Nightly 51.0a1 (2016-08-04)


Steps to Reproduce:
1.) Open Nightly with new profile.
2.) Nightly should open with default two tabs.
3.) Open 2 more tabs and navigate to random pages on the tabs.
4.) Go back to the first tab and open about:telemetry
5.) Check the scalar browser.engagement.tab_open_event_count. Value should be 2.
6.) Close Nightly via the toolbar.
7.) Open nightly (this time allow for the same profile to be used).
8.) One tab should open. On this page, there should be a "Restore Previous Session" button. Click that button.
9.) All previous tabs will be opened.
10.) Go to the about:telemetry page and refresh the page.

Expected Result: The value for  browser.engagement.tab_open_event_count should still be 2 since we did not open any new tabs and restored tabs should not be counted.

Actual Result: browser.engagement.tab_open_event_count shows 3

Comment 1

[John Dorlus [:Silne30]]

Created a bug to track this. If it's not a bug, we will just mark it Not a bug.

Comment 2

[Georg Fritzsche [:gfritzsche]]

I'm not sure if we can - or should - filter out explicit "restore previous session" actions.
I'll leave this for Alessio who has more context.
At the least we should make the documentation more explicit about those scenarios.

For comparison, does this work as expected with the session restore after crashes?

Comment 3

[Alessio Placitelli [:Dexter]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> I'm not sure if we can - or should - filter out explicit "restore previous
> session" actions.

Ideally, we would want to filter these out. I'm not sure if this is possible without messing with the SessionStore internals nor worth it, as I'd imagine these cases are far less frequent than opening Firefox with the tabs from the last session open or without any restoration at all.

> I'll leave this for Alessio who has more context.
> At the least we should make the documentation more explicit about those
> scenarios.

Indeed, this edge case needs to be documented.

> For comparison, does this work as expected with the session restore after
> crashes?

It works consistently: tabs opened when "manually" restoring a session (after a crash or through about:home) are counted.

Comment 4

[Alessio Placitelli [:Dexter]]

Brendan, a new edge case was found for the windows/tabs probes introduced by bug 1271304 (tab_open_event_count, window_open_event_count).

Basically, when manually issuing a session restoration through the about:home page (see comment 0) or after a crash, the number of restored tabs/windows are being counted in the tab_open_event_count and window_open_event_count probes.

This should be far less frequent than opening Firefox with the tabs from the last session open or without any restoration at all, so I'm not sure this is a problem.

Would it be fine for you if we documented this in the probe? I'm not sure we can easily tackle this edge case.

Comment 5

[brendan c]

I think this behavior is ok. Are the manually restored tabs counted in 
FX_SESSION_RESTORE_NUMBER_OF_TABS_RESTORED
FX_SESSION_RESTORE_NUMBER_OF_WINDOWS_RESTORED ?
(Mentioned by Georg here: https://bugzilla.mozilla.org/show_bug.cgi?id=1271304#c3 )

It's definitely a gray area. In some ways it's a signal of user intent, b/c the user did choose to manually open a bunch of tabs; on the other hand it kind of signals using the tabs to save state.

I don't really recall what the UX for tab restoration after a crash is like (I use a session manager add on). I guess if there is a crash and users are prompted to restore tabs from the crashed session in a dialogue upon opening, we wouldn't really want to count that as a regular tab_open_event, but rather as a special crash_restored_tab_open_event.

Overall, I think this is probably a genuine bug, but probably low priority right now -- I think that ultimately we would want to count those tab opens as a separate category, but counting them as regular tab_open_events will be fine for now, and I think there are more important things to worry about. I would suggest leaving this bug open for now and we can revisit next quarter, but imo ATM getting new measures in place takes priority over getting the last 1% of edge cases perfect for existing measures.

Thanks Alessio!

Comment 6

[Alessio Placitelli [:Dexter]]

(In reply to brendan c from comment #5)
> I think this behavior is ok. Are the manually restored tabs counted in 
> FX_SESSION_RESTORE_NUMBER_OF_TABS_RESTORED
> FX_SESSION_RESTORE_NUMBER_OF_WINDOWS_RESTORED ?
> (Mentioned by Georg here:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1271304#c3 )

Yes!

> It's definitely a gray area. In some ways it's a signal of user intent, b/c
> the user did choose to manually open a bunch of tabs; on the other hand it
> kind of signals using the tabs to save state.
> 
> I don't really recall what the UX for tab restoration after a crash is like
> (I use a session manager add on). I guess if there is a crash and users are
> prompted to restore tabs from the crashed session in a dialogue upon
> opening, we wouldn't really want to count that as a regular tab_open_event,
> but rather as a special crash_restored_tab_open_event.

Yeah, users are prompted to restore the tabs as soon as the browser starts.

> Overall, I think this is probably a genuine bug, but probably low priority
> right now -- I think that ultimately we would want to count those tab opens
> as a separate category, but counting them as regular tab_open_events will be
> fine for now, and I think there are more important things to worry about. I
> would suggest leaving this bug open for now and we can revisit next quarter,
> but imo ATM getting new measures in place takes priority over getting the
> last 1% of edge cases perfect for existing measures.
> 
> Thanks Alessio!

Cool, let's leave it open for now and come back to it later on.

Thank you Brendan!

Comment 7

[Georg Fritzsche [:gfritzsche]]

(In reply to Alessio Placitelli [:Dexter] from comment #6)
> > Overall, I think this is probably a genuine bug, but probably low priority
> > right now -- I think that ultimately we would want to count those tab opens
> > as a separate category, but counting them as regular tab_open_events will be
> > fine for now, and I think there are more important things to worry about. I
> > would suggest leaving this bug open for now and we can revisit next quarter,
> > but imo ATM getting new measures in place takes priority over getting the
> > last 1% of edge cases perfect for existing measures.
> > 
> > Thanks Alessio!
> 
> Cool, let's leave it open for now and come back to it later on.

I'd rather call this out clearly in the probe documentation here now, close the bug and revisit possible improvements in future bugs (based on needs from working with the data).

Comment 8

[Alessio Placitelli [:Dexter]]

Attachment: bug1292337.patch

Comment 9

[John Dorlus [:Silne30]]

Adding link to the testrail case that covers this case. The case will need to be changed as this behavior will be viewed as expected for now.

Comment 10

[Alessio Placitelli [:Dexter]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff8ebe727b0fd14d898e76ed1ea584a07bbac934
Bug 1292337 - browser.engagement.tab_open_event_count Counts restored tab opening. r=gfritzsche

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/ff8ebe727b0f