Commit 2544a4d3 authored by Anna Malova's avatar Anna Malova Committed by Commit Bot

Revert "Remove PrefServiceBridge.java"

This reverts commit b17c0966.

Reason for revert: CL caused build failures on chrome/ci/android-internal-chromium-tot. 

Original change's description:
> 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}

TBR=battre@chromium.org,estade@chromium.org,chouinard@chromium.org

Change-Id: Ic25ffc3e2fc0b0bab9f280906afa8e4ea0ffc387
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1071603
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2308414Reviewed-by: default avatarAnna Malova <amalova@chromium.org>
Commit-Queue: Anna Malova <amalova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790327}
parent 53428651
...@@ -195,6 +195,7 @@ chrome_junit_test_java_sources = [ ...@@ -195,6 +195,7 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/photo_picker/FileEnumWorkerTaskTest.java", "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/photo_picker/PickerBitmapViewTest.java",
"junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.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/privacy/settings/PrivacyPreferencesManagerTest.java",
"junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetricsTest.java", "junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceMetricsTest.java",
"junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceNotificationTest.java", "junit/src/org/chromium/chrome/browser/search_engines/SearchEngineChoiceNotificationTest.java",
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
// generate_java_test.py // generate_java_test.py
package org.chromium.components.prefs; package org.chromium.chrome.browser.preferences;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
...@@ -22,87 +22,90 @@ import org.robolectric.annotation.Config; ...@@ -22,87 +22,90 @@ import org.robolectric.annotation.Config;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.JniMocker; import org.chromium.base.test.util.JniMocker;
/** Unit tests for {@link PrefService}. */ /** Unit tests for {@link PrefServiceBridge}. */
@RunWith(BaseRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE)
public class PrefServiceTest { public class PrefServiceBridgeTest {
private static final String PREF = "42"; private static final String PREF = "42";
private static final long NATIVE_HANDLE = 117;
@Rule @Rule
public JniMocker mocker = new JniMocker(); public JniMocker mocker = new JniMocker();
@Mock @Mock
private PrefService.Natives mNativeMock; private PrefServiceBridge.Natives mNativeMock;
PrefService mPrefService;
@Before @Before
public void setUp() { public void setUp() {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
mocker.mock(PrefServiceJni.TEST_HOOKS, mNativeMock); mocker.mock(PrefServiceBridgeJni.TEST_HOOKS, mNativeMock);
mPrefService = new PrefService(NATIVE_HANDLE);
} }
@Test @Test
public void testGetBoolean() { public void testGetBoolean() {
boolean expected = false; boolean expected = false;
doReturn(expected).when(mNativeMock).getBoolean(NATIVE_HANDLE, PREF); PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
doReturn(expected).when(mNativeMock).getBoolean(PREF);
assertEquals(expected, mPrefService.getBoolean(PREF)); assertEquals(expected, prefServiceBridge.getBoolean(PREF));
} }
@Test @Test
public void testSetBoolean() { public void testSetBoolean() {
boolean value = true; boolean value = true;
mPrefService.setBoolean(PREF, value); PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
prefServiceBridge.setBoolean(PREF, value);
verify(mNativeMock).setBoolean(eq(NATIVE_HANDLE), eq(PREF), eq(value)); verify(mNativeMock).setBoolean(eq(PREF), eq(value));
} }
@Test @Test
public void testGetInteger() { public void testGetInteger() {
int expected = 26; int expected = 26;
doReturn(expected).when(mNativeMock).getInteger(NATIVE_HANDLE, PREF); PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
doReturn(expected).when(mNativeMock).getInteger(PREF);
assertEquals(expected, mPrefService.getInteger(PREF)); assertEquals(expected, prefServiceBridge.getInteger(PREF));
} }
@Test @Test
public void testSetInteger() { public void testSetInteger() {
int value = 62; int value = 62;
mPrefService.setInteger(PREF, value); PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
prefServiceBridge.setInteger(PREF, value);
verify(mNativeMock).setInteger(eq(NATIVE_HANDLE), eq(PREF), eq(value)); verify(mNativeMock).setInteger(eq(PREF), eq(value));
} }
@Test @Test
public void testGetString() { public void testGetString() {
String expected = "foo"; String expected = "foo";
doReturn(expected).when(mNativeMock).getString(NATIVE_HANDLE, PREF); PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
doReturn(expected).when(mNativeMock).getString(PREF);
assertEquals(expected, mPrefService.getString(PREF)); assertEquals(expected, prefServiceBridge.getString(PREF));
} }
@Test @Test
public void testSetString() { public void testSetString() {
String value = "bar"; String value = "bar";
mPrefService.setString(PREF, value); PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
prefServiceBridge.setString(PREF, value);
verify(mNativeMock).setString(eq(NATIVE_HANDLE), eq(PREF), eq(value)); verify(mNativeMock).setString(eq(PREF), eq(value));
} }
@Test @Test
public void testIsManaged() { public void testIsManaged() {
boolean expected = true; boolean expected = true;
doReturn(expected).when(mNativeMock).isManagedPreference(NATIVE_HANDLE, PREF); PrefServiceBridge prefServiceBridge = new PrefServiceBridge();
doReturn(expected).when(mNativeMock).isManagedPreference(PREF);
assertEquals(expected, mPrefService.isManagedPreference(PREF)); assertEquals(expected, prefServiceBridge.isManagedPreference(PREF));
} }
} }
...@@ -2501,6 +2501,7 @@ static_library("browser") { ...@@ -2501,6 +2501,7 @@ static_library("browser") {
"android/preferences/cookie_controls_service_bridge.h", "android/preferences/cookie_controls_service_bridge.h",
"android/preferences/pref_change_registrar_android.cc", "android/preferences/pref_change_registrar_android.cc",
"android/preferences/pref_change_registrar_android.h", "android/preferences/pref_change_registrar_android.h",
"android/preferences/pref_service_bridge.cc",
"android/preferences/privacy_preferences_manager.cc", "android/preferences/privacy_preferences_manager.cc",
"android/profile_key_startup_accessor.cc", "android/profile_key_startup_accessor.cc",
"android/profile_key_startup_accessor.h", "android/profile_key_startup_accessor.h",
......
file://components/prefs/android/OWNERS twellington@chromium.org
chouinard@chromium.org
per-file cookie_controls*=dullweber@chromium.org per-file cookie_controls*=dullweber@chromium.org
......
// 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));
}
...@@ -13,6 +13,7 @@ android_library("java") { ...@@ -13,6 +13,7 @@ android_library("java") {
"android/java/src/org/chromium/chrome/browser/preferences/GrandfatheredChromePreferenceKeys.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/KeyPrefix.java",
"android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.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", "android/java/src/org/chromium/chrome/browser/preferences/SharedPreferencesManager.java",
] ]
deps = [ deps = [
...@@ -44,7 +45,10 @@ java_cpp_strings("java_pref_names_srcjar") { ...@@ -44,7 +45,10 @@ java_cpp_strings("java_pref_names_srcjar") {
} }
generate_jni("jni_headers") { generate_jni("jni_headers") {
sources = [ "android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.java" ] sources = [
"android/java/src/org/chromium/chrome/browser/preferences/PrefChangeRegistrar.java",
"android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java",
]
} }
java_library("preferences_junit_tests") { java_library("preferences_junit_tests") {
......
...@@ -10,12 +10,12 @@ and/or write small amounts of data from Java to a persistent key-value store. ...@@ -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 supports reading and writing simple key-value pairs to a file that is saved
across app sessions. across app sessions.
## PrefService ## PrefServiceBridge
[`PrefService`][2] is a JNI bridge providing access to a native Chrome [`PrefServiceBridge`][2] is a JNI bridge providing access to the native Chrome
[PrefService][3] instance associated. This interface can be used to read and [PrefService][3] instance associated with the active user profile. This
write prefs once they're registered through the `PrefRegistry` and exposed to interface can be used to read and write prefs once they're registered through
Java by way of a `java_cpp_strings` build target such as [this one][4]. the `PrefRegistry` and added to the [`@Pref` enum][4].
## FAQ ## FAQ
...@@ -33,24 +33,26 @@ should be stored in PrefService. If the answer to all of the above questions is ...@@ -33,24 +33,26 @@ should be stored in PrefService. If the answer to all of the above questions is
No, then SharedPreferences should be preferred. No, then SharedPreferences should be preferred.
**What if the PrefService type I need to access is not supported by **What if the PrefService type I need to access is not supported by
PrefService (i.e. double, Time, etc.)?** PrefServiceBridge (i.e. double, Time, etc.)?**
If a base value type is supported by PrefService, then PrefService should If a base value type is supported by PrefService, then PrefServiceBridge should
be extended to support it once it's needed. be extended to support it once it's needed.
**How do I access a PrefService pref associated with local state rather than **How do I access a PrefService pref associated with local state rather than
browser profile?** browser profile?**
Most Chrome for Android preferences should be associated with a specific Most Chrome for Android preferences should be associated with a specific
profile. If your preference should instead be associated with [local state][5] profile. If your preference should instead be associated with [local state][6]
(for example, if it is related to the First Run Experience), then you should not (for example, if it is related to the First Run Experience), then you should not
use PrefService and should instead create your own feature-specific JNI use PrefServiceBridge and should instead create your own feature-specific JNI
bridge to access the correct PrefService instance (see [`first_run_utils.cc`][6]). bridge to access the correct PrefService instance (see
[`first_run_utils.cc`][7]).
[0]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/android/java/src/org/chromium/chrome/browser/preferences/SharedPreferencesManager.java [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 [1]: https://developer.android.com/reference/android/content/SharedPreferences
[2]: https://source.chromium.org/chromium/chromium/src/+/master:components/prefs/android/java/src/org/chromium/components/prefs/PrefService.java [2]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/android/java/src/org/chromium/chrome/browser/preferences/PrefServiceBridge.java
[3]: https://chromium.googlesource.com/chromium/src/+/master/services/preferences/README.md [3]: https://chromium.googlesource.com/chromium/src/+/master/services/preferences/README.md
[4]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/BUILD.gn;drc=4ae1b7be67cd9b470ebcc90f2747a9f31f155b00;l=28 [4]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/android/preferences/prefs.h
[5]: https://www.chromium.org/developers/design-documents/preferences#TOC-Introduction [5]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/preferences/OWNERS
[6]: https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/first_run/android/first_run_utils.cc [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
// 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);
}
}
...@@ -763,7 +763,6 @@ if (is_android) { ...@@ -763,7 +763,6 @@ if (is_android) {
"//components/gcm_driver/android:components_gcm_driver_junit_tests", "//components/gcm_driver/android:components_gcm_driver_junit_tests",
"//components/permissions/android:components_permissions_junit_tests", "//components/permissions/android:components_permissions_junit_tests",
"//components/policy/android:components_policy_junit_tests", "//components/policy/android:components_policy_junit_tests",
"//components/prefs/android:junit",
"//components/query_tiles:query_tiles_junit_tests", "//components/query_tiles:query_tiles_junit_tests",
"//components/signin/core/browser/android:components_signin_junit_tests", "//components/signin/core/browser/android:components_signin_junit_tests",
"//components/variations/android:components_variations_junit_tests", "//components/variations/android:components_variations_junit_tests",
......
...@@ -17,20 +17,3 @@ android_library("java") { ...@@ -17,20 +17,3 @@ android_library("java") {
] ]
annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ] 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",
]
}
twellington@chromium.org
chouinard@chromium.org
# COMPONENT: Internals>Preferences
# OS: Android
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
package org.chromium.components.prefs; package org.chromium.components.prefs;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
...@@ -24,8 +23,7 @@ public class PrefService { ...@@ -24,8 +23,7 @@ public class PrefService {
mNativePrefServiceAndroid = 0; mNativePrefServiceAndroid = 0;
} }
@VisibleForTesting private PrefService(long nativePrefServiceAndroid) {
PrefService(long nativePrefServiceAndroid) {
mNativePrefServiceAndroid = nativePrefServiceAndroid; mNativePrefServiceAndroid = nativePrefServiceAndroid;
} }
......
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