1194932 - Fix racy initialization of the gfx.features.compositor field

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

TelemetryEnvironment waits for an observer notification to update the current compositor type under system.gfx.features. However, if the compositor gets created before we add the observer, we never register the compositor type.

I'm hoping this is the reason behind so many sessions (~40%) reporting a "none" compositor type. I can reproduce it locally.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

This patch tries to grab gfx featues when setup() runs, and then only adds the compositor observer if a compositor hasn't been created yet.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

correct patch

Comment 3

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8648340 [details] [diff] [review]
fix

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

::: toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ +818,5 @@
> +      // update the graphics features object again.
> +      let watch = {
> +        observe: (aSubject, aTopic, aData) => {
> +          this._updateGraphicsFeatures();
> +          Services.obs.removeObserver(watch, COMPOSITOR_CREATED_TOPIC);

What about unregistering it on shutdown?
I think we should still unregister this in _removeObservers.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: v2

Okay, simpler patch then.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c411087193c8

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8649704 [details] [diff] [review]
v2

Approval Request Comment
[Feature/regressing bug #]: Graphics telemetry
[User impact if declined]: None
[Describe test coverage new/current, TreeHerder]: Landed on Nightly
[Risks and why]: No risk. Aurora has new telemetry probes to give us very useful information on the graphics features that our users have enabled. Unfortunately, there was a bug in the probe that makes it misreport about 40% of the time. I'd like to uplift this fix so that we get more reliable data.
[String/UUID change made/needed]:

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8649704 [details] [diff] [review]
v2

Fix the important telemetry issue, taking it in 42.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/940bf65a058d