1598398 - Mitigate missing 20% on 3x1 and move to 3x7

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P1)

Description

[Jessilyn Davis]

Related to: Bug 1585742 - [Pocket] Investigate Pocket New Tab Rollout Targeting

Problem:
20% of users on Fx 70 eligible to see 3x7 are still seeing the old default of 3x1.

What happened:
People get put in a study that's on during a release change, the pref changes was an AS pref. During the release change that pref gets changed to a new default (with enabled:false AS Pref value), they get booted out, and somehow get left with a user changed pref (that's enabled:false) and not a default pref, thus no longer being able to be rolled out to.

Notes from Investigation Meeting:
https://docs.google.com/document/d/1FqQn_ZLs2CMsrV56palYUYir3Ot0pGnWGAePUBveTT0/edit#

Scott to articulate mitigation strategy options - and ideally get the fix in 72 Nightly by 11/29. If not, it'll be an uplift candidate into the 72 Beta cycle.

Comment 1

[Ed Lee :Mardak]

An unenrollment of "user-preference-changed-sideload" doesn't mean the "user" pref is no longer the default value. That unenrollment is triggered when the pref experiment no longer sees its expected value, which we saw in bug 1553243 comment 8 for the "control" branch as it was expecting the previous release's default value.

This means those unenrolled "control" users would be using the new default value and should be getting the default experience.

Comment 2

[Gavin Suntop [:gvn] (they/them)]

One possible approach:

• Deprecate the enabled sub-pref under browser.newtabpage.activity-stream.discoverystream.config. Ignore it entirely in the code.
• Retain browser.newtabpage.activity-stream.discoverystream.enabled as a universal DS on/off switch (as it's a Firefox pref)
• Add a new Firefox pref, such as enabled-geos, as a whitelist array for selectively enabling DS (eg: ["US", "CA", "DE"])

Comment 3

[Ed Lee :Mardak]

My current theory is that these users with {enabled:false} user pref values got them during normal experiment unenrollment at the end of an experiment where because these were "user" pref experiments, the original "user" value (which was the default value in 67) was 1) saved, 2) changed for the experimentt, 3) "restored" during 68. However, because the value restored is now different from 68's default value, Firefox will no longer use the default pref value and use the 67 default value now persisted as a user value.

But the current situation is the same no matter the actual cause: many users have wrong pref values.

One way to fix this is to stop trusting the pref as suggested in comment 2, and a simple way is to just rename the pref.

Alternatively is if we believe no user should have manually modified this pref anyway (and there are no experiments needing to change this pref), we can Services.prefs.clearUserPref() to force it to be the default.

Comment 4

[Ed Lee :Mardak]

This change should work to clear out any user pref:

diff --git a/browser/components/newtab/lib/ActivityStream.jsm b/browser/components/newtab/lib/ActivityStream.jsm
--- a/browser/components/newtab/lib/ActivityStream.jsm
+++ b/browser/components/newtab/lib/ActivityStream.jsm
@@ -655,8 +655,12 @@ this.ActivityStream = class ActivityStream {
   init() {
     try {
       this._updateDynamicPrefs();
       this._defaultPrefs.init();
+      this._migratePref(
+        "browser.newtabpage.activity-stream.discoverystream.config",
+        () => {}
+      );
 
       // Hook up the store and let all feeds and pages initialize
       this.store.init(
         this.feeds,

Comment 5

[Ed Lee :Mardak]

Alternatively, specifically targeting the 67 default pref value:

if (Services.prefs.getStringPref(DS_CONFIG_PREF) == DS_67_DEFAULT) Services.prefs.clearUserPref(DS_CONFIG_PREF);

Comment 6

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: A large chunk of users are not seeing the new discovery stream layout because they were in previous experiments.

Comment 7

[Ed Lee :Mardak]

Attachment: Bug 1598398 - Clear discovery stream config user pref if it's an old default value r=thecount

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/09deb28d77c8
Clear discovery stream config user pref if it's an old default value r=thecount

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/09deb28d77c8

Comment 10

[Ed Lee :Mardak]

QA steps:
0) set up to see discovery stream 7 rows, so US region en-US locale

1. set browser.newtabpage.activity-stream.discoverystream.config to fx67's default value: {"api_key_pref":"extensions.pocket.oAuthConsumerKey","enabled":false,"show_spocs":true,"layout_endpoint":"https://getpocket.cdn.mozilla.net/v3/newtab/layout?version=1&consumer_key=$apiKey&layout_variant=basic"}
2. see "activity stream" 1 row pocket with green links
3. restart firefox
4. see discovery stream with 7 rows and about:config shows pref is default

Extra:
like above except in step 1, have "enabled":true so it shows the basic "discovery stream" layout with blue links as if we were running an experiment
expect: after restart it still shows this "experiment" and about:config pref is still modified

Comment 11

[Pascal Chevrel:pascalc (PTO until April 26)]

This is a bit risky for our RC this Monday as it just landed and we haven't even shipped a nightly with the patch, we should let it ride the trains or have it in a 71 dot release as a ride-along. I am tracking the bug for evaluation in case we have a dot release in December.

Comment 12

[Marius Coman [:mcoman], Ecosystem QA]

I have verified this issue with the latest Firefox Nightly (72.0a1 Build ID - 20191125095200) installed, on Windows 10 x64, Arch Linux and Mac 10.15.1 using the steps from comment 10. Now, the Discovery Stream with 7 rows experience is displayed after a restart.
Also if the browser.newtabpage.activity-stream.discoverystream.config pref is set to {"api_key_pref":"extensions.pocket.oAuthConsumerKey","enabled":true,"show_spocs":true,"layout_endpoint":"https://getpocket.cdn.mozilla.net/v3/newtab/layout?version=1&consumer_key=$apiKey&layout_variant=basic"}, only one row is displayed after restart and the pref is still modified.

Comment 13

[Ed Lee :Mardak]

Looks like the investigation and fix seem to be correct. At least from initial fx72 rollout so far, 99.97% of users are seeing the expected layout while fx71 US users was closer to 83%.

I guess going forwards, be extra careful with experiments that span multiple versions that have default pref changes (which I suppose aren't supposed to change anyway for a valid experiment ?).