1352224 - Run a Telemetry Experiment on Nightly to test Flash set to Click-To-Activate by default

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, enhancement, P3)

Description

[:Felipe Gomes (needinfo for replies!)]

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

[:Felipe Gomes (needinfo for replies!)]

Attachment: flash-as-ctp

Benjamin, would you like to review it, or do you suggest someone else?

Comment 2

[:Felipe Gomes (needinfo for replies!)]

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

[:Felipe Gomes (needinfo for replies!)]

Attachment: Experiment to make Flash Click-to-Play

Comment 4

[Alex Thayer [:alexical] (she/her)]

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

[:Felipe Gomes (needinfo for replies!)]

Attachment: 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

[:Felipe Gomes (needinfo for replies!)]

(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

[:Felipe Gomes (needinfo for replies!)]

Attachment: experiment.xpi, unsigned

Wei, can you sign this addon for me?

Comment 8

[:Felipe Gomes (needinfo for replies!)]

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

[Jason Thomas [:jason]]

Attachment: experiment.xpi signed

Please see attached.

Comment 10

[Stefan [:StefanG_QA]]

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

[:Felipe Gomes (needinfo for replies!)]

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

[:Felipe Gomes (needinfo for replies!)]

This has been deployed on bug 1356713