Fix racy initialization of the gfx.features.compositor field

RESOLVED FIXED in Firefox 42

Status

()

Toolkit
Telemetry
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

unspecified
mozilla43
Points:
---

Firefox Tracking Flags

(firefox42 fixed, firefox43 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Created attachment 8648339 [details] [diff] [review]
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.
Attachment #8648339 - Flags: review?(gfritzsche)
Created attachment 8648340 [details] [diff] [review]
fix

correct patch
Attachment #8648339 - Attachment is obsolete: true
Attachment #8648339 - Flags: review?(gfritzsche)
Attachment #8648340 - Flags: review?(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.
Attachment #8648340 - Flags: review?(gfritzsche)
Created attachment 8649704 [details] [diff] [review]
v2

Okay, simpler patch then.
Attachment #8648340 - Attachment is obsolete: true
Attachment #8649704 - Flags: review?(gfritzsche)
Attachment #8649704 - Flags: review?(gfritzsche) → review+
https://hg.mozilla.org/mozilla-central/rev/c411087193c8
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
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]:
Attachment #8649704 - Flags: approval-mozilla-aurora?
status-firefox42: --- → affected
Comment on attachment 8649704 [details] [diff] [review]
v2

Fix the important telemetry issue, taking it in 42.
Attachment #8649704 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.