From 8c4aab3af6564dae17f924c576dd19c829657dbb Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Thu, 27 Feb 2020 16:50:44 -0800 Subject: [PATCH] Allow mutable overlays to be enabled by default When an overlay is not specified as immutable through OverlayConfig, enabling the overlay using the OverlayConfig `enable` attribute makes the overlay enabled by default. When an overlay is scanned by the OverlayManagerService for the first time, the default-enabled state will be applied to the overlay. If the configured default-enabled state changes in a subsequent boot, the default-enabled state will not be applied to the overlay. This means OTAs cannot change a mutable overlay from default-enabled to default-disabled (or vice-versa) and except the configured enabled state to take effect. When the device is factory reset, then the configured enabled state will be applied. If a package is removed from the device in one OTA and added back to the device in a future OTA, the configured enabled state will be applied. If an overlay changes its target package or target overlayable, then the configured enabled state will be applied. If an immutable overlay becomes mutable, then the configured enabled state will be applied. Bug: 149499802 Test: atest OverlayManagerServiceImplRebootTests Change-Id: I24f86591ac811ef2b836da36ef5574a82628b151 --- .../internal/content/om/OverlayConfig.java | 29 +++-- .../server/om/OverlayManagerServiceImpl.java | 11 +- .../server/om/OverlayManagerSettings.java | 56 ++++++--- .../OverlayManagerServiceImplRebootTests.java | 107 ++++++++++++++---- 4 files changed, 143 insertions(+), 60 deletions(-) diff --git a/core/java/com/android/internal/content/om/OverlayConfig.java b/core/java/com/android/internal/content/om/OverlayConfig.java index 72f16e4e4a82d..fbef027ac37f4 100644 --- a/core/java/com/android/internal/content/om/OverlayConfig.java +++ b/core/java/com/android/internal/content/om/OverlayConfig.java @@ -21,7 +21,6 @@ import android.annotation.Nullable; import android.content.pm.PackagePartitions; import android.content.pm.parsing.ParsingPackageRead; import android.os.Build; -import android.os.Process; import android.os.Trace; import android.util.ArrayMap; import android.util.Log; @@ -232,20 +231,26 @@ public class OverlayConfig { /** * Returns whether the overlay is enabled by default. - * Overlays that are not configured are disabled by default mutable. + * Overlays that are not configured are disabled by default. + * + * If an immutable overlay has its enabled state change, the new enabled state is applied to the + * overlay. + * + * When a mutable is first seen by the OverlayManagerService, the default-enabled state will be + * applied to the overlay. If the configured default-enabled state changes in a subsequent boot, + * the default-enabled state will not be applied to the overlay. + * + * The configured enabled state will only be applied when: + * */ public boolean isEnabled(String packageName) { final Configuration config = mConfigurations.get(packageName); - - // STOPSHIP(149499802): Enabling a mutable overlay currently has no effect. Either implement - // some behavior for default-enabled, mutable overlays or prevent parsing of the enabled - // attribute on overlays that are mutable. - if (config != null && config.parsedConfig.mutable) { - Log.w(TAG, "Default-enabled configuration for mutable overlay " - + config.parsedConfig.packageName + " has no effect"); - return OverlayConfigParser.DEFAULT_ENABLED_STATE; - } - return config == null? OverlayConfigParser.DEFAULT_ENABLED_STATE : config.parsedConfig.enabled; } diff --git a/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java b/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java index 2493057e01212..ebdd79a9409e7 100644 --- a/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java +++ b/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java @@ -102,10 +102,6 @@ final class OverlayManagerServiceImpl { return true; } - if (getPackageConfiguredPriority(theTruth.packageName) != oldSettings.priority) { - return true; - } - // If an immutable overlay changes its configured enabled state, reinitialize the overlay. if (!isMutable && isPackageConfiguredEnabled(theTruth.packageName) != oldSettings.isEnabled()) { @@ -161,6 +157,7 @@ final class OverlayManagerServiceImpl { final PackageInfo overlayPackage = overlayPackages.get(i); final OverlayInfo oi = storedOverlayInfos.get(overlayPackage.packageName); + int priority = getPackageConfiguredPriority(overlayPackage.packageName); if (mustReinitializeOverlay(overlayPackage, oi)) { // if targetPackageName has changed the package that *used* to // be the target must also update its assets @@ -174,8 +171,10 @@ final class OverlayManagerServiceImpl { overlayPackage.applicationInfo.getBaseCodePath(), isPackageConfiguredMutable(overlayPackage.packageName), isPackageConfiguredEnabled(overlayPackage.packageName), - getPackageConfiguredPriority(overlayPackage.packageName), - overlayPackage.overlayCategory); + priority, overlayPackage.overlayCategory); + } else if (priority != oi.priority) { + mSettings.setPriority(overlayPackage.packageName, newUserId, priority); + packagesToUpdateAssets.add(oi.targetPackageName); } storedOverlayInfos.remove(overlayPackage.packageName); diff --git a/services/core/java/com/android/server/om/OverlayManagerSettings.java b/services/core/java/com/android/server/om/OverlayManagerSettings.java index 6bccdfcf5bb2a..bdbaf78439a8c 100644 --- a/services/core/java/com/android/server/om/OverlayManagerSettings.java +++ b/services/core/java/com/android/server/om/OverlayManagerSettings.java @@ -71,24 +71,9 @@ final class OverlayManagerSettings { @NonNull final String baseCodePath, boolean isMutable, boolean isEnabled, int priority, @Nullable String overlayCategory) { remove(packageName, userId); - final SettingsItem item = - new SettingsItem(packageName, userId, targetPackageName, targetOverlayableName, - baseCodePath, OverlayInfo.STATE_UNKNOWN, isEnabled, isMutable, priority, - overlayCategory); - - int i; - for (i = mItems.size() - 1; i >= 0; i--) { - SettingsItem parentItem = mItems.get(i); - if (parentItem.mPriority <= priority) { - break; - } - } - int pos = i + 1; - if (pos == mItems.size()) { - mItems.add(item); - } else { - mItems.add(pos, item); - } + insert(new SettingsItem(packageName, userId, targetPackageName, targetOverlayableName, + baseCodePath, OverlayInfo.STATE_UNKNOWN, isEnabled, isMutable, priority, + overlayCategory)); } /** @@ -219,6 +204,21 @@ final class OverlayManagerSettings { return removed; } + /** + * Reassigns the priority of an overlay maintaining the values of the overlays other settings. + */ + void setPriority(@NonNull final String packageName, final int userId, final int priority) { + final int moveIdx = select(packageName, userId); + if (moveIdx < 0) { + throw new BadKeyException(packageName, userId); + } + + final SettingsItem itemToMove = mItems.get(moveIdx); + mItems.remove(moveIdx); + itemToMove.setPriority(priority); + insert(itemToMove); + } + /** * Returns true if the settings were modified, false if they remain the same. */ @@ -284,6 +284,21 @@ final class OverlayManagerSettings { return true; } + /** + * Inserts the item into the list of settings items. + */ + private void insert(@NonNull SettingsItem item) { + int i; + for (i = mItems.size() - 1; i >= 0; i--) { + SettingsItem parentItem = mItems.get(i); + if (parentItem.mPriority <= item.getPriority()) { + break; + } + } + + mItems.add(i + 1, item); + } + void dump(@NonNull final PrintWriter p, @NonNull DumpState dumpState) { // select items to display Stream items = mItems.stream(); @@ -583,6 +598,11 @@ final class OverlayManagerSettings { return mCache; } + private void setPriority(int priority) { + mPriority = priority; + invalidateCache(); + } + private void invalidateCache() { mCache = null; } diff --git a/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplRebootTests.java b/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplRebootTests.java index c4fea77cf1bae..f35eecf05f320 100644 --- a/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplRebootTests.java +++ b/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplRebootTests.java @@ -30,6 +30,7 @@ import org.junit.runner.RunWith; import java.util.Arrays; import java.util.List; +import java.util.function.BiConsumer; @RunWith(AndroidJUnit4.class) public class OverlayManagerServiceImplRebootTests extends OverlayManagerServiceImplTestsBase { @@ -132,57 +133,115 @@ public class OverlayManagerServiceImplRebootTests extends OverlayManagerServiceI } @Test - public void testMutabilityChange() { + public void testMutableEnabledToImmutableEnabled() { final OverlayManagerServiceImpl impl = getImpl(); installTargetPackage(TARGET, USER); - addOverlayPackage(OVERLAY, TARGET, USER, false, true, 0); - impl.updateOverlaysForUser(USER); - final OverlayInfo o1 = impl.getOverlayInfo(OVERLAY, USER); - assertNotNull(o1); - assertTrue(o1.isEnabled()); - assertFalse(o1.isMutable); + final BiConsumer setOverlay = (mutable, enabled) -> { + addOverlayPackage(OVERLAY, TARGET, USER, mutable, enabled, 0); + impl.updateOverlaysForUser(USER); + final OverlayInfo o1 = impl.getOverlayInfo(OVERLAY, USER); + assertNotNull(o1); + assertEquals(enabled, o1.isEnabled()); + assertEquals(mutable, o1.isMutable); + }; - addOverlayPackage(OVERLAY, TARGET, USER, true, false, 0); - impl.updateOverlaysForUser(USER); - final OverlayInfo o2 = impl.getOverlayInfo(OVERLAY, USER); - assertNotNull(o2); - assertFalse(o2.isEnabled()); - assertTrue(o2.isMutable); + // Immutable/enabled -> mutable/enabled + setOverlay.accept(false /* mutable */, true /* enabled */); + setOverlay.accept(true /* mutable */, true /* enabled */); - addOverlayPackage(OVERLAY, TARGET, USER, false, false, 0); - impl.updateOverlaysForUser(USER); - final OverlayInfo o3 = impl.getOverlayInfo(OVERLAY, USER); - assertNotNull(o3); - assertFalse(o3.isEnabled()); - assertFalse(o3.isMutable); + // Mutable/enabled -> immutable/enabled + setOverlay.accept(false /* mutable */, true /* enabled */); + + // Immutable/enabled -> mutable/disabled + setOverlay.accept(true /* mutable */, false /* enabled */); + + // Mutable/disabled -> immutable/enabled + setOverlay.accept(false /* mutable */, true /* enabled */); + + // Immutable/enabled -> immutable/disabled + setOverlay.accept(false /* mutable */, false /* enabled */); + + // Immutable/disabled -> mutable/enabled + setOverlay.accept(true /* mutable */, true /* enabled */); + + // Mutable/enabled -> immutable/disabled + setOverlay.accept(false /* mutable */, false /* enabled */); + + // Immutable/disabled -> mutable/disabled + setOverlay.accept(true /* mutable */, false /* enabled */); + + // Mutable/disabled -> immutable/disabled + setOverlay.accept(false /* mutable */, false /* enabled */); } @Test - public void testPriorityChange() { + public void testMutablePriorityChange() { final OverlayManagerServiceImpl impl = getImpl(); installTargetPackage(TARGET, USER); + addOverlayPackage(OVERLAY, TARGET, USER, true, true, 0); + addOverlayPackage(OVERLAY2, TARGET, USER, true, true, 1); + impl.updateOverlaysForUser(USER); + final OverlayInfo o1 = impl.getOverlayInfo(OVERLAY, USER); + assertNotNull(o1); + assertEquals(0, o1.priority); + + final OverlayInfo o2 = impl.getOverlayInfo(OVERLAY2, USER); + assertNotNull(o2); + assertEquals(1, o2.priority); + + // Overlay priority changing between reboots should not affect enable state of mutable + // overlays + impl.setEnabled(OVERLAY, true, USER); + + // Reorder the overlays + addOverlayPackage(OVERLAY, TARGET, USER, true, true, 1); + addOverlayPackage(OVERLAY2, TARGET, USER, true, true, 0); + impl.updateOverlaysForUser(USER); + + final OverlayInfo o3 = impl.getOverlayInfo(OVERLAY, USER); + assertNotNull(o3); + assertEquals(1, o3.priority); + + final OverlayInfo o4 = impl.getOverlayInfo(OVERLAY2, USER); + assertNotNull(o4); + assertEquals(0, o4.priority); + assertTrue(o1.isEnabled()); + } + + @Test + public void testImmutablePriorityChange() { + final OverlayManagerServiceImpl impl = getImpl(); + installTargetPackage(TARGET, USER); addOverlayPackage(OVERLAY, TARGET, USER, false, true, 0); addOverlayPackage(OVERLAY2, TARGET, USER, false, true, 1); impl.updateOverlaysForUser(USER); final OverlayInfo o1 = impl.getOverlayInfo(OVERLAY, USER); - final OverlayInfo o2 = impl.getOverlayInfo(OVERLAY2, USER); assertNotNull(o1); - assertNotNull(o2); assertEquals(0, o1.priority); + + final OverlayInfo o2 = impl.getOverlayInfo(OVERLAY2, USER); + assertNotNull(o2); assertEquals(1, o2.priority); + // Overlay priority changing between reboots should not affect enable state of mutable + // overlays + impl.setEnabled(OVERLAY, true, USER); + + // Reorder the overlays addOverlayPackage(OVERLAY, TARGET, USER, false, true, 1); addOverlayPackage(OVERLAY2, TARGET, USER, false, true, 0); impl.updateOverlaysForUser(USER); final OverlayInfo o3 = impl.getOverlayInfo(OVERLAY, USER); - final OverlayInfo o4 = impl.getOverlayInfo(OVERLAY2, USER); assertNotNull(o3); - assertNotNull(o4); assertEquals(1, o3.priority); + + final OverlayInfo o4 = impl.getOverlayInfo(OVERLAY2, USER); + assertNotNull(o4); assertEquals(0, o4.priority); + assertTrue(o1.isEnabled()); } }