1012526 - UITour.jsm only registers with UITelemetry when it's lazily imported on-demand

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

So, UITour.jsm is lazily imported on-demand whenever:
* A page uses the mozUITour API
* The person uses the Tour menuitem from the Help menu

And as part of the initialization process of UITour, it registers with UITelemetry.jsm a simple measure function. This is a problem, because we *always* want that to be registered with UITelemetry, so the data from UITour can be collected.

Note: This *only* affects seenPageIDs, as that's the only telemetry data collected by UITour.jsm

I think this would explain why we may not be seeing seenPageIDs not show up in telemetry as often as expected.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Gavin: Will need this fixed ASAP, as it affects telemetry collection of data that we're trying to analyse *now*. Can you add this to the priority backlog?

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Marco, can you please add this to the current iteration?

Comment 3

[Marco Mucci [:MarcoM]]

Added to Iteration 32.2

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

This also lets UITour.jsm remain being lazy-loaded.

Sadly, I don't think we can test this. If it were testable via xpcshell, it'd be easy - as we get a new environment for each test. But we can only test this via mochitest... in which case we can't guarantee that some other test hasn't already caused UITour.jsm to be loaded beforehand :\

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8425170 [details] [diff] [review]
Patch v1

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

Bah, sorry for not catching this the first time. :( This looks good - just some minor things.

::: browser/modules/BrowserUITelemetry.jsm
@@ +171,5 @@
>    init: function() {
>      UITelemetry.addSimpleMeasureFunction("toolbars",
>                                           this.getToolbarMeasures.bind(this));
> +    // Ensure that UITour.jsm remains lazy-loaded, yet always registers its
> +    // simple measure functiojn with UITelemetry.

Nit - typo: "functiojn" -> "function"

::: browser/modules/UITour.jsm
@@ +7,5 @@
>  this.EXPORTED_SYMBOLS = ["UITour"];
>  
>  const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
>  
> +Cu.reportError("UITour.jsm loaded");

Please remove before landing.

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/fx-team/rev/f1bd861d8acf

Comment 7

[Carsten Book [:Tomcat]]

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

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 952568
User impact if declined: No user impact. Impact is lack of/inconsistent Telemetry data. Unable to analyse anything but immediate (within 1 hour) impact of whats-new tour page on usage of UI, via Telemetry.
Testing completed (on m-c, etc.): Unable to automate test. Manually tested.
Risk to taking this patch (and alternatives if risky): Minimal - can only impact telemetry collection of what is already broken. Change to fix that is minimal low-risk change.
String or IDL/UUID changes made by this patch: None

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7d6cefca7432
https://hg.mozilla.org/releases/mozilla-beta/rev/b19932497b46

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/b19932497b46