Merge changes Iac69f180,I99b6cc0e into udc-dev

* changes:
  Revert "[SettingsProvider] tracking generation of non-predefined unset settings"
  Revert "[SettingsProvider] only increment generation of unset settings if new setting is non-predefined"

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
+        // doesn't have a value stored for it. Returns the generation key if the value isn't null
-        // if the caller asked for the generation key.
+        // 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;
+                        int index = key.equals(SETTING) ? 0 : 1;
-                        if (value != null) {
-                            index = key.equals(SETTING) ? 0 : 1;
-                        } else {
-                            // special index for unset settings
-                            index = 2;
-                        }
                         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
+        // Empty list won't be 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(returnedValue2).isNull();
+        assertThat(cachedValue).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

@@ -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 "
+                    Slog.i(LOG_TAG, "Incremented generation for setting:" + indexMapKey
-                            + (indexMapKey.isEmpty() ? "unset settings" : "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 "
+                    Slog.i(LOG_TAG, "Exported index:" + index
-                            + (indexMapKey.isEmpty() ? "unset settings" : "setting:" + indexMapKey)
+                            + " 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);

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
+            // 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 false;
-            return type == SETTINGS_TYPE_CONFIG;
         }
     }
 
@@ -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),
+                        mSettingsRegistry.getSettingsLocked(SETTINGS_TYPE_CONFIG,
-                        prefix);
+                                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,
+                        tag, makeDefault, forceNonSystemPackage, packageName, overrideableByRestore);
-                        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);

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;

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);