Skip to content

Commit

Permalink
[Android] Move SharedPreferencesManager to //base
Browse files Browse the repository at this point in the history
SharedPreferencesManager only needed to be //chrome-layer because
it has lists of the SharedPrefs keys used in Chrome. This CL:
- Detaches the key registry from the key checking
- Moves SharedPreferencesManager and Base/ChromePreferenceKeyChecker
  to //base
- Leaves behind a temporary facade where //chrome
  SharedPreferencesManager is until the migration is finished.
- Implements checking that all PreferenceKeyRegistries are
  listed in AllPreferenceKeyRegistries
- Modifies ChromePreferenceKeysTest to check uniqueness across
  all known registries

See go/base-shared-preferences for more details.

Bug: 1069897,1483469
Change-Id: I1010671a43c0c23d9953f119c1970142e33afcc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4866653
Code-Coverage: [email protected] <[email protected]>
Reviewed-by: Andrew Grieve <[email protected]>
Commit-Queue: Henrique Nakashima <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1202809}
  • Loading branch information
Henrique Nakashima authored and Chromium LUCI CQ committed Sep 28, 2023
1 parent 83168c3 commit d2cf18e
Show file tree
Hide file tree
Showing 32 changed files with 1,689 additions and 1,417 deletions.
32 changes: 32 additions & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,8 @@ component("base") {
"android/scoped_hardware_buffer_fence_sync.h",
"android/scoped_hardware_buffer_handle.cc",
"android/scoped_hardware_buffer_handle.h",
"android/shared_preferences/shared_preferences_manager.cc",
"android/shared_preferences/shared_preferences_manager.h",
"android/statistics_recorder_android.cc",
"android/sys_utils.cc",
"android/sys_utils.h",
Expand Down Expand Up @@ -1242,6 +1244,7 @@ component("base") {
]

deps += [
":base_shared_preferences_jni",
"//third_party/ashmem",
"//third_party/cpu_features:ndk_compat",
]
Expand Down Expand Up @@ -4226,6 +4229,10 @@ if (is_android || is_robolectric) {
generate_jni("process_launcher_jni") {
sources = [ "android/java/src/org/chromium/base/process_launcher/ChildProcessService.java" ]
}

generate_jni("base_shared_preferences_jni") {
sources = [ "android/java/src/org/chromium/base/shared_preferences/SharedPreferencesManager.java" ]
}
} # is_android || is_robolectric

if (is_android) {
Expand Down Expand Up @@ -4312,6 +4319,7 @@ if (is_android) {
"//build/android:build_java",
"//third_party/android_deps:com_google_code_findbugs_jsr305_java",
"//third_party/android_deps:com_google_errorprone_error_prone_annotations_java",
"//third_party/android_deps:guava_android_java",
"//third_party/androidx:androidx_annotation_annotation_experimental_java",
"//third_party/androidx:androidx_annotation_annotation_java",
"//third_party/androidx:androidx_collection_collection_java",
Expand Down Expand Up @@ -4473,6 +4481,25 @@ if (is_android) {
]
}

android_library("base_shared_preferences_java") {
deps = [
":base_java",
":jni_java",
"//third_party/android_deps:guava_android_java",
"//third_party/androidx:androidx_annotation_annotation_java",
]

sources = [
"android/java/src/org/chromium/base/shared_preferences/KeyPrefix.java",
"android/java/src/org/chromium/base/shared_preferences/KnownPreferenceKeyRegistries.java",
"android/java/src/org/chromium/base/shared_preferences/NoOpPreferenceKeyChecker.java",
"android/java/src/org/chromium/base/shared_preferences/PreferenceKeyChecker.java",
"android/java/src/org/chromium/base/shared_preferences/PreferenceKeyRegistry.java",
"android/java/src/org/chromium/base/shared_preferences/SharedPreferencesManager.java",
"android/java/src/org/chromium/base/shared_preferences/StrictPreferenceKeyChecker.java",
]
}

android_aidl("process_launcher_aidl") {
import_include = [ "android/java/src" ]
sources = [
Expand Down Expand Up @@ -4771,6 +4798,10 @@ if (is_android) {
"android/junit/src/org/chromium/base/metrics/CachingUmaRecorderTest.java",
"android/junit/src/org/chromium/base/process_launcher/ChildConnectionAllocatorTest.java",
"android/junit/src/org/chromium/base/process_launcher/ChildProcessConnectionTest.java",
"android/junit/src/org/chromium/base/shared_preferences/KeyPrefixTest.java",
"android/junit/src/org/chromium/base/shared_preferences/KnownPreferenceKeyRegistriesTest.java",
"android/junit/src/org/chromium/base/shared_preferences/SharedPreferencesManagerTest.java",
"android/junit/src/org/chromium/base/shared_preferences/StrictPreferenceKeyCheckerTest.java",
"android/junit/src/org/chromium/base/supplier/ObservableSupplierImplTest.java",
"android/junit/src/org/chromium/base/supplier/OneShotCallbackTest.java",
"android/junit/src/org/chromium/base/supplier/OneshotSupplierImplTest.java",
Expand Down Expand Up @@ -4801,6 +4832,7 @@ if (is_android) {
":base_java_test_support",
":base_java_test_support_uncommon",
":base_junit_test_support",
":base_shared_preferences_java",
"//base:jni_java",
"//base/test:test_support_java",
"//third_party/android_deps:guava_android_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
// 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;
package org.chromium.base.shared_preferences;

/**
* A prefix for a range of SharedPreferences keys generated dynamically.
*
* Instances should be declared as keys in {@link ChromePreferenceKeys}.
* Instances should be declared as keys in the PreferenceKeys registry.
*/
public class KeyPrefix {
private final String mPrefix;

KeyPrefix(String pattern) {
public KeyPrefix(String pattern) {
// More thorough checking is performed in ChromePreferenceKeysTest.
assert pattern.endsWith("*");
mPrefix = pattern.substring(0, pattern.length() - 1);
Expand All @@ -33,11 +33,11 @@ public String createKey(int index) {
return mPrefix + index;
}

String pattern() {
public String pattern() {
return mPrefix + "*";
}

boolean hasGenerated(String key) {
public boolean hasGenerated(String key) {
return key.startsWith(mPrefix);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.base.shared_preferences;

import com.google.common.collect.Sets;

import org.chromium.base.ResettersForTesting;
import org.chromium.build.BuildConfig;
import org.chromium.build.annotations.CheckDiscard;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

/**
* Ensures that all {@link PreferenceKeyRegistry}s used are known.
*
* A complement to ChromePreferenceKeysTest, which ensures that preference keys across all known
* registries are unique.
*
* This checking is done in tests in which |initializeKnownRegistries()| is called, which happens
* during browser process initialization.
*/
@CheckDiscard("Preference key checking should only happen on build with asserts")
public class KnownPreferenceKeyRegistries {
private static Set<PreferenceKeyRegistry> sKnownRegistries;
private static Set<PreferenceKeyRegistry> sRegistriesUsedBeforeInitialization = new HashSet<>();

public static void onRegistryUsed(PreferenceKeyRegistry registry) {
if (!BuildConfig.ENABLE_ASSERTS) {
return;
}

if (sKnownRegistries == null) {
// Before initialization, keep track of registries used.
sRegistriesUsedBeforeInitialization.add(registry);
} else {
// After initialization, check if registry is known.
if (!sKnownRegistries.contains(registry)) {
String message =
"An unknown registry was used, PreferenceKeyRegistries must be declared as "
+ "known in AllPreferenceKeyRegistries: "
+ String.join(",", registry.toDebugString());
assert false : message;
}
}
}

public static void initializeKnownRegistries(Set<PreferenceKeyRegistry> knownRegistries) {
if (!BuildConfig.ENABLE_ASSERTS) {
return;
}

if (sKnownRegistries != null) {
// Double initialization; make sure new known registries are the same.
assert sKnownRegistries.equals(knownRegistries);
return;
}

// Check that each registry already used is known; assert otherwise.
Set<PreferenceKeyRegistry> unknownRegistries =
Sets.difference(sRegistriesUsedBeforeInitialization, knownRegistries);
if (!unknownRegistries.isEmpty()) {
List<String> unknownRegistryNames = new ArrayList<>();
for (PreferenceKeyRegistry unknownRegistry : unknownRegistries) {
unknownRegistryNames.add(unknownRegistry.toDebugString());
}
String message =
"Unknown registries were used, PreferenceKeyRegistries must be declared as "
+ "known in AllPreferenceKeyRegistries: "
+ String.join(",", unknownRegistryNames);
assert false : message;
}

sKnownRegistries = knownRegistries;
sRegistriesUsedBeforeInitialization = null;
}

static void clearForTesting() {
Set<PreferenceKeyRegistry> previousKnownRegistries = sKnownRegistries;
Set<PreferenceKeyRegistry> registriesUsedBeforeInitialization =
sRegistriesUsedBeforeInitialization;

ResettersForTesting.register(() -> {
sKnownRegistries = previousKnownRegistries;
sRegistriesUsedBeforeInitialization = registriesUsedBeforeInitialization;
});
sKnownRegistries = null;
sRegistriesUsedBeforeInitialization = new HashSet<>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// 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;
package org.chromium.base.shared_preferences;

/**
* A placeholder key checker that never throws exceptions. Used in production builds.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// 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;
package org.chromium.base.shared_preferences;

/**
* A SharedPreferences key checker that may check if the key is in use.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.base.shared_preferences;

import androidx.annotation.NonNull;

import org.chromium.build.annotations.CheckDiscard;

import java.util.HashSet;
import java.util.List;
import java.util.Locale;

@CheckDiscard("Preference key checking should only happen on build with asserts")
public class PreferenceKeyRegistry {
private final String mModule;
public final HashSet<String> mKeysInUse;
public final HashSet<String> mLegacyFormatKeys;
public final List<KeyPrefix> mLegacyPrefixes;

public PreferenceKeyRegistry(String module, List<String> keysInUse, List<String> legacyKeys,
List<KeyPrefix> legacyPrefixes) {
mModule = module;
mKeysInUse = new HashSet<>(keysInUse);
mLegacyFormatKeys = new HashSet<>(legacyKeys);
mLegacyPrefixes = legacyPrefixes;
}

@NonNull
public String toDebugString() {
return String.format(Locale.getDefault(), "%s (%d in use, %d legacy, %d legacy prefixes)",
mModule, mKeysInUse.size(), mLegacyFormatKeys.size(), mLegacyPrefixes.size());
}
}
Loading

0 comments on commit d2cf18e

Please sign in to comment.