1348820 - Ship Activity Stream as an experiment to Nightly audience (50/50)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

We want to ship Activity Stream as an experiment to our Nightly users. 50% of the users will have it enabled. All users will be able to enable/disable Activity Stream in Settings (Advanced -> Experimental features).

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1348820 - Remove MOZ_ANDROID_ACTIVITY_STREAM build flag.

This flag wasn't used in the build system anyways. It was used to hide early builds of the
new Activity Stream UI. However this is now going to be controlled by Switchboard experiments.

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

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1348820 - Setup A/B experiment for enabling Activity Stream in Nightly.

This is a bit complicated. But most of that code should go away again as soon as
we can stop shipping the opt-out preference.

With this patch we have three flags that can be controlled via Switchboard:

* activity-stream: This is our global kill switch and allows us to pull the feature
  if needed. A user has to be in this experiment to ever see activity stream. The
  goal is to enable this experiment for 100% of the Nightly audience.

* activity-stream-opt-out: This is experiment will enable the Activity Stream by
  default. The goal is to enable this experiment for 50% of the Nightly audience.

* activity-stream-settings: This experiment controls the visibility of a setting
  to enable/disable activity stream (settings -> advanced -> experimental features).
  This allows us to control whether users can opt-in or opt-out of the activity
  experiment. The goal is to enable this for 100% of the Nightly audience.

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

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1348820 - Always collect metadata from visited pages.

We are going to need this in the future and starting collection even before releasing
Activity Stream will create a better experience once we turn it on.

And this flag is hard to miss. So let's just get rid of it.

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

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849516 [details]
Bug 1348820 - Setup A/B experiment for enabling Activity Stream in Nightly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122050/diff/1-2/

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849517 [details]
Bug 1348820 - Always collect metadata from visited pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122314/diff/1-2/

Comment 6

[:Grisha Kruglov]

Comment on attachment 8849515 [details]
Bug 1348820 - Remove MOZ_ANDROID_ACTIVITY_STREAM build flag.

https://reviewboard.mozilla.org/r/122048/#review124642

Comment 7

[:Grisha Kruglov]

Comment on attachment 8849516 [details]
Bug 1348820 - Setup A/B experiment for enabling Activity Stream in Nightly.

https://reviewboard.mozilla.org/r/122050/#review124648

::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStream.java:71
(Diff revision 2)
> +                .apply();
> +    }
> +
> +    /**
> +     * Returns true if Activity Stream has been enabled by the user. Before calling this method
> +     * hasUserEnabledOrDisabled() should be used to determine whether the user actually has made

You're not asserting this via .contains call here to avoid having to make another .contains call?

::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamPreference.java:16
(Diff revision 2)
> +
> +import org.mozilla.gecko.GeckoAppShell;
> +import org.mozilla.gecko.util.ThreadUtils;
> +
> +/**
> + * A custom switch preference that is used while we allow user's to opt-out from using Activity Stream.

nit: `users`, not `user's`

::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamPreference.java:45
(Diff revision 2)
> +        init(context);
> +    }
> +
> +    private void init(Context context) {
> +        // The SwitchPreference shouldn't do any persistence itself. We want to avoid that a value
> +        // is written that is not set by the user but set from an experiment.

"We want to avoid writing a value that is ..."

Comment 8

[:Grisha Kruglov]

Comment on attachment 8849517 [details]
Bug 1348820 - Always collect metadata from visited pages.

https://reviewboard.mozilla.org/r/122314/#review124700

I'm curious if we ever quantified perf impact of collecting metadata? As we let this ride the trains, it'll be good to know for certain what will happen: no significant impact, minor regression we can live with, significant regression we'll need to address...

Also, _can_ we quanitify this right now? Even if there are good metrics that will indicate this, comparing nightly vs others is probably not going to work. A switchboard flag to a/b test this and see what we're impacting..? I'm curious if you think this is worth doing.

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849516 [details]
Bug 1348820 - Setup A/B experiment for enabling Activity Stream in Nightly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122050/diff/2-3/

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849517 [details]
Bug 1348820 - Always collect metadata from visited pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122314/diff/2-3/

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to :Grisha Kruglov from comment #8)
> I'm curious if we ever quantified perf impact of collecting metadata? As we
> let this ride the trains, it'll be good to know for certain what will
> happen: no significant impact, minor regression we can live with,
> significant regression we'll need to address...
> 
> Also, _can_ we quanitify this right now? Even if there are good metrics that
> will indicate this, comparing nightly vs others is probably not going to
> work. A switchboard flag to a/b test this and see what we're impacting..?
> I'm curious if you think this is worth doing.

Do you mean perf impact on website loading? This should be covered by our autophone tests:
https://mzl.la/2nLtg1G

If you mean the performance of the collecting itself: Yeah, this would be quite interesting. I'll file a bug to measure that. I wonder how we can do that without sending telemetry whenever the user loads a website. That might be too much.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4775bb6e6b61
Remove MOZ_ANDROID_ACTIVITY_STREAM build flag. r=Grisha
https://hg.mozilla.org/integration/autoland/rev/387d0326e3a0
Setup A/B experiment for enabling Activity Stream in Nightly. r=Grisha
https://hg.mozilla.org/integration/autoland/rev/2fffcd4ce3e4
Always collect metadata from visited pages. r=Grisha

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=86041724&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/377c33c931fe

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/65a83a84898e
Backed out 3 changesets for testANRReporter failures on Android rc1 a=backout

Comment 15

[Sebastian Kaspari (:sebastian; :pocmo)]

This is a weird one. All tests pass and then afterwards the app crashes. And of course this doesn't happen locally. This will need some debugging.

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

After manual bisecting and pushing commits to try it looks like the build flag changes caused this.

Comment 17

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849516 [details]
Bug 1348820 - Setup A/B experiment for enabling Activity Stream in Nightly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122050/diff/3-4/

Comment 18

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849517 [details]
Bug 1348820 - Always collect metadata from visited pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122314/diff/3-4/

Comment 19

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849516 [details]
Bug 1348820 - Setup A/B experiment for enabling Activity Stream in Nightly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122050/diff/4-5/

Comment 20

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849517 [details]
Bug 1348820 - Always collect metadata from visited pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122314/diff/4-5/

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a51d4de1147
Setup A/B experiment for enabling Activity Stream in Nightly. r=Grisha
https://hg.mozilla.org/integration/autoland/rev/fc897556cfed
Always collect metadata from visited pages. r=Grisha

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0a51d4de1147
https://hg.mozilla.org/mozilla-central/rev/fc897556cfed