diff --git a/core/tests/coretests/src/android/provider/NameValueCacheTest.java b/core/tests/coretests/src/android/provider/NameValueCacheTest.java index ccf8085f87ff9..b6fc137471a42 100644 --- a/core/tests/coretests/src/android/provider/NameValueCacheTest.java +++ b/core/tests/coretests/src/android/provider/NameValueCacheTest.java @@ -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(); } } diff --git a/packages/SettingsProvider/src/com/android/providers/settings/GenerationRegistry.java b/packages/SettingsProvider/src/com/android/providers/settings/GenerationRegistry.java index db6cc1a39ef12..7f3b0ff8c8386 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/GenerationRegistry.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/GenerationRegistry.java @@ -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); diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index 27c8cdfe98f33..ba275ebca1682 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -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); diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index 4d8705f135af3..c3888268a61db 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -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; diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/GenerationRegistryTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/GenerationRegistryTest.java index 6ec8146baee0b..d34fe6943153c 100644 --- a/packages/SettingsProvider/test/src/com/android/providers/settings/GenerationRegistryTest.java +++ b/packages/SettingsProvider/test/src/com/android/providers/settings/GenerationRegistryTest.java @@ -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);