1218842 - TelemetryEnvironment submits inactive GMP plugins

Status: VERIFIED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Georg Fritzsche [:gfritzsche]]

We should filter out disabled GMP plugins here by also checking for |plugin.disabled|:
https://dxr.mozilla.org/mozilla-central/rev/6c7c983bce46a460c2766fbdd73283f6d2b03a69/toolkit/components/telemetry/TelemetryEnvironment.jsm#624

We should check that the Telemetry unit tests still run through properly after this change:
mach xpcshell-test toolkit/components/telemetry/tests/unit/

Comment 1

[Alessio Placitelli [:Dexter]]

Attachment: bug1218842.patch

Comment 2

[Alessio Placitelli [:Dexter]]

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

Comment 3

[Alessio Placitelli [:Dexter]]

https://hg.mozilla.org/integration/fx-team/rev/e402ef99492dc81b1b1f28d04a6baa96de0ec3d2
Bug 1218842 - Don't submit inactive or invalid GMP plugins in TelemetryEnvironment. r=gfritzsche

Comment 4

[Carsten Book [:Tomcat]]

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

Comment 5

[Alessio Placitelli [:Dexter]]

Attachment: Fx_43 - Patch for Firefox beta

Approval Request Comment
[Feature/regressing bug #]: This fixes the wrong GMPlugins being reported by Telemetry.
[User impact if declined]: The wrong number of plugins may be reported in about:healthreport.
[Describe test coverage new/current, TreeHerder]: Automated test coverage already present.
[Risks and why]: Low-risk - manually tested and basically just skipping GMPlugins explicitly marked as invalid or disabled.
[String/UUID change made/needed]: None

Comment 6

[Alessio Placitelli [:Dexter]]

Comment on attachment 8681964 [details] [diff] [review]
bug1218842.patch

Approval Request Comment
[Feature/regressing bug #]: This fixes the wrong GMPlugins being reported by Telemetry.
[User impact if declined]: The wrong number of plugins may be reported in about:healthreport.
[Describe test coverage new/current, TreeHerder]: Automated test coverage already present.
[Risks and why]: Low-risk - manually tested and basically just skipping GMPlugins explicitly marked as invalid or disabled.
[String/UUID change made/needed]: None

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e402ef99492d

Comment 8

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion

Comment 9

[Ada [:adalucinet]]

On latest 45.0a1 (from 2015-11-05), with alert(JSON.stringify(Cu.import("resource://gre/modules/TelemetryEnvironment.jsm").TelemetryEnvironment.currentEnvironment)) command the output is:
"activeGMPlugins": {
      
    } for both Always or Never activate states.

Tested with both about:healthreport page v2 and v4 content, across platforms [1]. Is it expected that even in active state the gmp plugins are not submitted? And please let me know if this testing is enough to call this verified.

[1] Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit

Comment 10

[Alessio Placitelli [:Dexter]]

Alexandra, good catch. This is a regression, I've filed bug 1222503 for it.

Comment 11

[Ada [:adalucinet]]

Confirming this fix with latest 45.0a1 (2015-11-11), across platforms, along with the verification for bug 1222503.