1270641 - Choose winner of expanded-firstrun and uplift through 47

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chenxia Liu [:liuche]]

We need to pick a first run (A, B, C) as the winner and uplift that through Beta.

Comment 1

[Chenxia Liu [:liuche]]

Barbara, let me know when we'll have an idea about this one, or when we'll have enough data from beta to make this decision.

Comment 2

[Chenxia Liu [:liuche]]

Alex, do you know which version won? Was it the one without the Data Saver panel?

Comment 3

[Alex Davis [:adavis]]

Are you referring to this onboarding test?
https://mana.mozilla.org/wiki/pages/viewpage.action?pageId=57508096

Barbara logged her results but didn't formulate a conclusion in the document so I am unsure what she decided to do moving forward.

Version B demonstrated a slight improvement but I don't know if she felt it was worth rolling out.

Comment 4

[Chenxia Liu [:liuche]]

Barbara, what do you think? Let me know if you have a timeline for when we can pick one of these! If it's long, we might want to just land bug 1268647 with Version B, and then decide later (or Version C, either is fine with me).

Comment 5

[Barbara Bermes [:barbara] - NI please!]

Up until today, I thought B would be a slight winner, however looking at the latest results, C is showing some success now too: https://sql.telemetry.mozilla.org/dashboard/experiment-onboarding-with-multiple-slides.

When do you need this decision by? I'm also fine with going with B for now if that's easier, but wanted to bring this up.

ADavis, what are you saying?

Comment 6

[Chenxia Liu [:liuche]]

Okay, thanks! In that case, if we won't know for a while, I'll just go with C - it'll be easier to remove code rather than add it back in. I'll just land things today, and start working on the uplift patch for getting en_US only in 48 (bug 1268647).

Comment 7

[Chenxia Liu [:liuche]]

To be clear, I'm not landing anything for this bug, but am rather talking about bug 1268647.

Comment 8

[Alex Davis [:adavis]]

In the dashboard I made, B & C were tied as winners for best retention.

C works for me.

Comment 9

[Chenxia Liu [:liuche]]

Attachment: MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

Review commit: https://reviewboard.mozilla.org/r/55546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55546/

Comment 10

[Andrzej Hunt :ahunt]

Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

https://reviewboard.mozilla.org/r/55546/#review52312

::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:91
(Diff revision 1)
> -        if (SwitchBoard.isInBucket(context, 0, 33)) {
> -            return Experiments.ONBOARDING2_A.equals(experiment);
> -        } else if (SwitchBoard.isInBucket(context, 33, 66)) {
> -            return Experiments.ONBOARDING2_B.equals(experiment);
> -        } else if (SwitchBoard.isInBucket(context, 66, 100)) {
> -            return Experiments.ONBOARDING2_C.equals(experiment);
> +        return Experiments.ONBOARDING2_C.equals(experiment);

Do we need to keep this at all? It looks like we removed the check in FirstrunPagerConfig.getDefault.

Comment 11

[Chenxia Liu [:liuche]]

(In reply to Andrzej Hunt :ahunt from comment #10)
> Comment on attachment 8757028 [details]
> MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt
> 
> https://reviewboard.mozilla.org/r/55546/#review52312
> 
> ::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:91
> (Diff revision 1)
> > -        if (SwitchBoard.isInBucket(context, 0, 33)) {
> > -            return Experiments.ONBOARDING2_A.equals(experiment);
> > -        } else if (SwitchBoard.isInBucket(context, 33, 66)) {
> > -            return Experiments.ONBOARDING2_B.equals(experiment);
> > -        } else if (SwitchBoard.isInBucket(context, 66, 100)) {
> > -            return Experiments.ONBOARDING2_C.equals(experiment);
> > +        return Experiments.ONBOARDING2_C.equals(experiment);
> 
> Do we need to keep this at all? It looks like we removed the check in
> FirstrunPagerConfig.getDefault.

It's still being used by the Kinto downloadable content (incorrectly, I think) but I commented on that to see if it was intended to be a live Switchboard query, or perhaps it's meant to be added to this call.

Aurora try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f73acc49f46

Comment 12

[Chenxia Liu [:liuche]]

Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

Approval Request Comment
[Feature/regressing bug #]: Turn off experiment
[User impact if declined]: Users will still be in an experiment
[Describe test coverage new/current, TreeHerder]: local, try runs
[Risks and why]: Low, removing code
[String/UUID change made/needed]: removal of unused strings, no need to re-localize

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

I spoke to Margaret and mentioned the fact that RC week is only for release blocking issues. Keeping that bar in mind, we may not have enough time to stop this experiment. Let's uplift only to Aurora48.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

any reason why this didn't land in nightly?

Comment 15

[:Margaret Leibovic]

(In reply to Sylvestre Ledru [:sylvestre] from comment #14)
> any reason why this didn't land in nightly?

The first run flow is different on 49+ (bug 1268641), so this bug didn't affect Nightly.

Why has this still not landed on Aurora? At this point this will need to be uplifted to Beta now.

Comment 16

[Chenxia Liu [:liuche]]

Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

Trying to flag this for beta, because this missed aurora merge, but it looks like I can't clear the b- flag.

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

This is approved for uplift to Beta48.

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Wes, just fyi, this needs to land on Beta48. Thanks!

Comment 19

[Francesco Lodolo [:flod]]

Is it possible to have a patch that doesn't touch localization files for string frozen branches (aurora, beta)? Removing strings from localization files is going to create unnecessary noise in tools and dashboards, so having a patch without changes to android_strings.dtd would be ideal.

The patch removing strings can then ride the trains.

To be honest it's also absolutely unclear why this never landed in nightly and it's jumping directly to beta.

Comment 20

[Carsten Book [:Tomcat]]

i guess we need to wait till comment#19 is fixed before we can uplift

Comment 21

[Chenxia Liu [:liuche]]

Attachment: Beta patch: Disable other experiments.

Carrying over review flag, removed dtd changes.

> To be honest it's also absolutely unclear why this never landed in nightly
> and it's jumping directly to beta.

Margaret addressed this in Comment #15, and the tracking flags also show that neither 49 nor 50 are affected. Nightly (at the time of the a? request) had a completely different flow, and this didn't get uplifted so now we're landing it on beta.

Comment 22

[Chenxia Liu [:liuche]]

Ready to land now, I removed the string changes.

Beta try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eff519e8b258

Comment 23

[Carsten Book [:Tomcat]]

landed! -> https://hg.mozilla.org/releases/mozilla-beta/rev/1510116bd5e5

Comment 24

[u549602]

Verified as fixed on latest Beta build, 48.0b2. This fix was tested on a Xiaomi mi i4 with Android 5.0.2 and on a Xiaomi mi Pad2 with Android 5.1.1.