Bug 1419581 - Part 1: Simplify MMA GCM sender IDs logic. r=nechen draft
authorTad <tad@spotco.us>
Fri, 12 Jan 2018 15:03:37 -0800
changeset 747336 ce7c1585529e61491a0133633b976b27083c2372
parent 747335 0ab618268eb680ca7dc0eb9911dc05e7e9b1ac77
child 747337 8cc4621269ac24abe889494ccfa80e6f90c7f428
push id96881
push usernalexander@mozilla.com
push dateThu, 25 Jan 2018 22:25:52 +0000
reviewersnechen
bugs1419581
milestone60.0a1
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
mobile/android/app/src/test/java/org/mozilla/gecko/push/TestPushManager.java
mobile/android/base/AppConstants.java.in
mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java
mobile/android/base/java/org/mozilla/gecko/mma/MmaInterface.java
mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java
mobile/android/base/java/org/mozilla/gecko/mma/MmaStubImp.java
mobile/android/base/java/org/mozilla/gecko/push/PushManager.java
--- 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
@@ -185,17 +185,17 @@ public class TestPushManager {
         // Un-subscribing from an unknown channel succeeds: we just ignore the request.
         manager.unsubscribeChannel(UUID.randomUUID().toString());
     }
 
     @Test
     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
     public void testStartupBeforeRegistration() throws Exception {
         PushRegistration registration = manager.configure("default", "http://localhost:8080", true, System.currentTimeMillis());
         assertOnlyConfigured(registration, "http://localhost:8080", true);
 
         manager.startup(System.currentTimeMillis());
--- a/mobile/android/base/AppConstants.java.in
+++ b/mobile/android/base/AppConstants.java.in
@@ -109,22 +109,26 @@ public class AppConstants {
 
     public static final boolean MOZ_ANDROID_GCM =
 //#ifdef MOZ_ANDROID_GCM
     true;
 //#else
     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@";
     public static final String MOZ_UPDATE_CHANNEL = "@MOZ_UPDATE_CHANNEL@";
     public static final String OMNIJAR_NAME = "@OMNIJAR_NAME@";
     public static final String OS_TARGET = "@OS_TARGET@";
     public static final String TARGET_XPCOM_ABI = "@TARGET_XPCOM_ABI@";
 
     public static final String USER_AGENT_BOT_LIKE = "Redirector/" + AppConstants.MOZ_APP_VERSION +
--- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaDelegate.java
@@ -20,17 +20,16 @@ import android.text.TextUtils;
 import org.mozilla.gecko.Experiments;
 import org.mozilla.gecko.MmaConstants;
 import org.mozilla.gecko.PrefsHelper;
 import org.mozilla.gecko.R;
 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;
 
 import java.lang.ref.WeakReference;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.UUID;
 
@@ -70,17 +69,16 @@ public class MmaDelegate {
     private static WeakReference<Context> applicationContext;
 
     public static void init(Activity activity) {
         applicationContext = new WeakReference<>(activity.getApplicationContext());
         // Since user attributes are gathered in Fennec, not in MMA implementation,
         // we gather the information here then pass to mmaHelper.init()
         // Note that generateUserAttribute always return a non null HashMap.
         final Map<String, Object> attributes = gatherUserAttributes(activity);
-        mmaHelper.setGcmSenderId(PushManager.getSenderIds());
         final String deviceId = getDeviceId(activity);
         mmaHelper.setDeviceId(deviceId);
         PrefsHelper.setPref(GeckoPreferences.PREFS_MMA_DEVICE_ID, deviceId);
         // above two config setup required to be invoked before mmaHelper.init.
         mmaHelper.init(activity, attributes);
 
         if (!isDefaultBrowser(activity)) {
             mmaHelper.event(MmaDelegate.LAUNCH_BUT_NOT_DEFAULT_BROWSER);
@@ -156,20 +154,16 @@ public class MmaDelegate {
         if (isMmaEnabled(context)) {
             mmaHelper.setCustomIcon(R.drawable.ic_status_logo);
             return mmaHelper.handleGcmMessage(context, from, bundle);
         } else {
             return false;
         }
     }
 
-    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;
         }
 
         final SharedPreferences sharedPreferences = activity.getPreferences(Context.MODE_PRIVATE);
         String deviceId = sharedPreferences.getString(KEY_ANDROID_PREF_STRING_LEANPLUM_DEVICE_ID, null);
         if (deviceId == null) {
--- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaInterface.java
+++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaInterface.java
@@ -15,26 +15,22 @@ import android.support.annotation.NonNul
 
 import java.util.Map;
 
 
 public interface MmaInterface {
 
     void init(Activity Activity, Map<String, ?> attributes);
 
-    void setGcmSenderId(String senderIds);
-
     void setCustomIcon(@DrawableRes int iconResId);
 
     void start(Context context);
 
     void event(String mmaEvent);
 
     void event(String mmaEvent, double value);
 
     void stop();
 
     @CheckResult boolean handleGcmMessage(Context context, String from, Bundle bundle);
 
-    String getMmaSenderId();
-
     void setDeviceId(@NonNull String deviceId);
 }
--- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java
@@ -41,16 +41,18 @@ public class MmaLeanplumImp implements M
         LeanplumActivityHelper.enableLifecycleCallbacks(activity.getApplication());
 
         if (AppConstants.MOZILLA_OFFICIAL) {
             Leanplum.setAppIdForProductionMode(MmaConstants.MOZ_LEANPLUM_SDK_CLIENTID, MmaConstants.MOZ_LEANPLUM_SDK_KEY);
         } else {
             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 {
             Leanplum.start(activity);
         }
 
         // this is special to Leanplum. Since we defer LeanplumActivityHelper's onResume call till
         // switchboard completes loading. We miss the call to LeanplumActivityHelper.onResume.
@@ -65,21 +67,16 @@ public class MmaLeanplumImp implements M
             @Override
             public void run() {
                 LeanplumActivityHelper.onResume(activity);
             }
         });
     }
 
     @Override
-    public void setGcmSenderId(String senderIds) {
-        LeanplumPushService.setGcmSenderId(senderIds);
-    }
-
-    @Override
     public void setCustomIcon(@DrawableRes final int iconResId) {
         LeanplumPushService.setCustomizer(new LeanplumPushNotificationCustomizer() {
             @Override
             public void customize(NotificationCompat.Builder builder, Bundle notificationPayload) {
                 builder.setSmallIcon(iconResId);
                 builder.setDefaults(Notification.DEFAULT_SOUND);
             }
 
@@ -113,18 +110,13 @@ public class MmaLeanplumImp implements M
         if (from != null && from.equals(MmaConstants.MOZ_MMA_SENDER_ID) && bundle.containsKey(Constants.Keys.PUSH_MESSAGE_TEXT)) {
             LeanplumPushService.handleNotification(context, bundle);
             return true;
         }
         return false;
     }
 
     @Override
-    public String getMmaSenderId() {
-        return MmaConstants.MOZ_MMA_SENDER_ID;
-    }
-
-    @Override
     public void setDeviceId(@NonNull String deviceId) {
         Leanplum.setDeviceId(deviceId);
     }
 
 }
--- a/mobile/android/base/java/org/mozilla/gecko/mma/MmaStubImp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/mma/MmaStubImp.java
@@ -17,21 +17,16 @@ import java.util.Map;
 
 public class MmaStubImp implements MmaInterface {
     @Override
     public void init(Activity activity, Map<String, ?> attributes) {
 
     }
 
     @Override
-    public void setGcmSenderId(String senderIds) {
-
-    }
-
-    @Override
     public void setCustomIcon(@DrawableRes int iconResId) {
 
     }
 
     @Override
     public void start(Context context) {
 
     }
@@ -52,18 +47,13 @@ public class MmaStubImp implements MmaIn
     }
 
     @Override
     public boolean handleGcmMessage(Context context, String from, Bundle bundle) {
         return false;
     }
 
     @Override
-    public String getMmaSenderId() {
-        return "";
-    }
-
-    @Override
     public void setDeviceId(@NonNull String deviceId) {
 
     }
 
 }
--- a/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java
+++ b/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java
@@ -52,25 +52,16 @@ public class PushManager {
     }
 
     public PushManager(@NonNull PushState state, @NonNull GcmTokenClient gcmClient, @NonNull PushClientFactory pushClientFactory) {
         this.state = state;
         this.gcmClient = gcmClient;
         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.
         for (Map.Entry<String, PushRegistration> entry : state.getRegistrations().entrySet()) {
             final PushSubscription subscription = entry.getValue().getSubscription(chid);
             if (subscription != null) {
                 return entry.getValue();
             }
@@ -248,17 +239,17 @@ public class PushManager {
         if (registration == null || registration.autopushEndpoint == null) {
             Log.i(LOG_TAG, "Cannot advance to registered: registration needs configuration.");
             throw new ProfileNeedsConfigurationException();
         }
         return advanceRegistration(registration, profileName, now);
     }
 
     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);
 
         if (registration.uaid.value == null) {
             if (registration.debug) {
                 Log.i(LOG_TAG, "No uaid; requesting from autopush endpoint: " + registration.autopushEndpoint);
             } else {
                 Log.i(LOG_TAG, "No uaid: requesting from autopush endpoint.");
@@ -300,17 +291,17 @@ public class PushManager {
 
     public void invalidateGcmToken() {
         gcmClient.invalidateToken();
     }
 
     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.
             Log.w(LOG_TAG, "Startup: needs Google Play Services.  Ignoring until GCM is requested in response to user activity.");
             return;
         } catch (IOException e) {
             // We're temporarily unable to get a GCM token.  There's nothing to be done; we'll
             // try to advance the App's state in response to user activity or at next startup.