[FCM] Migrate HasPersistedMessages pref to include subscription IDs

Before this CL:
There was a one global Boolean to signal if there are persisted FCM
messages.
But messages are replayed per subscription upon registration,
so it's in efficient to keep one global flag.

After this CL:
This is replaced with a set of subscription ids that have persisted
messages.

Bug: 946486
Change-Id: I36836d224561568b84472786d0828e4dd40531fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1569936
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#651757}

components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java

@@ -17,7 +17,6 @@ import org.chromium.base.task.PostTask;
 import org.chromium.base.task.TaskTraits;
 
 import java.io.IOException;
-import java.util.Set;
 
 /**
  * This class is the Java counterpart to the C++ GCMDriverAndroid class.
@@ -53,6 +52,10 @@ public class GCMDriver {
             throw new IllegalStateException("Already instantiated");
         }
         sInstance = new GCMDriver(nativeGCMDriverAndroid);
+        // TODO(crbug.com/946486): This has been in added in M75 to migrate the
+        // way we store if there are persisted messages. It should be removed in
+        // M77.
+        LazySubscriptionsManager.migrateHasPersistedMessagesPref();
         return sInstance;
     }
 
@@ -69,30 +72,24 @@ public class GCMDriver {
 
     @CalledByNative
     private void replayPersistedMessages(final String appId) {
-        if (LazySubscriptionsManager.hasPersistedMessages()) {
-            long time = SystemClock.elapsedRealtime();
-            Set<String> lazySubscriptionIds = LazySubscriptionsManager.getLazySubscriptionIds();
-            boolean hasRemainingMessages = false;
-            for (String id : lazySubscriptionIds) {
-                if (!id.startsWith(appId)) {
-                    hasRemainingMessages = (LazySubscriptionsManager.readMessages(id).length != 0);
-                    continue;
-                }
-                GCMMessage[] messages = LazySubscriptionsManager.readMessages(id);
-                for (GCMMessage message : messages) {
-                    dispatchMessage(message);
-                }
-                LazySubscriptionsManager.deletePersistedMessagesForSubscriptionId(id);
-            }
-            LazySubscriptionsManager.storeHasPersistedMessages(hasRemainingMessages);
-            long duration = SystemClock.elapsedRealtime() - time;
-            // Call RecordHistogram.recordTimesHistogram() on a background thread to avoid expensive
-            // JNI calls in the critical path.
-            PostTask.postTask(TaskTraits.BEST_EFFORT_MAY_BLOCK, () -> {
-                RecordHistogram.recordTimesHistogram(
-                        "PushMessaging.TimeToReadPersistedMessages", duration);
-            });
+        if (!LazySubscriptionsManager.hasPersistedMessagesForSubscription(appId)) {
+            return;
+        }
+
+        long time = SystemClock.elapsedRealtime();
+        GCMMessage[] messages = LazySubscriptionsManager.readMessages(appId);
+        for (GCMMessage message : messages) {
+            dispatchMessage(message);
         }
+        LazySubscriptionsManager.deletePersistedMessagesForSubscriptionId(appId);
+
+        long duration = SystemClock.elapsedRealtime() - time;
+        // Call RecordHistogram.recordTimesHistogram() on a background thread to avoid
+        // expensive JNI calls in the critical path.
+        PostTask.postTask(TaskTraits.BEST_EFFORT_MAY_BLOCK, () -> {
+            RecordHistogram.recordTimesHistogram(
+                    "PushMessaging.TimeToReadPersistedMessages", duration);
+        });
     }
 
     @CalledByNative

components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/LazySubscriptionsManager.java

@@ -30,7 +30,9 @@ import java.util.Set;
 public class LazySubscriptionsManager {
     private static final String TAG = "LazySubscriptions";
     private static final String FCM_LAZY_SUBSCRIPTIONS = "fcm_lazy_subscriptions";
-    private static final String HAS_PERSISTED_MESSAGES_KEY = "has_persisted_messages";
+    static final String LEGACY_HAS_PERSISTED_MESSAGES_KEY = "has_persisted_messages";
+    private static final String SUBSCRIPTIONS_WITH_PERSISTED_MESSAGES_KEY =
+            "subscriptions_with_persisted_messages";
     private static final String PREF_PACKAGE =
             "org.chromium.components.gcm_driver.lazy_subscriptions";
     private static final String INVALIDATION_APP_ID = "com.google.chrome.fcm.invalidations";
@@ -46,30 +48,71 @@ public class LazySubscriptionsManager {
     private LazySubscriptionsManager() {}
 
     /**
-     * Stores a global flag that indicates whether there are any persisted
-     * messages to read. The flag could be read using hasPersistedMessages().
+     * A one time migration from the deprecated "has persisted messages" boolean
+     * flag to a set of subscription ids that have persisted messages. If the
+     * global flag is set, it add all lazy subscription ids have persisted
+     * messages and then clears the global flag.
+     */
+    public static void migrateHasPersistedMessagesPref() {
+        SharedPreferences sharedPrefs = ContextUtils.getAppSharedPreferences();
+        boolean hasPersistedMessages =
+                sharedPrefs.getBoolean(LEGACY_HAS_PERSISTED_MESSAGES_KEY, false);
+        if (!hasPersistedMessages) {
+            return;
+        }
+        Set<String> lazySubscriptionIds = getLazySubscriptionIds();
+        sharedPrefs.edit()
+                .putStringSet(SUBSCRIPTIONS_WITH_PERSISTED_MESSAGES_KEY, lazySubscriptionIds)
+                .apply();
+        sharedPrefs.edit().remove(LEGACY_HAS_PERSISTED_MESSAGES_KEY).apply();
+    }
+
+    /**
+     * Adds/Removes the |subscriptionId| to indicate whether there are any
+     * persisted messages to read for this |subscriptionId|. This information
+     * could be read using hasPersistedMessagesForSubscription().
+     * @param subscriptionId
      * @param hasPersistedMessages
      */
-    public static void storeHasPersistedMessages(boolean hasPersistedMessages) {
-        // Store the global flag in the default preferences instead of special one
-        // for the GCM messages. The reason is the default preferences file is used in
-        // many places in Chrome and should be already cached in memory by the
-        // time this method is called. Therefore, it should provide a cheap way
-        // that (most probably) doesn't require disk access to read that global flag.
+    public static void storeHasPersistedMessagesForSubscription(
+            final String subscriptionId, boolean hasPersistedMessages) {
+        // Stores the information in the default preferences instead of special
+        // one for the GCM messages. The reason is the default preferences file
+        // is used in many places in Chrome and should be already cached in
+        // memory by the time this method is called. Therefore, it should
+        // provide a cheap way that (most probably) doesn't require disk access
+        // to read that flag.
         SharedPreferences sharedPrefs = ContextUtils.getAppSharedPreferences();
-        sharedPrefs.edit().putBoolean(HAS_PERSISTED_MESSAGES_KEY, hasPersistedMessages).apply();
+        Set<String> subscriptionsWithPersistedMessages = new HashSet<>(sharedPrefs.getStringSet(
+                SUBSCRIPTIONS_WITH_PERSISTED_MESSAGES_KEY, Collections.emptySet()));
+        if (subscriptionsWithPersistedMessages.contains(subscriptionId) == hasPersistedMessages) {
+            // Correct information are already stored, nothing to do.
+            return;
+        }
+        if (hasPersistedMessages) {
+            subscriptionsWithPersistedMessages.add(subscriptionId);
+        } else {
+            subscriptionsWithPersistedMessages.remove(subscriptionId);
+        }
+        sharedPrefs.edit()
+                .putStringSet(SUBSCRIPTIONS_WITH_PERSISTED_MESSAGES_KEY,
+                        subscriptionsWithPersistedMessages)
+                .apply();
     }
 
     /**
-     * Whether some messages are persisted and should be replayed next time
-     * Chrome is running. It should be cheaper to call than actually reading the
-     * stored messages. Call this method to decide whether there is a need to
-     * read any persisted messages.
-     * @return whether some messages are persisted.
+     * Whether some messages are persisted for |subscriptionId| and should be
+     * replayed next time Chrome is running. It should be cheaper to call than
+     * actually reading the stored messages. Call this method to decide whether
+     * there is a need to read any persisted messages for that subscription.
+     * @param subscriptionId
+     * @return whether some messages are persisted for that subscription.
      */
-    public static boolean hasPersistedMessages() {
+    public static boolean hasPersistedMessagesForSubscription(final String subscriptionId) {
         SharedPreferences sharedPrefs = ContextUtils.getAppSharedPreferences();
-        return sharedPrefs.getBoolean(HAS_PERSISTED_MESSAGES_KEY, false);
+        Set<String> subscriptionsWithPersistedMessages = new HashSet<>(sharedPrefs.getStringSet(
+                SUBSCRIPTIONS_WITH_PERSISTED_MESSAGES_KEY, Collections.emptySet()));
+        return subscriptionsWithPersistedMessages.contains(subscriptionId);
     }
 
     /**
@@ -185,7 +228,7 @@ public class LazySubscriptionsManager {
             // Add the new message to the end.
             queueJSON.put(message.toJSON());
             sharedPrefs.edit().putString(subscriptionId, queueJSON.toString()).apply();
-            storeHasPersistedMessages(/*hasPersistedMessages=*/true);
+            storeHasPersistedMessagesForSubscription(subscriptionId, /*hasPersistedMessages=*/true);
         } catch (JSONException e) {
             Log.e(TAG,
                     "Error when parsing the persisted message queue for subscriber:"
@@ -243,6 +286,8 @@ public class LazySubscriptionsManager {
         SharedPreferences sharedPrefs =
                 context.getSharedPreferences(PREF_PACKAGE, Context.MODE_PRIVATE);
         sharedPrefs.edit().remove(subscriptionId).apply();
+        LazySubscriptionsManager.storeHasPersistedMessagesForSubscription(
+                subscriptionId, /*hasPersistedMessages=*/false);
     }
 
     /**

components/gcm_driver/android/junit/src/org/chromium/components/gcm_driver/LazySubscriptionsManagerTest.java

@@ -9,6 +9,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import android.content.SharedPreferences;
 import android.os.Bundle;
 
 import org.junit.Before;
@@ -16,6 +17,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.annotation.Config;
 
+import org.chromium.base.ContextUtils;
 import org.chromium.base.metrics.CachedMetrics;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.test.ShadowRecordHistogram;
@@ -41,14 +43,44 @@ public class LazySubscriptionsManagerTest {
      */
     @Test
     public void testHasPersistedMessages() {
+        final String subscriptionId = "subscription_id";
         // Default is false.
-        assertFalse(LazySubscriptionsManager.hasPersistedMessages());
+        assertFalse(LazySubscriptionsManager.hasPersistedMessagesForSubscription(subscriptionId));
+
+        LazySubscriptionsManager.storeHasPersistedMessagesForSubscription(subscriptionId, true);
+        assertTrue(LazySubscriptionsManager.hasPersistedMessagesForSubscription(subscriptionId));
+
+        LazySubscriptionsManager.storeHasPersistedMessagesForSubscription(subscriptionId, false);
+        assertFalse(LazySubscriptionsManager.hasPersistedMessagesForSubscription(subscriptionId));
+    }
+
+    /**
+     * Tests the migration path from one boolean pref to a set subscription ids for persisted
+     * messages.
+     */
+    @Test
+    public void testMigrateHasPersistedMessagesPref() {
+        final String subscriptionId1 = "subscription_id1";
+        final String subscriptionId2 = "subscription_id2";
+        LazySubscriptionsManager.storeLazinessInformation(subscriptionId1, true);
+        LazySubscriptionsManager.storeLazinessInformation(subscriptionId2, true);
+
+        SharedPreferences sharedPrefs = ContextUtils.getAppSharedPreferences();
+        sharedPrefs.edit()
+                .putBoolean(LazySubscriptionsManager.LEGACY_HAS_PERSISTED_MESSAGES_KEY, false)
+                .apply();
+        LazySubscriptionsManager.migrateHasPersistedMessagesPref();
+
+        assertFalse(LazySubscriptionsManager.hasPersistedMessagesForSubscription(subscriptionId1));
+        assertFalse(LazySubscriptionsManager.hasPersistedMessagesForSubscription(subscriptionId2));
 
-        LazySubscriptionsManager.storeHasPersistedMessages(true);
-        assertTrue(LazySubscriptionsManager.hasPersistedMessages());
+        sharedPrefs.edit()
+                .putBoolean(LazySubscriptionsManager.LEGACY_HAS_PERSISTED_MESSAGES_KEY, true)
+                .apply();
+        LazySubscriptionsManager.migrateHasPersistedMessagesPref();
 
-        LazySubscriptionsManager.storeHasPersistedMessages(false);
-        assertFalse(LazySubscriptionsManager.hasPersistedMessages());
+        assertTrue(LazySubscriptionsManager.hasPersistedMessagesForSubscription(subscriptionId1));
+        assertTrue(LazySubscriptionsManager.hasPersistedMessagesForSubscription(subscriptionId2));
     }
 
     /**