From 5545cfcfe10a8064e4c61dc791531437c436364a Mon Sep 17 00:00:00 2001 From: Tad Date: Fri, 12 Jan 2018 15:03:37 -0800 Subject: [PATCH] Bug 1419581 - Part 1: Simplify MMA GCM sender IDs logic. r=nechen Right now, the MMA glue is built into constants.jar. constants.jar is the home of preprocessed Java code; it's built very early in the build process and intended to be a tiny kernel of shared definitions. The fact that the MMA glue has to live there is just a sad consequence of the non-Gradle build system, which makes dependency injection difficult. Unfortunately, another consequence is that it's not possible to move reference org.mozilla.gecko.{gcm,push} in the MMA glue, because those packages are built after constants.jar. Instead, this patch lifts some of the logic into AppConstants, which is part of constants.jar. We had grown a twisty maze of indirection around the GCM sender IDs and it just wasn't necessary; this just lifts the static pieces up a level and removes a bunch of interface indirection. What surprises me is that asking Google's InstanceId.getToken for a GCM token with a "comma,separated,list" of GCM sender IDs works -- and indeed, has worked since we added the second MMA sender ID. I didn't expect that and can't explain it, but this doesn't change that logic and local testing (both of the existing APKs, and APKs with this modification) looks good. MozReview-Commit-ID: 3hObfAwNlPH *** a0c07e53 o draft Bug 1419581 - Part 1: Move MMA setGcmSenderID from MmaDelegate to MmaLeanplumImp. r=nechen MozReview-Commit-ID: A4hrk6pVqGW --HG-- extra : rebase_source : ce7c1585529e61491a0133633b976b27083c2372 extra : intermediate-source : f8b3e95f18e4082ab8404187508d09eadba8612e extra : source : 8f1655752d43af33356d497d559888a967bbf6a0 --- .../org/mozilla/gecko/push/TestPushManager.java | 2 +- mobile/android/base/AppConstants.java.in | 6 +++++- .../java/org/mozilla/gecko/mma/MmaDelegate.java | 6 ------ .../java/org/mozilla/gecko/mma/MmaInterface.java | 4 ---- .../java/org/mozilla/gecko/mma/MmaLeanplumImp.java | 12 ++---------- .../base/java/org/mozilla/gecko/mma/MmaStubImp.java | 10 ---------- .../java/org/mozilla/gecko/push/PushManager.java | 13 ++----------- 7 files changed, 10 insertions(+), 43 deletions(-) diff --git a/mobile/android/app/src/test/java/org/mozilla/gecko/push/TestPushManager.java b/mobile/android/app/src/test/java/org/mozilla/gecko/push/TestPushManager.java index 42ae0f543fa7..4be60e70beda 100644 --- a/mobile/android/app/src/test/java/org/mozilla/gecko/push/TestPushManager.java +++ b/mobile/android/app/src/test/java/org/mozilla/gecko/push/TestPushManager.java @@ -190,7 +190,7 @@ public class TestPushManager { public void testStartupBeforeConfiguration() throws Exception { verify(gcmTokenClient, never()).getToken(anyString(), anyBoolean()); manager.startup(System.currentTimeMillis()); - verify(gcmTokenClient, times(1)).getToken(AppConstants.MOZ_ANDROID_GCM_SENDERID, false); + verify(gcmTokenClient, times(1)).getToken(AppConstants.MOZ_ANDROID_GCM_SENDERIDS, false); } @Test diff --git a/mobile/android/base/AppConstants.java.in b/mobile/android/base/AppConstants.java.in index 02cc4a70981f..34e483a6db77 100644 --- a/mobile/android/base/AppConstants.java.in +++ b/mobile/android/base/AppConstants.java.in @@ -114,11 +114,15 @@ public class AppConstants { false; //#endif - public static final String MOZ_ANDROID_GCM_SENDERID = + public static final String MOZ_ANDROID_GCM_SENDERIDS = +//#ifdef MOZ_MMA_GCM_SENDERID + "@MOZ_ANDROID_GCM_SENDERID@,@MOZ_MMA_GCM_SENDERID@"; +//#else //#ifdef MOZ_ANDROID_GCM_SENDERID "@MOZ_ANDROID_GCM_SENDERID@"; //#else null; +//#endif //#endif public static final String MOZ_CHILD_PROCESS_NAME = "@MOZ_CHILD_PROCESS_NAME@"; diff --git a/mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java b/mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java index 1d203d183696..fd248df0dd61 100644 --- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java +++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java @@ -25,7 +25,6 @@ import org.mozilla.gecko.Tab; import org.mozilla.gecko.Tabs; import org.mozilla.gecko.fxa.FirefoxAccounts; import org.mozilla.gecko.preferences.GeckoPreferences; -import org.mozilla.gecko.push.PushManager; import org.mozilla.gecko.switchboard.SwitchBoard; import org.mozilla.gecko.util.ContextUtils; @@ -75,7 +74,6 @@ public class MmaDelegate { // we gather the information here then pass to mmaHelper.init() // Note that generateUserAttribute always return a non null HashMap. final Map attributes = gatherUserAttributes(activity); - mmaHelper.setGcmSenderId(PushManager.getSenderIds()); final String deviceId = getDeviceId(activity); mmaHelper.setDeviceId(deviceId); PrefsHelper.setPref(GeckoPreferences.PREFS_MMA_DEVICE_ID, deviceId); @@ -161,10 +159,6 @@ public class MmaDelegate { } } - public static String getMmaSenderId() { - return mmaHelper.getMmaSenderId(); - } - private static String getDeviceId(Activity activity) { if (SwitchBoard.isInExperiment(activity, Experiments.LEANPLUM_DEBUG)) { return DEBUG_LEANPLUM_DEVICE_ID; diff --git a/mobile/android/base/java/org/mozilla/gecko/mma/MmaInterface.java b/mobile/android/base/java/org/mozilla/gecko/mma/MmaInterface.java index 317922f2fe71..c5c20ade6090 100644 --- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaInterface.java +++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaInterface.java @@ -20,8 +20,6 @@ public interface MmaInterface { void init(Activity Activity, Map attributes); - void setGcmSenderId(String senderIds); - void setCustomIcon(@DrawableRes int iconResId); void start(Context context); @@ -34,7 +32,5 @@ public interface MmaInterface { @CheckResult boolean handleGcmMessage(Context context, String from, Bundle bundle); - String getMmaSenderId(); - void setDeviceId(@NonNull String deviceId); } diff --git a/mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java b/mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java index b7943f0bc528..8da751b3c9b0 100644 --- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java +++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java @@ -46,6 +46,8 @@ public class MmaLeanplumImp implements MmaInterface { Leanplum.setAppIdForDevelopmentMode(MmaConstants.MOZ_LEANPLUM_SDK_CLIENTID, MmaConstants.MOZ_LEANPLUM_SDK_KEY); } + LeanplumPushService.setGcmSenderId(AppConstants.MOZ_ANDROID_GCM_SENDERIDS); + if (attributes != null) { Leanplum.start(activity, attributes); } else { @@ -69,11 +71,6 @@ public class MmaLeanplumImp implements MmaInterface { }); } - @Override - public void setGcmSenderId(String senderIds) { - LeanplumPushService.setGcmSenderId(senderIds); - } - @Override public void setCustomIcon(@DrawableRes final int iconResId) { LeanplumPushService.setCustomizer(new LeanplumPushNotificationCustomizer() { @@ -117,11 +114,6 @@ public class MmaLeanplumImp implements MmaInterface { return false; } - @Override - public String getMmaSenderId() { - return MmaConstants.MOZ_MMA_SENDER_ID; - } - @Override public void setDeviceId(@NonNull String deviceId) { Leanplum.setDeviceId(deviceId); diff --git a/mobile/android/base/java/org/mozilla/gecko/mma/MmaStubImp.java b/mobile/android/base/java/org/mozilla/gecko/mma/MmaStubImp.java index fec85177f94c..b84de3b9d386 100644 --- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaStubImp.java +++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaStubImp.java @@ -21,11 +21,6 @@ public class MmaStubImp implements MmaInterface { } - @Override - public void setGcmSenderId(String senderIds) { - - } - @Override public void setCustomIcon(@DrawableRes int iconResId) { @@ -56,11 +51,6 @@ public class MmaStubImp implements MmaInterface { return false; } - @Override - public String getMmaSenderId() { - return ""; - } - @Override public void setDeviceId(@NonNull String deviceId) { diff --git a/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java b/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java index 0522ac2b6139..47b18e5d96d3 100644 --- a/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java +++ b/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java @@ -57,15 +57,6 @@ public class PushManager { this.pushClientFactory = pushClientFactory; } - public static String getSenderIds() { - final String mmaSenderId = MmaDelegate.getMmaSenderId(); - if (mmaSenderId != null && mmaSenderId.length() > 0) { - return AppConstants.MOZ_ANDROID_GCM_SENDERID + "," + mmaSenderId; - } else { - return AppConstants.MOZ_ANDROID_GCM_SENDERID; - } - } - public PushRegistration registrationForSubscription(String chid) { // chids are globally unique, so we're not concerned about finding a chid associated to // any particular profile. @@ -253,7 +244,7 @@ public class PushManager { } protected @NonNull PushRegistration advanceRegistration(final PushRegistration registration, final @NonNull String profileName, final long now) throws AutopushClientException, PushClient.LocalException, GcmTokenClient.NeedsGooglePlayServicesException, IOException { - final Fetched gcmToken = gcmClient.getToken(getSenderIds(), registration.debug); + final Fetched gcmToken = gcmClient.getToken(AppConstants.MOZ_ANDROID_GCM_SENDERIDS, registration.debug); final PushClient pushClient = pushClientFactory.getPushClient(registration.autopushEndpoint, registration.debug); @@ -305,7 +296,7 @@ public class PushManager { public void startup(long now) { try { Log.i(LOG_TAG, "Startup: requesting GCM token."); - gcmClient.getToken(getSenderIds(), false); // For side-effects. + gcmClient.getToken(AppConstants.MOZ_ANDROID_GCM_SENDERIDS, false); // For side-effects. } catch (GcmTokenClient.NeedsGooglePlayServicesException e) { // Requires user intervention. At App startup, we don't want to address this. In // response to user activity, we do want to try to have the user address this.