Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge changes Iac69f180,I99b6cc0e into udc-dev

Browse Source 
* changes:
  Revert "[SettingsProvider] tracking generation of non-predefined unset settings"
  Revert "[SettingsProvider] only increment generation of unset settings if new setting is non-predefined"
This commit is contained in:
Remi NGUYEN VAN
2023-03-09 01:59:50 +00:00
committed by Android (Google) Code Review
parent 5b5b810866 1d4e2c9e27
commit c505edcb38
5 changed files with 20 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
core/tests/coretests/src/android/provider/NameValueCacheTest.java
View File

@@ -91,7 +91,7 @@ public class NameValueCacheTest {
mConfigsCacheGenerationStore = new MemoryIntArray(2);
mConfigsCacheGenerationStore.set(0, 123);
mConfigsCacheGenerationStore.set(1, 456);
mSettingsCacheGenerationStore = new MemoryIntArray(3);
mSettingsCacheGenerationStore = new MemoryIntArray(2);
mSettingsCacheGenerationStore.set(0, 234);
mSettingsCacheGenerationStore.set(1, 567);
mConfigsStorage = new HashMap<>();
@@ -163,10 +163,6 @@ public class NameValueCacheTest {
Bundle incomingBundle = invocationOnMock.getArgument(4);
String key = invocationOnMock.getArgument(3);
String value = incomingBundle.getString(Settings.NameValueTable.VALUE);
boolean newSetting = false;
if (!mSettingsStorage.containsKey(key)) {
newSetting = true;
}
mSettingsStorage.put(key, value);
int currentGeneration;
// Different settings have different generation codes
@@ -177,18 +173,12 @@ public class NameValueCacheTest {
currentGeneration = mSettingsCacheGenerationStore.get(1);
mSettingsCacheGenerationStore.set(1, ++currentGeneration);
}
if (newSetting) {
// Tracking the generation of all unset settings.
// Increment when a new setting is inserted
currentGeneration = mSettingsCacheGenerationStore.get(2);
mSettingsCacheGenerationStore.set(2, ++currentGeneration);
}
return null;
});
// Returns the value corresponding to a setting, or null if the setting
// doesn't have a value stored for it. Returns the generation key
// if the caller asked for the generation key.
// doesn't have a value stored for it. Returns the generation key if the value isn't null
// and the caller asked for the generation key.
when(mMockIContentProvider.call(any(), eq(Settings.Secure.CONTENT_URI.getAuthority()),
eq(Settings.CALL_METHOD_GET_SECURE), any(), any(Bundle.class))).thenAnswer(
invocationOnMock -> {
@@ -199,15 +189,9 @@ public class NameValueCacheTest {
Bundle bundle = new Bundle();
bundle.putSerializable(Settings.NameValueTable.VALUE, value);
if (incomingBundle.containsKey(
if (value != null && incomingBundle.containsKey(
Settings.CALL_METHOD_TRACK_GENERATION_KEY)) {
int index;
if (value != null) {
index = key.equals(SETTING) ? 0 : 1;
} else {
// special index for unset settings
index = 2;
}
int index = key.equals(SETTING) ? 0 : 1;
bundle.putParcelable(Settings.CALL_METHOD_TRACK_GENERATION_KEY,
mSettingsCacheGenerationStore);
bundle.putInt(Settings.CALL_METHOD_GENERATION_INDEX_KEY, index);
@@ -377,38 +361,16 @@ public class NameValueCacheTest {
}
@Test
public void testCaching_unsetSetting() throws Exception {
public void testCaching_nullSetting() throws Exception {
String returnedValue = Settings.Secure.getString(mMockContentResolver, SETTING);
verify(mMockIContentProvider, times(1)).call(any(), any(),
eq(Settings.CALL_METHOD_GET_SECURE), any(), any(Bundle.class));
assertThat(returnedValue).isNull();
String cachedValue = Settings.Secure.getString(mMockContentResolver, SETTING);
// The first unset setting's generation number is cached
verifyNoMoreInteractions(mMockIContentProvider);
assertThat(cachedValue).isNull();
String returnedValue2 = Settings.Secure.getString(mMockContentResolver, SETTING2);
// Empty list won't be cached
verify(mMockIContentProvider, times(2)).call(any(), any(),
eq(Settings.CALL_METHOD_GET_SECURE), any(), any(Bundle.class));
assertThat(returnedValue2).isNull();
String cachedValue2 = Settings.Secure.getString(mMockContentResolver, SETTING);
// The second unset setting's generation number is cached
verifyNoMoreInteractions(mMockIContentProvider);
assertThat(cachedValue2).isNull();
Settings.Secure.putString(mMockContentResolver, SETTING, "a");
// The generation for unset settings should have changed
returnedValue2 = Settings.Secure.getString(mMockContentResolver, SETTING2);
verify(mMockIContentProvider, times(3)).call(any(), any(),
eq(Settings.CALL_METHOD_GET_SECURE), any(), any(Bundle.class));
assertThat(returnedValue2).isNull();
// The generation tracker for the first setting should have change because it's set now
returnedValue = Settings.Secure.getString(mMockContentResolver, SETTING);
verify(mMockIContentProvider, times(4)).call(any(), any(),
eq(Settings.CALL_METHOD_GET_SECURE), any(), any(Bundle.class));
assertThat(returnedValue).isEqualTo("a");
assertThat(cachedValue).isNull();
}
}

32
packages/SettingsProvider/src/com/android/providers/settings/GenerationRegistry.java
View File

@@ -16,7 +16,6 @@
package com.android.providers.settings;
import android.annotation.NonNull;
import android.os.Bundle;
import android.provider.Settings;
import android.util.ArrayMap;
@@ -60,10 +59,6 @@ final class GenerationRegistry {
// Maximum size of an individual backing store
static final int MAX_BACKING_STORE_SIZE = MemoryIntArray.getMaxSize();
// Use an empty string to track the generation number of all non-predefined, unset settings
// The generation number is only increased when a new non-predefined setting is inserted
private static final String DEFAULT_MAP_KEY_FOR_UNSET_SETTINGS = "";
public GenerationRegistry(Object lock) {
mLock = lock;
}
@@ -77,10 +72,6 @@ final class GenerationRegistry {
(SettingsState.getTypeFromKey(key) == SettingsState.SETTINGS_TYPE_CONFIG);
// Only store the prefix if the mutated setting is a config
final String indexMapKey = isConfig ? (name.split("/")[0] + "/") : name;
incrementGenerationInternal(key, indexMapKey);
}
private void incrementGenerationInternal(int key, @NonNull String indexMapKey) {
synchronized (mLock) {
final MemoryIntArray backingStore = getBackingStoreLocked(key,
/* createIfNotExist= */ false);
@@ -96,8 +87,7 @@ final class GenerationRegistry {
final int generation = backingStore.get(index) + 1;
backingStore.set(index, generation);
if (DEBUG) {
Slog.i(LOG_TAG, "Incremented generation for "
+ (indexMapKey.isEmpty() ? "unset settings" : "setting:" + indexMapKey)
Slog.i(LOG_TAG, "Incremented generation for setting:" + indexMapKey
+ " key:" + SettingsState.keyToString(key)
+ " at index:" + index);
}
@@ -108,18 +98,6 @@ final class GenerationRegistry {
}
}
// A new, non-predefined setting has been inserted, increment the tracking number for all unset
// settings
public void incrementGenerationForUnsetSettings(int key) {
final boolean isConfig =
(SettingsState.getTypeFromKey(key) == SettingsState.SETTINGS_TYPE_CONFIG);
if (isConfig) {
// No need to track new settings for configs
return;
}
incrementGenerationInternal(key, DEFAULT_MAP_KEY_FOR_UNSET_SETTINGS);
}
/**
* Return the backing store's reference, the index and the current generation number
* of a cached setting. If it was not in the backing store, first create the entry in it before
@@ -146,8 +124,8 @@ final class GenerationRegistry {
bundle.putInt(Settings.CALL_METHOD_GENERATION_KEY,
backingStore.get(index));
if (DEBUG) {
Slog.i(LOG_TAG, "Exported index:" + index + " for "
+ (indexMapKey.isEmpty() ? "unset settings" : "setting:" + indexMapKey)
Slog.i(LOG_TAG, "Exported index:" + index
+ " for setting:" + indexMapKey
+ " key:" + SettingsState.keyToString(key));
}
} catch (IOException e) {
@@ -157,10 +135,6 @@ final class GenerationRegistry {
}
}
public void addGenerationDataForUnsetSettings(Bundle bundle, int key) {
addGenerationData(bundle, key, /* indexMapKey= */ DEFAULT_MAP_KEY_FOR_UNSET_SETTINGS);
}
public void onUserRemoved(int userId) {
final int secureKey = SettingsState.makeKey(
SettingsState.SETTINGS_TYPE_SECURE, userId);

32
packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
View File

@@ -2327,15 +2327,11 @@ public class SettingsProvider extends ContentProvider {
result.putString(Settings.NameValueTable.VALUE,
(setting != null && !setting.isNull()) ? setting.getValue() : null);
synchronized (mLock) {
if ((setting != null && !setting.isNull()) || isSettingPreDefined(name, type)) {
// Individual generation tracking for predefined settings even if they are unset
if ((setting != null && !setting.isNull()) || isSettingPreDefined(name, type)) {
// Don't track generation for non-existent settings unless the name is predefined
synchronized (mLock) {
mSettingsRegistry.mGenerationRegistry.addGenerationData(result,
SettingsState.makeKey(type, userId), name);
} else {
// All non-predefined, unset settings are tracked using the same generation number
mSettingsRegistry.mGenerationRegistry.addGenerationDataForUnsetSettings(result,
SettingsState.makeKey(type, userId));
}
}
return result;
@@ -2349,8 +2345,7 @@ public class SettingsProvider extends ContentProvider {
} else if (type == SETTINGS_TYPE_SYSTEM) {
return sAllSystemSettings.contains(name);
} else {
// Consider all config settings predefined because they are used by system apps only
return type == SETTINGS_TYPE_CONFIG;
return false;
}
}
@@ -2359,13 +2354,14 @@ public class SettingsProvider extends ContentProvider {
Bundle result = new Bundle();
result.putSerializable(Settings.NameValueTable.VALUE, keyValues);
if (trackingGeneration) {
// Track generation even if the namespace is empty because this is for system apps
synchronized (mLock) {
// Track generation even if namespace is empty because this is for system apps only
mSettingsRegistry.mGenerationRegistry.addGenerationData(result,
SettingsState.makeKey(SETTINGS_TYPE_CONFIG, UserHandle.USER_SYSTEM),
prefix);
mSettingsRegistry.getSettingsLocked(SETTINGS_TYPE_CONFIG,
UserHandle.USER_SYSTEM).mKey, prefix);
}
}
return result;
}
@@ -3056,15 +3052,10 @@ public class SettingsProvider extends ContentProvider {
final int key = makeKey(type, userId);
boolean success = false;
boolean wasUnsetNonPredefinedSetting = false;
SettingsState settingsState = peekSettingsStateLocked(key);
if (settingsState != null) {
if (!isSettingPreDefined(name, type) && !settingsState.hasSetting(name)) {
wasUnsetNonPredefinedSetting = true;
}
success = settingsState.insertSettingLocked(name, value,
tag, makeDefault, forceNonSystemPackage, packageName,
overrideableByRestore);
tag, makeDefault, forceNonSystemPackage, packageName, overrideableByRestore);
}
if (success && criticalSettings != null && criticalSettings.contains(name)) {
@@ -3073,11 +3064,6 @@ public class SettingsProvider extends ContentProvider {
if (forceNotify || success) {
notifyForSettingsChange(key, name);
if (wasUnsetNonPredefinedSetting) {
// Increment the generation number for all non-predefined, unset settings,
// because a new non-predefined setting has been inserted
mGenerationRegistry.incrementGenerationForUnsetSettings(key);
}
}
if (success) {
logSettingChanged(userId, name, type, CHANGE_TYPE_INSERT);

6
packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
View File

@@ -759,12 +759,6 @@ final class SettingsState {
mPackageToMemoryUsage.put(packageName, newSize);
}
public boolean hasSetting(String name) {
synchronized (mLock) {
return hasSettingLocked(name);
}
}
@GuardedBy("mLock")
private boolean hasSettingLocked(String name) {
return mSettings.indexOfKey(name) >= 0;

20
packages/SettingsProvider/test/src/com/android/providers/settings/GenerationRegistryTest.java
View File

@@ -151,26 +151,6 @@ public class GenerationRegistryTest {
checkBundle(b, 0, 1, false);
}
@Test
public void testUnsetSettings() throws IOException {
final GenerationRegistry generationRegistry = new GenerationRegistry(new Object());
final int secureKey = SettingsState.makeKey(SettingsState.SETTINGS_TYPE_SECURE, 0);
final String testSecureSetting = "test_secure_setting";
Bundle b = new Bundle();
generationRegistry.addGenerationData(b, secureKey, testSecureSetting);
checkBundle(b, 0, 1, false);
generationRegistry.addGenerationDataForUnsetSettings(b, secureKey);
checkBundle(b, 1, 1, false);
generationRegistry.addGenerationDataForUnsetSettings(b, secureKey);
// Test that unset settings always have the same index
checkBundle(b, 1, 1, false);
generationRegistry.incrementGenerationForUnsetSettings(secureKey);
// Test that the generation number of the unset settings have increased
generationRegistry.addGenerationDataForUnsetSettings(b, secureKey);
checkBundle(b, 1, 2, false);
}
private void checkBundle(Bundle b, int expectedIndex, int expectedGeneration, boolean isNull)
throws IOException {
final MemoryIntArray array = getArray(b);