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
This commit is contained in:
Winson
2021-06-25 15:28:51 -07:00
committed by Patrick Baumann
parent 6eac5ad95a
commit 6e7dced612
2 changed files with 81 additions and 65 deletions

View File

@@ -21481,7 +21481,7 @@ public class PackageManagerService extends IPackageManager.Stub
// user handle installed state
int[] allUsers;
final int freezeUser;
final SparseArray<Pair<Integer, String>> enabledStateAndCallerPerUser;
final SparseArray<TempUserState> 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<Integer, String> 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<Integer, String> 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 {

View File

@@ -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<CodePath>
) {
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))
}