From 6e7dced612ef0be1260af8a4ea092f7086837e06 Mon Sep 17 00:00:00 2001 From: Winson Date: Fri, 25 Jun 2021 15:28:51 -0700 Subject: [PATCH] Fix uninstall system updates disabling stub for single user A previous fix was introduced to re-enable compressed packages if any user on the device had the package installed + enabled, but this introduced a regression if there was only 1 user on device. It treated an uninstall updates command as if it was disabling for the primary user, whereas those should actually be handled independently, allowing an app to remain installed and enabled even after uninstalling updates. This was caught in the initial change, but ignored as an unimportant quirk of the uninstall updates system. This was in error and thus the tests for this feature were also broken and needed to be fixed to match the preserving behavior. This tacks onto a previous change that records the user state for a package before deletePackageX actually runs, and resets the state objects afterwards. This informs whether any user has the package installed + enabled to re-uncompress the stub to /data. Bug: 191988668 Test: atest SystemStubMultiUserDisableUninstallTest Change-Id: I670163e01d13ee5fdd2ebc6a046fa55a5398b290 Merged-In: I670163e01d13ee5fdd2ebc6a046fa55a5398b290 --- .../server/pm/PackageManagerService.java | 77 +++++++++++-------- ...SystemStubMultiUserDisableUninstallTest.kt | 69 +++++++++-------- 2 files changed, 81 insertions(+), 65 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 2419873cdc416..60bebe8b09146 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -21481,7 +21481,7 @@ public class PackageManagerService extends IPackageManager.Stub // user handle installed state int[] allUsers; final int freezeUser; - final SparseArray> enabledStateAndCallerPerUser; + final SparseArray priorUserStates; /** enabled state of the uninstalled application */ synchronized (mLock) { uninstalledPs = mSettings.getPackageLPr(packageName); @@ -21532,16 +21532,16 @@ public class PackageManagerService extends IPackageManager.Stub // We're downgrading a system app, which will apply to all users, so // freeze them all during the downgrade freezeUser = UserHandle.USER_ALL; - enabledStateAndCallerPerUser = new SparseArray<>(); + priorUserStates = new SparseArray<>(); for (int i = 0; i < allUsers.length; i++) { PackageUserState userState = uninstalledPs.readUserState(allUsers[i]); - Pair enabledStateAndCaller = - new Pair<>(userState.enabled, userState.lastDisableAppCaller); - enabledStateAndCallerPerUser.put(allUsers[i], enabledStateAndCaller); + priorUserStates.put(allUsers[i], + new TempUserState(userState.enabled, userState.lastDisableAppCaller, + userState.installed)); } } else { freezeUser = removeUser; - enabledStateAndCallerPerUser = null; + priorUserStates = null; } } @@ -21578,6 +21578,30 @@ public class PackageManagerService extends IPackageManager.Stub if (info.args != null) { info.args.doPostDeleteLI(true); } + + boolean reEnableStub = false; + + if (priorUserStates != null) { + synchronized (mLock) { + for (int i = 0; i < allUsers.length; i++) { + TempUserState priorUserState = priorUserStates.get(allUsers[i]); + int enabledState = priorUserState.enabledState; + PackageSetting pkgSetting = getPackageSetting(packageName); + pkgSetting.setEnabled(enabledState, allUsers[i], + priorUserState.lastDisableAppCaller); + + AndroidPackage aPkg = pkgSetting.getPkg(); + boolean pkgEnabled = aPkg != null && aPkg.isEnabled(); + if (!reEnableStub && priorUserState.installed + && ((enabledState == COMPONENT_ENABLED_STATE_DEFAULT && pkgEnabled) + || enabledState == COMPONENT_ENABLED_STATE_ENABLED)) { + reEnableStub = true; + } + } + mSettings.writeAllUsersPackageRestrictionsLPr(); + } + } + final AndroidPackage stubPkg = (disabledSystemPs == null) ? null : disabledSystemPs.pkg; if (stubPkg != null && stubPkg.isStub()) { @@ -21587,19 +21611,7 @@ public class PackageManagerService extends IPackageManager.Stub } if (stubPs != null) { - boolean enable = false; - for (int aUserId : allUsers) { - if (stubPs.getInstalled(aUserId)) { - int enabled = stubPs.getEnabled(aUserId); - if (enabled == COMPONENT_ENABLED_STATE_DEFAULT - || enabled == COMPONENT_ENABLED_STATE_ENABLED) { - enable = true; - break; - } - } - } - - if (enable) { + if (reEnableStub) { if (DEBUG_COMPRESSION) { Slog.i(TAG, "Enabling system stub after removal; pkg: " + stubPkg.getPackageName()); @@ -21611,19 +21623,6 @@ public class PackageManagerService extends IPackageManager.Stub } } } - if (enabledStateAndCallerPerUser != null) { - synchronized (mLock) { - for (int i = 0; i < allUsers.length; i++) { - Pair enabledStateAndCaller = - enabledStateAndCallerPerUser.get(allUsers[i]); - getPackageSetting(packageName) - .setEnabled(enabledStateAndCaller.first, - allUsers[i], - enabledStateAndCaller.second); - } - mSettings.writeAllUsersPackageRestrictionsLPr(); - } - } } return res ? PackageManager.DELETE_SUCCEEDED : PackageManager.DELETE_FAILED_INTERNAL_ERROR; @@ -28928,6 +28927,20 @@ public class PackageManagerService extends IPackageManager.Stub } } } + + private static class TempUserState { + public final int enabledState; + @Nullable + public final String lastDisableAppCaller; + public final boolean installed; + + private TempUserState(int enabledState, @Nullable String lastDisableAppCaller, + boolean installed) { + this.enabledState = enabledState; + this.lastDisableAppCaller = lastDisableAppCaller; + this.installed = installed; + } + } } interface PackageSender { diff --git a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/SystemStubMultiUserDisableUninstallTest.kt b/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/SystemStubMultiUserDisableUninstallTest.kt index 46120af065508..45e0d09a17533 100644 --- a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/SystemStubMultiUserDisableUninstallTest.kt +++ b/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/SystemStubMultiUserDisableUninstallTest.kt @@ -281,7 +281,7 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { assertState( primaryInstalled = false, primaryEnabled = true, secondaryInstalled = true, secondaryEnabled = true, - codePaths = listOf(CodePath.DIFFERENT, CodePath.SYSTEM) + codePaths = listOf(CodePath.SAME, CodePath.SYSTEM) ) uninstall(User.SECONDARY) @@ -289,7 +289,7 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { assertState( primaryInstalled = false, primaryEnabled = true, secondaryInstalled = false, secondaryEnabled = true, - codePaths = listOf(CodePath.DIFFERENT, CodePath.SYSTEM) + codePaths = listOf(CodePath.SAME, CodePath.SYSTEM) ) installExisting(User.PRIMARY) @@ -311,20 +311,20 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { @Test fun uninstallSecondaryFirstByUserAndInstallExistingSecondaryFirst() { + uninstall(User.SECONDARY) + + assertState( + primaryInstalled = true, primaryEnabled = true, + secondaryInstalled = false, secondaryEnabled = true, + codePaths = listOf(CodePath.SAME, CodePath.SYSTEM) + ) + uninstall(User.PRIMARY) - assertState( - primaryInstalled = false, primaryEnabled = true, - secondaryInstalled = true, secondaryEnabled = true, - codePaths = listOf(CodePath.DIFFERENT, CodePath.SYSTEM) - ) - - uninstall(User.SECONDARY) - assertState( primaryInstalled = false, primaryEnabled = true, secondaryInstalled = false, secondaryEnabled = true, - codePaths = listOf(CodePath.DIFFERENT, CodePath.SYSTEM) + codePaths = listOf(CodePath.SAME, CodePath.SYSTEM) ) installExisting(User.SECONDARY) @@ -348,15 +348,14 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { fun uninstallUpdatesAndEnablePrimaryFirst() { device.executeShellCommand("pm uninstall-system-updates $TEST_PKG_NAME") - // Uninstall-system-updates always disables system user 0 - // TODO: Is this intentional? There is no user argument for this command. assertState( - primaryInstalled = true, primaryEnabled = false, + primaryInstalled = true, primaryEnabled = true, secondaryInstalled = true, secondaryEnabled = true, // If any user is enabled when uninstalling updates, /data is re-uncompressed codePaths = listOf(CodePath.DIFFERENT, CodePath.SYSTEM) ) + toggleEnabled(false, User.PRIMARY) toggleEnabled(true, User.PRIMARY) assertState( @@ -379,14 +378,15 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { fun uninstallUpdatesAndEnableSecondaryFirst() { device.executeShellCommand("pm uninstall-system-updates $TEST_PKG_NAME") - // Uninstall-system-updates always disables system user 0 assertState( - primaryInstalled = true, primaryEnabled = false, + primaryInstalled = true, primaryEnabled = true, secondaryInstalled = true, secondaryEnabled = true, // If any user is enabled when uninstalling updates, /data is re-uncompressed codePaths = listOf(CodePath.DIFFERENT, CodePath.SYSTEM) ) + toggleEnabled(false, User.PRIMARY) + toggleEnabled(true, User.SECONDARY) assertState( @@ -417,6 +417,7 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { codePaths = listOf(CodePath.SYSTEM) ) + toggleEnabled(false, User.PRIMARY) toggleEnabled(true, User.PRIMARY) assertState( @@ -447,6 +448,7 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { codePaths = listOf(CodePath.SYSTEM) ) + toggleEnabled(false, User.PRIMARY) toggleEnabled(true, User.SECONDARY) assertState( @@ -471,13 +473,13 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { device.executeShellCommand("pm uninstall-system-updates $TEST_PKG_NAME") - // Uninstall-system-updates always disables system user 0 assertState( - primaryInstalled = false, primaryEnabled = false, + primaryInstalled = false, primaryEnabled = true, secondaryInstalled = false, secondaryEnabled = true, codePaths = listOf(CodePath.SYSTEM) ) + toggleEnabled(false, User.PRIMARY) toggleEnabled(true, User.PRIMARY) assertState( @@ -502,9 +504,8 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { device.executeShellCommand("pm uninstall-system-updates $TEST_PKG_NAME") - // Uninstall-system-updates always disables system user 0 assertState( - primaryInstalled = false, primaryEnabled = false, + primaryInstalled = false, primaryEnabled = true, secondaryInstalled = false, secondaryEnabled = true, codePaths = listOf(CodePath.SYSTEM) ) @@ -512,7 +513,7 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { toggleEnabled(true, User.SECONDARY) assertState( - primaryInstalled = false, primaryEnabled = false, + primaryInstalled = false, primaryEnabled = true, secondaryInstalled = false, secondaryEnabled = true, codePaths = listOf(CodePath.DIFFERENT, CodePath.SYSTEM) ) @@ -582,22 +583,24 @@ class SystemStubMultiUserDisableUninstallTest : BaseHostJUnit4Test() { codePaths: List ) { HostUtils.getUserIdToPkgInstalledState(device, TEST_PKG_NAME) - .forEach { (userId, installed) -> - if (userId == 0) { - assertThat(installed).isEqualTo(primaryInstalled) - } else { - assertThat(installed).isEqualTo(secondaryInstalled) - } + .also { assertThat(it.size).isAtLeast(USER_COUNT) } + .forEach { (userId, installed) -> + if (userId == 0) { + assertThat(installed).isEqualTo(primaryInstalled) + } else { + assertThat(installed).isEqualTo(secondaryInstalled) } + } HostUtils.getUserIdToPkgEnabledState(device, TEST_PKG_NAME) - .forEach { (userId, enabled) -> - if (userId == 0) { - assertThat(enabled).isEqualTo(primaryEnabled) - } else { - assertThat(enabled).isEqualTo(secondaryEnabled) - } + .also { assertThat(it.size).isAtLeast(USER_COUNT) } + .forEach { (userId, enabled) -> + if (userId == 0) { + assertThat(enabled).isEqualTo(primaryEnabled) + } else { + assertThat(enabled).isEqualTo(secondaryEnabled) } + } assertCodePaths(codePaths.first(), codePaths.getOrNull(1)) }