:Felipe Gomes (needinfo for replies!) Assignee
	Description • 7 years ago

Bug 1317856 just switched Flash as Click-to-Play on Nightly. This bug is meant to track:
 - the extra requirements of shipping that to release, through the dependencies
 - the actual work of switching this on release, maybe directly by changing prefs, maybe with a staged rollout.

	Benjamin Smedberg
	Comment 1 • 7 years ago

My current thinking:

* roll the blocklist out without staging
* roll the CtA setting out gradually
** to 50% of beta starting immediately with 55b1
** to release 55 gradually over 6 weeks, starting at 10% of release

Monitoring:
* overall page loads (engagement)
* users who flip Flash back to enabled
* infobar shown/accept/hide rates
* notification shown/action rates

I filed bug 1362520 on datasets for release monitoring, since that's too much data for simple notebooks.

	Benjamin Smedberg
	Comment 2 • 7 years ago

Final deployment strategy is in the user story. I am agnostic as to the specific deployment vehicle: system addon, pref experiment, pref rollout are all fine from my perspective. Schedule risk because pref rollout isn't ready is not ok ;-)

Felipe and Doug, this is ready for engineering decisions and action.

	:Felipe Gomes (needinfo for replies!) Assignee
	Comment 3 • 7 years ago

Other dependencies are tracked on other bugs, so I'll use this one to implement the rollout

	Benjamin Smedberg
	Comment 7 • 7 years ago
mozreview-review

Comment on attachment 8875897 [details]
Bug 1365714 - Part 1 - Adjust prefs to roll Click-to-Play by default on release.

https://reviewboard.mozilla.org/r/147284/#review151824

	Benjamin Smedberg
	Comment 8 • 7 years ago
mozreview-review

Comment on attachment 8875898 [details]
Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature.

https://reviewboard.mozilla.org/r/147286/#review151826

::: browser/extensions/clicktoplay-rollout/bootstrap.js:17
(Diff revision 2)
> +Cu.import("resource://gre/modules/AppConstants.jsm");
> +
> +// The amount of people to be part of the rollout
> +const TEST_THRESHOLD = {
> +  "beta": 0.5,  // 50%
> +  "release": 0.1,  // 10%

Should be 5% for release

::: browser/extensions/clicktoplay-rollout/bootstrap.js:116
(Diff revision 2)
> +  value = Math.random();
> +  Preferences.set(pref, value.toString().substr(0, 8));
> +  return value;
> +}
> +
> +function setCohort(cohortName) {

I believe this function is missing code to add the cohort to environment/experiments.

::: browser/extensions/clicktoplay-rollout/bootstrap.js:126
(Diff revision 2)
> +    }
> +  } catch (e) {}
> +}
> +
> +function watchForPrefChanges() {
> +  Preferences.observe(PREF_FLASH_STATE, function()) {

I don't think the pref observer or this special cohort is necessary: I can distinguish whether users make a change via the environment, and being able to explicitly monitor that per original cohort might be helpful.

	:Felipe Gomes (needinfo for replies!) Assignee
	Comment 9 • 7 years ago
mozreview-review

Comment on attachment 8875898 [details]
Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature.

https://reviewboard.mozilla.org/r/147286/#review151836

::: browser/extensions/clicktoplay-rollout/bootstrap.js:17
(Diff revision 2)
> +Cu.import("resource://gre/modules/AppConstants.jsm");
> +
> +// The amount of people to be part of the rollout
> +const TEST_THRESHOLD = {
> +  "beta": 0.5,  // 50%
> +  "release": 0.1,  // 10%

ok

::: browser/extensions/clicktoplay-rollout/bootstrap.js:116
(Diff revision 2)
> +  value = Math.random();
> +  Preferences.set(pref, value.toString().substr(0, 8));
> +  return value;
> +}
> +
> +function setCohort(cohortName) {

I was gonna add it, but the pref value is already being recorded in TelemetryEnvironment.jsm:

`["plugins.ctprollout.cohort", {what: RECORD_PREF_VALUE}],`

And I was wondering if that is enough?

If it's not enough I can add it using the new setExperimentActive function

::: browser/extensions/clicktoplay-rollout/bootstrap.js:126
(Diff revision 2)
> +    }
> +  } catch (e) {}
> +}
> +
> +function watchForPrefChanges() {
> +  Preferences.observe(PREF_FLASH_STATE, function()) {

The problem is that, since I'm using defaultPrefs to set PREF_FLASH_STATE, it's set on every startup. If the user changes it to something else, we'll keep changing it back again.  So this different cohort is a way to tell "leave this user alone".

What I could do is store in the cohort name the name of the original cohort too. What do you think?

	Benjamin Smedberg
	Comment 10 • 7 years ago

> `["plugins.ctprollout.cohort", {what: RECORD_PREF_VALUE}],`
> 
> And I was wondering if that is enough?

No that's not enough, because I'd have to get the data team to modify main_summary to include this new field. The experiments field is included by default.

(In fact you can revert that TelemetryEnvironment change if you like.

> If it's not enough I can add it using the new setExperimentActive function

please

> What I could do is store in the cohort name the name of the original cohort
> too. What do you think?

That would solve the problem, yes.

	Benjamin Smedberg
	Comment 14 • 7 years ago
mozreview-review

Comment on attachment 8876203 [details]
Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature.

https://reviewboard.mozilla.org/r/147644/#review151926

::: browser/extensions/clicktoplay-rollout/bootstrap.js:123
(Diff revision 1)
> +function setCohort(cohortName) {
> +  let currentCohort = Preferences.get(PREF_COHORT_NAME, undefined);
> +
> +  if (cohortName != currentCohort) {
> +    Preferences.set(PREF_COHORT_NAME, cohortName);
> +    // Only do this if the cohort name is changing, to avoid triggering

This needs to be set on every startup: setExperimentActive is a non-persistent API.

	Benjamin Smedberg
	Comment 17 • 7 years ago
mozreview-review

Comment on attachment 8876203 [details]
Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature.

https://reviewboard.mozilla.org/r/147644/#review151948

	Pulsebot
	Comment 18 • 7 years ago

Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1731ad31e6a5
Part 1 - Adjust prefs to roll Click-to-Play by default on release. r=bsmedberg
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5ed57189ad
Part 2 - System add-on to control rollout of the CTP feature. r=bsmedberg

	Benjamin Smedberg
	Comment 19 • 7 years ago

Important risks in terms of testing:

* people shouldn't switch cohorts. Once we've picked that you're going to be on or off, it should stay that way.
* the cohort needs to show up in the main ping environment.experiments section. about:telemetry should show you that.

	Stefan [:StefanG_QA]
	Comment 20 • 7 years ago

All test has passed on the provided Beta TRY build. I can provide the results upon request. 

When restarting the browser, the prefs are not reverted to the initial value.
User action are remembered when changing the Flash options from about:addons.
The "plugins.ctprollout.cohort" is changed respectively depending on the cohorts.

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 21 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/1731ad31e6a5
https://hg.mozilla.org/mozilla-central/rev/4f5ed57189ad

	Chris Mills [:cmills]
	Comment 22 • 7 years ago

I've expanded on the plan for rollout of "click-to-play" Flash in the release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/55#Plugins

Does that cover it for now?