Closed Bug 1348820 Opened 3 years ago Closed 3 years ago

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

Categories

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

All
Android
enhancement

Tracking

()

RESOLVED FIXED
Firefox 55
Iteration:
1.18
Tracking Status
firefox55 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

(Whiteboard: [MobileAS])

Attachments

(2 files, 1 obsolete file)

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 on attachment 8849515 [details]
Bug 1348820 - Remove MOZ_ANDROID_ACTIVITY_STREAM build flag.

https://reviewboard.mozilla.org/r/122048/#review124642
Attachment #8849515 - Flags: review?(gkruglov) → review+
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 ..."
Attachment #8849516 - Flags: review?(gkruglov) → review+
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.
Attachment #8849517 - Flags: review?(gkruglov) → review+
(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.
Pushed by s.kaspari@gmail.com:
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
Backout by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/65a83a84898e
Backed out 3 changesets for testANRReporter failures on Android rc1 a=backout
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.
Flags: needinfo?(s.kaspari)
After manual bisecting and pushing commits to try it looks like the build flag changes caused this.
Attachment #8849515 - Attachment is obsolete: true
Pushed by s.kaspari@gmail.com:
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
See Also: → 1350886
https://hg.mozilla.org/mozilla-central/rev/0a51d4de1147
https://hg.mozilla.org/mozilla-central/rev/fc897556cfed
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Depends on: 1351295
You need to log in before you can comment on or make changes to this bug.