1390584 - [Shield] Pref Flip Study: Unified Search

Status: RESOLVED FIXED

(Shield :: Shield Study, enhancement)

Description

[Shilpi Gupta [:shilpi]]

This is the bug to launch opt out pref flipping study for unified search. In this study, we will test a limited set of cohorts with 3 prefs. 


Note: Originally planned superset is with 8 cohorts which will still be tested via an opt in addon and later via an opt-out (native study). This bug is for 2 of 3 part study (2/3)

1. Opt-in addon
2. Optout pref flipping (limited set)
3. Optout addon Native Study (expansive set)

Comment 1

[Shilpi Gupta [:shilpi]]

Javaun, Can you please add the doc with limited cohorts here?

Comment 2

[Shilpi Gupta [:shilpi]]

Unified Shield v3 brief here:
https://docs.google.com/document/d/1AstlOsrwDXQFR2ruLEy4PpeqPiJA-36XtZUZyr3e2bc/edit#heading=h.o62ux7i4ei9e

Comment 3

[Javaun Moradi [:javaun]]

I will confirm the reduced set of branches here. Thanks!

Comment 4

[Matt Grimes [:Matt_G]]

I've looked at the study design and am giving my sign off. Please be sure to use the template for the intent to ship email from the docs so that rel-man knows what populations we're targeting and exactly what prefs we'll be changing. They'll also want to know the proposed end date.

Comment 5

[Shilpi Gupta [:shilpi]]

Thanks Matt!

Javaun - Confirming we are targeting beta 56. 
Can you share the final reduced set of branches along with the prefs? I will mention the proposed end date to be after 4 weeks from launch. Let me know if this is fine. I will get the intent to ship email going.

Comment 6

[Javaun Moradi [:javaun]]

Branches
* Control: No changes to Firefox
* Unified Search with default settings (history first): 5 history, 4 search
* Unified Search, (search first): 4 search, 5 history
* Unified Search, reduced result set, search first: 3 search, 3 history
* MAY CUT: Unified Search, Dynamic results: 3 history,3 search if Autocomplete is direct URL nav. 3 search, 3 history if autocomplete is search.


Prefs in Play:

//default = true. false unifies the two bars. Boolean
"browser.search.widget.inNavBar"

//total number N of Awesome Bar result, where first count is Autocomplete. Default = 10. Int.
"browser.urlbar.maxRichResults": 10

//Doesn't exist by default.  New String pref allowing us to "bucket" awesome Bar results. Suggestion = search, General = places.db
//Example:3 places.db then 3 search suggestions
//If matchBucketsSearch is not set, matchBuckets controls composition for all awesome bar queries
"browser.urlbar.matchBuckets": "general:3, suggestion:3"

//Doesn't exist by default. if matchBucketsSearch is set, it controls composition when autocomplete is default
"browser.urlbar.matchBucketsSearch": "general:1,suggestion:4,general:4"

==========

Exact combinations here: (Small Branch Test for Pref flip)
https://docs.google.com/document/d/1AstlOsrwDXQFR2ruLEy4PpeqPiJA-36XtZUZyr3e2bc/edit#heading=h.2ja9upt0zq1t

Comment 7

[Shilpi Gupta [:shilpi]]

We have rel man approval from Liz this AM. The study is planned to be shipped today 8/16

Comment 8

[Javaun Moradi [:javaun]]

Slight edit to the above. CUTTING the "Unified/Dynamic" and "top hit" branch for complexity

Branches
* Control: No changes to Firefox
* Unified Search with default settings (history first): 5 history, 4 search
* Unified Search, (search first): 4 search, 5 history
* Unified Search, reduced result set, search first: 3 search, 4 history
* NEW: Unified Search, reduced result set, history first: 3 history, 4 search


NOTE: In the matrix I reused "suggestion:3, general:4" for both 10 and 7 result branches. I did this to consolidate so we'd have fewer collateral branches created. What this means is that 10 result users will see 3 searches, 4 history, then 2 more search suggestion at bottom. That's fine.

REVISED MATRIX HERE, now color coded...(Small Branch Test for Pref flip)
https://docs.google.com/document/d/1AstlOsrwDXQFR2ruLEy4PpeqPiJA-36XtZUZyr3e2bc/edit#heading=h.2ja9upt0zq1t

Comment 9

[Matt Grimes [:Matt_G]]

The study is live on Beta with the revised matrix from the doc.

Participants will receive all possible combinations of the following:

browser.search.widget.inNavBar - true / false
browser.urlbar.maxRichResults - 7 / 10
browser.urlbar.matchBuckets - general:5,suggestion:4 / suggestion:3,general:4 / general:3, search:4 / general:1,search:3,general:5

Comment 10

[Javaun Moradi [:javaun]]

This is my fault, I screwed these up (fat finger) in the doc. I typed "search" when it should be "suggestion" in all case.


For the second to last "general:3, search:4" it will still work, since it will fill the remainder with search suggestions. But the last one will not work ( general:1,search:3,general:5) without the word suggestion.

> browser.urlbar.matchBuckets - general:5,suggestion:4 /
> suggestion:3,general:4 / general:3, search:4 / general:1,search:3,general:5

Comment 11

[Matt Grimes [:Matt_G]]

Here are the new values. Let me know if that looks right:

* general:1, suggestion:3, general:5
* general:3, suggestion:4 
* suggestion:3, general:4
* general:5,suggestion:4

Comment 12

[:Paolo Amadini]

With maxRichResults equal to 7:

[1] general:1,suggestion:3,general:5  =  1 heuristic, 1 "top" history, 3 suggestions, 2 history
[2] general:3,suggestion:4            =  1 heuristic, 3 history, 3 suggestions
[3] suggestion:3,general:4            =  1 heuristic, 3 suggestions, 3 history
[4] general:5,suggestion:4            =  1 heuristic, 5 history, 1 suggestions

With maxRichResults equal to 10:

[5] general:1,suggestion:3,general:5  =  1 heuristic, 1 "top" history, 3 suggestions, 5 history
[6] general:3,suggestion:4            =  1 heuristic, 3 history, 4 suggestions, 2 history
[7] suggestion:3,general:4            =  1 heuristic, 3 suggestions, 6 history
[8] general:5,suggestion:4            =  1 heuristic, 5 history, 4 suggestions

Some notes comparing what we're shipping to what is in the document:

- Branch [1] is grayed out in the document, but in practice we're still testing it.
- Branch [6] has 2 general entries at the end, likely an artifact of having "suggestion:4" instead of "suggestion:6". This is probably not particularly relevant because maxRichResults equal to 10 is not the main focus of the study.
- Branch [4] seems to be intended to be excluded by design, or to be a control branch.

The most relevant branches thus seem to be [1], [2], [3], and [8].

I'm going to use what we're shipping here as the starting point for the add-on version of the study, combined with the information in the document, so we will have at least some branches we can compare between the two studies.

Comment 13

[:Paolo Amadini]

(In reply to Matt Grimes [:Matt_G] from comment #9)
> browser.search.widget.inNavBar - true / false

One of the known caveats with the implementation of this preference is that it must be flipped after the observer for it has been registered during window initialization, or it won't take effect. Is there a way to test these pref flip scenarios, so we know that the users who had the pref flipped also had the search bar removed? Did you QA this already?

Comment 14

[:Paolo Amadini]

Also worth noting, as mentioned in the document, that branch [8] is the default behavior we've tested before.

Comment 15

[Michael Cooper [:mythmon]]

(In reply to :Paolo Amadini from comment #13)
> (In reply to Matt Grimes [:Matt_G] from comment #9)
> > browser.search.widget.inNavBar - true / false
> 
> One of the known caveats with the implementation of this preference is that
> it must be flipped after the observer for it has been registered during
> window initialization, or it won't take effect. Is there a way to test these
> pref flip scenarios, so we know that the users who had the pref flipped also
> had the search bar removed? Did you QA this already?

This is a behavior that we did not anticipate when designing Normandy. We assumed that code that used prefs would do a direct read of prefs before registering listeners, which this implementation does not. (As an aside, I think this could cause some other bugs. For example, if the pref is changed in the profile directory while Firefox isn't running, this implementation doesn't notice.)

There are two major ways that Shield changes preferences that interact with this code.

1) The very first time a profile is run in Firefox, Shield runs very soon after startup (before first paint). After a few HTTP requests it flips the preferences that it has been instructed to do so. In this case, I think there is a risk of the pref flipping before the listener is registered.

2) On subsequent runs of Firefox with a given profile, Shield runs on a 24 hour timer. This always happens at least a few seconds after startup. In this case, I think the preference observer will have been registered, and so everything will work fine.

Users that run beta with a new profile could possibly be opted in to the experiment without the UI treatment. Telemetry will mark them as participating, and have will be changed pref changed, but won't affect the user. I don't have an estimate for how prevalent this will be.

Note that users will only use method #1 or #2. After an experiment starts, it won't be restarted by the other method.

Comment 16

[Matt Grimes [:Matt_G]]

Flagging dzeber here as this will impact analysis. I assume it would be safe to throw out new profiles from analysis or we can run additional analysis that validates branch and preference value match and toss out anything that doesn't. If I understand this correctly, it *shouldn't* be an issue with the other preferences.

Comment 17

[:Paolo Amadini]

(In reply to Mike Cooper [:mythmon] from comment #15)
> This is a behavior that we did not anticipate when designing Normandy. We
> assumed that code that used prefs would do a direct read of prefs before
> registering listeners, which this implementation does not. (As an aside, I
> think this could cause some other bugs. For example, if the pref is changed
> in the profile directory while Firefox isn't running, this implementation
> doesn't notice.)

Thanks for the information. For the record, the reason we took out the synchronization on startup in bug 694291 is that we didn't want to accidentally remove or add back the search bar, which might happen in production for a number of known cases of profile migrations across versions.

Is there a way to trigger the flipping code deterministically so we can verify what happens in practice for the two cases you mentioned?

Comment 18

[Michael Cooper [:mythmon]]

There isn't a great way to deterministically test this for first startup, because it relies on a race condition during the first browser startup. Any way we could trigger this manually would avoid the race conditions, invalidating the test.

Comment 19

[Dave Zeber [:dzeber]]

We discussed this in a meeting last week, and the decision was to modify the recipe so that the pref changes are made 24 hours after firstrun. This should avoid the problem of new profiles looking like they're participating on a treatment branch but not actually receiving the UI changes.

The update was pushed Aug 17, the day after study launch. Thus, the only profiles that would be affected by this issue are study participants whose profile was created within a day of Aug 16. The plan for anaylsis is just to exclude them, which should not be an issue given the expected population sizes for opt-out.

Comment 20

[Matt Grimes [:Matt_G]]

We are ending this experiment today. Closing this bug.