Remove PrefServiceBridge.java

Port PrefServiceBridgeTest to PrefServiceTest.
Update docs.
Update OWNERS.

Bug: 1071603
Change-Id: I48f83e1faade5263a6b105268bfd57ded3db27e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307609
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790210}

chrome/android/chrome_junit_test_java_sources.gni

@@ -195,7 +195,6 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/photo_picker/FileEnumWorkerTaskTest.java",
   "junit/src/org/chromium/chrome/browser/photo_picker/PickerBitmapViewTest.java",
   "junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.java",
-  "junit/src/org/chromium/chrome/browser/preferences/PrefServiceBridgeTest.java",
   "junit/src/org/chromium/chrome/browser/privacy/settings/PrivacyPreferencesManagerTest.java",
   "junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetricsTest.java",
   "junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceNotificationTest.java",

chrome/browser/BUILD.gn

@@ -2499,7 +2499,6 @@ static_library("browser") {
       "android/preferences/cookie_controls_service_bridge.h",
       "android/preferences/pref_change_registrar_android.cc",
       "android/preferences/pref_change_registrar_android.h",
-      "android/preferences/pref_service_bridge.cc",
       "android/preferences/privacy_preferences_manager.cc",
       "android/profile_key_startup_accessor.cc",
       "android/profile_key_startup_accessor.h",

chrome/browser/android/preferences/OWNERS

-twellington@chromium.org
-chouinard@chromium.org
+file://components/prefs/android/OWNERS
 
 per-file cookie_controls*=dullweber@chromium.org
 

chrome/browser/android/preferences/pref_service_bridge.cc

-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <jni.h>
-
-#include <string>
-
-#include "base/android/jni_string.h"
-#include "base/android/scoped_java_ref.h"
-#include "chrome/browser/preferences/jni_headers/PrefServiceBridge_jni.h"
-#include "chrome/browser/profiles/profile_manager.h"
-#include "components/prefs/pref_service.h"
-
-namespace {
-
-using base::android::JavaParamRef;
-
-PrefService* GetPrefService() {
-  return ProfileManager::GetActiveUserProfile()
-      ->GetOriginalProfile()
-      ->GetPrefs();
-}
-
-}  // namespace
-
-// ----------------------------------------------------------------------------
-// Native JNI methods
-// ----------------------------------------------------------------------------
-
-static void JNI_PrefServiceBridge_ClearPref(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& j_preference) {
-  GetPrefService()->ClearPref(
-      base::android::ConvertJavaStringToUTF8(env, j_preference));
-}
-
-static jboolean JNI_PrefServiceBridge_GetBoolean(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& j_preference) {
-  return GetPrefService()->GetBoolean(
-      base::android::ConvertJavaStringToUTF8(env, j_preference));
-}
-
-static void JNI_PrefServiceBridge_SetBoolean(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& j_preference,
-    const jboolean j_value) {
-  GetPrefService()->SetBoolean(
-      base::android::ConvertJavaStringToUTF8(env, j_preference), j_value);
-}
-
-static jint JNI_PrefServiceBridge_GetInteger(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& j_preference) {
-  return GetPrefService()->GetInteger(
-      base::android::ConvertJavaStringToUTF8(env, j_preference));
-}
-
-static void JNI_PrefServiceBridge_SetInteger(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& j_preference,
-    const jint j_value) {
-  GetPrefService()->SetInteger(
-      base::android::ConvertJavaStringToUTF8(env, j_preference), j_value);
-}
-
-static base::android::ScopedJavaLocalRef<jstring>
-JNI_PrefServiceBridge_GetString(JNIEnv* env,
-                                const JavaParamRef<jstring>& j_preference) {
-  return base::android::ConvertUTF8ToJavaString(
-      env, GetPrefService()->GetString(
-               base::android::ConvertJavaStringToUTF8(env, j_preference)));
-}
-
-static void JNI_PrefServiceBridge_SetString(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& j_preference,
-    const base::android::JavaParamRef<jstring>& j_value) {
-  GetPrefService()->SetString(
-      base::android::ConvertJavaStringToUTF8(env, j_preference),
-      base::android::ConvertJavaStringToUTF8(env, j_value));
-}
-
-static jboolean JNI_PrefServiceBridge_IsManagedPreference(
-    JNIEnv* env,
-    const JavaParamRef<jstring>& j_preference) {
-  return GetPrefService()->IsManagedPreference(
-      base::android::ConvertJavaStringToUTF8(env, j_preference));
-}

chrome/browser/preferences/BUILD.gn

@@ -13,7 +13,6 @@ android_library("java") {
     "android/java/src/org/chromium/chrome/browser/preferences/GrandfatheredChromePreferenceKeys.java",
     "android/java/src/org/chromium/chrome/browser/preferences/KeyPrefix.java",
     "android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.java",
-    "android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java",
     "android/java/src/org/chromium/chrome/browser/preferences/SharedPreferencesManager.java",
   ]
   deps = [
@@ -45,10 +44,7 @@ java_cpp_strings("java_pref_names_srcjar") {
 }
 
 generate_jni("jni_headers") {
-  sources = [
-    "android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.java",
-    "android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java",
-  ]
+  sources = [ "android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.java" ]
 }
 
 java_library("preferences_junit_tests") {

chrome/browser/preferences/android/README.md

@@ -10,12 +10,12 @@ and/or write small amounts of data from Java to a persistent key-value store.
 supports reading and writing simple key-value pairs to a file that is saved
 across app sessions.
 
-## PrefServiceBridge
+## PrefService
 
-[`PrefServiceBridge`][2] is a JNI bridge providing access to the native Chrome
-[PrefService][3] instance associated with the active user profile. This
-interface can be used to read and write prefs once they're registered through
-the `PrefRegistry` and added to the [`@Pref` enum][4].
+[`PrefService`][2] is a JNI bridge providing access to a native Chrome
+[PrefService][3] instance associated. This interface can be used to read and
+write prefs once they're registered through the `PrefRegistry` and exposed to
+Java by way of a `java_cpp_strings` build target such as [this one][4].
 
 ## FAQ
 
@@ -33,26 +33,24 @@ should be stored in PrefService. If the answer to all of the above questions is
 No, then SharedPreferences should be preferred.
 
 **What if the PrefService type I need to access is not supported by
-PrefServiceBridge (i.e. double, Time, etc.)?**
+PrefService (i.e. double, Time, etc.)?**
 
-If a base value type is supported by PrefService, then PrefServiceBridge should
+If a base value type is supported by PrefService, then PrefService should
 be extended to support it once it's needed.
 
 **How do I access a PrefService pref associated with local state rather than
 browser profile?**
 
 Most Chrome for Android preferences should be associated with a specific
-profile. If your preference should instead be associated with [local state][6]
+profile. If your preference should instead be associated with [local state][5]
 (for example, if it is related to the First Run Experience), then you should not
-use PrefServiceBridge and should instead create your own feature-specific JNI
-bridge to access the correct PrefService instance (see
-[`first_run_utils.cc`][7]).
+use PrefService and should instead create your own feature-specific JNI
+bridge to access the correct PrefService instance (see [`first_run_utils.cc`][6]).
 
 [0]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/android/java/src/org/chromium/chrome/browser/preferences/SharedPreferencesManager.java
 [1]: https://developer.android.com/reference/android/content/SharedPreferences
-[2]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
+[2]: https://source.chromium.org/chromium/chromium/src/+/master:components/prefs/android/java/src/org/chromium/components/prefs/PrefService.java
 [3]: https://chromium.googlesource.com/chromium/src/+/master/services/preferences/README.md
-[4]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/android/preferences/prefs.h
-[5]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/OWNERS
-[6]: https://www.chromium.org/developers/design-documents/preferences#TOC-Introduction
-[7]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/first_run/android/first_run_utils.cc
+[4]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/BUILD.gn;drc=4ae1b7be67cd9b470ebcc90f2747a9f31f155b00;l=28
+[5]: https://www.chromium.org/developers/design-documents/preferences#TOC-Introduction
+[6]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/first_run/android/first_run_utils.cc

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

-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.chrome.browser.preferences;
-
-import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
-
-import org.chromium.base.ThreadUtils;
-import org.chromium.base.annotations.NativeMethods;
-
-/**
- * PrefServiceBridge is a singleton which provides read and write access to native PrefService.
- */
-public class PrefServiceBridge {
-    // Singleton constructor. Do not call directly unless for testing purpose.
-    @VisibleForTesting
-    protected PrefServiceBridge() {}
-
-    private static PrefServiceBridge sInstance;
-
-    /**
-     * @return The singleton PrefServiceBridge instance.
-     */
-    public static PrefServiceBridge getInstance() {
-        ThreadUtils.assertOnUiThread();
-        if (sInstance == null) {
-            sInstance = new PrefServiceBridge();
-        }
-        return sInstance;
-    }
-
-    /**
-     * @param preference The name of the preference.
-     */
-    public void clearPref(@NonNull String preference) {
-        PrefServiceBridgeJni.get().clearPref(preference);
-    }
-
-    /**
-     * @param preference The name of the preference.
-     * @return Whether the specified preference is enabled.
-     */
-    public boolean getBoolean(@NonNull String preference) {
-        return PrefServiceBridgeJni.get().getBoolean(preference);
-    }
-
-    /**
-     * @param preference The name of the preference.
-     * @param value The value the specified preference will be set to.
-     */
-    public void setBoolean(@NonNull String preference, boolean value) {
-        PrefServiceBridgeJni.get().setBoolean(preference, value);
-    }
-
-    /**
-     * @param preference The name of the preference.
-     * @return value The value of the specified preference.
-     */
-    public int getInteger(@NonNull String preference) {
-        return PrefServiceBridgeJni.get().getInteger(preference);
-    }
-
-    /**
-     * @param preference The name of the preference.
-     * @param value The value the specified preference will be set to.
-     */
-    public void setInteger(@NonNull String preference, int value) {
-        PrefServiceBridgeJni.get().setInteger(preference, value);
-    }
-
-    /**
-     * @param preference The name of the preference.
-     * @return value The value of the specified preference.
-     */
-    @NonNull
-    public String getString(@NonNull String preference) {
-        return PrefServiceBridgeJni.get().getString(preference);
-    }
-
-    /**
-     * @param preference The name of the preference.
-     * @param value The value the specified preference will be set to.
-     */
-    public void setString(@NonNull String preference, @NonNull String value) {
-        PrefServiceBridgeJni.get().setString(preference, value);
-    }
-
-    /**
-     * @param preference The name of the preference.
-     * @return Whether the specified preference is managed.
-     */
-    public boolean isManagedPreference(@NonNull String preference) {
-        return PrefServiceBridgeJni.get().isManagedPreference(preference);
-    }
-
-    @VisibleForTesting
-    public static void setInstanceForTesting(@Nullable PrefServiceBridge instanceForTesting) {
-        sInstance = instanceForTesting;
-    }
-
-    @NativeMethods
-    interface Natives {
-        void clearPref(String preference);
-        boolean getBoolean(String preference);
-        void setBoolean(String preference, boolean value);
-        int getInteger(String preference);
-        void setInteger(String preference, int value);
-        String getString(String preference);
-        void setString(String preference, String value);
-        boolean isManagedPreference(String preference);
-    }
-}

components/BUILD.gn

@@ -763,6 +763,7 @@ if (is_android) {
       "//components/gcm_driver/android:components_gcm_driver_junit_tests",
       "//components/permissions/android:components_permissions_junit_tests",
       "//components/policy/android:components_policy_junit_tests",
+      "//components/prefs/android:junit",
       "//components/query_tiles:query_tiles_junit_tests",
       "//components/signin/core/browser/android:components_signin_junit_tests",
       "//components/variations/android:components_variations_junit_tests",

components/prefs/android/BUILD.gn

@@ -17,3 +17,20 @@ android_library("java") {
   ]
   annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
 }
+
+java_library("junit") {
+  # Skip platform checks since Robolectric depends on requires_android targets.
+  bypass_platform_checks = true
+  testonly = true
+  sources = [ "java/src/org/chromium/components/prefs/PrefServiceTest.java" ]
+  deps = [
+    ":java",
+    "//base:base_java",
+    "//base:base_java_test_support",
+    "//base:base_junit_test_support",
+    "//base/test:test_support_java",
+    "//third_party/android_deps:robolectric_all_java",
+    "//third_party/junit",
+    "//third_party/mockito:mockito_java",
+  ]
+}

components/prefs/android/OWNERS

+twellington@chromium.org
+chouinard@chromium.org
+
+# COMPONENT: Internals>Preferences
+# OS: Android

components/prefs/android/java/src/org/chromium/components/prefs/PrefService.java

@@ -5,6 +5,7 @@
 package org.chromium.components.prefs;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
@@ -23,7 +24,8 @@ public class PrefService {
         mNativePrefServiceAndroid = 0;
     }
 
-    private PrefService(long nativePrefServiceAndroid) {
+    @VisibleForTesting
+    PrefService(long nativePrefServiceAndroid) {
         mNativePrefServiceAndroid = nativePrefServiceAndroid;
     }
 

chrome/android/junit/src/org/chromium/chrome/browser/preferences/PrefServiceBridgeTest.java → components/prefs/android/java/src/org/chromium/components/prefs/PrefServiceTest.java

@@ -4,7 +4,7 @@
 
 // generate_java_test.py
 
-package org.chromium.chrome.browser.preferences;
+package org.chromium.components.prefs;
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.eq;
@@ -22,90 +22,87 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.JniMocker;
 
-/** Unit tests for {@link PrefServiceBridge}. */
+/** Unit tests for {@link PrefService}. */
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-public class PrefServiceBridgeTest {
+public class PrefServiceTest {
     private static final String PREF = "42";
+    private static final long NATIVE_HANDLE = 117;
 
     @Rule
     public JniMocker mocker = new JniMocker();
     @Mock
-    private PrefServiceBridge.Natives mNativeMock;
+    private PrefService.Natives mNativeMock;
+
+    PrefService mPrefService;
 
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
-        mocker.mock(PrefServiceBridgeJni.TEST_HOOKS, mNativeMock);
+        mocker.mock(PrefServiceJni.TEST_HOOKS, mNativeMock);
+        mPrefService = new PrefService(NATIVE_HANDLE);
     }
 
     @Test
     public void testGetBoolean() {
         boolean expected = false;
 
-        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
-        doReturn(expected).when(mNativeMock).getBoolean(PREF);
+        doReturn(expected).when(mNativeMock).getBoolean(NATIVE_HANDLE, PREF);
 
-        assertEquals(expected, prefServiceBridge.getBoolean(PREF));
+        assertEquals(expected, mPrefService.getBoolean(PREF));
     }
 
     @Test
     public void testSetBoolean() {
         boolean value = true;
 
-        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
-        prefServiceBridge.setBoolean(PREF, value);
+        mPrefService.setBoolean(PREF, value);
 
-        verify(mNativeMock).setBoolean(eq(PREF), eq(value));
+        verify(mNativeMock).setBoolean(eq(NATIVE_HANDLE), eq(PREF), eq(value));
     }
 
     @Test
     public void testGetInteger() {
         int expected = 26;
 
-        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
-        doReturn(expected).when(mNativeMock).getInteger(PREF);
+        doReturn(expected).when(mNativeMock).getInteger(NATIVE_HANDLE, PREF);
 
-        assertEquals(expected, prefServiceBridge.getInteger(PREF));
+        assertEquals(expected, mPrefService.getInteger(PREF));
     }
 
     @Test
     public void testSetInteger() {
         int value = 62;
 
-        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
-        prefServiceBridge.setInteger(PREF, value);
+        mPrefService.setInteger(PREF, value);
 
-        verify(mNativeMock).setInteger(eq(PREF), eq(value));
+        verify(mNativeMock).setInteger(eq(NATIVE_HANDLE), eq(PREF), eq(value));
     }
 
     @Test
     public void testGetString() {
         String expected = "foo";
 
-        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
-        doReturn(expected).when(mNativeMock).getString(PREF);
+        doReturn(expected).when(mNativeMock).getString(NATIVE_HANDLE, PREF);
 
-        assertEquals(expected, prefServiceBridge.getString(PREF));
+        assertEquals(expected, mPrefService.getString(PREF));
     }
 
     @Test
     public void testSetString() {
         String value = "bar";
 
-        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
-        prefServiceBridge.setString(PREF, value);
+        mPrefService.setString(PREF, value);
 
-        verify(mNativeMock).setString(eq(PREF), eq(value));
+        verify(mNativeMock).setString(eq(NATIVE_HANDLE), eq(PREF), eq(value));
     }
 
     @Test
     public void testIsManaged() {
         boolean expected = true;
 
-        PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
-        doReturn(expected).when(mNativeMock).isManagedPreference(PREF);
+        doReturn(expected).when(mNativeMock).isManagedPreference(NATIVE_HANDLE, PREF);
 
-        assertEquals(expected, prefServiceBridge.isManagedPreference(PREF));
+        assertEquals(expected, mPrefService.isManagedPreference(PREF));
     }
 }