Telemetry Data Point Request - Flash Plugin Version

RESOLVED FIXED in mozilla16

Status

()

Toolkit
Telemetry
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mcoates, Assigned: gfritzsche)

Tracking

unspecified
mozilla16
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Outdated plugins are one of the most common ways our users are compromised.  We will be mounting targeted campaigns to educate users and help guide them to update their plugins.

However, we will not be able to determine if these efforts are successful or if the problem is subsiding unless we are able to measure the total count of flash by version number.

To simplify the data gathering it is sufficient to only apply this data point to flash.

Comment 1

6 years ago
Are you going to attach a patch, we do not have bandwidth for this in the next couple of weeks.
Be sure to collect at least a week of data before mounting any campaigns.  Telemetry data can be noisy.
(In reply to Taras Glek (:taras) from comment #1)
> Are you going to attach a patch, we do not have bandwidth for this in the
> next couple of weeks.

Understood. I'm getting the bug filed and working to get resources to complete the patch.

Updated

6 years ago
Assignee: nobody → georg.fritzsche
(Assignee)

Comment 4

6 years ago
Michael, do we need that telemetry long-term or will it be only in for a limited time?

Comment 5

6 years ago
Yes, we should plan on keeping this enabled "forever". If it's not a privacy problem, we probably want to submit all plugin versions, otherwise just Flash.
(Assignee)

Comment 6

6 years ago
As a privacy review in the bug might be sufficient, i am adding the requested data.

Life-time of the probe: 
We want this to stay in with no time limit.

Examples of data being collected:
If it is no privacy concern, we want to collect the names and versions of all installed plugins, e.g.:
 * "Shockwave Flash", "11.2.202.235" 
 * "Java(TM) Platform SE 7 U4", "10.4.1.255"
 * "Adobe Acrobat", "9.5.1.283"
If collecting the names and versions of all installed plugins is a privacy problem, we want to collect only the data for the installed Flash plugin, e.g.
 * "flashVersion", "11.2.202.235"
Status: NEW → ASSIGNED
Keywords: privacy-review-needed
(Assignee)

Updated

6 years ago
Depends on: 767077
(Assignee)

Comment 7

6 years ago
Created attachment 635403 [details] [diff] [review]
Add plugin versions to telemetry ping

This is currently blocked by bug 767077 as plugin names may contain special characters.
(In reply to Georg Fritzsche [:gfritzsche] from comment #6)
> As a privacy review in the bug might be sufficient, i am adding the
> requested data.

Thanks, Georg.  

We should focus just on Flash for now.  Collecting info about all plug-ins has more privacy implications (and is broader than this bug).  See: https://wiki.mozilla.org/Privacy/Reviews/Telemetry/Encountered_Plugin_Types

Your comment 6 the first question, "precisely what data will we collect?"
For completeness, please verify you agree with my answers to these questions:

1. What problem are we trying to solve with this data?  We want to identify whether or not users are more rapidly upgrading off buggy versions of flash.

2. What benefit will a solution provide to our users?  We can focus our engagement efforts on those that work best (show good results in telmetry data) to keep users upgrading to safe versions of flash.
(Assignee)

Comment 9

6 years ago
(In reply to Sid Stamm [:geekboy] from comment #8)
> We should focus just on Flash for now.  Collecting info about all plug-ins
> has more privacy implications (and is broader than this bug).  See:
> https://wiki.mozilla.org/Privacy/Reviews/Telemetry/Encountered_Plugin_Types

Ah, i see - so we limit this bug to Flash.
 
> Your comment 6 the first question, "precisely what data will we collect?"
> For completeness, please verify you agree with my answers to these questions:
> 
> 1. What problem are we trying to solve with this data?  We want to identify
> whether or not users are more rapidly upgrading off buggy versions of flash.
> 
> 2. What benefit will a solution provide to our users?  We can focus our
> engagement efforts on those that work best (show good results in telmetry
> data) to keep users upgrading to safe versions of flash.

Agreed on both of these as per comment 1.
(Assignee)

Updated

6 years ago
No longer depends on: 767077
(Assignee)

Comment 10

6 years ago
Created attachment 637510 [details] [diff] [review]
Submit flash version from telemetry ping

Taras, can you review this?
Attachment #637510 - Flags: review?(taras.mozilla)
(Assignee)

Updated

6 years ago
See Also: → bug 695589
(Assignee)

Updated

6 years ago
Attachment #635403 - Attachment is obsolete: true
Comment on attachment 637510 [details] [diff] [review]
Submit flash version from telemetry ping

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

::: toolkit/components/telemetry/TelemetryPing.js
@@ +346,5 @@
>  
>      if (this._addons)
>        ret.addons = this._addons;
>  
> +    ret.flashVersion = this.getFlashVersion();

I'd make this conditional on actually having Flash, just like we do for addons, persona, etc.

Do we have a good way of providing a fake plugin for testing purposes?  Surely other tests have addressed this problem.

::: toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
@@ +134,5 @@
>  
>    do_check_eq(payload.info.reason, reason);
>    do_check_true("appUpdateChannel" in payload.info);
>    do_check_true("locale" in payload.info);
> +  do_check_eq(payload.info.flashVersion, getFlashVersion());

Once we make setting flashVersion conditional, I think it will be sufficient to check for the existence of flashVersion and not a specific value (unless it's a test-specific value, not something returned by getFlashVersion).
Attachment #637510 - Flags: review?(taras.mozilla) → review-
(Assignee)

Comment 12

6 years ago
Created attachment 638138 [details] [diff] [review]
Submit flash version from telemetry ping, v2

Addressed previous review comments.
Attachment #637510 - Attachment is obsolete: true
Attachment #638138 - Flags: review?(nfroyd)
Attachment #638138 - Attachment is patch: true
Comment on attachment 638138 [details] [diff] [review]
Submit flash version from telemetry ping, v2

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

Looks good to me, just a couple minor changes.

::: toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
@@ +125,5 @@
>      appVersion: "1", 
>      appName: "XPCShell", 
>      appBuildID: "2007010101",
> +    platformBuildID: "2007010101",
> +    flashVersion: "1.1.1.1"

Please pull "1.1.1.1" out into a const FLASH_VERSION at the top of the file and use FLASH_VERSION in the definition of PluginHost.

@@ +135,5 @@
>  
>    do_check_eq(payload.info.reason, reason);
>    do_check_true("appUpdateChannel" in payload.info);
>    do_check_true("locale" in payload.info);
> +  

Nit: unnecessary whitespace change.

@@ +336,5 @@
> +  },
> +
> +  QueryInterface: function(iid) {
> +    if (iid.equals(Components.interfaces.nsIPluginHost)
> +     || iid.equals(Components.interfaces.nsISupports))

Nit: please use Ci. instead.

@@ +355,5 @@
> +const PLUGINHOST_CONTRACTID = "@mozilla.org/plugin/host;1";
> +const PLUGINHOST_CID = Components.ID("{2329e6ea-1f15-4cbe-9ded-6e98e842de0e}");
> +
> +function registerFakePluginHost() {
> +  var registrar = Components.manager.QueryInterface(Components.interfaces.nsIComponentRegistrar);

Nit: Ci.nsIComponentRegistrar.
Attachment #638138 - Flags: review?(nfroyd) → review+
Keywords: privacy-review-needed
(Assignee)

Comment 14

6 years ago
Created attachment 638425 [details] [diff] [review]
Submit flash version from telemetry ping, v3

Adressed review comments.
Attachment #638138 - Attachment is obsolete: true
Attachment #638425 - Flags: review+
(Assignee)

Comment 15

6 years ago
Comment on attachment 638425 [details] [diff] [review]
Submit flash version from telemetry ping, v3

Remove failed attempt to carry review+ over.
Attachment #638425 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/936ee90e6e54

Please remember to include all of the necessary commit information in the patches you submit. It makes life much easier for those checking in on your behalf. Thanks!
https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/936ee90e6e54
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Blocks: 772143
You need to log in before you can comment on or make changes to this bug.