Commit 584216e1 authored by Henrique Nakashima's avatar Henrique Nakashima Committed by Commit Bot

Move SharedPrefs from ChromeSurveyController to ChromePreferenceKeys

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

Bug: 1022108
Change-Id: I3dcf06de160044cc0abd423ede2738ef212b21de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2005368
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733403}
parent dbf78e6e
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.survey; package org.chromium.chrome.browser.survey;
import android.content.Context; import android.content.Context;
import android.content.SharedPreferences;
import android.os.Handler; import android.os.Handler;
import android.text.TextUtils; import android.text.TextUtils;
...@@ -27,6 +26,8 @@ import org.chromium.chrome.browser.infobar.InfoBarContainerLayout.Item; ...@@ -27,6 +26,8 @@ import org.chromium.chrome.browser.infobar.InfoBarContainerLayout.Item;
import org.chromium.chrome.browser.infobar.InfoBarIdentifier; import org.chromium.chrome.browser.infobar.InfoBarIdentifier;
import org.chromium.chrome.browser.infobar.SurveyInfoBar; import org.chromium.chrome.browser.infobar.SurveyInfoBar;
import org.chromium.chrome.browser.infobar.SurveyInfoBarDelegate; import org.chromium.chrome.browser.infobar.SurveyInfoBarDelegate;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.settings.privacy.PrivacyPreferencesManager; import org.chromium.chrome.browser.settings.privacy.PrivacyPreferencesManager;
import org.chromium.chrome.browser.tab.EmptyTabObserver; import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
...@@ -46,13 +47,6 @@ import java.util.Random; ...@@ -46,13 +47,6 @@ import java.util.Random;
* Class that controls if and when to show surveys related to the Chrome Home experiment. * Class that controls if and when to show surveys related to the Chrome Home experiment.
*/ */
public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimationListener { public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimationListener {
/**
* The survey questions for this survey are the same as those in the survey used for Chrome
* Home, so we reuse the old infobar key to prevent the users from seeing the same survey more
* than once.
*/
static final String SURVEY_INFO_BAR_DISPLAYED_KEY = "chrome_home_survey_info_bar_displayed";
static final String DATE_LAST_ROLLED_KEY = "last_rolled_for_chrome_survey_key";
private static final String CHROME_SURVEY_TRIAL_NAME = "ChromeSurvey"; private static final String CHROME_SURVEY_TRIAL_NAME = "ChromeSurvey";
private static final String MAX_NUMBER = "max-number"; private static final String MAX_NUMBER = "max-number";
...@@ -269,8 +263,8 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation ...@@ -269,8 +263,8 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation
/** @return If the survey info bar for this survey was logged as seen before. */ /** @return If the survey info bar for this survey was logged as seen before. */
@VisibleForTesting @VisibleForTesting
boolean hasInfoBarBeenDisplayed() { boolean hasInfoBarBeenDisplayed() {
SharedPreferences sharedPreferences = ContextUtils.getAppSharedPreferences(); SharedPreferencesManager preferences = SharedPreferencesManager.getInstance();
if (sharedPreferences.getLong(SURVEY_INFO_BAR_DISPLAYED_KEY, -1L) != -1L) { if (preferences.readLong(ChromePreferenceKeys.SURVEY_INFO_BAR_DISPLAYED, -1L) != -1L) {
recordSurveyFilteringResult(FilteringResult.SURVEY_INFOBAR_ALREADY_DISPLAYED); recordSurveyFilteringResult(FilteringResult.SURVEY_INFOBAR_ALREADY_DISPLAYED);
return true; return true;
} }
...@@ -317,8 +311,8 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation ...@@ -317,8 +311,8 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation
*/ */
@VisibleForTesting @VisibleForTesting
boolean isRandomlySelectedForSurvey() { boolean isRandomlySelectedForSurvey() {
SharedPreferences preferences = ContextUtils.getAppSharedPreferences(); SharedPreferencesManager preferences = SharedPreferencesManager.getInstance();
int lastDate = preferences.getInt(DATE_LAST_ROLLED_KEY, -1); int lastDate = preferences.readInt(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED, -1);
int today = getDayOfYear(); int today = getDayOfYear();
if (lastDate == today) { if (lastDate == today) {
recordSurveyFilteringResult(FilteringResult.USER_ALREADY_SAMPLED_TODAY); recordSurveyFilteringResult(FilteringResult.USER_ALREADY_SAMPLED_TODAY);
...@@ -345,7 +339,7 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation ...@@ -345,7 +339,7 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation
return false; return false;
} }
preferences.edit().putInt(DATE_LAST_ROLLED_KEY, today).apply(); preferences.writeInt(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED, today);
if (getRandomNumberUpTo(maxNumber) == 0) { if (getRandomNumberUpTo(maxNumber) == 0) {
recordSurveyFilteringResult(FilteringResult.USER_SELECTED_FOR_SURVEY); recordSurveyFilteringResult(FilteringResult.USER_SELECTED_FOR_SURVEY);
return true; return true;
...@@ -436,7 +430,7 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation ...@@ -436,7 +430,7 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation
}; };
} }
/** Logs in {@link SharedPreferences} that the info bar was displayed. */ /** Logs in SharedPreferences that the info bar was displayed. */
private void recordInfoBarDisplayed() { private void recordInfoBarDisplayed() {
// This can be called multiple times e.g. by mLoggingHandler & onSurveyInfoBarClosed(). // This can be called multiple times e.g. by mLoggingHandler & onSurveyInfoBarClosed().
// Return early to allow only one call to this method (http://crbug.com/791076). // Return early to allow only one call to this method (http://crbug.com/791076).
...@@ -447,10 +441,9 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation ...@@ -447,10 +441,9 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation
mLoggingHandler.removeCallbacksAndMessages(null); mLoggingHandler.removeCallbacksAndMessages(null);
SharedPreferences sharedPreferences = ContextUtils.getAppSharedPreferences(); SharedPreferencesManager preferences = SharedPreferencesManager.getInstance();
sharedPreferences.edit() preferences.writeLong(
.putLong(SURVEY_INFO_BAR_DISPLAYED_KEY, System.currentTimeMillis()) ChromePreferenceKeys.SURVEY_INFO_BAR_DISPLAYED, System.currentTimeMillis());
.apply();
mSurveyInfoBarTab = null; mSurveyInfoBarTab = null;
} }
...@@ -518,7 +511,7 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation ...@@ -518,7 +511,7 @@ public class ChromeSurveyController implements InfoBarContainer.InfoBarAnimation
@VisibleForTesting @VisibleForTesting
public static String getChromeSurveyInfoBarDisplayedKey() { public static String getChromeSurveyInfoBarDisplayedKey() {
return SURVEY_INFO_BAR_DISPLAYED_KEY; return ChromePreferenceKeys.SURVEY_INFO_BAR_DISPLAYED;
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -10,7 +10,6 @@ import static org.mockito.Mockito.doReturn; ...@@ -10,7 +10,6 @@ import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import android.content.SharedPreferences;
import android.util.ArrayMap; import android.util.ArrayMap;
import org.junit.After; import org.junit.After;
...@@ -22,10 +21,11 @@ import org.mockito.Mock; ...@@ -22,10 +21,11 @@ import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.chromium.base.ContextUtils;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
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.Tab;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
...@@ -43,7 +43,7 @@ public class ChromeSurveyControllerTest { ...@@ -43,7 +43,7 @@ public class ChromeSurveyControllerTest {
private TestChromeSurveyController mTestController; private TestChromeSurveyController mTestController;
private RiggedSurveyController mRiggedController; private RiggedSurveyController mRiggedController;
private SharedPreferences mSharedPreferences; private SharedPreferencesManager mSharedPreferences;
@Mock @Mock
Tab mTab; Tab mTab;
...@@ -61,7 +61,7 @@ public class ChromeSurveyControllerTest { ...@@ -61,7 +61,7 @@ public class ChromeSurveyControllerTest {
mTestController = new TestChromeSurveyController(); mTestController = new TestChromeSurveyController();
mTestController.setTabModelSelector(mSelector); mTestController.setTabModelSelector(mSelector);
mSharedPreferences = ContextUtils.getAppSharedPreferences(); mSharedPreferences = SharedPreferencesManager.getInstance();
Assert.assertNull("Tab should be null", mTestController.getLastTabInfobarShown()); Assert.assertNull("Tab should be null", mTestController.getLastTabInfobarShown());
Map<String, Boolean> featureMap = new ArrayMap<>(); Map<String, Boolean> featureMap = new ArrayMap<>();
featureMap.put(ChromeFeatureList.HORIZONTAL_TAB_SWITCHER_ANDROID, true); featureMap.put(ChromeFeatureList.HORIZONTAL_TAB_SWITCHER_ANDROID, true);
...@@ -76,12 +76,10 @@ public class ChromeSurveyControllerTest { ...@@ -76,12 +76,10 @@ public class ChromeSurveyControllerTest {
@Test @Test
public void testInfoBarDisplayedBefore() { public void testInfoBarDisplayedBefore() {
Assert.assertFalse( Assert.assertFalse(
mSharedPreferences.contains(ChromeSurveyController.SURVEY_INFO_BAR_DISPLAYED_KEY)); mSharedPreferences.contains(ChromePreferenceKeys.SURVEY_INFO_BAR_DISPLAYED));
Assert.assertFalse(mTestController.hasInfoBarBeenDisplayed()); Assert.assertFalse(mTestController.hasInfoBarBeenDisplayed());
mSharedPreferences.edit() mSharedPreferences.writeLong(
.putLong(ChromeSurveyController.SURVEY_INFO_BAR_DISPLAYED_KEY, ChromePreferenceKeys.SURVEY_INFO_BAR_DISPLAYED, System.currentTimeMillis());
System.currentTimeMillis())
.apply();
Assert.assertTrue(mTestController.hasInfoBarBeenDisplayed()); Assert.assertTrue(mTestController.hasInfoBarBeenDisplayed());
} }
...@@ -171,7 +169,7 @@ public class ChromeSurveyControllerTest { ...@@ -171,7 +169,7 @@ public class ChromeSurveyControllerTest {
@Test @Test
public void testEligibilityRolledYesterday() { public void testEligibilityRolledYesterday() {
mRiggedController = new RiggedSurveyController(0, 5, 10); mRiggedController = new RiggedSurveyController(0, 5, 10);
mSharedPreferences.edit().putInt(ChromeSurveyController.DATE_LAST_ROLLED_KEY, 4); mSharedPreferences.writeInt(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED, 4);
Assert.assertTrue( Assert.assertTrue(
"Random selection should be true", mRiggedController.isRandomlySelectedForSurvey()); "Random selection should be true", mRiggedController.isRandomlySelectedForSurvey());
} }
...@@ -179,7 +177,7 @@ public class ChromeSurveyControllerTest { ...@@ -179,7 +177,7 @@ public class ChromeSurveyControllerTest {
@Test @Test
public void testEligibilityRollingTwiceSameDay() { public void testEligibilityRollingTwiceSameDay() {
mRiggedController = new RiggedSurveyController(0, 5, 10); mRiggedController = new RiggedSurveyController(0, 5, 10);
mSharedPreferences.edit().putInt(ChromeSurveyController.DATE_LAST_ROLLED_KEY, 5).apply(); mSharedPreferences.writeInt(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED, 5);
Assert.assertFalse("Random selection should be false", Assert.assertFalse("Random selection should be false",
mRiggedController.isRandomlySelectedForSurvey()); mRiggedController.isRandomlySelectedForSurvey());
} }
...@@ -188,22 +186,22 @@ public class ChromeSurveyControllerTest { ...@@ -188,22 +186,22 @@ public class ChromeSurveyControllerTest {
public void testEligibilityFirstTimeRollingQualifies() { public void testEligibilityFirstTimeRollingQualifies() {
mRiggedController = new RiggedSurveyController(0, 5, 10); mRiggedController = new RiggedSurveyController(0, 5, 10);
Assert.assertFalse( Assert.assertFalse(
mSharedPreferences.contains(ChromeSurveyController.DATE_LAST_ROLLED_KEY)); mSharedPreferences.contains(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED));
Assert.assertTrue( Assert.assertTrue(
"Random selection should be true", mRiggedController.isRandomlySelectedForSurvey()); "Random selection should be true", mRiggedController.isRandomlySelectedForSurvey());
Assert.assertEquals("Numbers should match", 5, Assert.assertEquals("Numbers should match", 5,
mSharedPreferences.getInt(ChromeSurveyController.DATE_LAST_ROLLED_KEY, -1)); mSharedPreferences.readInt(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED, -1));
} }
@Test @Test
public void testEligibilityFirstTimeRollingDoesNotQualify() { public void testEligibilityFirstTimeRollingDoesNotQualify() {
mRiggedController = new RiggedSurveyController(5, 1, 10); mRiggedController = new RiggedSurveyController(5, 1, 10);
Assert.assertFalse( Assert.assertFalse(
mSharedPreferences.contains(ChromeSurveyController.DATE_LAST_ROLLED_KEY)); mSharedPreferences.contains(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED));
Assert.assertFalse( Assert.assertFalse(
"Random selection should be true", mRiggedController.isRandomlySelectedForSurvey()); "Random selection should be true", mRiggedController.isRandomlySelectedForSurvey());
Assert.assertEquals("Numbers should match", 1, Assert.assertEquals("Numbers should match", 1,
mSharedPreferences.getInt(ChromeSurveyController.DATE_LAST_ROLLED_KEY, -1)); mSharedPreferences.readInt(ChromePreferenceKeys.SURVEY_DATE_LAST_ROLLED, -1));
} }
class RiggedSurveyController extends ChromeSurveyController { class RiggedSurveyController extends ChromeSurveyController {
......
...@@ -530,6 +530,14 @@ public final class ChromePreferenceKeys { ...@@ -530,6 +530,14 @@ public final class ChromePreferenceKeys {
public static final String SNAPSHOT_DATABASE_REMOVED = "snapshot_database_removed"; public static final String SNAPSHOT_DATABASE_REMOVED = "snapshot_database_removed";
public static final String SURVEY_DATE_LAST_ROLLED = "last_rolled_for_chrome_survey_key";
/**
* The survey questions for this survey are the same as those in the survey used for Chrome
* Home, so we reuse the old infobar key to prevent the users from seeing the same survey more
* than once.
*/
public static final String SURVEY_INFO_BAR_DISPLAYED = "chrome_home_survey_info_bar_displayed";
public static final String SYNC_SESSIONS_UUID = "chromium.sync.sessions.id"; public static final String SYNC_SESSIONS_UUID = "chromium.sync.sessions.id";
public static final String TABBED_ACTIVITY_LAST_BACKGROUNDED_TIME_MS_PREF = public static final String TABBED_ACTIVITY_LAST_BACKGROUNDED_TIME_MS_PREF =
...@@ -806,6 +814,8 @@ public final class ChromePreferenceKeys { ...@@ -806,6 +814,8 @@ public final class ChromePreferenceKeys {
SIGNIN_PROMO_SETTINGS_PERSONALIZED_DISMISSED, SIGNIN_PROMO_SETTINGS_PERSONALIZED_DISMISSED,
SNAPSHOT_DATABASE_REMOVED, SNAPSHOT_DATABASE_REMOVED,
START_SURFACE_SINGLE_PANE_ENABLED_KEY, START_SURFACE_SINGLE_PANE_ENABLED_KEY,
SURVEY_DATE_LAST_ROLLED,
SURVEY_INFO_BAR_DISPLAYED,
SYNC_SESSIONS_UUID, SYNC_SESSIONS_UUID,
TABBED_ACTIVITY_LAST_BACKGROUNDED_TIME_MS_PREF, TABBED_ACTIVITY_LAST_BACKGROUNDED_TIME_MS_PREF,
TAB_ID_MANAGER_NEXT_ID, TAB_ID_MANAGER_NEXT_ID,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment