Closed
Bug 1248813
Opened 8 years ago
Closed 8 years ago
Telemetry experiment for plugin content blocking
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(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 |
This is for a telemetry experiment to measure the effect of blocking plugin content on crashes, hangs, and other browser jank.
Assignee | ||
Comment 1•8 years ago
|
||
Updated list names.
Attachment #8720056 -
Attachment is obsolete: true
Comment 2•8 years ago
|
||
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)
Assignee | ||
Comment 3•8 years ago
|
||
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 4•8 years ago
|
||
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)
Comment 5•8 years ago
|
||
Update user story for Beta 46 experiment.
status-firefox45:
--- → wontfix
status-firefox46:
--- → affected
status-firefox47:
--- → wontfix
status-firefox48:
--- → wontfix
User Story: (updated)
Comment 6•8 years ago
|
||
Test code will also land disabled on Nightly and Aurora.
Group: mozilla-employee-confidential
User Story: (updated)
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8723152 [details] [diff] [review] plugin-block-experiment (v2) Felipe, asking for your feedback on this too.
Attachment #8723152 -
Flags: review?(felipc)
Assignee | ||
Comment 8•8 years ago
|
||
Geord: The bugs referred in the manifest are now open to the public.
Comment 9•8 years ago
|
||
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+
Assignee | ||
Comment 10•8 years ago
|
||
Addressed feedback.
Attachment #8723152 -
Attachment is obsolete: true
Assignee | ||
Comment 11•8 years ago
|
||
Experiment add-on.
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8731925 [details]
experiment.xpi (unsigned)
Jason: Can you help us getting the extension signed?
Flags: needinfo?(jthomas)
Comment 14•8 years ago
|
||
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`
Assignee | ||
Comment 15•8 years ago
|
||
Skip experiment if e10s is enabled.
Attachment #8731924 -
Attachment is obsolete: true
Attachment #8732397 -
Flags: review?(felipc)
Assignee | ||
Comment 16•8 years ago
|
||
Sorry, updated wrong patch before.
Attachment #8732397 -
Attachment is obsolete: true
Attachment #8732397 -
Flags: review?(felipc)
Attachment #8732398 -
Flags: review?(felipc)
Assignee | ||
Comment 17•8 years ago
|
||
Updated addon.
Attachment #8731925 -
Attachment is obsolete: true
Attachment #8732169 -
Attachment is obsolete: true
Comment 18•8 years ago
|
||
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+
Assignee | ||
Comment 19•8 years ago
|
||
Renamed addon folder.
Attachment #8732398 -
Attachment is obsolete: true
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8732421 [details]
experiment.xpi unsigned (v3)
Jason, need you to do your signing magic one more time.
Flags: needinfo?(jthomas)
Assignee | ||
Comment 23•8 years ago
|
||
Services component was not included.
Attachment #8732419 -
Attachment is obsolete: true
Assignee | ||
Comment 24•8 years ago
|
||
Updated add-on.
Attachment #8732421 -
Attachment is obsolete: true
Attachment #8732857 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8732915 -
Attachment description: experiment.xpi signed (v4) → experiment.xpi unsigned (v4)
Assignee | ||
Comment 25•8 years ago
|
||
Attachment #8732915 -
Attachment is obsolete: true
Assignee | ||
Comment 26•8 years ago
|
||
Sorry, I created the previous extension using the wrong patch. Current one needs to be signed again.
Flags: needinfo?(jthomas)
Comment 28•8 years ago
|
||
[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
Comment 29•8 years ago
|
||
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)
Assignee | ||
Comment 30•8 years ago
|
||
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)
Assignee | ||
Comment 31•8 years ago
|
||
Updated add-on.
Attachment #8732916 -
Attachment is obsolete: true
Attachment #8733051 -
Attachment is obsolete: true
Assignee | ||
Comment 32•8 years ago
|
||
Make experiment actually run for 21 days.
Attachment #8742415 -
Attachment is obsolete: true
Assignee | ||
Comment 33•8 years ago
|
||
Updated add-on.
Attachment #8742416 -
Attachment is obsolete: true
Assignee | ||
Comment 34•8 years ago
|
||
Jason, the add-on needs to be signed again so we can hopefully start the experiment on Monday.
Flags: needinfo?(jthomas)
Assignee | ||
Comment 36•8 years ago
|
||
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
Assignee | ||
Comment 37•8 years ago
|
||
Updated add-on.
Attachment #8744439 -
Attachment is obsolete: true
Attachment #8744449 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8745026 -
Flags: review?(benjamin)
Comment 38•8 years ago
|
||
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.
Assignee | ||
Comment 40•8 years ago
|
||
Specified final experiment dates and target versions.
Attachment #8745026 -
Attachment is obsolete: true
Assignee | ||
Comment 41•8 years ago
|
||
Updated add-on.
Attachment #8745027 -
Attachment is obsolete: true
Assignee | ||
Comment 42•8 years ago
|
||
Attachment #8747292 -
Attachment is obsolete: true
Assignee | ||
Comment 43•8 years ago
|
||
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)
Comment 45•8 years ago
|
||
Thanks, Jason. I pushed the signed add-on to the telemetry experiment server repo: https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/e59fef380ae0
Comment 46•8 years ago
|
||
Move `release_tag` to plugin-block-beta47 experiment: https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/3491bc57e173
Comment 47•8 years ago
|
||
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
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•