Add getInteger to PrefServiceBridge

Add a generic getInteger method to PrefServiceBridge, making use of the
prefs.h enums.

Migrate usages of the Prompt for Download pref to use the generic
methods, and move wrapper function with feature-specific type
annotations to DownloadUtils.

Bug: 1016957
Change-Id: I46f9f33f14467aa628767f722c099a39b813dee4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879552Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Commit-Queue: Natalie Chouinard <chouinard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709515}

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationCustomView.java

@@ -18,7 +18,6 @@ import android.widget.TextView;
 
 import androidx.annotation.Nullable;
 
-import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.preferences.download.DownloadDirectoryAdapter;
 import org.chromium.chrome.browser.ui.widget.text.AlertDialogEditText;
 import org.chromium.chrome.download.R;
@@ -57,8 +56,8 @@ public class DownloadLocationCustomView
         mDialogType = dialogType;
 
         // Automatically check "don't show again" the first time the user is seeing the dialog.
-        boolean isInitial = PrefServiceBridge.getInstance().getPromptForDownloadAndroid()
-                == DownloadPromptStatus.SHOW_INITIAL;
+        boolean isInitial =
+                DownloadUtils.getPromptForDownloadAndroid() == DownloadPromptStatus.SHOW_INITIAL;
         mDontShowAgain.setChecked(isInitial);
         mDontShowAgain.setOnCheckedChangeListener(this);
 
@@ -89,9 +88,8 @@ public class DownloadLocationCustomView
     // CompoundButton.OnCheckedChangeListener implementation.
     @Override
     public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
-        int newStatus =
-                isChecked ? DownloadPromptStatus.DONT_SHOW : DownloadPromptStatus.SHOW_PREFERENCE;
-        PrefServiceBridge.getInstance().setPromptForDownloadAndroid(newStatus);
+        DownloadUtils.setPromptForDownloadAndroid(
+                isChecked ? DownloadPromptStatus.DONT_SHOW : DownloadPromptStatus.SHOW_PREFERENCE);
     }
 
     // Helper methods available to DownloadLocationDialogBridge.

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialogBridge.java

@@ -209,11 +209,9 @@ public class DownloadLocationDialogBridge implements ModalDialogProperties.Contr
         // Update preference to show prompt based on whether checkbox is checked only when the user
         // click the positive button.
         if (dontShowAgain) {
-            PrefServiceBridge.getInstance().setPromptForDownloadAndroid(
-                    DownloadPromptStatus.DONT_SHOW);
+            DownloadUtils.setPromptForDownloadAndroid(DownloadPromptStatus.DONT_SHOW);
         } else {
-            PrefServiceBridge.getInstance().setPromptForDownloadAndroid(
-                    DownloadPromptStatus.SHOW_PREFERENCE);
+            DownloadUtils.setPromptForDownloadAndroid(DownloadPromptStatus.SHOW_PREFERENCE);
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java

@@ -48,6 +48,8 @@ import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.offlinepages.OfflinePageOrigin;
 import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
 import org.chromium.chrome.browser.offlinepages.downloads.OfflinePageDownloadBridge;
+import org.chromium.chrome.browser.preferences.Pref;
+import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.TabLaunchType;
@@ -1059,6 +1061,21 @@ public class DownloadUtils {
         return originalUri;
     }
 
+    /**
+     * @return The status of prompt for download pref, defined by {@link DownloadPromptStatus}.
+     */
+    @DownloadPromptStatus
+    public static int getPromptForDownloadAndroid() {
+        return PrefServiceBridge.getInstance().getInteger(Pref.PROMPT_FOR_DOWNLOAD_ANDROID);
+    }
+
+    /**
+     * @param status New status to update the prompt for download preference.
+     */
+    public static void setPromptForDownloadAndroid(@DownloadPromptStatus int status) {
+        PrefServiceBridge.getInstance().setInteger(Pref.PROMPT_FOR_DOWNLOAD_ANDROID, status);
+    }
+
     @NativeMethods
     interface Natives {
         String getFailStateMessage(@FailState int failState);

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

@@ -16,7 +16,6 @@ import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.chrome.browser.ContentSettingsType;
 import org.chromium.chrome.browser.browsing_data.TimePeriod;
-import org.chromium.chrome.browser.download.DownloadPromptStatus;
 import org.chromium.chrome.browser.preferences.languages.LanguageItem;
 import org.chromium.chrome.browser.preferences.website.ContentSettingException;
 import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
@@ -130,6 +129,14 @@ public class PrefServiceBridge {
         PrefServiceBridgeJni.get().setBoolean(PrefServiceBridge.this, preference, value);
     }
 
+    /**
+     * @param preference The name of the preference.
+     * @return value The value of the specified preference.
+     */
+    public int getInteger(@Pref int preference) {
+        return PrefServiceBridgeJni.get().getInteger(PrefServiceBridge.this, preference);
+    }
+
     /**
      * @param preference The name of the preference.
      * @param value The value the specified preference will be set to.
@@ -1037,21 +1044,6 @@ public class PrefServiceBridge {
                 PrefServiceBridge.this, directory);
     }
 
-    /**
-     * @return The status of prompt for download pref, defined by {@link DownloadPromptStatus}.
-     */
-    @DownloadPromptStatus
-    public int getPromptForDownloadAndroid() {
-        return PrefServiceBridgeJni.get().getPromptForDownloadAndroid(PrefServiceBridge.this);
-    }
-
-    /**
-     * @param status New status to update the prompt for download preference.
-     */
-    public void setPromptForDownloadAndroid(@DownloadPromptStatus int status) {
-        PrefServiceBridgeJni.get().setPromptForDownloadAndroid(PrefServiceBridge.this, status);
-    }
-
     /**
      * @return Whether the explicit language prompt was shown at least once.
      */
@@ -1094,6 +1086,7 @@ public class PrefServiceBridge {
         void setContentSetting(PrefServiceBridge caller, int contentSettingType, int setting);
         boolean getBoolean(PrefServiceBridge caller, int preference);
         void setBoolean(PrefServiceBridge caller, int preference, boolean value);
+        int getInteger(PrefServiceBridge caller, int preference);
         void setInteger(PrefServiceBridge caller, int preference, int value);
         boolean isManagedPreference(PrefServiceBridge caller, int preference);
         boolean getAcceptCookiesEnabled(PrefServiceBridge caller);
@@ -1204,8 +1197,6 @@ public class PrefServiceBridge {
         void setLanguageBlockedState(PrefServiceBridge caller, String language, boolean blocked);
         String getDownloadDefaultDirectory(PrefServiceBridge caller);
         void setDownloadAndSaveFileDefaultDirectory(PrefServiceBridge caller, String directory);
-        int getPromptForDownloadAndroid(PrefServiceBridge caller);
-        void setPromptForDownloadAndroid(PrefServiceBridge caller, int status);
         boolean getExplicitLanguageAskPromptShown(PrefServiceBridge caller);
         void setExplicitLanguageAskPromptShown(PrefServiceBridge caller, boolean shown);
         void setForceWebContentsDarkModeEnabled(PrefServiceBridge caller, boolean enabled);

chrome/android/java/src/org/chromium/chrome/browser/preferences/download/DownloadPreferences.java

@@ -12,9 +12,9 @@ import androidx.annotation.Nullable;
 
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.download.DownloadPromptStatus;
+import org.chromium.chrome.browser.download.DownloadUtils;
 import org.chromium.chrome.browser.offlinepages.prefetch.PrefetchConfiguration;
 import org.chromium.chrome.browser.preferences.ChromeSwitchPreference;
-import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.preferences.PreferenceUtils;
 
 /**
@@ -78,8 +78,7 @@ public class DownloadPreferences
         if (mLocationPromptEnabledPref != null) {
             // Location prompt is marked enabled if the prompt status is not DONT_SHOW.
             boolean isLocationPromptEnabled =
-                    PrefServiceBridge.getInstance().getPromptForDownloadAndroid()
-                    != DownloadPromptStatus.DONT_SHOW;
+                    DownloadUtils.getPromptForDownloadAndroid() != DownloadPromptStatus.DONT_SHOW;
             mLocationPromptEnabledPref.setChecked(isLocationPromptEnabled);
         }
 
@@ -114,14 +113,12 @@ public class DownloadPreferences
         if (PREF_LOCATION_PROMPT_ENABLED.equals(preference.getKey())) {
             if ((boolean) newValue) {
                 // Only update if the interstitial has been shown before.
-                if (PrefServiceBridge.getInstance().getPromptForDownloadAndroid()
+                if (DownloadUtils.getPromptForDownloadAndroid()
                         != DownloadPromptStatus.SHOW_INITIAL) {
-                    PrefServiceBridge.getInstance().setPromptForDownloadAndroid(
-                            DownloadPromptStatus.SHOW_PREFERENCE);
+                    DownloadUtils.setPromptForDownloadAndroid(DownloadPromptStatus.SHOW_PREFERENCE);
                 }
             } else {
-                PrefServiceBridge.getInstance().setPromptForDownloadAndroid(
-                        DownloadPromptStatus.DONT_SHOW);
+                DownloadUtils.setPromptForDownloadAndroid(DownloadPromptStatus.DONT_SHOW);
             }
         } else if (PREF_PREFETCHING_ENABLED.equals(preference.getKey())) {
             PrefetchConfiguration.setPrefetchingEnabledInSettings((boolean) newValue);

chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadLocationChangeTest.java

@@ -25,7 +25,6 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.download.DownloadTestRule.CustomMainActivityStart;
-import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.download.R;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.Features;
@@ -79,8 +78,8 @@ public class DownloadLocationChangeTest implements CustomMainActivityStart {
     @Features.EnableFeatures(ChromeFeatureList.DOWNLOADS_LOCATION_CHANGE)
     public void testDefaultDialogPositiveButtonClickThrough() {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
-            Assert.assertEquals(DownloadPromptStatus.SHOW_INITIAL,
-                    PrefServiceBridge.getInstance().getPromptForDownloadAndroid());
+            Assert.assertEquals(
+                    DownloadPromptStatus.SHOW_INITIAL, DownloadUtils.getPromptForDownloadAndroid());
 
             simulateDownloadDirectories(true /* hasSDCard */);
 
@@ -116,8 +115,8 @@ public class DownloadLocationChangeTest implements CustomMainActivityStart {
         int currentCallCount = mDownloadTestRule.getChromeDownloadCallCount();
 
         TestThreadUtils.runOnUiThreadBlocking(() -> {
-            Assert.assertEquals(DownloadPromptStatus.SHOW_INITIAL,
-                    PrefServiceBridge.getInstance().getPromptForDownloadAndroid());
+            Assert.assertEquals(
+                    DownloadPromptStatus.SHOW_INITIAL, DownloadUtils.getPromptForDownloadAndroid());
 
             simulateDownloadDirectories(false /* hasSDCard */);
 
@@ -160,8 +159,7 @@ public class DownloadLocationChangeTest implements CustomMainActivityStart {
     }
 
     private void promptDownloadLocationDialog(@DownloadPromptStatus int promptStatus) {
-        TestThreadUtils.runOnUiThreadBlocking(() -> {
-            PrefServiceBridge.getInstance().setPromptForDownloadAndroid(promptStatus);
-        });
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> { DownloadUtils.setPromptForDownloadAndroid(promptStatus); });
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTestRule.java

@@ -19,7 +19,6 @@ import org.chromium.base.Log;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.download.items.OfflineContentAggregatorFactory;
-import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.components.offline_items_collection.ContentId;
 import org.chromium.components.offline_items_collection.OfflineContentProvider;
@@ -244,8 +243,7 @@ public class DownloadTestRule extends ChromeActivityTestRule<ChromeActivity> {
         mActivityStart.customMainActivityStart();
 
         TestThreadUtils.runOnUiThreadBlocking(() -> {
-            PrefServiceBridge.getInstance().setPromptForDownloadAndroid(
-                    DownloadPromptStatus.DONT_SHOW);
+            DownloadUtils.setPromptForDownloadAndroid(DownloadPromptStatus.DONT_SHOW);
         });
 
         cleanUpAllDownloads();

chrome/android/junit/src/org/chromium/chrome/browser/preferences/PrefServiceBridgeTest.java

@@ -59,6 +59,26 @@ public class PrefServiceBridgeTest {
         verify(mNativeMock).setBoolean(eq(prefServiceBridge), eq(PREF), eq(value));
     }
 
+    @Test
+    public void testGetInteger() {
+        int expected = 26;
+
+        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
+        doReturn(expected).when(mNativeMock).getInteger(prefServiceBridge, PREF);
+
+        assertEquals(expected, prefServiceBridge.getInteger(PREF));
+    }
+
+    @Test
+    public void testSetInteger() {
+        int value = 62;
+
+        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
+        prefServiceBridge.setInteger(PREF, value);
+
+        verify(mNativeMock).setInteger(eq(prefServiceBridge), eq(PREF), eq(value));
+    }
+
     @Test
     public void testIsManaged() {
         boolean expected = true;

chrome/browser/android/preferences/pref_service_bridge.cc

@@ -150,6 +150,13 @@ static void JNI_PrefServiceBridge_SetBoolean(JNIEnv* env,
       PrefServiceBridge::GetPrefNameExposedToJava(j_pref_index), j_value);
 }
 
+static jint JNI_PrefServiceBridge_GetInteger(JNIEnv* env,
+                                             const JavaParamRef<jobject>& obj,
+                                             const jint j_pref_index) {
+  return GetPrefService()->GetInteger(
+      PrefServiceBridge::GetPrefNameExposedToJava(j_pref_index));
+}
+
 static void JNI_PrefServiceBridge_SetInteger(JNIEnv* env,
                                              const JavaParamRef<jobject>& obj,
                                              const jint j_pref_index,
@@ -1318,19 +1325,6 @@ static void JNI_PrefServiceBridge_SetDownloadAndSaveFileDefaultDirectory(
   GetPrefService()->SetFilePath(prefs::kSaveFileDefaultDirectory, path);
 }
 
-static jint JNI_PrefServiceBridge_GetPromptForDownloadAndroid(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& obj) {
-  return GetPrefService()->GetInteger(prefs::kPromptForDownloadAndroid);
-}
-
-static void JNI_PrefServiceBridge_SetPromptForDownloadAndroid(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& obj,
-    const jint status) {
-  GetPrefService()->SetInteger(prefs::kPromptForDownloadAndroid, status);
-}
-
 static jboolean JNI_PrefServiceBridge_GetExplicitLanguageAskPromptShown(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {