Skip to content

GitLab

T
tangled
Commit 01cf4392 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot
Browse files
Options

Reland "Remove PrefServiceBridge.java" 

This reverts commit 2544a4d3.

Reason for revert: Clank was updated; no change to original patch

> 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,chouinard@chromium.org

Bug: 1071603
Change-Id: I1687c20d38ac72131b874b8222127cb23329d30e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310316Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Reviewed-by: default avatarNatalie Chouinard <chouinard@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791007}
parent 755a41c0
Show whitespace changes
Inline Side-by-side
Showing with 65 additions and 257 deletions
chrome/android/chrome_junit_test_java_sources.gni
View file @ 01cf4392
......@@ -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
View file @ 01cf4392
......@@ -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
View file @ 01cf4392
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 deleted 100644 → 0
View file @ 755a41c0
// 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
View file @ 01cf4392
......@@ -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
View file @ 01cf4392
......@@ -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 deleted 100644 → 0
View file @ 755a41c0
// 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
View file @ 01cf4392
......@@ -767,6 +767,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
View file @ 01cf4392
......@@ -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 0 → 100644
View file @ 01cf4392
twellington@chromium.org
chouinard@chromium.org
# COMPONENT: Internals>Preferences
# OS: Android
components/prefs/android/java/src/org/chromium/components/prefs/PrefService.java
View file @ 01cf4392
......@@ -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
View file @ 01cf4392
......@@ -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));
}
}
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