Closed Bug 1365714 Opened 4 years ago Closed 3 years ago

Release Flash plugin Click-to-Play (aka "Ask to Activate")

Categories

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

Product:
Core
Component:
Plug-ins
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: Felipe, Assigned: Felipe)

References

(
URL
)

Details

(4 keywords)

User Story

Rollout plan specification:

For beta 55, the entire cycle:

* deploy domain blocking to everyone (direct landing, ride the trains)
* deploy click-to-activate 50/50 on/off
** cohorts must appear in environment.experiments for monitoring

For release 55

* domain blocking to everyone (ride the trains)
* Deploy click to activate over the release cycle in the following stages:
** 5% of users - two weeks - monitoring engagement and other metrics
** 25% of users - two weeks
** 100% of users before Firefox 56
** cohorts must appear in environment.experiments for monitoring

Attachments

(2 files, 1 obsolete file)

Bug 1365714 - Part 1 - Adjust prefs to roll Click-to-Play by default on release.
59 bytes, text/x-review-board-request
benjamin
: review+
Details
Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature.
59 bytes, text/x-review-board-request
benjamin
: review+
Details 
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.
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.
Depends on: 1367457
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.
User Story: (updated)
Flags: needinfo?(felipc)
Flags: needinfo?(dothayer)
Depends on: 1369755
Other dependencies are tracked on other bugs, so I'll use this one to implement the rollout
Assignee: nobody → felipc
Status: NEW → ASSIGNED
Depends on: 1369160, 1368060
Flags: needinfo?(felipc)
Flags: needinfo?(dothayer)
No longer depends on: 1362149
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
Attachment #8875897 - Flags: review?(benjamin) → 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.
Attachment #8875898 - Flags: review?(benjamin) → 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?
> `["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.
Attachment #8875898 - Attachment is obsolete: true
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.
Attachment #8876203 - Flags: review?(benjamin) → 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
Attachment #8876203 - Flags: review?(benjamin) → review+
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
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.
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.
https://hg.mozilla.org/mozilla-central/rev/1731ad31e6a5
https://hg.mozilla.org/mozilla-central/rev/4f5ed57189ad
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1372237
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?
Keywords: dev-doc-needed → dev-doc-complete
Depends on: 1370350
Depends on: 1374289
Depends on: 1379175
Depends on: 1390703
Depends on: 1390705
Depends on: 1390706
You need to log in before you can comment on or make changes to this bug.