1285921 - setEmptyPrefWatchList() doesn't wait for TelemetryEnvironment to initialize

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Georg Fritzsche [:gfritzsche]]

After bug 1273889, Preferences.ignore() on unwatched preferences will lead to errors reported in the console.

> This shows many exceptions like this:
> > "CONSOLE_MESSAGE: (info) 1468205679500	Toolkit.Telemetry	TRACE	TelemetryEnvironment::_stopWatchingPrefs"
> > "CONSOLE_MESSAGE: (error) [JavaScript Error: "Attempt to stop observing a preference "app.feedback.baseURL" that's not being observed" {file: "resource://gre/modules/Preferences.jsm" line: 323}]
> > Preferences.ignore@resource://gre/modules/Preferences.jsm:323:5
> > EnvironmentCache.prototype._stopWatchingPrefs@resource://gre/modules/TelemetryEnvironment.jsm:913:9
> 
> These are explained by bug 1273889, which made `Preferences.ignore()` report
> an error on trying to stop watching unwatched prefs.
> That is just error output though and shouldn't make the test fail.

This shows that our test fix for TelemetryEnvironment not watching prefs by default [1] is racey.
If TelemetryEnvironment is still async initializing, it might not have called `_startWatchingPrefs()` yet [2].
In that case, `_watchPreferences()` (called from `testWatchPreferences()`) tries to .ignore() preferences that are not watched yet at [3].

[1] https://dxr.mozilla.org/mozilla-central/rev/23dc78b7b57e9f91798ea44c242a04e112c37db0/toolkit/components/telemetry/tests/unit/head.js#288
[2] https://dxr.mozilla.org/mozilla-central/rev/23dc78b7b57e9f91798ea44c242a04e112c37db0/toolkit/components/telemetry/TelemetryEnvironment.jsm#769
[3] https://dxr.mozilla.org/mozilla-central/rev/23dc78b7b57e9f91798ea44c242a04e112c37db0/toolkit/components/telemetry/TelemetryEnvironment.jsm#852

Comment 1

[Georg Fritzsche [:gfritzsche]]

One solution is to always wait for TelemetryEnvironment to finish initializing.

Comment 2

[Georg Fritzsche [:gfritzsche]]

Attachment: setEmptyPrefWatchList() doesn't wait for TelemetryEnvironment to initialize

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2232f567f98

Comment 3

[Alessio Placitelli [:Dexter]]

Comment on attachment 8769649 [details] [diff] [review]
setEmptyPrefWatchList() doesn't wait for TelemetryEnvironment to initialize

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

This looks good, do we need to remove the other use of |do_test_pending|?

::: toolkit/components/telemetry/tests/unit/test_TelemetryController_idle.js
@@ +20,1 @@
>    do_test_pending();

Should this be removed, along with |do_test_finished| a few lines below?

Comment 4

[Georg Fritzsche [:gfritzsche]]

Attachment: setEmptyPrefWatchList() doesn't wait for TelemetryEnvironment to initialize

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/9dae91a2c31d
setEmptyPrefWatchList() doesn't wait for TelemetryEnvironment to initialize. r=dexter

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/9dae91a2c31d