Closed Bug 1248813 Opened 8 years ago Closed 8 years ago

Telemetry experiment for plugin content blocking

Categories

(Core Graveyard :: Plug-ins, defect)

45 Branch
defect
Not set
normal

Tracking

(firefox45 wontfix, firefox46 affected, firefox47 affected, firefox48 affected)

RESOLVED FIXED
Tracking Status
firefox45 --- wontfix
firefox46 --- affected
firefox47 --- affected
firefox48 --- affected

People

(Reporter: tschneider, Assigned: tschneider)

References

Details

User Story

This beta experiment will test multiple blocklist configurations for invisible plugin content in Firefox Beta 46. We will compare the different lists' effects on stability and performance metrics like crash rate and page load time. The experiment's blocklists are hosted on GitHub:

https://github.com/mozilla-services/shavar-plugin-blocklist

The beta experiment will measure three cohorts:

1. Block no plugin content. This is the control group.

2. Block some plugin content used for fingerprinting and super cookies.

3. Additionally block some plugin content used to watch the visibility of page elements. This type of plugin content sometimes continuously polls the page elements, causing inefficient DOM invalidations. A better solution for watching elements will be the proposed Intersection Observer API (bug 1243846).

Attachments

(3 files, 23 obsolete files)

5.13 KB, patch
Details | Diff | Splinter Review
1.63 KB, application/x-xpinstall
Details
5.52 KB, application/x-xpinstall
Details
Attached patch plugin-block-experiment (obsolete) — Splinter Review
This is for a telemetry experiment to measure the effect of blocking plugin content on crashes, hangs, and other browser jank.
Attached patch plugin-block-experiment (v2) (obsolete) — Splinter Review
Updated list names.
Attachment #8720056 - Attachment is obsolete: true
Assignee: nobody → tschneider
Depends on: 1237198
I assume you're going to want QA to take a look at this? Is there a test plan for this somewhere?
Flags: needinfo?(tschneider)
Comment on attachment 8723152 [details] [diff] [review]
plugin-block-experiment (v2)

Georg, can you please have a look at this? I think you can ignore the specified timestamps for now, we will update them once we have final green light to run the experiment.
Flags: needinfo?(tschneider)
Attachment #8723152 - Flags: review?(gfritzsche)
Comment on attachment 8723152 [details] [diff] [review]
plugin-block-experiment (v2)

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

This looks sensible on a quick glance. For specific questions or review for experiments :Felipe can give better input.

Did you test this locally?
There is some documentation on experiments testing here: https://wiki.mozilla.org/QA/Telemetry
Note that you need to disable xpi signing locally for testing.

::: browser/experiments/plugin-block/bootstrap.js
@@ +25,5 @@
> +        return;
> +      case "conservative":
> +        defaultPrefs.set("browser.safebrowsing.blockedURIs.enabled", true);
> +        defaultPrefs.set("urlclassifier.blockedTable",
> +                         "test-block-simple,mozplugin-block-digest256");

Is there any documentation anywhere on what those do?
I don't know what they are about.

@@ +52,5 @@
> +  // TESTING CODE
> +    try {
> +      let forcedBranch =
> +        Services.prefs.getCharPref("browser.safebrowsing.experiment.blocking");
> +      resolve(forcedBranch);

This is just for testing?

::: browser/experiments/plugin-block/install.rdf
@@ +18,5 @@
> +
> +    <!-- Front End MetaData -->
> +    <em:name>Plugin Block Testing</em:name>
> +    <em:description>Measuring the effect of blocking plugin content on crashes, hangs, and other browser jank.</em:description>
> +    <em:aboutURL>https://bugzilla.mozilla.org/show_bug.cgi?id=1237198</em:aboutURL>

We will need to use an aboutURL that does not limit access to Mozilla employees.

::: browser/experiments/plugin-block/manifest.json
@@ +2,5 @@
> +  "publish"     : true,
> +  "priority"    : 2,
> +  "name"        : "Plugin Block Testing",
> +  "description" : "Measuring the effect of blocking plugin content on crashes, hangs, and other browser jank.",
> +  "info"        : "<p><a href=\"https://bugzilla.mozilla.org/show_bug.cgi?id=1237198\">Related bug</a></p>",

This is employee-confidential - do we need a publically accessible info page for the manifest entry too?
Attachment #8723152 - Flags: review?(gfritzsche)
Update user story for Beta 46 experiment.
User Story: (updated)
Test code will also land disabled on Nightly and Aurora.
Group: mozilla-employee-confidential
User Story: (updated)
Comment on attachment 8723152 [details] [diff] [review]
plugin-block-experiment (v2)

Felipe, asking for your feedback on this too.
Attachment #8723152 - Flags: review?(felipc)
Geord: The bugs referred in the manifest are now open to the public.
Comment on attachment 8723152 [details] [diff] [review]
plugin-block-experiment (v2)

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

bootstrap code looks fine.  But it looks like this is a patch against mozilla-central? ("browser/experiments").  Experiments code actually lives in http://hg.mozilla.org/webtools/telemetry-experiment-server/

Have you already packaged this add-on xpi as an experiment? You can use this Makefile to do it easily: http://hg.mozilla.org/webtools/telemetry-experiment-server/file/tip/experiments/e10s-beta45-withoutaddons/code/Makefile

And here are some instructions for testing: https://wiki.mozilla.org/QA/Telemetry/Developing_a_Telemetry_Experiment
Note that since this is for beta, you'll need to get the experiment signed or make some changes locally to test it.

::: browser/experiments/plugin-block/bootstrap.js
@@ +11,5 @@
> +function startup() {
> +  // Seems startup() function is launched twice after install, we're
> +  // unsure why so far. We only want it to run once.
> +  if (gStarted) {
> +    Cu.reportError("startup() function was launched twice; making second run return.");

no need to do the Cu.reportError here

::: browser/experiments/plugin-block/install.rdf
@@ +11,5 @@
> +    <em:targetApplication>
> +      <Description>
> +        <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
> +        <em:minVersion>45.0</em:minVersion>
> +        <em:maxVersion>52.*</em:maxVersion>

please only use realistic values.  minVersion = 45.0, maxVersion 46.*.  Same for the manifest.json

::: browser/experiments/plugin-block/manifest.json
@@ +4,5 @@
> +  "name"        : "Plugin Block Testing",
> +  "description" : "Measuring the effect of blocking plugin content on crashes, hangs, and other browser jank.",
> +  "info"        : "<p><a href=\"https://bugzilla.mozilla.org/show_bug.cgi?id=1237198\">Related bug</a></p>",
> +  "manifest"    : {
> +    "id"               : "plugin-block-beta45@experiments.mozilla.org",

could you name this as beta45-46?

@@ +6,5 @@
> +  "info"        : "<p><a href=\"https://bugzilla.mozilla.org/show_bug.cgi?id=1237198\">Related bug</a></p>",
> +  "manifest"    : {
> +    "id"               : "plugin-block-beta45@experiments.mozilla.org",
> +    "startTime"        : 1455651552,
> +    "endTime"          : 1456861178,

you'll need to update startTime/endTime with proper values, as these are in the past
Attachment #8723152 - Flags: review?(felipc) → feedback+
Attached patch plugin-block-experiment (v3) (obsolete) — Splinter Review
Addressed feedback.
Attachment #8723152 - Attachment is obsolete: true
Attached file experiment.xpi (unsigned) (obsolete) —
Experiment add-on.
Comment on attachment 8731925 [details]
experiment.xpi (unsigned)

Jason: Can you help us getting the extension signed?
Flags: needinfo?(jthomas)
Attached file experiment.xpi signed (obsolete) —
Please see attached.
Flags: needinfo?(jthomas)
Comment on attachment 8731924 [details] [diff] [review]
plugin-block-experiment (v3)

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

::: experiments/plugin-block/code/bootstrap.js
@@ +13,5 @@
> +  // unsure why so far. We only want it to run once.
> +  if (gStarted) {
> +    return;
> +  }
> +  gStarted = true;

Tobias, can you please add a check to see if e10s is running, and if yes, skip and don't do anything?
Don't worry, if an experiment gets installed, e10s will deactivate on the next restart. This is just to prevent changing the prefs or setting the groups in the time interval while the experiment was installed but the user hasn't restarted yet, where they would be running both. And we don't want to get data from this situation mixed in the data that you will analyze later.

You can check if e10s is running with: `Services.appinfo.browserTabsRemoteAutostart`
Attached patch plugin-block-experiment (v4) (obsolete) — Splinter Review
Skip experiment if e10s is enabled.
Attachment #8731924 - Attachment is obsolete: true
Attachment #8732397 - Flags: review?(felipc)
Attached patch plugin-block-experiment (v4) (obsolete) — Splinter Review
Sorry, updated wrong patch before.
Attachment #8732397 - Attachment is obsolete: true
Attachment #8732397 - Flags: review?(felipc)
Attachment #8732398 - Flags: review?(felipc)
Attached file experiment.xpi unsigned (v2) (obsolete) —
Updated addon.
Attachment #8731925 - Attachment is obsolete: true
Attachment #8732169 - Attachment is obsolete: true
Comment on attachment 8732398 [details] [diff] [review]
plugin-block-experiment (v4)

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

Thanks! Only minor nit would be to name the folder you're creating a bit better, so it's easier to tell between the other folders.  "experiments/plugin-block-beta4546" or so..
Attachment #8732398 - Flags: review?(felipc) → review+
Attached patch plugin-block-experiment (v5) (obsolete) — Splinter Review
Renamed addon folder.
Attachment #8732398 - Attachment is obsolete: true
Attached file experiment.xpi unsigned (v3) (obsolete) —
Updated addon.
Attachment #8732400 - Attachment is obsolete: true
Comment on attachment 8732421 [details]
experiment.xpi unsigned (v3)

Jason, need you to do your signing magic one more time.
Flags: needinfo?(jthomas)
Attached file experiment.xpi signed (v3) (obsolete) —
Please see attached.
Flags: needinfo?(jthomas)
Attached patch plugin-block-experiment (v6) (obsolete) — Splinter Review
Services component was not included.
Attachment #8732419 - Attachment is obsolete: true
Attached file experiment.xpi unsigned (v4) (obsolete) —
Updated add-on.
Attachment #8732421 - Attachment is obsolete: true
Attachment #8732857 - Attachment is obsolete: true
Attachment #8732915 - Attachment description: experiment.xpi signed (v4) → experiment.xpi unsigned (v4)
Attached file experiment.xpi unsigned (v4) (obsolete) —
Attachment #8732915 - Attachment is obsolete: true
Sorry, I created the previous extension using the wrong patch. Current one needs to be signed again.
Flags: needinfo?(jthomas)
Attached file experiment.xpi signed (v4) (obsolete) —
Please see attached.
Flags: needinfo?(jthomas)
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Note: Developer specific testing.

Component: 
Name 	        Firefox
Version 	46.0b2
Build ID 	20160314144540
Update Channel 	beta
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS              Windows 7 SP1 x86_64
Tobias, Felipe recommends that we remove the e10s check from our experiment add-on. Previously, the e10s team wanted their e10s experiment to avoid other experiments, but they want to remove that restriction so they get more users.
Flags: needinfo?(tschneider)
Attached patch plugin-block-experiment (v7) (obsolete) — Splinter Review
Removed e10s check. Also updated the experiment to target FF 47 and starting from April 25 for a duration of 3 weeks.
Attachment #8732913 - Attachment is obsolete: true
Flags: needinfo?(tschneider)
Attached file experiment.xpi unsigned (v5) (obsolete) —
Updated add-on.
Attachment #8732916 - Attachment is obsolete: true
Attachment #8733051 - Attachment is obsolete: true
Make experiment actually run for 21 days.
Attachment #8742415 - Attachment is obsolete: true
Attached file experiment.xpi unsigned (v6) (obsolete) —
Updated add-on.
Attachment #8742416 - Attachment is obsolete: true
Jason, the add-on needs to be signed again so we can hopefully start the experiment on Monday.
Flags: needinfo?(jthomas)
Attached file experiment.xpi signed (v6) (obsolete) —
Please see attached.
Flags: needinfo?(jthomas)
Attached patch plugin-block-experiment (v9) (obsolete) — Splinter Review
Bringing back e10s check to exclude users from experiment if enabled. In reaction to https://bugzilla.mozilla.org/show_bug.cgi?id=1266889.
Attachment #8744437 - Attachment is obsolete: true
Attached file experiment.xpi unsigned (v7) (obsolete) —
Updated add-on.
Attachment #8744439 - Attachment is obsolete: true
Attachment #8744449 - Attachment is obsolete: true
Attachment #8745026 - Flags: review?(benjamin)
Comment on attachment 8745026 [details] [diff] [review]
plugin-block-experiment (v9)

Per conversation with Toby, this is already reviewed and I don't think we need an additional review from me for this.
Attachment #8745026 - Flags: review?(benjamin)
We are blocked on uplift of bug 1266889 to kick-off this telemetry experiment. From release management point of view, this experiment is a GO for Beta47 cycle.
Specified final experiment dates and target versions.
Attachment #8745026 - Attachment is obsolete: true
Attached file experiment.xpi unsigned (v8) (obsolete) —
Updated add-on.
Attachment #8745027 - Attachment is obsolete: true
Attachment #8747292 - Attachment is obsolete: true
Comment on attachment 8747306 [details]
experiment.xpi unsigned (v8)

Jason, sorry, but due too compatibility issues with e10s we had to move the start of our experiment again and change the add-on. We have to ask again for singing the XPI. Thanks for bearing with us here.
Flags: needinfo?(jthomas)
Please see attached.
Flags: needinfo?(jthomas)
Thanks, Jason.

I pushed the signed add-on to the telemetry experiment server repo:

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/e59fef380ae0
Depends on: 1271384
The experiment add-on was pushed to the production server in bug 1271384:

https://telemetry-experiment-dev.allizom.org/
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: