Bug 1419581 - Part 1: Simplify MMA GCM sender IDs logic. r=nechen
☠☠ backed out by a6a0d6dc6548 ☠ ☠
authorTad <tad@spotco.us>
Fri, 12 Jan 2018 15:03:37 -0800
changeset 453811 93547276fba805541a03a47c99516a61846e0cc5
parent 453810 45b5309a73cbde68fab5b75168bbeaba076ff238
child 453812 bc4cda1cc57cf032ac373911a372f45d1c6d4da8
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnechen
bugs1419581
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.