Move a SharedPref from ChannelsUpdater to ChromePreferenceKeys

Register it in ChromePreferenceKeys and use SharedPreferencesManager
consistently instead of SharedPreferences directly.

Bug: 1022108
Change-Id: Id810a00d8135d6e742f314bc01573aa3fe7c463c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032330
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737482}

chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelsUpdater.java

@@ -4,25 +4,24 @@
 
 package org.chromium.chrome.browser.notifications.channels;
 
-import android.content.SharedPreferences;
 import android.os.Build;
 
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.chrome.browser.notifications.NotificationManagerProxyImpl;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 
 /**
  * Contains helper methods for checking if we should update channels and updating them if so.
  */
 public class ChannelsUpdater {
-    @VisibleForTesting
-    static final String CHANNELS_VERSION_KEY = "channels_version_key";
 
     private static final Object sLock = new Object();
 
     private final ChannelsInitializer mChannelsInitializer;
-    private final SharedPreferences mSharedPreferences;
+    private final SharedPreferencesManager mSharedPreferences;
     private final boolean mIsAtLeastO;
     private final int mChannelsVersion;
 
@@ -35,15 +34,15 @@ public class ChannelsUpdater {
         // when we won't need it. It's ok for these parameters to be null when mIsAtLeastO is false.
         public static final ChannelsUpdater INSTANCE = Build.VERSION.SDK_INT < Build.VERSION_CODES.O
                 ? new ChannelsUpdater(false /* isAtLeastO */, null, null, -1)
-                : new ChannelsUpdater(true /* isAtLeastO */, ContextUtils.getAppSharedPreferences(),
-                          new ChannelsInitializer(new NotificationManagerProxyImpl(
-                                                          ContextUtils.getApplicationContext()),
-                                  ContextUtils.getApplicationContext().getResources()),
-                          ChannelDefinitions.CHANNELS_VERSION);
+                : new ChannelsUpdater(true /* isAtLeastO */, SharedPreferencesManager.getInstance(),
+                        new ChannelsInitializer(new NotificationManagerProxyImpl(
+                                                        ContextUtils.getApplicationContext()),
+                                ContextUtils.getApplicationContext().getResources()),
+                        ChannelDefinitions.CHANNELS_VERSION);
     }
 
     @VisibleForTesting
-    ChannelsUpdater(boolean isAtLeastO, SharedPreferences sharedPreferences,
+    ChannelsUpdater(boolean isAtLeastO, SharedPreferencesManager sharedPreferences,
             ChannelsInitializer channelsInitializer, int channelsVersion) {
         mIsAtLeastO = isAtLeastO;
         mSharedPreferences = sharedPreferences;
@@ -53,7 +52,9 @@ public class ChannelsUpdater {
 
     public boolean shouldUpdateChannels() {
         return mIsAtLeastO
-                && mSharedPreferences.getInt(CHANNELS_VERSION_KEY, -1) != mChannelsVersion;
+                && mSharedPreferences.readInt(
+                           ChromePreferenceKeys.NOTIFICATIONS_CHANNELS_VERSION, -1)
+                != mChannelsVersion;
     }
 
     public void updateChannels() {
@@ -76,6 +77,7 @@ public class ChannelsUpdater {
 
     private void storeChannelVersionInPrefs() {
         assert mSharedPreferences != null;
-        mSharedPreferences.edit().putInt(CHANNELS_VERSION_KEY, mChannelsVersion).apply();
+        mSharedPreferences.writeInt(
+                ChromePreferenceKeys.NOTIFICATIONS_CHANNELS_VERSION, mChannelsVersion);
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/notifications/channels/ChannelsUpdaterTest.java

@@ -26,10 +26,11 @@ import org.junit.rules.TestRule;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.test.BaseJUnit4ClassRunner;
-import org.chromium.base.test.util.InMemorySharedPreferences;
 import org.chromium.base.test.util.MinAndroidSdkLevel;
 import org.chromium.chrome.browser.notifications.NotificationManagerProxy;
 import org.chromium.chrome.browser.notifications.NotificationManagerProxyImpl;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.content_public.browser.test.NativeLibraryTestRule;
 
@@ -44,7 +45,7 @@ import java.util.List;
 @TargetApi(Build.VERSION_CODES.O)
 public class ChannelsUpdaterTest {
     private NotificationManagerProxy mNotificationManagerProxy;
-    private InMemorySharedPreferences mMockSharedPreferences;
+    private SharedPreferencesManager mSharedPreferences;
     private ChannelsInitializer mChannelsInitializer;
     private Resources mMockResources;
 
@@ -67,7 +68,7 @@ public class ChannelsUpdaterTest {
 
         mChannelsInitializer =
                 new ChannelsInitializer(mNotificationManagerProxy, context.getResources());
-        mMockSharedPreferences = new InMemorySharedPreferences();
+        mSharedPreferences = SharedPreferencesManager.getInstance();
 
         // Delete any channels that may already have been initialized. Cleaning up here rather than
         // in tearDown in case tests running before these ones caused channels to be created.
@@ -84,7 +85,7 @@ public class ChannelsUpdaterTest {
     @TargetApi(Build.VERSION_CODES.O)
     public void testShouldUpdateChannels_returnsFalsePreO() {
         ChannelsUpdater updater = new ChannelsUpdater(
-                false /* isAtLeastO */, mMockSharedPreferences, mChannelsInitializer, 0);
+                false /* isAtLeastO */, mSharedPreferences, mChannelsInitializer, 0);
         assertThat(updater.shouldUpdateChannels(), is(false));
     }
 
@@ -94,7 +95,7 @@ public class ChannelsUpdaterTest {
     @TargetApi(Build.VERSION_CODES.O)
     public void testShouldUpdateChannels_returnsTrueIfOAndNoSavedVersionInPrefs() {
         ChannelsUpdater updater = new ChannelsUpdater(
-                true /* isAtLeastO */, mMockSharedPreferences, mChannelsInitializer, 0);
+                true /* isAtLeastO */, mSharedPreferences, mChannelsInitializer, 0);
         assertThat(updater.shouldUpdateChannels(), is(true));
     }
 
@@ -103,9 +104,9 @@ public class ChannelsUpdaterTest {
     @MinAndroidSdkLevel(Build.VERSION_CODES.O)
     @TargetApi(Build.VERSION_CODES.O)
     public void testShouldUpdateChannels_returnsTrueIfOAndDifferentVersionInPrefs() {
-        mMockSharedPreferences.edit().putInt(ChannelsUpdater.CHANNELS_VERSION_KEY, 4).apply();
+        mSharedPreferences.writeInt(ChromePreferenceKeys.NOTIFICATIONS_CHANNELS_VERSION, 4);
         ChannelsUpdater updater = new ChannelsUpdater(
-                true /* isAtLeastO */, mMockSharedPreferences, mChannelsInitializer, 5);
+                true /* isAtLeastO */, mSharedPreferences, mChannelsInitializer, 5);
         assertThat(updater.shouldUpdateChannels(), is(true));
     }
 
@@ -114,9 +115,9 @@ public class ChannelsUpdaterTest {
     @MinAndroidSdkLevel(Build.VERSION_CODES.O)
     @TargetApi(Build.VERSION_CODES.O)
     public void testShouldUpdateChannels_returnsFalseIfOAndSameVersionInPrefs() {
-        mMockSharedPreferences.edit().putInt(ChannelsUpdater.CHANNELS_VERSION_KEY, 3).apply();
+        mSharedPreferences.writeInt(ChromePreferenceKeys.NOTIFICATIONS_CHANNELS_VERSION, 3);
         ChannelsUpdater updater = new ChannelsUpdater(
-                true /* isAtLeastO */, mMockSharedPreferences, mChannelsInitializer, 3);
+                true /* isAtLeastO */, mSharedPreferences, mChannelsInitializer, 3);
         assertThat(updater.shouldUpdateChannels(), is(false));
     }
 
@@ -126,11 +127,13 @@ public class ChannelsUpdaterTest {
     @TargetApi(Build.VERSION_CODES.O)
     public void testUpdateChannels_noopPreO() {
         ChannelsUpdater updater = new ChannelsUpdater(
-                false /* isAtLeastO */, mMockSharedPreferences, mChannelsInitializer, 21);
+                false /* isAtLeastO */, mSharedPreferences, mChannelsInitializer, 21);
         updater.updateChannels();
 
         assertThat(getChannelsIgnoringDefault(), hasSize(0));
-        assertThat(mMockSharedPreferences.getInt(ChannelsUpdater.CHANNELS_VERSION_KEY, -1), is(-1));
+        assertThat(
+                mSharedPreferences.readInt(ChromePreferenceKeys.NOTIFICATIONS_CHANNELS_VERSION, -1),
+                is(-1));
     }
 
     @Test
@@ -139,7 +142,7 @@ public class ChannelsUpdaterTest {
     @TargetApi(Build.VERSION_CODES.O)
     public void testUpdateChannels_createsExpectedChannelsAndUpdatesPref() {
         ChannelsUpdater updater = new ChannelsUpdater(
-                true /* isAtLeastO */, mMockSharedPreferences, mChannelsInitializer, 21);
+                true /* isAtLeastO */, mSharedPreferences, mChannelsInitializer, 21);
         updater.updateChannels();
 
         assertThat(getChannelsIgnoringDefault(), hasSize((greaterThan(0))));
@@ -148,7 +151,9 @@ public class ChannelsUpdaterTest {
                         ChannelDefinitions.ChannelId.DOWNLOADS,
                         ChannelDefinitions.ChannelId.INCOGNITO,
                         ChannelDefinitions.ChannelId.MEDIA));
-        assertThat(mMockSharedPreferences.getInt(ChannelsUpdater.CHANNELS_VERSION_KEY, -1), is(21));
+        assertThat(
+                mSharedPreferences.readInt(ChromePreferenceKeys.NOTIFICATIONS_CHANNELS_VERSION, -1),
+                is(21));
     }
 
     @Test
@@ -167,7 +172,7 @@ public class ChannelsUpdaterTest {
             mNotificationManagerProxy.createNotificationChannel(channel);
         }
 
-        ChannelsUpdater updater = new ChannelsUpdater(true /* isAtLeastO */, mMockSharedPreferences,
+        ChannelsUpdater updater = new ChannelsUpdater(true /* isAtLeastO */, mSharedPreferences,
                 new ChannelsInitializer(mNotificationManagerProxy, mMockResources), 12);
         updater.updateChannels();
 

chrome/browser/preferences/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceKeys.java

@@ -431,6 +431,7 @@ public final class ChromePreferenceKeys {
 
     public static final String MEDIA_WEBRTC_NOTIFICATION_IDS = "WebRTCNotificationIds";
 
+    public static final String NOTIFICATIONS_CHANNELS_VERSION = "channels_version_key";
     public static final String NOTIFICATIONS_LAST_SHOWN_NOTIFICATION_TYPE =
             "NotificationUmaTracker.LastShownNotificationType";
     public static final String NOTIFICATIONS_NEXT_TRIGGER =
@@ -836,6 +837,7 @@ public final class ChromePreferenceKeys {
                 LOCALE_MANAGER_SEARCH_ENGINE_PROMO_SHOW_STATE,
                 LOCALE_MANAGER_WAS_IN_SPECIAL_LOCALE,
                 MEDIA_WEBRTC_NOTIFICATION_IDS,
+                NOTIFICATIONS_CHANNELS_VERSION,
                 NOTIFICATIONS_LAST_SHOWN_NOTIFICATION_TYPE,
                 NOTIFICATIONS_NEXT_TRIGGER,
                 NTP_SNIPPETS_IS_SCHEDULED,