From 1a19e33dbd042cbb8fc712cbda6ac6b1c0f008f2 Mon Sep 17 00:00:00 2001
From: tomnatan <tomnatan@google.com>
Date: Mon, 10 Jan 2022 13:11:01 +0000
Subject: [PATCH] Remove all overrides of an app that isn't installed when flag
 changes

There can be a race condition between a package override flag becoming empty and that package being uninstalled. An issue will occur if the change to the flag is processed after the package is uninstalled (and therefore this package won't be processed) but before the package receiver received a package removed intent (and won't remove overrides because the package override flag is empty).

Fix: 201059968
Test: atest FrameworksMockingServicesTests:AppCompatOverridesServiceTest
Change-Id: I9ccc52b6448eabe76053af9e03686a8409805848
---
 .../overrides/AppCompatOverridesService.java  | 19 ++++++++++---------
 .../AppCompatOverridesServiceTest.java        |  6 +++++-
 2 files changed, 15 insertions(+), 10 deletions(-)

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 880dbf6421ddd..fe002ce00d324 100644
--- a/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java
+++ b/services/core/java/com/android/server/compat/overrides/AppCompatOverridesService.java
@@ -25,6 +25,7 @@ import static android.provider.DeviceConfig.NAMESPACE_APP_COMPAT_OVERRIDES;
 import static com.android.server.compat.overrides.AppCompatOverridesParser.FLAG_OWNED_CHANGE_IDS;
 import static com.android.server.compat.overrides.AppCompatOverridesParser.FLAG_REMOVE_OVERRIDES;
 
+import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
 
 import android.annotation.NonNull;
@@ -157,17 +158,17 @@ public final class AppCompatOverridesService {
         Map<String, CompatibilityOverridesToRemoveConfig> packageNameToOverridesToRemove =
                 new ArrayMap<>();
         for (String packageName : packageNames) {
-            Long versionCode = getVersionCodeOrNull(packageName);
-            if (versionCode == null) {
-                // Package isn't installed yet.
-                continue;
-            }
-
             Set<Long> changeIdsToSkip = packageToChangeIdsToSkip.getOrDefault(packageName,
                     emptySet());
-            Map<Long, PackageOverride> overridesToAdd = mOverridesParser.parsePackageOverrides(
-                    properties.getString(packageName, /* defaultValue= */ ""), packageName,
-                    versionCode, changeIdsToSkip);
+
+            Map<Long, PackageOverride> overridesToAdd = emptyMap();
+            Long versionCode = getVersionCodeOrNull(packageName);
+            if (versionCode != null) {
+                // Only if package installed add overrides, otherwise just remove.
+                overridesToAdd = mOverridesParser.parsePackageOverrides(
+                        properties.getString(packageName, /* defaultValue= */ ""), packageName,
+                        versionCode, changeIdsToSkip);
+            }
             if (!overridesToAdd.isEmpty()) {
                 packageNameToOverridesToAdd.put(packageName,
                         new CompatibilityOverrideConfig(overridesToAdd));
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 edf68166bb7d1..1a5888e7dd76e 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
@@ -183,7 +183,8 @@ public class AppCompatOverridesServiceTest {
                 mOverridesToRemoveByPackageConfigCaptor.getValue().packageNameToOverridesToRemove;
         Map<Long, PackageOverride> addedOverrides;
         assertThat(packageNameToAddedOverrides.keySet()).containsExactly(PACKAGE_1, PACKAGE_3);
-        assertThat(packageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_3, PACKAGE_4);
+        assertThat(packageNameToRemovedOverrides.keySet()).containsExactly(PACKAGE_2, PACKAGE_3,
+                PACKAGE_4);
         // Package 1
         addedOverrides = packageNameToAddedOverrides.get(PACKAGE_1).overrides;
         assertThat(addedOverrides).hasSize(3);
@@ -193,6 +194,9 @@ public class AppCompatOverridesServiceTest {
                 new PackageOverride.Builder().setMinVersionCode(2).setEnabled(true).build());
         assertThat(addedOverrides.get(789L)).isEqualTo(
                 new PackageOverride.Builder().setEnabled(false).build());
+        // Package 2
+        assertThat(packageNameToRemovedOverrides.get(PACKAGE_2).changeIds).containsExactly(123L,
+                456L, 789L);
         // Package 3
         addedOverrides = packageNameToAddedOverrides.get(PACKAGE_3).overrides;
         assertThat(addedOverrides).hasSize(1);