Last Comment Bug 1352224 - Run a Telemetry Experiment on Nightly to test Flash set to Click-To-Activate by default
: Run a Telemetry Experiment on Nightly to test Flash set to Click-To-Activate ...
Status: RESOLVED FIXED
: flashplayer, site-compat
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: unspecified
: All All
P3 normal with 1 vote (vote)
: ---
Assigned To: :Felipe Gomes (needinfo me!)
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
Depends on: 1277346 1307604 1314094 1350381 1356713
Blocks: flashstudy flash-click-to-play
  Show dependency treegraph
 
Reported: 2017-03-30 13:57 PDT by :Felipe Gomes (needinfo me!)
Modified: 2017-04-14 17:07 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
flash-as-ctp (12.87 KB, patch)
2017-04-06 12:33 PDT, :Felipe Gomes (needinfo me!)
no flags Details | Diff | Splinter Review
Experiment to make Flash Click-to-Play (15.10 KB, patch)
2017-04-11 11:50 PDT, :Felipe Gomes (needinfo me!)
dothayer: review+
Details | Diff | Splinter Review
Experiment to make Flash Click-to-Play, r=dothayer (16.90 KB, patch)
2017-04-12 11:09 PDT, :Felipe Gomes (needinfo me!)
felipc: review+
Details | Diff | Splinter Review
experiment.xpi, unsigned (4.16 KB, application/x-xpinstall)
2017-04-12 11:12 PDT, :Felipe Gomes (needinfo me!)
no flags Details
experiment.xpi signed (8.04 KB, application/x-xpinstall)
2017-04-12 12:57 PDT, Jason Thomas [:jason]
no flags Details

User Story
- Nightly users
- minBuildID must include bug 1350381
- Only users who have Flash installed
- Only users with Flash as "Always Activate" (default)

- Allow lists: empty
- First-party block list: empty
- 3rd-party block list: the disconnect.me list

- Favor html5 fallback over plugin content: on the presence of a <video>

- Population size: 25% of nightly, split 50/50 on test/control
- Non-blocked versions of Flash
- From Apr 14 to Apr 23

- Data collection:
no extra data collection is necessary outside of what's already collected by Telemetry      
Description User image :Felipe Gomes (needinfo me!) 2017-03-30 13:57:47 PDT
We now have all the pieces in place to run an experiment on Nightly to set Flash as Click-to-Activate by default. That includes a dynamic allow/blocklist of sites that are allowed to run Flash (in first-party or third-party frames) and a system to prefer html5 fallback over Flash.

We will want to fine tune all these things, and will get wider data about how this is behaving out of a shield study on release (bug 1335232), but to get basic info about how this works, we should run this on Nightly first.

I'll keep the User Story updated with the experiment parameters.
Comment 1 User image :Felipe Gomes (needinfo me!) 2017-04-06 12:33:47 PDT
Created attachment 8855446 [details] [diff] [review]
flash-as-ctp

Benjamin, would you like to review it, or do you suggest someone else?
Comment 2 User image :Felipe Gomes (needinfo me!) 2017-04-06 13:31:02 PDT
Comment on attachment 8855446 [details] [diff] [review]
flash-as-ctp

I need to change something as the plugins.flashBlock.enabled pref needs to be changed on "test-waiting" time in order for the lists to download.
Comment 3 User image :Felipe Gomes (needinfo me!) 2017-04-11 11:50:17 PDT
Created attachment 8857090 [details] [diff] [review]
Experiment to make Flash Click-to-Play
Comment 4 User image Doug Thayer [:dthayer] 2017-04-11 13:48:41 PDT
Comment on attachment 8857090 [details] [diff] [review]
Experiment to make Flash Click-to-Play

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

::: experiments/flash-as-ctp-nightly55/code/bootstrap.js
@@ +203,5 @@
> +  let classifier = Cc['@mozilla.org/url-classifier/dbservice;1']
> +                     .getService(Ci.nsIURIClassifier);
> +
> +  let { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
> +  let table = Services.prefs.getCharPref('urlclassifier.flashSubDocTable');

For completeness's sake, should we be checking if this pref has a user value in checkHasAnyUserPref?

Also nit: for consistency this should maybe be declared as a constant above with all the other pref names.

@@ +290,5 @@
> +function install(a,b) {
> +}
> +
> +function uninstall(a,b) {
> +  stopExperiment();

I don't think it matters, since I think the only side effect would be rechecking the shavar lists, but this is run on upgrade as well, right?

Is there any other case where this can be hit without the stopExperiment call in shutdown being run?
Comment 5 User image :Felipe Gomes (needinfo me!) 2017-04-12 11:09:33 PDT
Created attachment 8857590 [details] [diff] [review]
Experiment to make Flash Click-to-Play, r=dothayer

I've made some minor changes but I don't think it needs re-review. Just improved the tagging for people who change prefs in the middle of the experiment. Instead of being lumped in a single "disqualified" cohort, they will be tagged as "disqualified-prefchanged".. Just to make it easier to understand how many people undo the changes.
Comment 6 User image :Felipe Gomes (needinfo me!) 2017-04-12 11:10:53 PDT
(In reply to Doug Thayer [:dthayer] from comment #4)
> > +  let classifier = Cc['@mozilla.org/url-classifier/dbservice;1']
> > +                     .getService(Ci.nsIURIClassifier);
> > +
> > +  let { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
> > +  let table = Services.prefs.getCharPref('urlclassifier.flashSubDocTable');
> 
> For completeness's sake, should we be checking if this pref has a user value
> in checkHasAnyUserPref?
> 
> Also nit: for consistency this should maybe be declared as a constant above
> with all the other pref names.

Done

> 
> @@ +290,5 @@
> > +function install(a,b) {
> > +}
> > +
> > +function uninstall(a,b) {
> > +  stopExperiment();
> 
> I don't think it matters, since I think the only side effect would be
> rechecking the shavar lists, but this is run on upgrade as well, right?
> 
> Is there any other case where this can be hit without the stopExperiment
> call in shutdown being run?

Yeah, when an experiment expires while the user is not running Firefox, the next time that they open Firefox, only the uninstall() Function will be called  (i.e., startup() and shutdown() won't). So that's why it's important to have it there.
Comment 7 User image :Felipe Gomes (needinfo me!) 2017-04-12 11:12:36 PDT
Created attachment 8857592 [details]
experiment.xpi, unsigned

Wei, can you sign this addon for me?
Comment 8 User image :Felipe Gomes (needinfo me!) 2017-04-12 12:32:07 PDT
Hello Stefan, could you QA this for us?


Using a fresh profile and a recent Nightly build:

#### Set up ####

** Go to about:config

- Ensure that toolkit.telemetry.enabled = true
- Change devtools.chrome.enabled to true

** Open the Web Developer -> Browser Console tool

- Copy & paste & <Enter> the following: (it can be all at once, doesn't need to be line by line)

Services.prefs.setBoolPref("xpinstall.signatures.required", false);
Services.prefs.setBoolPref("experiments.logging.enabled", true);
Services.prefs.setBoolPref("extensions.logging.enabled", true);
Services.prefs.setIntPref("experiments.logging.level", 0);
Services.prefs.setCharPref("experiments.force-sample-value", "0.1");
Services.prefs.setCharPref("experiments.manifest.uri", "https://people.mozilla.org/~fgomes/flash-as-ctp-55/firefox-manifest.json");

- Afterwards, copy & paste & <Enter> the following: (**)

Cu.import("resource:///modules/experiments/Experiments.jsm");
Experiments.instance().updateManifest();


#### Test cases ####

* After all of that runs, go to about:support, scroll to the very bottom and check if "Flash as Click-To-Activate" is listed in "Experimental Features", and Active = true.  See what the Branch says:

* If "control":
 -- nothing should have changed. Navigate to a site known to have Flash, and see if it's loaded without asking the user anything. Check that Flash is still "Always Activate" in about:addons

* If "test-waiting":
 -- Restart Firefox, wait some seconds (might take up to 1min after restart), and open/refresh about:support to see if branch changes to "test"

 -- When it has changed to "test":
   -- Check that Flash is behaving in Ask to Activate mode, by going to some random Flash website and see if it asks for activation
   -- Check https://itisatrap.org/firefox/flashblock.html : The "Actual" column should match the "Expected for Ask-to-Activate" column.
   -- Go to about:config and change the pref "plugins.flashBlock.enabled" to false, and check that the branch has changed to "disqualified-prefchanged"

* On a clean profile again, before running the (**) command above, change the pref "plugins.flashBlock.enabled" to true. This will guarantee that there's a non-standard pref before the experiment starts. In this case, the experiment should directly go to a branch named "disqualified".


* If you're able to uninstall Flash, please do so for this test. Make sure Firefox is restarted after Flash is uninstalled, then run the experiment and check that it goes to a branch named "no-flash".
Comment 9 User image Jason Thomas [:jason] 2017-04-12 12:57:49 PDT
Created attachment 8857638 [details]
experiment.xpi signed

Please see attached.
Comment 10 User image Stefan [:StefanG_QA] 2017-04-12 16:20:44 PDT
We have executed the test on Windows, Ubuntu and Mac. We encountered two issues:

1. In case If test-waiting after 3 or 4 restarts the branch field never turn to "test" on all tested platform
2. In case If "no-flash" after removing flash, the branch shows "control' or "test-waiting"
Comment 11 User image :Felipe Gomes (needinfo me!) 2017-04-14 15:44:38 PDT
QA has concluded testings (Stefan will send out an e-mail shortly), so I've pushed it to the repo and will file a new bug for deployment:

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/aef9c8055a672a330260c3219f16798c5db98d14

Comment about QA: QA an issue, bug 1338638, which is unrelated to the experiment code, but which made QA'ing this a bit of pain, since that bug caused the Shavar service to take unusually long to update in certain conditions. This bug existed on Nightlies from Apr 6 - Apr 14 and is now fixed, but it nonetheless does not affect the experiment itself.. Users might just take a bit longer to get into the "test" cohort (users chosen for test are put in a "test-waiting" cohort until we are sure they have received the right flash blocklist).
Comment 12 User image :Felipe Gomes (needinfo me!) 2017-04-14 17:07:10 PDT
This has been deployed on bug 1356713

Note You need to log in before you can comment on or make changes to this bug.