1351581 - Limit Leanplum to specific regions (via Switchboard)

Status: RESOLVED DUPLICATE of bug 1351585

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

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

We do not want to enable Leanplum for all users. Instead the marketing/product teams want to use it in specific markets.

To determine the region we can use the same approach we already use in SearchEngineManager to determine the (default) search engine(s) for the user/installation.

We want to be able to update those regions without app updates. Therefore we would like to make this configurable by a Switchboard experiment. In Switchboard we can specify the regions as additional values of the experiment.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Note that using Switchboard means it will take some time until we know whether we can use Leanplum. Therefore it won't be available on the first start and we might not be able to use it in the first-run immediately.

Comment 2

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1351581 - Use Switchboard to limit Leanplum startup/init

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

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1351581 - Add build flag to inject mma implementation.

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

Comment 4

[Nevin Chen(Not active on Bugzilla)]

Attachment: WIP. Please help... Did I miss anything?

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

Comment 5

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8867155 [details]
Bug 1351581 - Add build flag to inject mma implementation.

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

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8867155 [details]
Bug 1351581 - Add build flag to inject mma implementation.

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

Comment 7

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8867155 [details]
Bug 1351581 - Add build flag to inject mma implementation.

https://reviewboard.mozilla.org/r/138750/#review142180

::: commit-message-5864d:1
(Diff revision 3)
> +Bug 1351581 - Add build flag to inject mma implementation. r?maliu,sebastian,nalexander

I'd really prefer we don't use `mma` everywhere -- it's not a term I'd expect most people to know.  (To me, it means "Mixed Martial Arts", which is ... not right.)

Why not just call this `leanplum`?

::: mobile/android/base/MmaConstants.java.in:17
(Diff revision 3)
> +//#else
> +import org.mozilla.gecko.mma.MmaStubImp;
> +//#endif
> +
> +public class MmaConstants {
> +    public static final String MOZ_INSTALL_MMA_SDK_APP_TOKEN =

I don't see anything about a Leanplum token in the other patches.  What's the status here?  There's work needed to get tokens on the releng builder machines which we should track, if you're not already doing so.

::: mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java:17
(Diff revision 3)
> +
> +public class MmaDelegate {
> +
> +    private static MmaInterface mmaHelper = MmaConstants.getMma();
> +
> +    public static String FIRST_RUN = "First Run";

Are these events that are relevant to Leanplum?  What happens as these evolve over time?

Comment 8

[Nick Alexander :nalexander [he/him]]

Gah, hit publish before writing my summary comment (different between MozReview and Github reviews!).

I mean to say that this is r- only 'cuz I want to get clarity on the name from our other reviewers and see what you intend with the Leanplum token.  Everything else here looks good.

Comment 9

[Nevin Chen(Not active on Bugzilla)]

Bug 1351585 also covered this bug. Please mark dup.

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8867052 [details]
Bug 1351581 - Use Switchboard to limit Leanplum startup/init

https://reviewboard.mozilla.org/r/138666/#review145510

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:886
(Diff revision 1)
> +            @Override
> +            protected Void doInBackground(Void... params) {
> +                super.doInBackground(params);
> +                if (SwitchBoard.isInExperiment(context, Experiments.LEANPLUM) &&
> +                        GeckoPreferences.getBooleanPref(context, GeckoPreferences.PREFS_HEALTHREPORT_UPLOAD_ENABLED, true)) {
> +                    // Do LeanPlum start/init here

I thought this needs to be done in the Application class? Is it enough to do it here in BrowserApp only?

::: mobile/android/base/java/org/mozilla/gecko/Experiments.java:75
(Diff revision 1)
>  
>      // Enable full bookmark management(full-page dialog, bookmark/folder modification, etc.)
>      public static final String FULL_BOOKMARK_MANAGEMENT = "full-bookmark-management";
>  
> +    // Enable Leanplum SDK
> +    public static final String LEANPLUM = "leanplum-start";

Maybe just "leanplum"?

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Wait. This is already closed. This is pretty confusing. That's the second bug with reviews that has been closed. Please clear the review flags too.