Move SharedPrefs from tabmodel to ChromePreferenceKeys.

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

Bug: 1022108
Change-Id: I696158b3391df4fe37a4ccab43e9e9e2a38424f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1994097
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736618}

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.tabmodel;
 
 import android.content.Context;
-import android.content.SharedPreferences;
 import android.os.StrictMode;
 import android.os.SystemClock;
 import android.support.v4.util.AtomicFile;
@@ -35,6 +34,8 @@ import org.chromium.base.task.TaskRunner;
 import org.chromium.base.task.TaskTraits;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.ntp.NewTabPage;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabIdManager;
 import org.chromium.chrome.browser.tab.TabImpl;
@@ -76,17 +77,6 @@ public class TabPersistentStore extends TabPersister {
 
     private static final String BASE_STATE_FOLDER = "tabs";
 
-    /** The name of the directory where the state is saved. */
-    @VisibleForTesting
-    static final String SAVED_STATE_DIRECTORY = "0";
-
-    @VisibleForTesting
-    static final String PREF_ACTIVE_TAB_ID =
-            "org.chromium.chrome.browser.tabmodel.TabPersistentStore.ACTIVE_TAB_ID";
-
-    private static final String PREF_HAS_COMPUTED_MAX_ID =
-            "org.chromium.chrome.browser.tabmodel.TabPersistentStore.HAS_COMPUTED_MAX_ID";
-
     /** Prevents two TabPersistentStores from saving the same file simultaneously. */
     private static final Object SAVE_LIST_LOCK = new Object();
 
@@ -189,7 +179,6 @@ public class TabPersistentStore extends TabPersister {
     private SparseIntArray mNormalTabsRestored;
     private SparseIntArray mIncognitoTabsRestored;
 
-    private SharedPreferences mPreferences;
     private SequencedTaskRunner mSequencedTaskRunner;
     private AsyncTask<DataInputStream> mPrefetchTabListTask;
     private List<Pair<AsyncTask<DataInputStream>, String>> mPrefetchTabListToMergeTasks;
@@ -224,7 +213,6 @@ public class TabPersistentStore extends TabPersister {
         mTabIdsToRestore = new HashSet<>();
         mObservers = new ObserverList<>();
         mObservers.addObserver(observer);
-        mPreferences = ContextUtils.getAppSharedPreferences();
         TaskTraits taskTraits = TaskTraits.USER_BLOCKING_MAY_BLOCK;
         mSequencedTaskRunner = PostTask.createSequencedTaskRunner(taskTraits);
         mPrefetchTabListToMergeTasks = new ArrayList<>();
@@ -560,7 +548,8 @@ public class TabPersistentStore extends TabPersister {
         try {
             long time = SystemClock.uptimeMillis();
             TabState state;
-            int restoredTabId = mPreferences.getInt(PREF_ACTIVE_TAB_ID, Tab.INVALID_TAB_ID);
+            int restoredTabId = SharedPreferencesManager.getInstance().readInt(
+                    ChromePreferenceKeys.TABMODEL_ACTIVE_TAB_ID, Tab.INVALID_TAB_ID);
             if (restoredTabId == tabToRestore.id && mPrefetchActiveTabTask != null) {
                 long timeWaitingForPrefetch = SystemClock.uptimeMillis();
                 state = mPrefetchActiveTabTask.get();
@@ -861,8 +850,8 @@ public class TabPersistentStore extends TabPersister {
             activeTabId = normalModel.getTabAt(activeIndex).getId();
         }
         // Always override the existing value in case there is no active tab.
-        ContextUtils.getAppSharedPreferences().edit().putInt(
-                PREF_ACTIVE_TAB_ID, activeTabId).apply();
+        SharedPreferencesManager.getInstance().writeInt(
+                ChromePreferenceKeys.TABMODEL_ACTIVE_TAB_ID, activeTabId);
 
         byte[] listData = serializeMetadata(normalInfo, incognitoInfo, tabsBeingRestored);
         return new TabModelSelectorMetadata(listData, normalInfo, incognitoInfo, tabsBeingRestored);
@@ -1012,7 +1001,10 @@ public class TabPersistentStore extends TabPersister {
      * @throws IOException
      */
     private void checkAndUpdateMaxTabId() throws IOException {
-        if (mPreferences.getBoolean(PREF_HAS_COMPUTED_MAX_ID, false)) return;
+        if (SharedPreferencesManager.getInstance().readBoolean(
+                    ChromePreferenceKeys.TABMODEL_HAS_COMPUTED_MAX_ID, false)) {
+            return;
+        }
 
         int maxId = 0;
         // Calculation of the max tab ID is done only once per user and is stored in
@@ -1054,7 +1046,8 @@ public class TabPersistentStore extends TabPersister {
             StrictMode.setThreadPolicy(oldPolicy);
         }
         TabIdManager.getInstance().incrementIdCounterTo(maxId);
-        mPreferences.edit().putBoolean(PREF_HAS_COMPUTED_MAX_ID, true).apply();
+        SharedPreferencesManager.getInstance().writeBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_COMPUTED_MAX_ID, true);
     }
 
     /**
@@ -1426,7 +1419,8 @@ public class TabPersistentStore extends TabPersister {
     }
 
     private void startPrefetchActiveTabTask(TaskRunner taskRunner) {
-        final int activeTabId = mPreferences.getInt(PREF_ACTIVE_TAB_ID, Tab.INVALID_TAB_ID);
+        final int activeTabId = SharedPreferencesManager.getInstance().readInt(
+                ChromePreferenceKeys.TABMODEL_ACTIVE_TAB_ID, Tab.INVALID_TAB_ID);
         if (activeTabId == Tab.INVALID_TAB_ID) return;
         mPrefetchActiveTabTask = new BackgroundOnlyAsyncTask<TabState>() {
             @Override

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabbedModeTabPersistencePolicy.java

@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.tabmodel;
 
-import android.content.SharedPreferences;
 import android.os.StrictMode;
 import android.util.Pair;
 import android.util.SparseBooleanArray;
@@ -24,6 +23,8 @@ import org.chromium.base.task.BackgroundOnlyAsyncTask;
 import org.chromium.base.task.TaskRunner;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.flags.FeatureUtilities;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.tab.TabState;
 
 import java.io.BufferedInputStream;
@@ -46,15 +47,6 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
     @VisibleForTesting
     static final String LEGACY_SAVED_STATE_FILE = "tab_state";
 
-    @VisibleForTesting
-    static final String PREF_HAS_RUN_FILE_MIGRATION =
-            "org.chromium.chrome.browser.tabmodel.TabPersistentStore.HAS_RUN_FILE_MIGRATION";
-
-    @VisibleForTesting
-    static final String PREF_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION =
-            "org.chromium.chrome.browser.tabmodel.TabPersistentStore."
-            + "HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION";
-
     /** The name of the directory where the state is saved. */
     @VisibleForTesting
     static final String SAVED_STATE_DIRECTORY = "0";
@@ -76,7 +68,6 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
 
     private static File sStateDirectory;
 
-    private final SharedPreferences mPreferences;
     private final int mSelectorIndex;
     private final int mOtherSelectorIndex;
     private final boolean mMergeTabs;
@@ -92,7 +83,6 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
      *                      tabbed mode files.
      */
     public TabbedModeTabPersistencePolicy(int selectorIndex, boolean mergeTabs) {
-        mPreferences = ContextUtils.getAppSharedPreferences();
         mSelectorIndex = selectorIndex;
         mOtherSelectorIndex = selectorIndex == 0 ? 1 : 0;
         mMergeTabs = mergeTabs;
@@ -158,10 +148,11 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
     public boolean performInitialization(TaskRunner taskRunner) {
         ThreadUtils.assertOnUiThread();
 
-        final boolean hasRunLegacyMigration =
-                mPreferences.getBoolean(PREF_HAS_RUN_FILE_MIGRATION, false);
+        final boolean hasRunLegacyMigration = SharedPreferencesManager.getInstance().readBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_RUN_FILE_MIGRATION, false);
         final boolean hasRunMultiInstanceMigration =
-                mPreferences.getBoolean(PREF_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, false);
+                SharedPreferencesManager.getInstance().readBoolean(
+                        ChromePreferenceKeys.TABMODEL_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, false);
 
         if (hasRunLegacyMigration && hasRunMultiInstanceMigration) return false;
 
@@ -304,11 +295,13 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
     }
 
     private void setLegacyFileMigrationPref() {
-        mPreferences.edit().putBoolean(PREF_HAS_RUN_FILE_MIGRATION, true).apply();
+        SharedPreferencesManager.getInstance().writeBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_RUN_FILE_MIGRATION, true);
     }
 
     private void setMultiInstanceFileMigrationPref() {
-        mPreferences.edit().putBoolean(PREF_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, true).apply();
+        SharedPreferencesManager.getInstance().writeBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, true);
     }
 
     @Override

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/MultiInstanceMigrationTest.java

@@ -17,6 +17,8 @@ import org.junit.runner.RunWith;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.test.util.AdvancedMockContext;
 import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.tab.TabState;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.tabmodel.MockTabModelSelector;
@@ -43,8 +45,8 @@ public class MultiInstanceMigrationTest {
 
         // Set the shared pref stating that the legacy file migration has occurred. The
         // multi-instance migration won't happen if the legacy path is taken.
-        ContextUtils.getAppSharedPreferences().edit().putBoolean(
-                TabbedModeTabPersistencePolicy.PREF_HAS_RUN_FILE_MIGRATION, true).apply();
+        SharedPreferencesManager.getInstance().writeBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_RUN_FILE_MIGRATION, true);
     }
 
     @After
@@ -106,8 +108,8 @@ public class MultiInstanceMigrationTest {
         buildPersistentStoreAndWaitForMigration();
 
         // Make sure we don't hit the migration path again.
-        Assert.assertTrue(ContextUtils.getAppSharedPreferences().getBoolean(
-                TabbedModeTabPersistencePolicy.PREF_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, false));
+        Assert.assertTrue(SharedPreferencesManager.getInstance().readBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, false));
 
         // Check that all metadata files moved.
         File newStateFile0 = new File(

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/RestoreMigrateTest.java

@@ -22,6 +22,8 @@ import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.test.util.AdvancedMockContext;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.RetryOnFailure;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabIdManager;
 import org.chromium.chrome.browser.tab.TabState;
@@ -126,8 +128,8 @@ public class RestoreMigrateTest {
         store.waitForMigrationToFinish();
 
         // Make sure we don't hit the migration path again.
-        Assert.assertTrue(ContextUtils.getAppSharedPreferences().getBoolean(
-                TabbedModeTabPersistencePolicy.PREF_HAS_RUN_FILE_MIGRATION, false));
+        Assert.assertTrue(SharedPreferencesManager.getInstance().readBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_RUN_FILE_MIGRATION, false));
 
         // Check that the files were moved.
         File newDir = TabbedModeTabPersistencePolicy.getOrCreateTabbedModeStateDirectory();

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelMergingTest.java

@@ -23,7 +23,6 @@ import org.junit.runner.RunWith;
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.ApplicationStatus.ActivityStateListener;
-import org.chromium.base.ContextUtils;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
@@ -36,6 +35,8 @@ import org.chromium.chrome.browser.ChromeTabbedActivity2;
 import org.chromium.chrome.browser.multiwindow.MultiInstanceManager;
 import org.chromium.chrome.browser.multiwindow.MultiWindowTestHelper;
 import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.tab.TabLaunchType;
 import org.chromium.chrome.browser.tabmodel.TabPersistentStoreTest.MockTabPersistentStoreObserver;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
@@ -84,11 +85,10 @@ public class TabModelMergingTest {
     public void setUp() throws Exception {
         mActivityTestRule.startMainActivityOnBlankPage();
         // Make sure file migrations don't run as they are unnecessary since app data was cleared.
-        ContextUtils.getAppSharedPreferences().edit().putBoolean(
-                TabbedModeTabPersistencePolicy.PREF_HAS_RUN_FILE_MIGRATION, true).apply();
-        ContextUtils.getAppSharedPreferences().edit().putBoolean(
-                TabbedModeTabPersistencePolicy.PREF_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, true)
-                        .apply();
+        SharedPreferencesManager prefs = SharedPreferencesManager.getInstance();
+        prefs.writeBoolean(ChromePreferenceKeys.TABMODEL_HAS_RUN_FILE_MIGRATION, true);
+        prefs.writeBoolean(
+                ChromePreferenceKeys.TABMODEL_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION, true);
 
         // Some of the logic for when to trigger a merge depends on whether the activity is in
         // multi-window mode. Set isInMultiWindowMode to true to avoid merging unexpectedly.

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.tabmodel;
 
 import android.app.Activity;
-import android.content.SharedPreferences;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
 import android.util.Pair;
@@ -33,6 +32,8 @@ import org.chromium.chrome.browser.accessibility_tab_switcher.OverviewListLayout
 import org.chromium.chrome.browser.compositor.overlays.strip.StripLayoutHelper;
 import org.chromium.chrome.browser.flags.ActivityType;
 import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.tab.MockTab;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabLaunchType;
@@ -287,7 +288,7 @@ public class TabPersistentStoreTest {
     /** Class for mocking out the directory containing all of the TabState files. */
     private TestTabModelDirectory mMockDirectory;
     private AdvancedMockContext mAppContext;
-    private SharedPreferences mPreferences;
+    private SharedPreferencesManager mPreferences;
 
     @Before
     public void setUp() {
@@ -326,7 +327,7 @@ public class TabPersistentStoreTest {
                                                       .getTargetContext()
                                                       .getApplicationContext());
         ContextUtils.initApplicationContextForTests(mAppContext);
-        mPreferences = ContextUtils.getAppSharedPreferences();
+        mPreferences = SharedPreferencesManager.getInstance();
         mMockDirectory = new TestTabModelDirectory(
                 mAppContext, "TabPersistentStoreTest", Integer.toString(SELECTOR_INDEX));
         TabPersistentStore.setBaseStateDirectoryForTests(mMockDirectory.getBaseDirectory());
@@ -571,8 +572,7 @@ public class TabPersistentStoreTest {
         mMockDirectory.writeTabModelFiles(info, true);
 
         // Set to pre-fetch
-        mPreferences.edit().putInt(
-                TabPersistentStore.PREF_ACTIVE_TAB_ID, info.selectedTabId).apply();
+        mPreferences.writeInt(ChromePreferenceKeys.TABMODEL_ACTIVE_TAB_ID, info.selectedTabId);
 
         // Initialize the classes.
         MockTabModelSelector mockSelector = new MockTabModelSelector(0, 0, null);
@@ -595,13 +595,13 @@ public class TabPersistentStoreTest {
                     AsyncTask.Status.FINISHED, store.mPrefetchActiveTabTask.getStatus());
 
             // Confirm that the correct active tab ID is updated when saving state.
-            mPreferences.edit().putInt(TabPersistentStore.PREF_ACTIVE_TAB_ID, -1).apply();
+            mPreferences.writeInt(ChromePreferenceKeys.TABMODEL_ACTIVE_TAB_ID, -1);
 
             store.saveState();
         });
 
-        Assert.assertEquals(
-                info.selectedTabId, mPreferences.getInt(TabPersistentStore.PREF_ACTIVE_TAB_ID, -1));
+        Assert.assertEquals(info.selectedTabId,
+                mPreferences.readInt(ChromePreferenceKeys.TABMODEL_ACTIVE_TAB_ID, -1));
     }
 
     /**

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

@@ -570,6 +570,16 @@ public final class ChromePreferenceKeys {
     public static final String TABBED_ACTIVITY_LAST_BACKGROUNDED_TIME_MS_PREF =
             "ChromeTabbedActivity.BackgroundTimeMs";
 
+    public static final String TABMODEL_ACTIVE_TAB_ID =
+            "org.chromium.chrome.browser.tabmodel.TabPersistentStore.ACTIVE_TAB_ID";
+    public static final String TABMODEL_HAS_COMPUTED_MAX_ID =
+            "org.chromium.chrome.browser.tabmodel.TabPersistentStore.HAS_COMPUTED_MAX_ID";
+    public static final String TABMODEL_HAS_RUN_FILE_MIGRATION =
+            "org.chromium.chrome.browser.tabmodel.TabPersistentStore.HAS_RUN_FILE_MIGRATION";
+    public static final String TABMODEL_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION =
+            "org.chromium.chrome.browser.tabmodel.TabPersistentStore."
+            + "HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION";
+
     public static final String TAB_ID_MANAGER_NEXT_ID =
             "org.chromium.chrome.browser.tab.TabIdManager.NEXT_ID";
 
@@ -849,6 +859,10 @@ public final class ChromePreferenceKeys {
                 SURVEY_INFO_BAR_DISPLAYED,
                 SYNC_SESSIONS_UUID,
                 TABBED_ACTIVITY_LAST_BACKGROUNDED_TIME_MS_PREF,
+                TABMODEL_ACTIVE_TAB_ID,
+                TABMODEL_HAS_COMPUTED_MAX_ID,
+                TABMODEL_HAS_RUN_FILE_MIGRATION,
+                TABMODEL_HAS_RUN_MULTI_INSTANCE_FILE_MIGRATION,
                 TAB_ID_MANAGER_NEXT_ID,
                 TOS_ACKED_ACCOUNTS,
                 TWA_DIALOG_NUMBER_OF_DISMISSALS_ON_CLEAR_DATA,