1408433 - Telemetry Experiments no longer needs to check toolkit.telemetry.enabled

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Chris H-C :chutten]

After bug 1406391, toolkit.telemetry.enabled will no longer be able to be changed. Already all UI allowing users to change it has been removed.

browser/experiments/Experiments.jsm no longer needs to observe or respond to changes in this pref. Code concerning these actions should be removed.

The test toolkit/mozapps/extensions/test/browser/browser_experiments.js can also have its toolkit.telemetry.enabled-related code removed. This might break some tests in this suite, so be sure to check that they still work after the removal.

Comment 1

[Georg Fritzsche [:gfritzsche]]

Note that Experiments.jsm still needs to decide on startup whether it will enable (pre-release) or disable (release).

Comment 2

[Chris H-C :chutten]

Attachment: bug 1408433 - Remove toolkit.telemetry.enabled code from Experiments

Review commit: https://reviewboard.mozilla.org/r/192586/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192586/

Comment 3

[Alessio Placitelli [:Dexter]]

Comment on attachment 8921566 [details]
bug 1408433 - Remove toolkit.telemetry.enabled code from Experiments

https://reviewboard.mozilla.org/r/192586/#review197954

::: browser/experiments/Experiments.jsm:396
(Diff revision 1)
>  
>    init() {
>      this._shutdown = false;
>      configureLogging();
>  
> -    gExperimentsEnabled = gPrefs.getBoolPref(PREF_ENABLED, false) && TelemetryUtils.isTelemetryEnabled;
> +    gExperimentsEnabled = gPrefs.getBoolPref(PREF_ENABLED, false);

I think this should be changed to `gExperimentsEnabled = gPrefs.getBoolPref(PREF_ENABLED, false) && Services.telemetry.canRecordExtended;` since we don't want experiments to run with telemetry extended disabled (that's the reason why it was checking the pref before).

I know that nobody should be really running experiments anymore, but let's preserve this  behaviour while the code is still around.

Since `canRecordExtended` is only set once, we don't  really need to watch for the pref changes: we're good to remove the rest of the things you removed!

Comment 4

[Chris H-C :chutten]

Comment on attachment 8921566 [details]
bug 1408433 - Remove toolkit.telemetry.enabled code from Experiments

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/192586/diff/1-2/

Comment 5

[Alessio Placitelli [:Dexter]]

Comment on attachment 8921566 [details]
bug 1408433 - Remove toolkit.telemetry.enabled code from Experiments

https://reviewboard.mozilla.org/r/192586/#review198462

Looks good now, thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bfd4b0927dc
Remove toolkit.telemetry.enabled code from Experiments r=Dexter

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 8

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/mozilla-central/rev/ad08d9064a7d while backing out bug 1406391

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/633443ef47bb
Remove toolkit.telemetry.enabled code from Experiments r=Dexter

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/633443ef47bb

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/633443ef47bb