1827766 - Rename the TelemetryEvent class to better reflect its broader scope

Status: NEW

(Firefox :: Address Bar, task, P1)

Description

[Drew Willcoxon :adw]

TelemetryEvent does more than record telemetry events: It also notifies providers of engagements, abandonments, and the start of search sessions. After bug 1827762 this is especially important because UrlbarProvider.onEngagement() is now the preferred way to handle result engagements that require actions other than loading a URL.

Additionally, Wil is working on exposure telemetry and he'll be modifying TelemetryEvent to keep a list of all results that were shown to the user during a search session. It's natural for TelemetryEvent to be the component that keeps this list since it's the place that records the start and end of sessions.

To better reflect this broader scope, I propose the following renames:

• TelemetryEvent -> SearchSession
• record() -> end()

didEnd() might be a better name because end() makes it sound like the class is in charge of ending the session rather than being the entry point for reacting to the fact it did end. Similarly, didStart() might be better than start().

Marco and Dão, wdyt?

(This bug is spun out from bug 1827762, see comments in D174941)

Comment 1

[Drew Willcoxon :adw]

NI'ing Daisuke too, please see comment 0

Comment 2

[Marco Bonardo [:mak]]

I think the renaming makes sense, its scope changed from when it was created
I don't have a strong opinion over those method names. I think start is not problematic. For end maybe a more verbose .concludeAndNotify?
what about discard, will it remain the same?

Comment 3

[Dão Gottwald [:dao]]

(In reply to Drew Willcoxon :adw from comment #0)

To better reflect this broader scope, I propose the following renames:

• TelemetryEvent -> SearchSession

I'm not particularly thrilled about this suggestion, partly because of the concern that Mark raised. Perhaps also worth pointing out that most of our related classes start with Urlbar. How about UrlbarSession? UrlbarSearchSession?

Comment 4

[Drew Willcoxon :adw]

UrlbarSession is better, I like it! (For posterity, Mark suggested "search" might be mistaken for search service type stuff, a good point)

It would be nice to move it to its own file too IMO.

(In reply to Marco Bonardo [:mak] from comment #2)

I don't have a strong opinion over those method names. I think start is not problematic. For end maybe a more verbose .concludeAndNotify?

endAndNotify?

what about discard, will it remain the same?

I can't think of a better name and it seems OK to me but definitely open to ideas.

Comment 5

[Daisuke Akatsuka (:daisuke)]

UrlbarSession sounds good to me too!
And also, I agree that implementing in a new JS file.