Merge "Revert "Revert "[SettingsProvider] tracking generation of non-pr...""... (dc4d9643) · Commits · e / os / android_frameworks_base

core/java/android/provider/Settings.java

+19 −14

Original line number	Diff line number	Diff line
		@@ -3021,7 +3021,11 @@ public final class Settings {

		public void destroy() {
		try {
		// If this process is the system server process, mArray is the same object as
		// the memory int array kept inside SetingsProvider, so skipping the close()
		if (!Settings.isInSystemServer()) {
		mArray.close();
		}
		} catch (IOException e) {
		Log.e(TAG, "Error closing backing array", e);
		}
		@@ -3190,9 +3194,7 @@ public final class Settings {
		@UnsupportedAppUsage
		public String getStringForUser(ContentResolver cr, String name, final int userHandle) {
		final boolean isSelf = (userHandle == UserHandle.myUserId());
		int currentGeneration = -1;
		boolean needsGenerationTracker = false;

		if (isSelf) {
		synchronized (NameValueCache.this) {
		final GenerationTracker generationTracker = mGenerationTrackers.get(name);
		@@ -3204,27 +3206,31 @@ public final class Settings {
		+ " in package:" + cr.getPackageName()
		+ " and user:" + userHandle);
		}
		// When a generation number changes, remove cached value, remove the old
		// generation tracker and request a new one
		mValues.remove(name);
		generationTracker.destroy();
		mGenerationTrackers.remove(name);
		} else if (mValues.containsKey(name)) {
		if (DEBUG) {
		Log.i(TAG, "Cache hit for setting:" + name);
		}
		return mValues.get(name);
		}
		currentGeneration = generationTracker.getCurrentGeneration();
		} else {
		needsGenerationTracker = true;
		}
		}
		if (DEBUG) {
		Log.i(TAG, "Cache miss for setting:" + name + " for user:"
		+ userHandle);
		}
		// Generation tracker doesn't exist or the value isn't cached
		needsGenerationTracker = true;
		} else {
		if (DEBUG \|\| LOCAL_LOGV) {
		Log.v(TAG, "get setting for user " + userHandle
		+ " by user " + UserHandle.myUserId() + " so skipping cache");
		}
		}
		if (DEBUG) {
		Log.i(TAG, "Cache miss for setting:" + name + " for user:" + userHandle);
		}

		// Check if the target settings key is readable. Reject if the caller is not system and
		// is trying to access a settings key defined in the Settings.Secure, Settings.System or
		@@ -3324,11 +3330,10 @@ public final class Settings {
		mGenerationTrackers.put(name, new GenerationTracker(name,
		array, index, generation,
		mGenerationTrackerErrorHandler));
		currentGeneration = generation;
		}
		}
		if (mGenerationTrackers.get(name) != null && currentGeneration
		== mGenerationTrackers.get(name).getCurrentGeneration()) {
		if (mGenerationTrackers.get(name) != null
		&& !mGenerationTrackers.get(name).isGenerationChanged()) {
		if (DEBUG) {
		Log.i(TAG, "Updating cache for setting:" + name);
		}
		@@ -3374,8 +3379,8 @@ public final class Settings {

		String value = c.moveToNext() ? c.getString(0) : null;
		synchronized (NameValueCache.this) {
		if (mGenerationTrackers.get(name) != null && currentGeneration
		== mGenerationTrackers.get(name).getCurrentGeneration()) {
		if (mGenerationTrackers.get(name) != null
		&& !mGenerationTrackers.get(name).isGenerationChanged()) {
		if (DEBUG) {
		Log.i(TAG, "Updating cache for setting:" + name + " using query");
		}

core/tests/coretests/src/android/provider/NameValueCacheTest.java

+56 −9

Original line number	Diff line number	Diff line
		@@ -28,6 +28,7 @@ import static org.mockito.Mockito.when;
		import android.content.ContentProvider;
		import android.content.IContentProvider;
		import android.os.Bundle;
		import android.os.Parcel;
		import android.platform.test.annotations.Presubmit;
		import android.test.mock.MockContentResolver;
		import android.util.MemoryIntArray;
		@@ -91,7 +92,7 @@ public class NameValueCacheTest {
		mConfigsCacheGenerationStore = new MemoryIntArray(2);
		mConfigsCacheGenerationStore.set(0, 123);
		mConfigsCacheGenerationStore.set(1, 456);
		mSettingsCacheGenerationStore = new MemoryIntArray(2);
		mSettingsCacheGenerationStore = new MemoryIntArray(3);
		mSettingsCacheGenerationStore.set(0, 234);
		mSettingsCacheGenerationStore.set(1, 567);
		mConfigsStorage = new HashMap<>();
		@@ -163,6 +164,10 @@ 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
		@@ -173,12 +178,18 @@ 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 value isn't null
		// and the caller asked for the generation key.
		// doesn't have a value stored for it. Returns the generation key
		// if 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 -> {
		@@ -189,14 +200,26 @@ public class NameValueCacheTest {
		Bundle bundle = new Bundle();
		bundle.putSerializable(Settings.NameValueTable.VALUE, value);

		if (value != null && incomingBundle.containsKey(
		if (incomingBundle.containsKey(
		Settings.CALL_METHOD_TRACK_GENERATION_KEY)) {
		int index = key.equals(SETTING) ? 0 : 1;
		int index;
		if (value != null) {
		index = key.equals(SETTING) ? 0 : 1;
		} else {
		// special index for unset settings
		index = 2;
		}
		// Manually make a copy of the memory int array to mimic sending it over IPC
		Parcel p = Parcel.obtain();
		mSettingsCacheGenerationStore.writeToParcel(p, 0);
		p.setDataPosition(0);
		MemoryIntArray parcelArray = MemoryIntArray.CREATOR.createFromParcel(p);
		bundle.putParcelable(Settings.CALL_METHOD_TRACK_GENERATION_KEY,
		mSettingsCacheGenerationStore);
		parcelArray);
		bundle.putInt(Settings.CALL_METHOD_GENERATION_INDEX_KEY, index);
		bundle.putInt(Settings.CALL_METHOD_GENERATION_KEY,
		mSettingsCacheGenerationStore.get(index));
		p.recycle();
		}
		return bundle;
		});
		@@ -206,6 +229,8 @@ public class NameValueCacheTest {
		public void cleanUp() throws IOException {
		mConfigsStorage.clear();
		mSettingsStorage.clear();
		mSettingsCacheGenerationStore.close();
		mConfigsCacheGenerationStore.close();
		}

		@Test
		@@ -361,16 +386,38 @@ public class NameValueCacheTest {
		}

		@Test
		public void testCaching_nullSetting() throws Exception {
		public void testCaching_unsetSetting() 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);
		// Empty list won't be cached
		// The first unset setting's generation number is cached
		verifyNoMoreInteractions(mMockIContentProvider);
		assertThat(cachedValue).isNull();

		String returnedValue2 = Settings.Secure.getString(mMockContentResolver, SETTING2);
		verify(mMockIContentProvider, times(2)).call(any(), any(),
		eq(Settings.CALL_METHOD_GET_SECURE), any(), any(Bundle.class));
		assertThat(cachedValue).isNull();
		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");
		}
		}

packages/SettingsProvider/src/com/android/providers/settings/GenerationRegistry.java

+29 −3

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.providers.settings;

		import android.annotation.NonNull;
		import android.os.Bundle;
		import android.provider.Settings;
		import android.util.ArrayMap;
		@@ -59,6 +60,10 @@ 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;
		}
		@@ -72,6 +77,10 @@ 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);
		@@ -87,7 +96,8 @@ final class GenerationRegistry {
		final int generation = backingStore.get(index) + 1;
		backingStore.set(index, generation);
		if (DEBUG) {
		Slog.i(LOG_TAG, "Incremented generation for setting:" + indexMapKey
		Slog.i(LOG_TAG, "Incremented generation for "
		+ (indexMapKey.isEmpty() ? "unset settings" : "setting:" + indexMapKey)
		+ " key:" + SettingsState.keyToString(key)
		+ " at index:" + index);
		}
		@@ -98,6 +108,18 @@ 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
		@@ -124,8 +146,8 @@ final class GenerationRegistry {
		bundle.putInt(Settings.CALL_METHOD_GENERATION_KEY,
		backingStore.get(index));
		if (DEBUG) {
		Slog.i(LOG_TAG, "Exported index:" + index
		+ " for setting:" + indexMapKey
		Slog.i(LOG_TAG, "Exported index:" + index + " for "
		+ (indexMapKey.isEmpty() ? "unset settings" : "setting:" + indexMapKey)
		+ " key:" + SettingsState.keyToString(key));
		}
		} catch (IOException e) {
		@@ -135,6 +157,10 @@ 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);

packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java

+23 −9

Original line number	Diff line number	Diff line
		@@ -2378,11 +2378,15 @@ public class SettingsProvider extends ContentProvider {
		result.putString(Settings.NameValueTable.VALUE,
		(setting != null && !setting.isNull()) ? setting.getValue() : null);

		if ((setting != null && !setting.isNull()) \|\| isSettingPreDefined(name, type)) {
		// Don't track generation for non-existent settings unless the name is predefined
		synchronized (mLock) {
		if ((setting != null && !setting.isNull()) \|\| isSettingPreDefined(name, type)) {
		// Individual generation tracking for predefined settings even if they are unset
		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;
		@@ -2396,7 +2400,8 @@ public class SettingsProvider extends ContentProvider {
		} else if (type == SETTINGS_TYPE_SYSTEM) {
		return sAllSystemSettings.contains(name);
		} else {
		return false;
		// Consider all config settings predefined because they are used by system apps only
		return type == SETTINGS_TYPE_CONFIG;
		}
		}

		@@ -2405,14 +2410,13 @@ 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,
		mSettingsRegistry.getSettingsLocked(SETTINGS_TYPE_CONFIG,
		UserHandle.USER_SYSTEM).mKey, prefix);
		SettingsState.makeKey(SETTINGS_TYPE_CONFIG, UserHandle.USER_SYSTEM),
		prefix);
		}
		}

		return result;
		}

		@@ -3103,10 +3107,15 @@ 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)) {
		@@ -3115,6 +3124,11 @@ 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);

packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java

+6 −0

Original line number	Diff line number	Diff line
		@@ -759,6 +759,12 @@ 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;