diff --git a/services/core/java/com/android/server/compat/overrides/AppCompatOverridesParser.java b/services/core/java/com/android/server/compat/overrides/AppCompatOverridesParser.java index a81213df6fe30..11dc1dbd25ccf 100644 --- a/services/core/java/com/android/server/compat/overrides/AppCompatOverridesParser.java +++ b/services/core/java/com/android/server/compat/overrides/AppCompatOverridesParser.java @@ -175,29 +175,25 @@ final class AppCompatOverridesParser { /** * Parses the given {@code configStr}, that is expected to be a comma separated list of changes - * overrides, and returns a {@link PackageOverrides}. + * overrides, and returns a map from change ID to {@link PackageOverride} instances to add. * *

Each change override is in the following format: - * ':::'. If is empty, - * this indicates that any override for the specified change ID should be removed. + * ':::'. * *

If there are multiple overrides that should be added with the same change ID, the one * that best fits the given {@code versionCode} is added. * *

Any overrides whose change ID is in {@code changeIdsToSkip} are ignored. * - *

If a change override entry in {@code configStr} is invalid, it will be ignored. If the - * same change ID is both added and removed, i.e., has a change override entry with an empty - * enabled and another with a non-empty enabled, the change ID will only be removed. + *

If a change override entry in {@code configStr} is invalid, it will be ignored. */ - static PackageOverrides parsePackageOverrides( - String configStr, long versionCode, Set changeIdsToSkip) { + static Map parsePackageOverrides(String configStr, long versionCode, + Set changeIdsToSkip) { if (configStr.isEmpty()) { - return new PackageOverrides(); + return emptyMap(); } PackageOverrideComparator comparator = new PackageOverrideComparator(versionCode); Map overridesToAdd = new ArrayMap<>(); - Set overridesToRemove = new ArraySet<>(); for (String overrideEntryString : configStr.split(",")) { List changeIdAndVersions = Arrays.asList(overrideEntryString.split(":", 4)); if (changeIdAndVersions.size() != 4) { @@ -220,16 +216,6 @@ final class AppCompatOverridesParser { String maxVersionCodeStr = changeIdAndVersions.get(2); String enabledStr = changeIdAndVersions.get(3); - if (enabledStr.isEmpty()) { - if (!minVersionCodeStr.isEmpty() || !maxVersionCodeStr.isEmpty()) { - Slog.w( - TAG, - "min/max version code should be empty if enabled is empty: " - + overrideEntryString); - } - overridesToRemove.add(changeId); - continue; - } if (!BOOLEAN_PATTERN.matcher(enabledStr).matches()) { Slog.w(TAG, "Invalid enabled string in override entry: " + overrideEntryString); continue; @@ -262,39 +248,7 @@ final class AppCompatOverridesParser { } } - for (Long changeId : overridesToRemove) { - if (overridesToAdd.containsKey(changeId)) { - Slog.w( - TAG, - "Change ID [" - + changeId - + "] is both added and removed in package override flag: " - + configStr); - overridesToAdd.remove(changeId); - } - } - - return new PackageOverrides(overridesToAdd, overridesToRemove); - } - - /** - * A container for a map from change ID to {@link PackageOverride} to add and a set of change - * IDs to remove overrides for. - * - *

The map of overrides to add and the set of overrides to remove are mutually exclusive. - */ - static final class PackageOverrides { - public final Map overridesToAdd; - public final Set overridesToRemove; - - PackageOverrides() { - this(emptyMap(), emptySet()); - } - - PackageOverrides(Map overridesToAdd, Set overridesToRemove) { - this.overridesToAdd = overridesToAdd; - this.overridesToRemove = overridesToRemove; - } + return overridesToAdd; } /** diff --git a/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java b/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java index 63ae1af42f086..6aed4b023297b 100644 --- a/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java +++ b/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java @@ -49,7 +49,6 @@ import com.android.internal.compat.CompatibilityOverrideConfig; import com.android.internal.compat.CompatibilityOverridesToRemoveConfig; import com.android.internal.compat.IPlatformCompat; import com.android.server.SystemService; -import com.android.server.compat.overrides.AppCompatOverridesParser.PackageOverrides; import java.util.ArrayList; import java.util.Arrays; @@ -128,20 +127,25 @@ public final class AppCompatOverridesService { } /** - * Same as {@link #applyOverrides(Properties, Map)} except all properties of the given {@code - * namespace} are fetched via {@link DeviceConfig#getProperties}. + * Same as {@link #applyOverrides(Properties, Set, Map)} except all properties of the given + * {@code namespace} are fetched via {@link DeviceConfig#getProperties}. */ - private void applyAllOverrides(String namespace, + private void applyAllOverrides(String namespace, Set ownedChangeIds, Map> packageToChangeIdsToSkip) { - applyOverrides(DeviceConfig.getProperties(namespace), packageToChangeIdsToSkip); + applyOverrides(DeviceConfig.getProperties(namespace), ownedChangeIds, + packageToChangeIdsToSkip); } /** * Iterates all package override flags in the given {@code properties}, and for each flag whose - * package is installed on the device, parses its value and applies the overrides in it with + * package is installed on the device, parses its value and adds the overrides in it with * respect to the package's current installed version. + * + *

In addition, for each package, removes any override that wasn't just added, whose change + * ID is in {@code ownedChangeIds} but not in the respective set in {@code + * packageToChangeIdsToSkip}. */ - private void applyOverrides(Properties properties, + private void applyOverrides(Properties properties, Set ownedChangeIds, Map> packageToChangeIdsToSkip) { Set packageNames = new ArraySet<>(properties.getKeyset()); packageNames.remove(FLAG_OWNED_CHANGE_IDS); @@ -154,15 +158,16 @@ public final class AppCompatOverridesService { } applyPackageOverrides(properties.getString(packageName, /* defaultValue= */ ""), - packageName, versionCode, - packageToChangeIdsToSkip.getOrDefault(packageName, emptySet())); + packageName, versionCode, ownedChangeIds, + packageToChangeIdsToSkip.getOrDefault(packageName, emptySet()), + /* removeOtherOwnedOverrides= */ true); } } /** - * Applies all overrides in all supported namespaces for the given {@code packageName}. + * Adds all overrides in all supported namespaces for the given {@code packageName}. */ - private void applyAllPackageOverrides(String packageName) { + private void addAllPackageOverrides(String packageName) { Long versionCode = getVersionCodeOrNull(packageName); if (versionCode == null) { return; @@ -171,26 +176,40 @@ public final class AppCompatOverridesService { for (String namespace : mSupportedNamespaces) { // We apply overrides for each namespace separately so that if there is a failure for // one namespace, the other namespaces won't be affected. + Set ownedChangeIds = getOwnedChangeIds(namespace); applyPackageOverrides( DeviceConfig.getString(namespace, packageName, /* defaultValue= */ ""), - packageName, versionCode, - getOverridesToRemove(namespace).getOrDefault(packageName, emptySet())); + packageName, versionCode, ownedChangeIds, + getOverridesToRemove(namespace, ownedChangeIds).getOrDefault(packageName, + emptySet()), /* removeOtherOwnedOverrides */ false); } } /** - * Calls {@link AppCompatOverridesParser#parsePackageOverrides} on the given arguments, adds the - * resulting {@link PackageOverrides#overridesToAdd} via {@link - * IPlatformCompat#putOverridesOnReleaseBuilds}, and removes the resulting {@link - * PackageOverrides#overridesToRemove} via {@link - * IPlatformCompat#removeOverridesOnReleaseBuilds}. + * Calls {@link AppCompatOverridesParser#parsePackageOverrides} on the given arguments and adds + * the resulting overrides via {@link IPlatformCompat#putOverridesOnReleaseBuilds}. + * + *

In addition, if {@code removeOtherOwnedOverrides} is true, removes any override that + * wasn't just added, whose change ID is in {@code ownedChangeIds} but not in {@code + * changeIdsToSkip}, via {@link IPlatformCompat#removeOverridesOnReleaseBuilds}. */ - private void applyPackageOverrides(String configStr, String packageName, - long versionCode, Set changeIdsToSkip) { - PackageOverrides packageOverrides = AppCompatOverridesParser.parsePackageOverrides( + private void applyPackageOverrides(String configStr, String packageName, long versionCode, + Set ownedChangeIds, Set changeIdsToSkip, + boolean removeOtherOwnedOverrides) { + Map overridesToAdd = AppCompatOverridesParser.parsePackageOverrides( configStr, versionCode, changeIdsToSkip); - putPackageOverrides(packageName, packageOverrides.overridesToAdd); - removePackageOverrides(packageName, packageOverrides.overridesToRemove); + putPackageOverrides(packageName, overridesToAdd); + + if (!removeOtherOwnedOverrides) { + return; + } + Set overridesToRemove = new ArraySet<>(); + for (Long changeId : ownedChangeIds) { + if (!overridesToAdd.containsKey(changeId) && !changeIdsToSkip.contains(changeId)) { + overridesToRemove.add(changeId); + } + } + removePackageOverrides(packageName, overridesToRemove); } /** @@ -227,10 +246,11 @@ public final class AppCompatOverridesService { * {@code namespace} and parses it into a map from package name to a set of change IDs to * remove for that package. */ - private Map> getOverridesToRemove(String namespace) { + private Map> getOverridesToRemove(String namespace, + Set ownedChangeIds) { return mOverridesParser.parseRemoveOverrides( DeviceConfig.getString(namespace, FLAG_REMOVE_OVERRIDES, /* defaultValue= */ ""), - getOwnedChangeIds(namespace)); + ownedChangeIds); } /** @@ -333,7 +353,9 @@ public final class AppCompatOverridesService { boolean ownedChangedIdsFlagChanged = properties.getKeyset().contains( FLAG_OWNED_CHANGE_IDS); - Map> overridesToRemove = getOverridesToRemove(mNamespace); + Set ownedChangeIds = getOwnedChangeIds(mNamespace); + Map> overridesToRemove = getOverridesToRemove(mNamespace, + ownedChangeIds); if (removeOverridesFlagChanged || ownedChangedIdsFlagChanged) { // In both cases it's possible that overrides that weren't removed before should // now be removed. @@ -343,9 +365,9 @@ public final class AppCompatOverridesService { if (removeOverridesFlagChanged) { // We need to re-apply all overrides in the namespace since the remove overrides // flag might have blocked some of them from being applied before. - applyAllOverrides(mNamespace, overridesToRemove); + applyAllOverrides(mNamespace, ownedChangeIds, overridesToRemove); } else { - applyOverrides(properties, overridesToRemove); + applyOverrides(properties, ownedChangeIds, overridesToRemove); } } } @@ -392,7 +414,7 @@ public final class AppCompatOverridesService { switch (action) { case ACTION_PACKAGE_ADDED: case ACTION_PACKAGE_CHANGED: - applyAllPackageOverrides(packageName); + addAllPackageOverrides(packageName); break; case ACTION_PACKAGE_REMOVED: if (!isInstalledForAnyUser(packageName)) { diff --git a/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesParserTest.java b/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesParserTest.java index bf97042d7b884..df19be4a9cfe6 100644 --- a/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesParserTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesParserTest.java @@ -31,8 +31,6 @@ import android.util.ArraySet; import androidx.test.filters.SmallTest; -import com.android.server.compat.overrides.AppCompatOverridesParser.PackageOverrides; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -183,115 +181,84 @@ public class AppCompatOverridesParserTest { } @Test - public void parsePackageOverrides_emptyConfig_returnsEmpty() { - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "", /* versionCode= */ 0, /* changeIdsToSkip= */ emptySet()); + public void parsePackageOverrides_emptyConfigNoOwnedChangeIds_returnsEmpty() { + Map result = AppCompatOverridesParser.parsePackageOverrides( + /* configStr= */ "", /* versionCode= */ 0, /* changeIdsToSkip= */ emptySet()); - assertThat(result.overridesToAdd).isEmpty(); - assertThat(result.overridesToRemove).isEmpty(); + assertThat(result).isEmpty(); } @Test public void parsePackageOverrides_configWithSingleOverride_returnsOverride() { - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "123:::true", /* versionCode= */ 5, /* changeIdsToSkip= */ emptySet()); - - assertThat(result.overridesToAdd).hasSize(1); - assertThat(result.overridesToAdd.get(123L)).isEqualTo( - new PackageOverride.Builder().setEnabled(true).build()); - } - - @Test - public void parsePackageOverrides_configWithMultipleOverridesToAdd_returnsOverrides() { - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "910:3:4:false,78:10::false,12:::false,34:1:2:true,34:10::true,56::2:true," - + "56:3:4:false,34:4:8:true,78:6:7:true,910:5::true,1112::5:true," - + "56:6::true,1112:6:7:false", /* versionCode= */ - 5, /* changeIdsToSkip= */ emptySet()); - - assertThat(result.overridesToAdd).hasSize(6); - assertThat(result.overridesToAdd.get(12L)).isEqualTo( - new PackageOverride.Builder().setEnabled(false).build()); - assertThat(result.overridesToAdd.get(34L)).isEqualTo( - new PackageOverride.Builder().setMinVersionCode(4).setMaxVersionCode(8).setEnabled( - true).build()); - assertThat(result.overridesToAdd.get(56L)).isEqualTo( - new PackageOverride.Builder().setMinVersionCode(3).setMaxVersionCode(4).setEnabled( - false).build()); - assertThat(result.overridesToAdd.get(78L)).isEqualTo( - new PackageOverride.Builder().setMinVersionCode(6).setMaxVersionCode(7).setEnabled( - true).build()); - assertThat(result.overridesToAdd.get(910L)).isEqualTo( - new PackageOverride.Builder().setMinVersionCode(5).setEnabled(true).build()); - assertThat(result.overridesToAdd.get(1112L)).isEqualTo( - new PackageOverride.Builder().setMaxVersionCode(5).setEnabled(true).build()); - assertThat(result.overridesToRemove).isEmpty(); - } - - @Test - public void parsePackageOverrides_configWithMultipleOverridesToRemove_returnsOverrides() { - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "12:::,34:1:2:", /* versionCode= */ 5, /* changeIdsToSkip= */ emptySet()); - - assertThat(result.overridesToRemove).containsExactly(12L, 34L); - assertThat(result.overridesToAdd).isEmpty(); - } - - @Test - public void parsePackageOverrides_configWithBothOverridesToAddAndRemove_returnsOverrides() { - // Note that change 56 is both added and removed, therefore it will only be removed. - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "56:::,12:::true,34:::,56:3:7:true", /* versionCode= */ 5, /* changeIdsToSkip= */ + Map result = AppCompatOverridesParser.parsePackageOverrides( + /* configStr= */ "123:::true", /* versionCode= */ 5, /* changeIdsToSkip= */ emptySet()); - assertThat(result.overridesToAdd).hasSize(1); - assertThat(result.overridesToAdd.get(12L)).isEqualTo( + assertThat(result).hasSize(1); + assertThat(result.get(123L)).isEqualTo( new PackageOverride.Builder().setEnabled(true).build()); - assertThat(result.overridesToRemove).containsExactly(34L, 56L); + } + + @Test + public void parsePackageOverrides_configWithMultipleOverrides_returnsOverrides() { + Map result = AppCompatOverridesParser.parsePackageOverrides( + /* configStr= */ "910:3:4:false,78:10::false,12:::false,34:1:2:true,34:10::true," + + "56::2:true,56:3:4:false,34:4:8:true,78:6:7:true,910:5::true," + + "1112::5:true,56:6::true,1112:6:7:false", /* versionCode= */ + 5, /* changeIdsToSkip= */ emptySet()); + + assertThat(result).hasSize(6); + assertThat(result.get(12L)).isEqualTo( + new PackageOverride.Builder().setEnabled(false).build()); + assertThat(result.get(34L)).isEqualTo( + new PackageOverride.Builder().setMinVersionCode(4).setMaxVersionCode(8).setEnabled( + true).build()); + assertThat(result.get(56L)).isEqualTo( + new PackageOverride.Builder().setMinVersionCode(3).setMaxVersionCode(4).setEnabled( + false).build()); + assertThat(result.get(78L)).isEqualTo( + new PackageOverride.Builder().setMinVersionCode(6).setMaxVersionCode(7).setEnabled( + true).build()); + assertThat(result.get(910L)).isEqualTo( + new PackageOverride.Builder().setMinVersionCode(5).setEnabled(true).build()); + assertThat(result.get(1112L)).isEqualTo( + new PackageOverride.Builder().setMaxVersionCode(5).setEnabled(true).build()); } @Test public void parsePackageOverrides_changeIdsToSkipSpecified_returnsWithoutChangeIdsToSkip() { - ArraySet changeIdsToSkip = new ArraySet<>(); - changeIdsToSkip.add(34L); - changeIdsToSkip.add(56L); - changeIdsToSkip.add(910L); - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "12:::true,34:::,56:3:7:true,78:::", /* versionCode= */ 5, changeIdsToSkip); + ArraySet changeIdsToSkip = new ArraySet<>(Arrays.asList(34L, 56L)); + Map result = AppCompatOverridesParser.parsePackageOverrides( + /* configStr= */ "12:::true,56:3:7:true", /* versionCode= */ 5, changeIdsToSkip); - assertThat(result.overridesToAdd).hasSize(1); - assertThat(result.overridesToAdd.get(12L)).isEqualTo( + assertThat(result).hasSize(1); + assertThat(result.get(12L)).isEqualTo( new PackageOverride.Builder().setEnabled(true).build()); - assertThat(result.overridesToRemove).containsExactly(78L); } @Test public void parsePackageOverrides_changeIdsToSkipContainsAllIds_returnsEmpty() { - ArraySet changeIdsToSkip = new ArraySet<>(); - changeIdsToSkip.add(12L); - changeIdsToSkip.add(34L); - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "12:::true,34:::", /* versionCode= */ 5, changeIdsToSkip); + ArraySet changeIdsToSkip = new ArraySet<>(Arrays.asList(12L, 34L)); + Map result = AppCompatOverridesParser.parsePackageOverrides( + /* configStr= */ "12:::true", /* versionCode= */ 5, changeIdsToSkip); - assertThat(result.overridesToAdd).isEmpty(); - assertThat(result.overridesToRemove).isEmpty(); + assertThat(result).isEmpty(); } @Test public void parsePackageOverrides_someOverridesAreInvalid_returnsWithoutInvalidOverrides() { // We add a valid entry before and after the invalid ones to make sure they are applied. - PackageOverrides result = AppCompatOverridesParser.parsePackageOverrides(/* configStr= */ - "12:::True,56:1:2:FALSE,56:3:true,78:4:8:true:,C1:::true,910:::no," - + "1112:1:ten:true,1112:one:10:true,,1314:7:3:false,34:one:ten:", + Map result = AppCompatOverridesParser.parsePackageOverrides( + /* configStr= */ "12:::True,56:1:2:FALSE,56:3:true,78:4:8:true:,C1:::true,910:::no," + + "1112:1:ten:true,1112:one:10:true,,1314:7:3:false,34:::", /* versionCode= */ 5, /* changeIdsToSkip= */ emptySet()); - assertThat(result.overridesToAdd).hasSize(2); - assertThat(result.overridesToAdd.get(12L)).isEqualTo( + assertThat(result).hasSize(2); + assertThat(result.get(12L)).isEqualTo( new PackageOverride.Builder().setEnabled(true).build()); - assertThat(result.overridesToAdd.get(56L)).isEqualTo( + assertThat(result.get(56L)).isEqualTo( new PackageOverride.Builder().setMinVersionCode(1).setMaxVersionCode(2).setEnabled( false).build()); - assertThat(result.overridesToRemove).containsExactly(34L); } private static ApplicationInfo createAppInfo(String packageName) { diff --git a/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java index 312927206a801..007191f026319 100644 --- a/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/compat/overrides/AppCompatOverridesServiceTest.java @@ -95,7 +95,6 @@ public class AppCompatOverridesServiceTest { private static final String PACKAGE_2 = "com.android.test2"; private static final String PACKAGE_3 = "com.android.test3"; private static final String PACKAGE_4 = "com.android.test4"; - private static final String PACKAGE_5 = "com.android.test5"; private MockContext mMockContext; private BroadcastReceiver mPackageReceiver; @@ -157,16 +156,14 @@ public class AppCompatOverridesServiceTest { mockGetApplicationInfoNotInstalled(PACKAGE_2); mockGetApplicationInfo(PACKAGE_3, /* versionCode= */ 10); mockGetApplicationInfo(PACKAGE_4, /* versionCode= */ 1); - mockGetApplicationInfo(PACKAGE_5, /* versionCode= */ 1); mService.registerDeviceConfigListeners(); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) - .setString(PACKAGE_1, "123:::true,456::1:false,456:2::true") + .setString(FLAG_OWNED_CHANGE_IDS, "123,456,789") + .setString(PACKAGE_1, "123:::true,456::1:false,456:2::true,789:::false") .setString(PACKAGE_2, "123:::true") - .setString(PACKAGE_3, "123:1:9:true,123:10:11:false,123:11::true,456:::") - .setString(PACKAGE_4, "") - .setString(PACKAGE_5, "123:::,789:::") - .setString(FLAG_OWNED_CHANGE_IDS, "123,456,789").build()); + .setString(PACKAGE_3, "123:1:9:true,123:10:11:false,123:11::true") + .setString(PACKAGE_4, "").build()); Map addedOverrides; // Package 1 @@ -175,11 +172,13 @@ public class AppCompatOverridesServiceTest { verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); addedOverrides = mOverridesToAddConfigCaptor.getValue().overrides; - assertThat(addedOverrides).hasSize(2); + assertThat(addedOverrides).hasSize(3); assertThat(addedOverrides.get(123L)).isEqualTo( new PackageOverride.Builder().setEnabled(true).build()); assertThat(addedOverrides.get(456L)).isEqualTo( new PackageOverride.Builder().setMinVersionCode(2).setEnabled(true).build()); + assertThat(addedOverrides.get(789L)).isEqualTo( + new PackageOverride.Builder().setEnabled(false).build()); // Package 2 verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); @@ -195,24 +194,37 @@ public class AppCompatOverridesServiceTest { assertThat(addedOverrides.get(123L)).isEqualTo( new PackageOverride.Builder().setMinVersionCode(10).setMaxVersionCode( 11).setEnabled(false).build()); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L); + assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L, 789L); // Package 4 verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( any(CompatibilityOverrideConfig.class), eq(PACKAGE_4)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_4)); - // Package 5 - verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( - any(CompatibilityOverrideConfig.class), eq(PACKAGE_5)); verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_5)); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 789L); + mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_4)); + assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 456L, + 789L); + } + + @Test + public void onPropertiesChanged_ownedChangeIdsFlagNotSet_onlyAddsOverrides() + throws Exception { + mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); + + mService.registerDeviceConfigListeners(); + DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) + .setString(PACKAGE_1, "123:::true").build()); + + verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), + eq(PACKAGE_1)); + verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); + assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(123L); } @Test public void onPropertiesChanged_removeOverridesFlagSetBefore_skipsOverridesToRemove() throws Exception { DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) + .setString(FLAG_OWNED_CHANGE_IDS, "123,456,789") .setString(FLAG_REMOVE_OVERRIDES, PACKAGE_1 + "=123:456," + PACKAGE_2 + "=123") .setString(PACKAGE_1, "123:::true") .setString(PACKAGE_4, "123:::true").build()); @@ -222,7 +234,7 @@ public class AppCompatOverridesServiceTest { mService.registerDeviceConfigListeners(); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) - .setString(PACKAGE_1, "123:::true,456:::,789:::false") + .setString(PACKAGE_1, "123:::true,789:::false") .setString(PACKAGE_2, "123:::true") .setString(PACKAGE_3, "456:::true").build()); @@ -235,14 +247,16 @@ public class AppCompatOverridesServiceTest { // Package 2 verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_2)); + verify(mPlatformCompat).removeOverridesOnReleaseBuilds( + mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_2)); + assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L, 789L); // Package 3 verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), eq(PACKAGE_3)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_3)); + verify(mPlatformCompat).removeOverridesOnReleaseBuilds( + mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_3)); assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(456L); + assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 789L); // Package 4 (not applied because it hasn't changed after the listener was added) verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( any(CompatibilityOverrideConfig.class), eq(PACKAGE_4)); @@ -253,27 +267,44 @@ public class AppCompatOverridesServiceTest { @Test public void onPropertiesChanged_removeOverridesFlagChangedNoPackageOverridesFlags_removesOnly() throws Exception { + DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) + .setString(FLAG_OWNED_CHANGE_IDS, "123,456,789") + .setString(PACKAGE_1, "") + .setString(PACKAGE_2, "").build()); + mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); + mockGetApplicationInfo(PACKAGE_2, /* versionCode= */ 0); + mService.registerDeviceConfigListeners(); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) .setString(FLAG_REMOVE_OVERRIDES, - PACKAGE_1 + "=123:456," + PACKAGE_2 + "=789").build()); + PACKAGE_1 + "=123:456," + PACKAGE_2 + "=*").build()); // Package 1 - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( + verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( + any(CompatibilityOverrideConfig.class), eq(PACKAGE_1)); + verify(mPlatformCompat, times(2)).removeOverridesOnReleaseBuilds( mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_1)); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 456L); + List configs = + mOverridesToRemoveConfigCaptor.getAllValues(); + assertThat(configs.size()).isAtLeast(2); + assertThat(configs.get(configs.size() - 2).changeIds).containsExactly(123L, 456L); + assertThat(configs.get(configs.size() - 1).changeIds).containsExactly(789L); // Package 2 + verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( + any(CompatibilityOverrideConfig.class), eq(PACKAGE_2)); verify(mPlatformCompat).removeOverridesOnReleaseBuilds( mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_2)); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(789L); + assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(123L, 456L, + 789L); } @Test public void onPropertiesChanged_removeOverridesFlagAndSomePackageOverrideFlagsChanged_ok() throws Exception { DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) + .setString(FLAG_OWNED_CHANGE_IDS, "123,456,789") .setString(FLAG_REMOVE_OVERRIDES, PACKAGE_1 + "=123:456") - .setString(PACKAGE_1, "123:::true,456:::,789:::false") + .setString(PACKAGE_1, "123:::true,789:::false") .setString(PACKAGE_3, "456:::false,789:::true").build()); mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); mockGetApplicationInfo(PACKAGE_2, /* versionCode= */ 0); @@ -282,7 +313,7 @@ public class AppCompatOverridesServiceTest { mService.registerDeviceConfigListeners(); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) .setString(FLAG_REMOVE_OVERRIDES, PACKAGE_2 + "=123," + PACKAGE_3 + "=789") - .setString(PACKAGE_2, "123:::true,456:::").build()); + .setString(PACKAGE_2, "123:::true").build()); // Package 1 verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), @@ -301,14 +332,17 @@ public class AppCompatOverridesServiceTest { mOverridesToRemoveConfigCaptor.getAllValues(); assertThat(configs.size()).isAtLeast(2); assertThat(configs.get(configs.size() - 2).changeIds).containsExactly(123L); - assertThat(configs.get(configs.size() - 1).changeIds).containsExactly(456L); + assertThat(configs.get(configs.size() - 1).changeIds).containsExactly(456L, 789L); // Package 3 verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), eq(PACKAGE_3)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( + verify(mPlatformCompat, times(2)).removeOverridesOnReleaseBuilds( mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_3)); assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(456L); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(789L); + configs = mOverridesToRemoveConfigCaptor.getAllValues(); + assertThat(configs.size()).isAtLeast(2); + assertThat(configs.get(configs.size() - 2).changeIds).containsExactly(789L); + assertThat(configs.get(configs.size() - 1).changeIds).containsExactly(123L); } @Test @@ -338,9 +372,10 @@ public class AppCompatOverridesServiceTest { // Package 2 verify(mPlatformCompat).putOverridesOnReleaseBuilds(mOverridesToAddConfigCaptor.capture(), eq(PACKAGE_2)); - verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( - any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_2)); + verify(mPlatformCompat).removeOverridesOnReleaseBuilds( + mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_2)); assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(123L); + assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(456L, 789L); // Package 3 verify(mPlatformCompat, never()).putOverridesOnReleaseBuilds( any(CompatibilityOverrideConfig.class), eq(PACKAGE_3)); @@ -362,10 +397,11 @@ public class AppCompatOverridesServiceTest { mService.registerDeviceConfigListeners(); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) - .setString(PACKAGE_1, "123:::true,456:::") - .setString(PACKAGE_2, "123:::true,456:::") - .setString(PACKAGE_3, "123:::true,456:::") - .setString(PACKAGE_4, "123:::true,456:::").build()); + .setString(FLAG_OWNED_CHANGE_IDS, "123,456") + .setString(PACKAGE_1, "123:::true") + .setString(PACKAGE_2, "123:::true") + .setString(PACKAGE_3, "123:::true") + .setString(PACKAGE_4, "123:::true").build()); // Package 1 verify(mPlatformCompat).putOverridesOnReleaseBuilds(any(CompatibilityOverrideConfig.class), @@ -478,12 +514,16 @@ public class AppCompatOverridesServiceTest { @Test public void packageReceiver_packageAddedIntent_appliesOverridesFromAllNamespaces() throws Exception { + // We're adding the owned_change_ids flag to make sure it's ignored. DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) - .setString(PACKAGE_1, "101:::true,103:::") + .setString(FLAG_OWNED_CHANGE_IDS, "101,102,103") + .setString(PACKAGE_1, "101:::true") .setString(PACKAGE_2, "102:::false").build()); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_2) + .setString(FLAG_OWNED_CHANGE_IDS, "201,202,203") .setString(PACKAGE_3, "201:::false").build()); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_3) + .setString(FLAG_OWNED_CHANGE_IDS, "301,302") .setString(PACKAGE_1, "301:::true,302:::false") .setString(PACKAGE_2, "302:::false").build()); mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); @@ -493,19 +533,18 @@ public class AppCompatOverridesServiceTest { verify(mPlatformCompat, times(2)).putOverridesOnReleaseBuilds( mOverridesToAddConfigCaptor.capture(), eq(PACKAGE_1)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_1)); + verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); List configs = mOverridesToAddConfigCaptor.getAllValues(); assertThat(configs.get(0).overrides.keySet()).containsExactly(101L); assertThat(configs.get(1).overrides.keySet()).containsExactly(301L, 302L); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(103L); } @Test public void packageReceiver_packageChangedIntent_appliesOverrides() throws Exception { DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) - .setString(PACKAGE_1, "101:::true,103:::").build()); + .setString(PACKAGE_1, "101:::true,103:::false").build()); mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); mPackageReceiver.onReceive(mMockContext, @@ -513,10 +552,10 @@ public class AppCompatOverridesServiceTest { verify(mPlatformCompat).putOverridesOnReleaseBuilds( mOverridesToAddConfigCaptor.capture(), eq(PACKAGE_1)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_1)); - assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(101L); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(103L); + verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); + assertThat(mOverridesToAddConfigCaptor.getValue().overrides.keySet()).containsExactly(101L, + 103L); } @Test @@ -524,13 +563,13 @@ public class AppCompatOverridesServiceTest { throws Exception { DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) .setString(FLAG_REMOVE_OVERRIDES, PACKAGE_1 + "=103," + PACKAGE_2 + "=101") - .setString(PACKAGE_1, "101:::true,103:::") + .setString(PACKAGE_1, "101:::true,103:::false") .setString(PACKAGE_2, "102:::false").build()); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_2) .setString(PACKAGE_1, "201:::false").build()); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_3) .setString(FLAG_REMOVE_OVERRIDES, PACKAGE_1 + "=301," + PACKAGE_3 + "=302") - .setString(PACKAGE_1, "301:::true,302:::false,303:::") + .setString(PACKAGE_1, "301:::true,302:::false,303:::true") .setString(PACKAGE_3, "302:::false").build()); mockGetApplicationInfo(PACKAGE_1, /* versionCode= */ 0); @@ -539,13 +578,12 @@ public class AppCompatOverridesServiceTest { verify(mPlatformCompat, times(3)).putOverridesOnReleaseBuilds( mOverridesToAddConfigCaptor.capture(), eq(PACKAGE_1)); - verify(mPlatformCompat).removeOverridesOnReleaseBuilds( - mOverridesToRemoveConfigCaptor.capture(), eq(PACKAGE_1)); + verify(mPlatformCompat, never()).removeOverridesOnReleaseBuilds( + any(CompatibilityOverridesToRemoveConfig.class), eq(PACKAGE_1)); List configs = mOverridesToAddConfigCaptor.getAllValues(); assertThat(configs.get(0).overrides.keySet()).containsExactly(101L); assertThat(configs.get(1).overrides.keySet()).containsExactly(201L); - assertThat(configs.get(2).overrides.keySet()).containsExactly(302L); - assertThat(mOverridesToRemoveConfigCaptor.getValue().changeIds).containsExactly(303L); + assertThat(configs.get(2).overrides.keySet()).containsExactly(302L, 303L); } @Test @@ -573,7 +611,7 @@ public class AppCompatOverridesServiceTest { throws Exception { DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) .setString(FLAG_OWNED_CHANGE_IDS, "101,102,103") - .setString(PACKAGE_1, "101:::true,103:::").build()); + .setString(PACKAGE_1, "101:::true,103:::false").build()); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_2) .setString(FLAG_OWNED_CHANGE_IDS, "201,202") .setString(PACKAGE_1, "202:::false").build()); @@ -593,14 +631,14 @@ public class AppCompatOverridesServiceTest { throws Exception { DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_1) .setString(FLAG_OWNED_CHANGE_IDS, "101,102,103") - .setString(PACKAGE_1, "101:::true,103:::") + .setString(PACKAGE_1, "101:::true,103:::false") .setString(PACKAGE_2, "102:::false").build()); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_2) .setString(FLAG_OWNED_CHANGE_IDS, "201") .setString(PACKAGE_3, "201:::false").build()); DeviceConfig.setProperties(new Properties.Builder(NAMESPACE_3) .setString(FLAG_OWNED_CHANGE_IDS, "301,302") - .setString(PACKAGE_1, "302:::") + .setString(PACKAGE_1, "302:::false") .setString(PACKAGE_2, "301:::true").build()); mockGetApplicationInfoNotInstalled(PACKAGE_1);