From ecae429892a3d9c361407ba80beb90de22ab2ee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 20 Nov 2020 07:50:07 +0000 Subject: [PATCH 1/6] OverlayDeviceTests: add missing TEST_MAPPING Bug: N/A Test: atest --test-mapping Change-Id: Idacc0c67e06612c6c1fc021cb3ca0876274c36a7 --- core/tests/overlaytests/device/TEST_MAPPING | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 core/tests/overlaytests/device/TEST_MAPPING diff --git a/core/tests/overlaytests/device/TEST_MAPPING b/core/tests/overlaytests/device/TEST_MAPPING new file mode 100644 index 0000000000000..43ee00f47e849 --- /dev/null +++ b/core/tests/overlaytests/device/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name" : "OverlayDeviceTests" + } + ] +} From f4312ccb5e0f008b8df0f353fb9583638df418ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 14 Feb 2020 15:49:48 +0100 Subject: [PATCH 2/6] OMS: replace OMSImpl.OverlayChangeListener with return values Remove the listener callback triggered when the OMS settings have changed. Teach the OMSImpl methods to return the equivalent information instead: an Optional of the target package and Android user who's overlays need to be refreshed, or an exception in case of error. This is the first step towards an OMS transactional API, where multiple requests can be batched together and the outcome is either that all operations succeed, or some operation fails and (to the outside system) nothing happens. Replacing callbacks with explicit return values allow the caller more control of what data is exposed to the rest of Android. Bug: 119916381 Test: atest FrameworksServicesTests:com.android.server.om OverlayDeviceTests OverlayHostTests CtsAppSecurityHostTestCases:OverlayHostTest Change-Id: If58d5e9ddeec649e708224bb29efcc38ebe51bf7 --- .../server/om/OverlayManagerService.java | 168 ++++++++++++----- .../server/om/OverlayManagerServiceImpl.java | 172 ++++++++++-------- .../com/android/server/om/PackageAndUser.java | 57 ++++++ .../OverlayManagerServiceImplRebootTests.java | 10 +- .../om/OverlayManagerServiceImplTests.java | 76 ++++---- .../OverlayManagerServiceImplTestsBase.java | 41 ++--- 6 files changed, 335 insertions(+), 189 deletions(-) create mode 100644 services/core/java/com/android/server/om/PackageAndUser.java diff --git a/services/core/java/com/android/server/om/OverlayManagerService.java b/services/core/java/com/android/server/om/OverlayManagerService.java index 0f8c9c789a3f5..40ac204e440cf 100644 --- a/services/core/java/com/android/server/om/OverlayManagerService.java +++ b/services/core/java/com/android/server/om/OverlayManagerService.java @@ -29,6 +29,8 @@ import static android.os.Trace.TRACE_TAG_RRO; import static android.os.Trace.traceBegin; import static android.os.Trace.traceEnd; +import static com.android.server.om.OverlayManagerServiceImpl.OperationFailedException; + import android.annotation.NonNull; import android.annotation.Nullable; import android.app.ActivityManager; @@ -88,6 +90,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; /** * Service to manage asset overlays. @@ -238,6 +241,10 @@ public final class OverlayManagerService extends SystemService { private final AtomicBoolean mPersistSettingsScheduled = new AtomicBoolean(false); + private final Consumer mOnOverlaysChanged = (pair) -> { + onOverlaysChanged(pair.packageName, pair.userId); + }; + public OverlayManagerService(@NonNull final Context context) { super(context); try { @@ -249,8 +256,7 @@ public final class OverlayManagerService extends SystemService { IdmapManager im = new IdmapManager(IdmapDaemon.getInstance(), mPackageManager); mSettings = new OverlayManagerSettings(); mImpl = new OverlayManagerServiceImpl(mPackageManager, im, mSettings, - OverlayConfig.getSystemInstance(), getDefaultOverlayPackages(), - new OverlayChangeListener()); + OverlayConfig.getSystemInstance(), getDefaultOverlayPackages()); mActorEnforcer = new OverlayActorEnforcer(mPackageManager); final IntentFilter packageFilter = new IntentFilter(); @@ -396,10 +402,17 @@ public final class OverlayManagerService extends SystemService { false); if (pi != null && !pi.applicationInfo.isInstantApp()) { mPackageManager.cachePackageInfo(packageName, userId, pi); - if (pi.isOverlayPackage()) { - mImpl.onOverlayPackageAdded(packageName, userId); - } else { - mImpl.onTargetPackageAdded(packageName, userId); + + try { + if (pi.isOverlayPackage()) { + mImpl.onOverlayPackageAdded(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } else { + mImpl.onTargetPackageAdded(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } + } catch (OperationFailedException e) { + Slog.e(TAG, "onPackageAdded internal error", e); } } } @@ -419,10 +432,17 @@ public final class OverlayManagerService extends SystemService { false); if (pi != null && pi.applicationInfo.isInstantApp()) { mPackageManager.cachePackageInfo(packageName, userId, pi); - if (pi.isOverlayPackage()) { - mImpl.onOverlayPackageChanged(packageName, userId); - } else { - mImpl.onTargetPackageChanged(packageName, userId); + + try { + if (pi.isOverlayPackage()) { + mImpl.onOverlayPackageChanged(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } else { + mImpl.onTargetPackageChanged(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } + } catch (OperationFailedException e) { + Slog.e(TAG, "onPackageChanged internal error", e); } } } @@ -441,7 +461,12 @@ public final class OverlayManagerService extends SystemService { mPackageManager.forgetPackageInfo(packageName, userId); final OverlayInfo oi = mImpl.getOverlayInfo(packageName, userId); if (oi != null) { - mImpl.onOverlayPackageReplacing(packageName, userId); + try { + mImpl.onOverlayPackageReplacing(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } catch (OperationFailedException e) { + Slog.e(TAG, "onPackageReplacing internal error", e); + } } } } @@ -460,10 +485,16 @@ public final class OverlayManagerService extends SystemService { false); if (pi != null && !pi.applicationInfo.isInstantApp()) { mPackageManager.cachePackageInfo(packageName, userId, pi); - if (pi.isOverlayPackage()) { - mImpl.onOverlayPackageReplaced(packageName, userId); - } else { - mImpl.onTargetPackageReplaced(packageName, userId); + try { + if (pi.isOverlayPackage()) { + mImpl.onOverlayPackageReplaced(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } else { + mImpl.onTargetPackageReplaced(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } + } catch (OperationFailedException e) { + Slog.e(TAG, "onPackageReplaced internal error", e); } } } @@ -481,10 +512,17 @@ public final class OverlayManagerService extends SystemService { synchronized (mLock) { mPackageManager.forgetPackageInfo(packageName, userId); final OverlayInfo oi = mImpl.getOverlayInfo(packageName, userId); - if (oi != null) { - mImpl.onOverlayPackageRemoved(packageName, userId); - } else { - mImpl.onTargetPackageRemoved(packageName, userId); + + try { + if (oi != null) { + mImpl.onOverlayPackageRemoved(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } else { + mImpl.onTargetPackageRemoved(packageName, userId) + .ifPresent(mOnOverlaysChanged); + } + } catch (OperationFailedException e) { + Slog.e(TAG, "onPackageRemoved internal error", e); } } } @@ -602,7 +640,13 @@ public final class OverlayManagerService extends SystemService { final long ident = Binder.clearCallingIdentity(); try { synchronized (mLock) { - return mImpl.setEnabled(packageName, enable, realUserId); + try { + mImpl.setEnabled(packageName, enable, realUserId) + .ifPresent(mOnOverlaysChanged); + return true; + } catch (OperationFailedException e) { + return false; + } } } finally { Binder.restoreCallingIdentity(ident); @@ -627,8 +671,14 @@ public final class OverlayManagerService extends SystemService { final long ident = Binder.clearCallingIdentity(); try { synchronized (mLock) { - return mImpl.setEnabledExclusive(packageName, false /* withinCategory */, - realUserId); + try { + mImpl.setEnabledExclusive(packageName, + false /* withinCategory */, realUserId) + .ifPresent(mOnOverlaysChanged); + return true; + } catch (OperationFailedException e) { + return false; + } } } finally { Binder.restoreCallingIdentity(ident); @@ -654,8 +704,14 @@ public final class OverlayManagerService extends SystemService { final long ident = Binder.clearCallingIdentity(); try { synchronized (mLock) { - return mImpl.setEnabledExclusive(packageName, true /* withinCategory */, - realUserId); + try { + mImpl.setEnabledExclusive(packageName, + true /* withinCategory */, realUserId) + .ifPresent(mOnOverlaysChanged); + return true; + } catch (OperationFailedException e) { + return false; + } } } finally { Binder.restoreCallingIdentity(ident); @@ -681,7 +737,13 @@ public final class OverlayManagerService extends SystemService { final long ident = Binder.clearCallingIdentity(); try { synchronized (mLock) { - return mImpl.setPriority(packageName, parentPackageName, realUserId); + try { + mImpl.setPriority(packageName, parentPackageName, realUserId) + .ifPresent(mOnOverlaysChanged); + return true; + } catch (OperationFailedException e) { + return false; + } } } finally { Binder.restoreCallingIdentity(ident); @@ -705,7 +767,13 @@ public final class OverlayManagerService extends SystemService { final long ident = Binder.clearCallingIdentity(); try { synchronized (mLock) { - return mImpl.setHighestPriority(packageName, realUserId); + try { + mImpl.setHighestPriority(packageName, realUserId) + .ifPresent(mOnOverlaysChanged); + return true; + } catch (OperationFailedException e) { + return false; + } } } finally { Binder.restoreCallingIdentity(ident); @@ -729,7 +797,13 @@ public final class OverlayManagerService extends SystemService { final long ident = Binder.clearCallingIdentity(); try { synchronized (mLock) { - return mImpl.setLowestPriority(packageName, realUserId); + try { + mImpl.setLowestPriority(packageName, realUserId) + .ifPresent(mOnOverlaysChanged); + return true; + } catch (OperationFailedException e) { + return false; + } } } finally { Binder.restoreCallingIdentity(ident); @@ -898,31 +972,27 @@ public final class OverlayManagerService extends SystemService { } }; - private final class OverlayChangeListener - implements OverlayManagerServiceImpl.OverlayChangeListener { - @Override - public void onOverlaysChanged(@NonNull final String targetPackageName, final int userId) { - schedulePersistSettings(); - FgThread.getHandler().post(() -> { - updateAssets(userId, targetPackageName); + private void onOverlaysChanged(@NonNull final String targetPackageName, final int userId) { + schedulePersistSettings(); + FgThread.getHandler().post(() -> { + updateAssets(userId, targetPackageName); - final Intent intent = new Intent(ACTION_OVERLAY_CHANGED, - Uri.fromParts("package", targetPackageName, null)); - intent.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); + final Intent intent = new Intent(ACTION_OVERLAY_CHANGED, + Uri.fromParts("package", targetPackageName, null)); + intent.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); - if (DEBUG) { - Slog.d(TAG, "send broadcast " + intent); - } + if (DEBUG) { + Slog.d(TAG, "send broadcast " + intent); + } - try { - ActivityManager.getService().broadcastIntentWithFeature(null, null, intent, - null, null, 0, null, null, null, android.app.AppOpsManager.OP_NONE, - null, false, false, userId); - } catch (RemoteException e) { - // Intentionally left empty. - } - }); - } + try { + ActivityManager.getService().broadcastIntent(null, intent, null, null, 0, + null, null, null, android.app.AppOpsManager.OP_NONE, null, false, false, + userId); + } catch (RemoteException e) { + // Intentionally left empty. + } + }); } /** diff --git a/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java b/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java index 05a4a38feef12..e60411bb78c5a 100644 --- a/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java +++ b/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java @@ -45,6 +45,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; /** @@ -71,7 +72,6 @@ final class OverlayManagerServiceImpl { private final OverlayManagerSettings mSettings; private final OverlayConfig mOverlayConfig; private final String[] mDefaultOverlays; - private final OverlayChangeListener mListener; /** * Helper method to merge the overlay manager's (as read from overlays.xml) @@ -114,14 +114,12 @@ final class OverlayManagerServiceImpl { @NonNull final IdmapManager idmapManager, @NonNull final OverlayManagerSettings settings, @NonNull final OverlayConfig overlayConfig, - @NonNull final String[] defaultOverlays, - @NonNull final OverlayChangeListener listener) { + @NonNull final String[] defaultOverlays) { mPackageManager = packageManager; mIdmapManager = idmapManager; mSettings = settings; mOverlayConfig = overlayConfig; mDefaultOverlays = defaultOverlays; - mListener = listener; } /** @@ -259,52 +257,58 @@ final class OverlayManagerServiceImpl { mSettings.removeUser(userId); } - void onTargetPackageAdded(@NonNull final String packageName, final int userId) { + Optional onTargetPackageAdded(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onTargetPackageAdded packageName=" + packageName + " userId=" + userId); } - updateAndRefreshOverlaysForTarget(packageName, userId, 0); + return updateAndRefreshOverlaysForTarget(packageName, userId, 0); } - void onTargetPackageChanged(@NonNull final String packageName, final int userId) { + Optional onTargetPackageChanged(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onTargetPackageChanged packageName=" + packageName + " userId=" + userId); } - updateAndRefreshOverlaysForTarget(packageName, userId, 0); + return updateAndRefreshOverlaysForTarget(packageName, userId, 0); } - void onTargetPackageReplacing(@NonNull final String packageName, final int userId) { + Optional onTargetPackageReplacing(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onTargetPackageReplacing packageName=" + packageName + " userId=" + userId); } - updateAndRefreshOverlaysForTarget(packageName, userId, 0); + return updateAndRefreshOverlaysForTarget(packageName, userId, 0); } - void onTargetPackageReplaced(@NonNull final String packageName, final int userId) { + Optional onTargetPackageReplaced(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onTargetPackageReplaced packageName=" + packageName + " userId=" + userId); } - updateAndRefreshOverlaysForTarget(packageName, userId, 0); + return updateAndRefreshOverlaysForTarget(packageName, userId, 0); } - void onTargetPackageRemoved(@NonNull final String packageName, final int userId) { + Optional onTargetPackageRemoved(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onTargetPackageRemoved packageName=" + packageName + " userId=" + userId); } - updateAndRefreshOverlaysForTarget(packageName, userId, 0); + return updateAndRefreshOverlaysForTarget(packageName, userId, 0); } /** * Update the state of any overlays for this target. */ - private void updateAndRefreshOverlaysForTarget(@NonNull final String targetPackageName, - final int userId, final int flags) { + private Optional updateAndRefreshOverlaysForTarget( + @NonNull final String targetPackageName, final int userId, final int flags) + throws OperationFailedException { final List targetOverlays = mSettings.getOverlaysForTarget(targetPackageName, userId); @@ -364,11 +368,13 @@ final class OverlayManagerServiceImpl { } if (modified) { - mListener.onOverlaysChanged(targetPackageName, userId); + return Optional.of(new PackageAndUser(targetPackageName, userId)); } + return Optional.empty(); } - void onOverlayPackageAdded(@NonNull final String packageName, final int userId) { + Optional onOverlayPackageAdded(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onOverlayPackageAdded packageName=" + packageName + " userId=" + userId); } @@ -376,8 +382,7 @@ final class OverlayManagerServiceImpl { final PackageInfo overlayPackage = mPackageManager.getPackageInfo(packageName, userId); if (overlayPackage == null) { Slog.w(TAG, "overlay package " + packageName + " was added, but couldn't be found"); - onOverlayPackageRemoved(packageName, userId); - return; + return onOverlayPackageRemoved(packageName, userId); } mSettings.init(packageName, userId, overlayPackage.overlayTarget, @@ -389,15 +394,17 @@ final class OverlayManagerServiceImpl { overlayPackage.overlayCategory); try { if (updateState(overlayPackage.overlayTarget, packageName, userId, 0)) { - mListener.onOverlaysChanged(overlayPackage.overlayTarget, userId); + return Optional.of(new PackageAndUser(overlayPackage.overlayTarget, userId)); } + return Optional.empty(); } catch (OverlayManagerSettings.BadKeyException e) { - Slog.e(TAG, "failed to update settings", e); mSettings.remove(packageName, userId); + throw new OperationFailedException("failed to update settings", e); } } - void onOverlayPackageChanged(@NonNull final String packageName, final int userId) { + Optional onOverlayPackageChanged(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onOverlayPackageChanged packageName=" + packageName + " userId=" + userId); } @@ -405,14 +412,16 @@ final class OverlayManagerServiceImpl { try { final OverlayInfo oi = mSettings.getOverlayInfo(packageName, userId); if (updateState(oi.targetPackageName, packageName, userId, 0)) { - mListener.onOverlaysChanged(oi.targetPackageName, userId); + return Optional.of(new PackageAndUser(oi.targetPackageName, userId)); } + return Optional.empty(); } catch (OverlayManagerSettings.BadKeyException e) { - Slog.e(TAG, "failed to update settings", e); + throw new OperationFailedException("failed to update settings", e); } } - void onOverlayPackageReplacing(@NonNull final String packageName, final int userId) { + Optional onOverlayPackageReplacing(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onOverlayPackageReplacing packageName=" + packageName + " userId=" + userId); @@ -423,14 +432,16 @@ final class OverlayManagerServiceImpl { if (updateState(oi.targetPackageName, packageName, userId, FLAG_OVERLAY_IS_BEING_REPLACED)) { removeIdmapIfPossible(oi); - mListener.onOverlaysChanged(oi.targetPackageName, userId); + return Optional.of(new PackageAndUser(oi.targetPackageName, userId)); } + return Optional.empty(); } catch (OverlayManagerSettings.BadKeyException e) { - Slog.e(TAG, "failed to update settings", e); + throw new OperationFailedException("failed to update settings", e); } } - void onOverlayPackageReplaced(@NonNull final String packageName, final int userId) { + Optional onOverlayPackageReplaced(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "onOverlayPackageReplaced packageName=" + packageName + " userId=" + userId); @@ -439,16 +450,12 @@ final class OverlayManagerServiceImpl { final PackageInfo pkg = mPackageManager.getPackageInfo(packageName, userId); if (pkg == null) { Slog.w(TAG, "overlay package " + packageName + " was replaced, but couldn't be found"); - onOverlayPackageRemoved(packageName, userId); - return; + return onOverlayPackageRemoved(packageName, userId); } try { final OverlayInfo oldOi = mSettings.getOverlayInfo(packageName, userId); if (mustReinitializeOverlay(pkg, oldOi)) { - if (oldOi != null && !oldOi.targetPackageName.equals(pkg.overlayTarget)) { - mListener.onOverlaysChanged(pkg.overlayTarget, userId); - } mSettings.init(packageName, userId, pkg.overlayTarget, pkg.targetOverlayableName, pkg.applicationInfo.getBaseCodePath(), isPackageConfiguredMutable(pkg.packageName), @@ -457,22 +464,25 @@ final class OverlayManagerServiceImpl { } if (updateState(pkg.overlayTarget, packageName, userId, 0)) { - mListener.onOverlaysChanged(pkg.overlayTarget, userId); + return Optional.of(new PackageAndUser(pkg.overlayTarget, userId)); } + return Optional.empty(); } catch (OverlayManagerSettings.BadKeyException e) { - Slog.e(TAG, "failed to update settings", e); + throw new OperationFailedException("failed to update settings", e); } } - void onOverlayPackageRemoved(@NonNull final String packageName, final int userId) { + Optional onOverlayPackageRemoved(@NonNull final String packageName, + final int userId) throws OperationFailedException { try { final OverlayInfo overlayInfo = mSettings.getOverlayInfo(packageName, userId); if (mSettings.remove(packageName, userId)) { removeIdmapIfPossible(overlayInfo); - mListener.onOverlaysChanged(overlayInfo.targetPackageName, userId); + return Optional.of(new PackageAndUser(overlayInfo.targetPackageName, userId)); } + return Optional.empty(); } catch (OverlayManagerSettings.BadKeyException e) { - Slog.e(TAG, "failed to remove overlay", e); + throw new OperationFailedException("failed to remove overlay", e); } } @@ -493,8 +503,8 @@ final class OverlayManagerServiceImpl { return mSettings.getOverlaysForUser(userId); } - boolean setEnabled(@NonNull final String packageName, final boolean enable, - final int userId) { + Optional setEnabled(@NonNull final String packageName, final boolean enable, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, String.format("setEnabled packageName=%s enable=%s userId=%d", packageName, enable, userId)); @@ -502,30 +512,33 @@ final class OverlayManagerServiceImpl { final PackageInfo overlayPackage = mPackageManager.getPackageInfo(packageName, userId); if (overlayPackage == null) { - return false; + throw new OperationFailedException( + String.format("failed to find overlay package %s for user %d", + packageName, userId)); } try { final OverlayInfo oi = mSettings.getOverlayInfo(packageName, userId); if (!oi.isMutable) { // Ignore immutable overlays. - return false; + throw new OperationFailedException( + "cannot enable immutable overlay packages in runtime"); } boolean modified = mSettings.setEnabled(packageName, userId, enable); modified |= updateState(oi.targetPackageName, oi.packageName, userId, 0); if (modified) { - mListener.onOverlaysChanged(oi.targetPackageName, userId); + return Optional.of(new PackageAndUser(oi.targetPackageName, userId)); } - return true; + return Optional.empty(); } catch (OverlayManagerSettings.BadKeyException e) { - return false; + throw new OperationFailedException("failed to update settings", e); } } - boolean setEnabledExclusive(@NonNull final String packageName, boolean withinCategory, - final int userId) { + Optional setEnabledExclusive(@NonNull final String packageName, + boolean withinCategory, final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, String.format("setEnabledExclusive packageName=%s" + " withinCategory=%s userId=%d", packageName, withinCategory, userId)); @@ -533,7 +546,8 @@ final class OverlayManagerServiceImpl { final PackageInfo overlayPackage = mPackageManager.getPackageInfo(packageName, userId); if (overlayPackage == null) { - return false; + throw new OperationFailedException(String.format( + "failed to find overlay package %s for user %d", packageName, userId)); } try { @@ -576,11 +590,11 @@ final class OverlayManagerServiceImpl { modified |= updateState(targetPackageName, packageName, userId, 0); if (modified) { - mListener.onOverlaysChanged(targetPackageName, userId); + return Optional.of(new PackageAndUser(targetPackageName, userId)); } - return true; + return Optional.empty(); } catch (OverlayManagerSettings.BadKeyException e) { - return false; + throw new OperationFailedException("failed to update settings", e); } } @@ -596,66 +610,75 @@ final class OverlayManagerServiceImpl { return mOverlayConfig.isEnabled(packageName); } - boolean setPriority(@NonNull final String packageName, - @NonNull final String newParentPackageName, final int userId) { + Optional setPriority(@NonNull final String packageName, + @NonNull final String newParentPackageName, final int userId) + throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "setPriority packageName=" + packageName + " newParentPackageName=" + newParentPackageName + " userId=" + userId); } if (!isPackageConfiguredMutable(packageName)) { - return false; + throw new OperationFailedException(String.format( + "overlay package %s user %d is not updatable", packageName, userId)); } final PackageInfo overlayPackage = mPackageManager.getPackageInfo(packageName, userId); if (overlayPackage == null) { - return false; + throw new OperationFailedException(String.format( + "failed to find overlay package %s for user %d", packageName, userId)); } if (mSettings.setPriority(packageName, newParentPackageName, userId)) { - mListener.onOverlaysChanged(overlayPackage.overlayTarget, userId); + return Optional.of(new PackageAndUser(overlayPackage.overlayTarget, userId)); } - return true; + return Optional.empty(); } - boolean setHighestPriority(@NonNull final String packageName, final int userId) { + Optional setHighestPriority(@NonNull final String packageName, + final int userId) throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "setHighestPriority packageName=" + packageName + " userId=" + userId); } if (!isPackageConfiguredMutable(packageName)) { - return false; + throw new OperationFailedException(String.format( + "overlay package %s user %d is not updatable", packageName, userId)); } final PackageInfo overlayPackage = mPackageManager.getPackageInfo(packageName, userId); if (overlayPackage == null) { - return false; + throw new OperationFailedException(String.format( + "failed to find overlay package %s for user %d", packageName, userId)); } if (mSettings.setHighestPriority(packageName, userId)) { - mListener.onOverlaysChanged(overlayPackage.overlayTarget, userId); + return Optional.of(new PackageAndUser(overlayPackage.overlayTarget, userId)); } - return true; + return Optional.empty(); } - boolean setLowestPriority(@NonNull final String packageName, final int userId) { + Optional setLowestPriority(@NonNull final String packageName, final int userId) + throws OperationFailedException { if (DEBUG) { Slog.d(TAG, "setLowestPriority packageName=" + packageName + " userId=" + userId); } if (!isPackageConfiguredMutable(packageName)) { - return false; + throw new OperationFailedException(String.format( + "overlay package %s user %d is not updatable", packageName, userId)); } final PackageInfo overlayPackage = mPackageManager.getPackageInfo(packageName, userId); if (overlayPackage == null) { - return false; + throw new OperationFailedException(String.format( + "failed to find overlay package %s for user %d", packageName, userId)); } if (mSettings.setLowestPriority(packageName, userId)) { - mListener.onOverlaysChanged(overlayPackage.overlayTarget, userId); + return Optional.of(new PackageAndUser(overlayPackage.overlayTarget, userId)); } - return true; + return Optional.empty(); } void dump(@NonNull final PrintWriter pw, @NonNull DumpState dumpState) { @@ -797,12 +820,13 @@ final class OverlayManagerServiceImpl { mIdmapManager.removeIdmap(oi, oi.userId); } - interface OverlayChangeListener { + static final class OperationFailedException extends Exception { + OperationFailedException(@NonNull final String message) { + super(message); + } - /** - * An event triggered by changes made to overlay state or settings as well as changes that - * add or remove target packages of overlays. - **/ - void onOverlaysChanged(@NonNull String targetPackage, int userId); + OperationFailedException(@NonNull final String message, @NonNull Throwable cause) { + super(message, cause); + } } } diff --git a/services/core/java/com/android/server/om/PackageAndUser.java b/services/core/java/com/android/server/om/PackageAndUser.java new file mode 100644 index 0000000000000..5c38ba7ce97bc --- /dev/null +++ b/services/core/java/com/android/server/om/PackageAndUser.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.om; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.annotation.UserIdInt; + +final class PackageAndUser { + public final @NonNull String packageName; + public final @UserIdInt int userId; + + PackageAndUser(@NonNull String packageName, @UserIdInt int userId) { + this.packageName = packageName; + this.userId = userId; + } + + @Override + public boolean equals(@Nullable Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof PackageAndUser)) { + return false; + } + PackageAndUser other = (PackageAndUser) obj; + return packageName.equals(other.packageName) && userId == other.userId; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + packageName.hashCode(); + result = prime * result + userId; + return result; + } + + @Override + public String toString() { + return String.format("PackageAndUser{packageName=%s, userId=%d}", packageName, userId); + } +} 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 391611b72dab8..5468fba59c109 100644 --- a/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplRebootTests.java +++ b/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplRebootTests.java @@ -78,7 +78,7 @@ public class OverlayManagerServiceImplRebootTests extends OverlayManagerServiceI } @Test - public void testImmutableEnabledChange() { + public void testImmutableEnabledChange() throws Exception { final OverlayManagerServiceImpl impl = getImpl(); installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET), USER); @@ -106,7 +106,7 @@ public class OverlayManagerServiceImplRebootTests extends OverlayManagerServiceI } @Test - public void testMutableEnabledChangeHasNoEffect() { + public void testMutableEnabledChangeHasNoEffect() throws Exception { final OverlayManagerServiceImpl impl = getImpl(); installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET), USER); @@ -134,7 +134,7 @@ public class OverlayManagerServiceImplRebootTests extends OverlayManagerServiceI } @Test - public void testMutableEnabledToImmutableEnabled() { + public void testMutableEnabledToImmutableEnabled() throws Exception { final OverlayManagerServiceImpl impl = getImpl(); installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET), USER); @@ -178,7 +178,7 @@ public class OverlayManagerServiceImplRebootTests extends OverlayManagerServiceI } @Test - public void testMutablePriorityChange() { + public void testMutablePriorityChange() throws Exception { final OverlayManagerServiceImpl impl = getImpl(); installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET), USER); @@ -218,7 +218,7 @@ public class OverlayManagerServiceImplRebootTests extends OverlayManagerServiceI } @Test - public void testImmutablePriorityChange() { + public void testImmutablePriorityChange() throws Exception { final OverlayManagerServiceImpl impl = getImpl(); installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET), USER); diff --git a/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTests.java b/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTests.java index 4f882ce13dd4f..33dbcc0855be8 100644 --- a/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTests.java +++ b/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTests.java @@ -22,11 +22,14 @@ import static android.content.om.OverlayInfo.STATE_MISSING_TARGET; import static android.os.OverlayablePolicy.CONFIG_SIGNATURE; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.testng.Assert.assertThrows; import android.content.om.OverlayInfo; +import android.util.Pair; import androidx.test.runner.AndroidJUnit4; @@ -35,6 +38,7 @@ import org.junit.runner.RunWith; import java.util.List; import java.util.Map; +import java.util.Optional; @RunWith(AndroidJUnit4.class) public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTestsBase { @@ -55,7 +59,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes private static final String CERT_CONFIG_NOK = "config_certificate_nok"; @Test - public void testGetOverlayInfo() { + public void testGetOverlayInfo() throws Exception { installNewPackage(overlay(OVERLAY, TARGET), USER); final OverlayManagerServiceImpl impl = getImpl(); @@ -67,7 +71,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testGetOverlayInfosForTarget() { + public void testGetOverlayInfosForTarget() throws Exception { installNewPackage(overlay(OVERLAY, TARGET), USER); installNewPackage(overlay(OVERLAY2, TARGET), USER); installNewPackage(overlay(OVERLAY3, TARGET), USER2); @@ -92,7 +96,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testGetOverlayInfosForUser() { + public void testGetOverlayInfosForUser() throws Exception { installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET), USER); installNewPackage(overlay(OVERLAY2, TARGET), USER); @@ -119,7 +123,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testPriority() { + public void testPriority() throws Exception { installNewPackage(overlay(OVERLAY, TARGET), USER); installNewPackage(overlay(OVERLAY2, TARGET), USER); installNewPackage(overlay(OVERLAY3, TARGET), USER); @@ -131,18 +135,21 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes assertOverlayInfoForTarget(TARGET, USER, o1, o2, o3); - assertTrue(impl.setLowestPriority(OVERLAY3, USER)); + assertEquals(impl.setLowestPriority(OVERLAY3, USER), + Optional.of(new PackageAndUser(TARGET, USER))); assertOverlayInfoForTarget(TARGET, USER, o3, o1, o2); - assertTrue(impl.setHighestPriority(OVERLAY3, USER)); + assertEquals(impl.setHighestPriority(OVERLAY3, USER), + Optional.of(new PackageAndUser(TARGET, USER))); assertOverlayInfoForTarget(TARGET, USER, o1, o2, o3); - assertTrue(impl.setPriority(OVERLAY, OVERLAY2, USER)); + assertEquals(impl.setPriority(OVERLAY, OVERLAY2, USER), + Optional.of(new PackageAndUser(TARGET, USER))); assertOverlayInfoForTarget(TARGET, USER, o2, o1, o3); } @Test - public void testOverlayInfoStateTransitions() { + public void testOverlayInfoStateTransitions() throws Exception { final OverlayManagerServiceImpl impl = getImpl(); assertNull(impl.getOverlayInfo(OVERLAY, USER)); @@ -153,7 +160,8 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes installNewPackage(target, USER); assertState(STATE_DISABLED, OVERLAY, USER); - impl.setEnabled(OVERLAY, true, USER); + assertEquals(impl.setEnabled(OVERLAY, true, USER), + Optional.of(new PackageAndUser(TARGET, USER))); assertState(STATE_ENABLED, OVERLAY, USER); // target upgrades do not change the state of the overlay @@ -168,50 +176,40 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testOnOverlayPackageUpgraded() { - final FakeListener listener = getListener(); + public void testOnOverlayPackageUpgraded() throws Exception { final FakeDeviceState.PackageBuilder target = target(TARGET); final FakeDeviceState.PackageBuilder overlay = overlay(OVERLAY, TARGET); installNewPackage(target, USER); installNewPackage(overlay, USER); - listener.count = 0; upgradePackage(overlay, USER); - assertEquals(2, listener.count); // upgrade to a version where the overlay has changed its target - // expect once for the old target package, once for the new target package - listener.count = 0; final FakeDeviceState.PackageBuilder overlay2 = overlay(OVERLAY, "some.other.target"); - upgradePackage(overlay2, USER); - assertEquals(3, listener.count); - - listener.count = 0; - upgradePackage(overlay2, USER); - assertEquals(2, listener.count); + final Pair, Optional> pair = + upgradePackage(overlay2, USER); + assertEquals(pair.first, Optional.of(new PackageAndUser(TARGET, USER))); + assertEquals(pair.second, Optional.of(new PackageAndUser("some.other.target", USER))); } @Test - public void testListener() { + public void testSetEnabledAtVariousConditions() throws Exception { final OverlayManagerServiceImpl impl = getImpl(); - final FakeListener listener = getListener(); - installNewPackage(overlay(OVERLAY, TARGET), USER); - assertEquals(1, listener.count); - listener.count = 0; + assertThrows(OverlayManagerServiceImpl.OperationFailedException.class, + () -> impl.setEnabled(OVERLAY, true, USER)); + // request succeeded, and there was a change that needs to be + // propagated to the rest of the system installNewPackage(target(TARGET), USER); - assertEquals(1, listener.count); - listener.count = 0; + installNewPackage(overlay(OVERLAY, TARGET), USER); + assertEquals(impl.setEnabled(OVERLAY, true, USER), + Optional.of(new PackageAndUser(TARGET, USER))); - impl.setEnabled(OVERLAY, true, USER); - assertEquals(1, listener.count); - listener.count = 0; - - impl.setEnabled(OVERLAY, true, USER); - assertEquals(0, listener.count); + // request succeeded, but nothing changed + assertFalse(impl.setEnabled(OVERLAY, true, USER).isPresent()); } @Test - public void testConfigSignaturePolicyOk() { + public void testConfigSignaturePolicyOk() throws Exception { setConfigSignaturePackageName(CONFIG_SIGNATURE_REFERENCE_PKG); reinitializeImpl(); @@ -229,7 +227,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testConfigSignaturePolicyCertNok() { + public void testConfigSignaturePolicyCertNok() throws Exception { setConfigSignaturePackageName(CONFIG_SIGNATURE_REFERENCE_PKG); reinitializeImpl(); @@ -247,7 +245,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testConfigSignaturePolicyNoConfig() { + public void testConfigSignaturePolicyNoConfig() throws Exception { addPackage(target(CONFIG_SIGNATURE_REFERENCE_PKG).setCertificate(CERT_CONFIG_OK), USER); installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET).setCertificate(CERT_CONFIG_NOK), USER); @@ -262,7 +260,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testConfigSignaturePolicyNoRefPkg() { + public void testConfigSignaturePolicyNoRefPkg() throws Exception { installNewPackage(target(TARGET), USER); installNewPackage(overlay(OVERLAY, TARGET).setCertificate(CERT_CONFIG_NOK), USER); @@ -276,7 +274,7 @@ public class OverlayManagerServiceImplTests extends OverlayManagerServiceImplTes } @Test - public void testConfigSignaturePolicyRefPkgNotSystem() { + public void testConfigSignaturePolicyRefPkgNotSystem() throws Exception { setConfigSignaturePackageName(CONFIG_SIGNATURE_REFERENCE_PKG); reinitializeImpl(); diff --git a/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTestsBase.java b/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTestsBase.java index 006dda0f80e35..2c477c897b301 100644 --- a/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTestsBase.java +++ b/services/tests/servicestests/src/com/android/server/om/OverlayManagerServiceImplTestsBase.java @@ -16,6 +16,8 @@ package com.android.server.om; +import static com.android.server.om.OverlayManagerServiceImpl.OperationFailedException; + import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -30,6 +32,7 @@ import android.content.pm.PackageInfo; import android.text.TextUtils; import android.util.ArrayMap; import android.util.ArraySet; +import android.util.Pair; import androidx.annotation.Nullable; @@ -43,13 +46,13 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; /** Base class for creating {@link OverlayManagerServiceImplTests} tests. */ class OverlayManagerServiceImplTestsBase { private OverlayManagerServiceImpl mImpl; private FakeDeviceState mState; - private FakeListener mListener; private FakePackageManagerHelper mPackageManager; private FakeIdmapDaemon mIdmapDaemon; private OverlayConfig mOverlayConfig; @@ -58,7 +61,6 @@ class OverlayManagerServiceImplTestsBase { @Before public void setUp() { mState = new FakeDeviceState(); - mListener = new FakeListener(); mPackageManager = new FakePackageManagerHelper(mState); mIdmapDaemon = new FakeIdmapDaemon(mState); mOverlayConfig = mock(OverlayConfig.class); @@ -73,18 +75,13 @@ class OverlayManagerServiceImplTestsBase { new IdmapManager(mIdmapDaemon, mPackageManager), new OverlayManagerSettings(), mOverlayConfig, - new String[0], - mListener); + new String[0]); } OverlayManagerServiceImpl getImpl() { return mImpl; } - FakeListener getListener() { - return mListener; - } - FakeIdmapDaemon getIdmapd() { return mIdmapDaemon; } @@ -155,7 +152,8 @@ class OverlayManagerServiceImplTestsBase { * * @throws IllegalStateException if the package is currently installed */ - void installNewPackage(FakeDeviceState.PackageBuilder pkg, int userId) { + void installNewPackage(FakeDeviceState.PackageBuilder pkg, int userId) + throws OperationFailedException { if (mState.select(pkg.packageName, userId) != null) { throw new IllegalStateException("package " + pkg.packageName + " already installed"); } @@ -176,23 +174,30 @@ class OverlayManagerServiceImplTestsBase { * {@link android.content.Intent#ACTION_PACKAGE_ADDED} broadcast with the * {@link android.content.Intent#EXTRA_REPLACING} extra. * + * @return the two Optional objects from starting and finishing the upgrade + * * @throws IllegalStateException if the package is not currently installed */ - void upgradePackage(FakeDeviceState.PackageBuilder pkg, int userId) { + Pair, Optional> upgradePackage( + FakeDeviceState.PackageBuilder pkg, int userId) throws OperationFailedException { final FakeDeviceState.Package replacedPackage = mState.select(pkg.packageName, userId); if (replacedPackage == null) { throw new IllegalStateException("package " + pkg.packageName + " not installed"); } + Optional opt1 = Optional.empty(); if (replacedPackage.targetPackageName != null) { - mImpl.onOverlayPackageReplacing(pkg.packageName, userId); + opt1 = mImpl.onOverlayPackageReplacing(pkg.packageName, userId); } mState.add(pkg, userId); + Optional opt2; if (pkg.targetPackage == null) { - mImpl.onTargetPackageReplaced(pkg.packageName, userId); + opt2 = mImpl.onTargetPackageReplaced(pkg.packageName, userId); } else { - mImpl.onOverlayPackageReplaced(pkg.packageName, userId); + opt2 = mImpl.onOverlayPackageReplaced(pkg.packageName, userId); } + + return Pair.create(opt1, opt2); } /** @@ -203,7 +208,7 @@ class OverlayManagerServiceImplTestsBase { * * @throws IllegalStateException if the package is not currently installed */ - void uninstallPackage(String packageName, int userId) { + void uninstallPackage(String packageName, int userId) throws OperationFailedException { final FakeDeviceState.Package pkg = mState.select(packageName, userId); if (pkg == null) { throw new IllegalStateException("package " + packageName+ " not installed"); @@ -485,12 +490,4 @@ class OverlayManagerServiceImplTestsBase { } } } - - static class FakeListener implements OverlayManagerServiceImpl.OverlayChangeListener { - public int count; - - public void onOverlaysChanged(@NonNull String targetPackage, int userId) { - count++; - } - } } From 038b24eb67d0416869bd0130af9bdce8d0ba3f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 14 Feb 2020 15:49:51 +0100 Subject: [PATCH 3/6] OMS: block until settings are persisted Do not schedule a job to write the OMS settings file on a background thread any more. Instead, wait for the file to be written before returning. This is a step towards a transactional OMS API, where it is vital that the file is always up-to-date in case a transaction fails and the in-memory settings need to be rolled back. Also: move the PackageReceiver off the main thread to prevent blocking the main thread. (AIDL calls are already processed on one of the binder pool threads; UserReceiver is kept on the main thread.) Bug: 119916381 Test: atest FrameworksServicesTests:com.android.server.om OverlayDeviceTests OverlayHostTests CtsAppSecurityHostTestCases:OverlayHostTest Change-Id: I2452f789af1353fdde729bdc841b3417a2048e99 --- .../server/om/OverlayManagerService.java | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/services/core/java/com/android/server/om/OverlayManagerService.java b/services/core/java/com/android/server/om/OverlayManagerService.java index 40ac204e440cf..437e33d66c919 100644 --- a/services/core/java/com/android/server/om/OverlayManagerService.java +++ b/services/core/java/com/android/server/om/OverlayManagerService.java @@ -50,6 +50,7 @@ import android.content.res.ApkAssets; import android.net.Uri; import android.os.Binder; import android.os.Environment; +import android.os.HandlerThread; import android.os.IBinder; import android.os.RemoteException; import android.os.ResultReceiver; @@ -66,7 +67,6 @@ import android.util.SparseArray; import com.android.internal.content.om.OverlayConfig; import com.android.server.FgThread; -import com.android.server.IoThread; import com.android.server.LocalServices; import com.android.server.SystemConfig; import com.android.server.SystemService; @@ -89,7 +89,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; /** @@ -239,8 +238,6 @@ public final class OverlayManagerService extends SystemService { private final OverlayActorEnforcer mActorEnforcer; - private final AtomicBoolean mPersistSettingsScheduled = new AtomicBoolean(false); - private final Consumer mOnOverlaysChanged = (pair) -> { onOverlaysChanged(pair.packageName, pair.userId); }; @@ -259,13 +256,16 @@ public final class OverlayManagerService extends SystemService { OverlayConfig.getSystemInstance(), getDefaultOverlayPackages()); mActorEnforcer = new OverlayActorEnforcer(mPackageManager); + HandlerThread packageReceiverThread = new HandlerThread(TAG); + packageReceiverThread.start(); + final IntentFilter packageFilter = new IntentFilter(); packageFilter.addAction(ACTION_PACKAGE_ADDED); packageFilter.addAction(ACTION_PACKAGE_CHANGED); packageFilter.addAction(ACTION_PACKAGE_REMOVED); packageFilter.addDataScheme("package"); getContext().registerReceiverAsUser(new PackageReceiver(), UserHandle.ALL, - packageFilter, null, null); + packageFilter, null, packageReceiverThread.getThreadHandler()); final IntentFilter userFilter = new IntentFilter(); userFilter.addAction(ACTION_USER_ADDED); @@ -318,7 +318,7 @@ public final class OverlayManagerService extends SystemService { final List targets = mImpl.updateOverlaysForUser(newUserId); updateAssets(newUserId, targets); } - schedulePersistSettings(); + persistSettings(); } finally { traceEnd(TRACE_TAG_RRO); } @@ -973,7 +973,7 @@ public final class OverlayManagerService extends SystemService { }; private void onOverlaysChanged(@NonNull final String targetPackageName, final int userId) { - schedulePersistSettings(); + persistSettings(); FgThread.getHandler().post(() -> { updateAssets(userId, targetPackageName); @@ -1065,27 +1065,21 @@ public final class OverlayManagerService extends SystemService { } } - private void schedulePersistSettings() { - if (mPersistSettingsScheduled.getAndSet(true)) { - return; + private void persistSettings() { + if (DEBUG) { + Slog.d(TAG, "Writing overlay settings"); } - IoThread.getHandler().post(() -> { - mPersistSettingsScheduled.set(false); - if (DEBUG) { - Slog.d(TAG, "Writing overlay settings"); + synchronized (mLock) { + FileOutputStream stream = null; + try { + stream = mSettingsFile.startWrite(); + mSettings.persist(stream); + mSettingsFile.finishWrite(stream); + } catch (IOException | XmlPullParserException e) { + mSettingsFile.failWrite(stream); + Slog.e(TAG, "failed to persist overlay state", e); } - synchronized (mLock) { - FileOutputStream stream = null; - try { - stream = mSettingsFile.startWrite(); - mSettings.persist(stream); - mSettingsFile.finishWrite(stream); - } catch (IOException | XmlPullParserException e) { - mSettingsFile.failWrite(stream); - Slog.e(TAG, "failed to persist overlay state", e); - } - } - }); + } } private void restoreSettings() { From ac6bbdb6b08e131743b2869a7454a1df752edf50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 14 Feb 2020 15:49:53 +0100 Subject: [PATCH 4/6] OMS: add transactional API Implement a transactional API for the OMS. This allows a caller to batch together individual operations, and apply those operations atomically: either they all succeed (and notifications are sent out accordingly), or some operation fails (and the transaction is cancelled, any internal changes are rolled back, and no notifications are sent out). As an added bonus, batching OMS operations result in fewer requests to the system to refresh affected packages. This commit adds support for transactions but leaves the old one-shot API intact. The old API should be considered deprecated and will be removed at some point in the future. Note: the old API provided both setEnabled and setHighestPriority as separate calls. In the transactional API, setEnabled implies setHighestPriority. Also, refactor the methods to broadcast ACTION_OVERLAY_CHANGED and to reach out to the package and activity managers. The methods no longer call each other: instead, it is up to the caller to explicitly invoke each method. Bug: 119916381 Test: builds Change-Id: I63df58510efbc5f7a7d3146a122c3d6209e7478b --- .../android/content/om/IOverlayManager.aidl | 15 + .../android/content/om/OverlayManager.java | 23 + .../content/om/OverlayManagerTransaction.aidl | 19 + .../content/om/OverlayManagerTransaction.java | 216 ++++++++ .../server/om/OverlayManagerService.java | 462 +++++++++++------- 5 files changed, 568 insertions(+), 167 deletions(-) create mode 100644 core/java/android/content/om/OverlayManagerTransaction.aidl create mode 100644 core/java/android/content/om/OverlayManagerTransaction.java diff --git a/core/java/android/content/om/IOverlayManager.aidl b/core/java/android/content/om/IOverlayManager.aidl index 44b5c44825998..0b950b4612854 100644 --- a/core/java/android/content/om/IOverlayManager.aidl +++ b/core/java/android/content/om/IOverlayManager.aidl @@ -17,6 +17,7 @@ package android.content.om; import android.content.om.OverlayInfo; +import android.content.om.OverlayManagerTransaction; /** * Api for getting information about overlay packages. @@ -163,4 +164,18 @@ interface IOverlayManager { * @param packageName The name of the overlay package whose idmap should be deleted. */ void invalidateCachesForOverlay(in String packageName, in int userIs); + + /** + * Perform a series of requests related to overlay packages. This is an + * atomic operation: either all requests were performed successfully and + * the changes were propagated to the rest of the system, or at least one + * request could not be performed successfully and nothing is changed and + * nothing is propagated to the rest of the system. + * + * @see OverlayManagerTransaction + * + * @param transaction the series of overlay related requests to perform + * @throws SecurityException if the transaction failed + */ + void commit(in OverlayManagerTransaction transaction); } diff --git a/core/java/android/content/om/OverlayManager.java b/core/java/android/content/om/OverlayManager.java index 217f637cf9e39..7c14c2891d017 100644 --- a/core/java/android/content/om/OverlayManager.java +++ b/core/java/android/content/om/OverlayManager.java @@ -253,6 +253,29 @@ public class OverlayManager { } } + /** + * Perform a series of requests related to overlay packages. This is an + * atomic operation: either all requests were performed successfully and + * the changes were propagated to the rest of the system, or at least one + * request could not be performed successfully and nothing is changed and + * nothing is propagated to the rest of the system. + * + * @see OverlayManagerTransaction + * + * @param transaction the series of overlay related requests to perform + * @throws Exception if not all the requests could be successfully and + * atomically executed + * + * @hide + */ + public void commit(@NonNull final OverlayManagerTransaction transaction) { + try { + mService.commit(transaction); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + /** * Starting on R, actor enforcement and app visibility changes introduce additional failure * cases, but the SecurityException thrown with these checks is unexpected for existing diff --git a/core/java/android/content/om/OverlayManagerTransaction.aidl b/core/java/android/content/om/OverlayManagerTransaction.aidl new file mode 100644 index 0000000000000..6715c82d4e6ff --- /dev/null +++ b/core/java/android/content/om/OverlayManagerTransaction.aidl @@ -0,0 +1,19 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.content.om; + +parcelable OverlayManagerTransaction; diff --git a/core/java/android/content/om/OverlayManagerTransaction.java b/core/java/android/content/om/OverlayManagerTransaction.java new file mode 100644 index 0000000000000..1fa8973c35b56 --- /dev/null +++ b/core/java/android/content/om/OverlayManagerTransaction.java @@ -0,0 +1,216 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.content.om; + +import static com.android.internal.util.Preconditions.checkNotNull; + +import android.annotation.IntDef; +import android.annotation.NonNull; +import android.os.Parcel; +import android.os.Parcelable; +import android.os.UserHandle; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** + * Container for a batch of requests to the OverlayManagerService. + * + * Transactions are created using a builder interface. Example usage: + * + * final OverlayManager om = ctx.getSystemService(OverlayManager.class); + * final OverlayManagerTransaction t = new OverlayManagerTransaction.Builder() + * .setEnabled(...) + * .setEnabled(...) + * .build(); + * om.commit(t); + * + * @hide + */ +public class OverlayManagerTransaction + implements Iterable, Parcelable { + // TODO: remove @hide from this class when OverlayManager is added to the + // SDK, but keep OverlayManagerTransaction.Request @hidden + private final List mRequests; + + OverlayManagerTransaction(@NonNull final List requests) { + checkNotNull(requests); + if (requests.contains(null)) { + throw new IllegalArgumentException("null request"); + } + mRequests = requests; + } + + private OverlayManagerTransaction(@NonNull final Parcel source) { + final int size = source.readInt(); + mRequests = new ArrayList(size); + for (int i = 0; i < size; i++) { + final int request = source.readInt(); + final String packageName = source.readString(); + final int userId = source.readInt(); + mRequests.add(new Request(request, packageName, userId)); + } + } + + @Override + public Iterator iterator() { + return mRequests.iterator(); + } + + @Override + public String toString() { + return String.format("OverlayManagerTransaction { mRequests = %s }", mRequests); + } + + /** + * A single unit of the transaction, such as a request to enable an + * overlay, or to disable an overlay. + * + * @hide + */ + public static class Request { + @IntDef(prefix = "TYPE_", value = { + TYPE_SET_ENABLED, + TYPE_SET_DISABLED, + }) + @Retention(RetentionPolicy.SOURCE) + @interface RequestType {} + + public static final int TYPE_SET_ENABLED = 0; + public static final int TYPE_SET_DISABLED = 1; + + @RequestType public final int type; + public final String packageName; + public final int userId; + + public Request(@RequestType final int type, @NonNull final String packageName, + final int userId) { + this.type = type; + this.packageName = packageName; + this.userId = userId; + } + + @Override + public String toString() { + return String.format("Request{type=0x%02x (%s), packageName=%s, userId=%d}", + type, typeToString(), packageName, userId); + } + + /** + * Translate the request type into a human readable string. Only + * intended for debugging. + * + * @hide + */ + public String typeToString() { + switch (type) { + case TYPE_SET_ENABLED: return "TYPE_SET_ENABLED"; + case TYPE_SET_DISABLED: return "TYPE_SET_DISABLED"; + default: return String.format("TYPE_UNKNOWN (0x%02x)", type); + } + } + } + + /** + * Builder class for OverlayManagerTransaction objects. + * + * @hide + */ + public static class Builder { + private final List mRequests = new ArrayList<>(); + + /** + * Request that an overlay package be enabled and change its loading + * order to the last package to be loaded, or disabled + * + * If the caller has the correct permissions, it is always possible to + * disable an overlay. Due to technical and security reasons it may not + * always be possible to enable an overlay, for instance if the overlay + * does not successfully overlay any target resources due to + * overlayable policy restrictions. + * + * An enabled overlay is a part of target package's resources, i.e. it will + * be part of any lookups performed via {@link android.content.res.Resources} + * and {@link android.content.res.AssetManager}. A disabled overlay will no + * longer affect the resources of the target package. If the target is + * currently running, its outdated resources will be replaced by new ones. + * + * @param packageName The name of the overlay package. + * @param enable true to enable the overlay, false to disable it. + * @return this Builder object, so you can chain additional requests + */ + public Builder setEnabled(@NonNull String packageName, boolean enable) { + return setEnabled(packageName, enable, UserHandle.myUserId()); + } + + /** + * @hide + */ + public Builder setEnabled(@NonNull String packageName, boolean enable, int userId) { + checkNotNull(packageName); + @Request.RequestType final int type = + enable ? Request.TYPE_SET_ENABLED : Request.TYPE_SET_DISABLED; + mRequests.add(new Request(type, packageName, userId)); + return this; + } + + /** + * Create a new transaction out of the requests added so far. Execute + * the transaction by calling OverlayManager#commit. + * + * @see OverlayManager#commit + * @return a new transaction + */ + public OverlayManagerTransaction build() { + return new OverlayManagerTransaction(mRequests); + } + } + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) { + final int size = mRequests.size(); + dest.writeInt(size); + for (int i = 0; i < size; i++) { + final Request req = mRequests.get(i); + dest.writeInt(req.type); + dest.writeString(req.packageName); + dest.writeInt(req.userId); + } + } + + public static final Parcelable.Creator CREATOR = + new Parcelable.Creator() { + + @Override + public OverlayManagerTransaction createFromParcel(Parcel source) { + return new OverlayManagerTransaction(source); + } + + @Override + public OverlayManagerTransaction[] newArray(int size) { + return new OverlayManagerTransaction[size]; + } + }; +} diff --git a/services/core/java/com/android/server/om/OverlayManagerService.java b/services/core/java/com/android/server/om/OverlayManagerService.java index 437e33d66c919..7a6792c121c04 100644 --- a/services/core/java/com/android/server/om/OverlayManagerService.java +++ b/services/core/java/com/android/server/om/OverlayManagerService.java @@ -24,6 +24,8 @@ import static android.content.Intent.ACTION_PACKAGE_REMOVED; import static android.content.Intent.ACTION_USER_ADDED; import static android.content.Intent.ACTION_USER_REMOVED; import static android.content.Intent.EXTRA_REASON; +import static android.content.om.OverlayManagerTransaction.Request.TYPE_SET_DISABLED; +import static android.content.om.OverlayManagerTransaction.Request.TYPE_SET_ENABLED; import static android.content.pm.PackageManager.SIGNATURE_MATCH; import static android.os.Trace.TRACE_TAG_RRO; import static android.os.Trace.traceBegin; @@ -41,6 +43,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.content.om.IOverlayManager; import android.content.om.OverlayInfo; +import android.content.om.OverlayManagerTransaction; import android.content.om.OverlayableInfo; import android.content.pm.IPackageManager; import android.content.pm.PackageInfo; @@ -84,11 +87,14 @@ import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.function.Consumer; /** @@ -238,8 +244,13 @@ public final class OverlayManagerService extends SystemService { private final OverlayActorEnforcer mActorEnforcer; - private final Consumer mOnOverlaysChanged = (pair) -> { - onOverlaysChanged(pair.packageName, pair.userId); + private final Consumer mPropagateOverlayChange = (pair) -> { + persistSettings(); + FgThread.getHandler().post(() -> { + List affectedTargets = updatePackageManager(pair.packageName, pair.userId); + updateActivityManager(affectedTargets, pair.userId); + broadcastActionOverlayChanged(affectedTargets, pair.userId); + }); }; public OverlayManagerService(@NonNull final Context context) { @@ -298,11 +309,11 @@ public final class OverlayManagerService extends SystemService { for (int i = 0; i < userCount; i++) { final UserInfo userInfo = users.get(i); if (!userInfo.supportsSwitchTo() && userInfo.id != UserHandle.USER_SYSTEM) { - // Initialize any users that can't be switched to, as there state would + // Initialize any users that can't be switched to, as their state would // never be setup in onSwitchUser(). We will switch to the system user right // after this, and its state will be setup there. final List targets = mImpl.updateOverlaysForUser(users.get(i).id); - updateOverlayPaths(users.get(i).id, targets); + updatePackageManager(targets, users.get(i).id); } } } @@ -316,7 +327,8 @@ public final class OverlayManagerService extends SystemService { // any asset changes to the rest of the system synchronized (mLock) { final List targets = mImpl.updateOverlaysForUser(newUserId); - updateAssets(newUserId, targets); + final List affectedTargets = updatePackageManager(targets, newUserId); + updateActivityManager(affectedTargets, newUserId); } persistSettings(); } finally { @@ -406,10 +418,10 @@ public final class OverlayManagerService extends SystemService { try { if (pi.isOverlayPackage()) { mImpl.onOverlayPackageAdded(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } else { mImpl.onTargetPackageAdded(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } } catch (OperationFailedException e) { Slog.e(TAG, "onPackageAdded internal error", e); @@ -436,10 +448,10 @@ public final class OverlayManagerService extends SystemService { try { if (pi.isOverlayPackage()) { mImpl.onOverlayPackageChanged(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } else { mImpl.onTargetPackageChanged(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } } catch (OperationFailedException e) { Slog.e(TAG, "onPackageChanged internal error", e); @@ -463,7 +475,7 @@ public final class OverlayManagerService extends SystemService { if (oi != null) { try { mImpl.onOverlayPackageReplacing(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } catch (OperationFailedException e) { Slog.e(TAG, "onPackageReplacing internal error", e); } @@ -488,10 +500,10 @@ public final class OverlayManagerService extends SystemService { try { if (pi.isOverlayPackage()) { mImpl.onOverlayPackageReplaced(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } else { mImpl.onTargetPackageReplaced(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } } catch (OperationFailedException e) { Slog.e(TAG, "onPackageReplaced internal error", e); @@ -516,10 +528,10 @@ public final class OverlayManagerService extends SystemService { try { if (oi != null) { mImpl.onOverlayPackageRemoved(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } else { mImpl.onTargetPackageRemoved(packageName, userId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); } } catch (OperationFailedException e) { Slog.e(TAG, "onPackageRemoved internal error", e); @@ -545,7 +557,7 @@ public final class OverlayManagerService extends SystemService { synchronized (mLock) { targets = mImpl.updateOverlaysForUser(userId); } - updateOverlayPaths(userId, targets); + updatePackageManager(targets, userId); } finally { traceEnd(TRACE_TAG_RRO); } @@ -642,7 +654,7 @@ public final class OverlayManagerService extends SystemService { synchronized (mLock) { try { mImpl.setEnabled(packageName, enable, realUserId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); return true; } catch (OperationFailedException e) { return false; @@ -674,7 +686,7 @@ public final class OverlayManagerService extends SystemService { try { mImpl.setEnabledExclusive(packageName, false /* withinCategory */, realUserId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); return true; } catch (OperationFailedException e) { return false; @@ -707,7 +719,7 @@ public final class OverlayManagerService extends SystemService { try { mImpl.setEnabledExclusive(packageName, true /* withinCategory */, realUserId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); return true; } catch (OperationFailedException e) { return false; @@ -739,7 +751,7 @@ public final class OverlayManagerService extends SystemService { synchronized (mLock) { try { mImpl.setPriority(packageName, parentPackageName, realUserId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); return true; } catch (OperationFailedException e) { return false; @@ -769,7 +781,7 @@ public final class OverlayManagerService extends SystemService { synchronized (mLock) { try { mImpl.setHighestPriority(packageName, realUserId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); return true; } catch (OperationFailedException e) { return false; @@ -799,7 +811,7 @@ public final class OverlayManagerService extends SystemService { synchronized (mLock) { try { mImpl.setLowestPriority(packageName, realUserId) - .ifPresent(mOnOverlaysChanged); + .ifPresent(mPropagateOverlayChange); return true; } catch (OperationFailedException e) { return false; @@ -851,6 +863,120 @@ public final class OverlayManagerService extends SystemService { } } + @Override + public void commit(@NonNull final OverlayManagerTransaction transaction) + throws RemoteException { + try { + traceBegin(TRACE_TAG_RRO, "OMS#commit " + transaction); + try { + executeAllRequests(transaction); + } catch (Exception e) { + final long ident = Binder.clearCallingIdentity(); + try { + restoreSettings(); + } finally { + Binder.restoreCallingIdentity(ident); + } + Slog.d(TAG, "commit failed: " + e.getMessage(), e); + throw new SecurityException("commit failed" + + (DEBUG ? ": " + e.getMessage() : "")); + } + } finally { + traceEnd(TRACE_TAG_RRO); + } + } + + private Optional executeRequest( + @NonNull final OverlayManagerTransaction.Request request) throws Exception { + final int realUserId = handleIncomingUser(request.userId, request.typeToString()); + enforceActor(request.packageName, request.typeToString(), realUserId); + + final long ident = Binder.clearCallingIdentity(); + try { + switch (request.type) { + case TYPE_SET_ENABLED: + Optional opt1 = + mImpl.setEnabled(request.packageName, true, request.userId); + Optional opt2 = + mImpl.setHighestPriority(request.packageName, request.userId); + // Both setEnabled and setHighestPriority affected the same + // target package and user: if both return non-empty + // Optionals, they are identical + return opt1.isPresent() ? opt1 : opt2; + case TYPE_SET_DISABLED: + return mImpl.setEnabled(request.packageName, false, request.userId); + default: + throw new IllegalArgumentException("unsupported request: " + request); + } + } finally { + Binder.restoreCallingIdentity(ident); + } + } + + private void executeAllRequests(@NonNull final OverlayManagerTransaction transaction) + throws Exception { + if (DEBUG) { + Slog.d(TAG, "commit " + transaction); + } + if (transaction == null) { + throw new IllegalArgumentException("null transaction"); + } + + // map: userId -> list + SparseArray> affectedTargetsToUpdate = new SparseArray<>(); + + synchronized (mLock) { + // map: userId -> set + SparseArray> targetsToUpdate = new SparseArray<>(); + + // execute the requests (as calling user) + for (final OverlayManagerTransaction.Request request : transaction) { + executeRequest(request).ifPresent(target -> { + Set userTargets = targetsToUpdate.get(target.userId); + if (userTargets == null) { + userTargets = new ArraySet(); + targetsToUpdate.put(target.userId, userTargets); + } + userTargets.add(target.packageName); + }); + } + + // past the point of no return: the entire transaction has been + // processed successfully, we can no longer fail: continue as + // system_server + final long ident = Binder.clearCallingIdentity(); + try { + persistSettings(); + + // inform the package manager about the new paths + for (int index = 0; index < targetsToUpdate.size(); index++) { + final int userId = targetsToUpdate.keyAt(index); + final List affectedTargets = + updatePackageManager(targetsToUpdate.valueAt(index), userId); + affectedTargetsToUpdate.put(userId, affectedTargets); + } + } finally { + Binder.restoreCallingIdentity(ident); + } + } // synchronized (mLock) + + FgThread.getHandler().post(() -> { + final long ident = Binder.clearCallingIdentity(); + try { + // schedule apps to refresh + broadcast the ACTION_OVERLAY_CHANGED intents + for (int index = 0; index < affectedTargetsToUpdate.size(); index++) { + final int userId = affectedTargetsToUpdate.keyAt(index); + final List packageNames = affectedTargetsToUpdate.valueAt(index); + + updateActivityManager(packageNames, userId); + broadcastActionOverlayChanged(packageNames, userId); + } + } finally { + Binder.restoreCallingIdentity(ident); + } + }); + } + @Override public void onShellCommand(@NonNull final FileDescriptor in, @NonNull final FileDescriptor out, @NonNull final FileDescriptor err, @@ -972,152 +1098,7 @@ public final class OverlayManagerService extends SystemService { } }; - private void onOverlaysChanged(@NonNull final String targetPackageName, final int userId) { - persistSettings(); - FgThread.getHandler().post(() -> { - updateAssets(userId, targetPackageName); - - final Intent intent = new Intent(ACTION_OVERLAY_CHANGED, - Uri.fromParts("package", targetPackageName, null)); - intent.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); - - if (DEBUG) { - Slog.d(TAG, "send broadcast " + intent); - } - - try { - ActivityManager.getService().broadcastIntent(null, intent, null, null, 0, - null, null, null, android.app.AppOpsManager.OP_NONE, null, false, false, - userId); - } catch (RemoteException e) { - // Intentionally left empty. - } - }); - } - - /** - * Updates the target packages' set of enabled overlays in PackageManager. - */ - private ArrayList updateOverlayPaths(int userId, List targetPackageNames) { - try { - traceBegin(TRACE_TAG_RRO, "OMS#updateOverlayPaths " + targetPackageNames); - if (DEBUG) { - Slog.d(TAG, "Updating overlay assets"); - } - final PackageManagerInternal pm = - LocalServices.getService(PackageManagerInternal.class); - final boolean updateFrameworkRes = targetPackageNames.contains("android"); - if (updateFrameworkRes) { - targetPackageNames = pm.getTargetPackageNames(userId); - } - - final Map> pendingChanges = - new ArrayMap<>(targetPackageNames.size()); - synchronized (mLock) { - final List frameworkOverlays = - mImpl.getEnabledOverlayPackageNames("android", userId); - final int n = targetPackageNames.size(); - for (int i = 0; i < n; i++) { - final String targetPackageName = targetPackageNames.get(i); - List list = new ArrayList<>(); - if (!"android".equals(targetPackageName)) { - list.addAll(frameworkOverlays); - } - list.addAll(mImpl.getEnabledOverlayPackageNames(targetPackageName, userId)); - pendingChanges.put(targetPackageName, list); - } - } - - final HashSet updatedPackages = new HashSet<>(); - final int n = targetPackageNames.size(); - for (int i = 0; i < n; i++) { - final String targetPackageName = targetPackageNames.get(i); - if (DEBUG) { - Slog.d(TAG, "-> Updating overlay: target=" + targetPackageName + " overlays=[" - + TextUtils.join(",", pendingChanges.get(targetPackageName)) - + "] userId=" + userId); - } - - if (!pm.setEnabledOverlayPackages( - userId, targetPackageName, pendingChanges.get(targetPackageName), - updatedPackages)) { - Slog.e(TAG, String.format("Failed to change enabled overlays for %s user %d", - targetPackageName, userId)); - } - } - return new ArrayList<>(updatedPackages); - } finally { - traceEnd(TRACE_TAG_RRO); - } - } - - private void updateAssets(final int userId, final String targetPackageName) { - updateAssets(userId, Collections.singletonList(targetPackageName)); - } - - private void updateAssets(final int userId, List targetPackageNames) { - final IActivityManager am = ActivityManager.getService(); - try { - final ArrayList updatedPaths = updateOverlayPaths(userId, targetPackageNames); - am.scheduleApplicationInfoChanged(updatedPaths, userId); - } catch (RemoteException e) { - // Intentionally left empty. - } - } - - private void persistSettings() { - if (DEBUG) { - Slog.d(TAG, "Writing overlay settings"); - } - synchronized (mLock) { - FileOutputStream stream = null; - try { - stream = mSettingsFile.startWrite(); - mSettings.persist(stream); - mSettingsFile.finishWrite(stream); - } catch (IOException | XmlPullParserException e) { - mSettingsFile.failWrite(stream); - Slog.e(TAG, "failed to persist overlay state", e); - } - } - } - - private void restoreSettings() { - try { - traceBegin(TRACE_TAG_RRO, "OMS#restoreSettings"); - synchronized (mLock) { - if (!mSettingsFile.getBaseFile().exists()) { - return; - } - try (FileInputStream stream = mSettingsFile.openRead()) { - mSettings.restore(stream); - - // We might have data for dying users if the device was - // restarted before we received USER_REMOVED. Remove data for - // users that will not exist after the system is ready. - - final List liveUsers = mUserManager.getUsers(true /*excludeDying*/); - final int[] liveUserIds = new int[liveUsers.size()]; - for (int i = 0; i < liveUsers.size(); i++) { - liveUserIds[i] = liveUsers.get(i).getUserHandle().getIdentifier(); - } - Arrays.sort(liveUserIds); - - for (int userId : mSettings.getUsers()) { - if (Arrays.binarySearch(liveUserIds, userId) < 0) { - mSettings.removeUser(userId); - } - } - } catch (IOException | XmlPullParserException e) { - Slog.e(TAG, "failed to restore overlay state", e); - } - } - } finally { - traceEnd(TRACE_TAG_RRO); - } - } - - private static final class PackageManagerHelperImpl implements PackageManagerHelper { + private static final class PackageManagerHelperImpl implements PackageManagerHelper { private final Context mContext; private final IPackageManager mPackageManager; @@ -1327,4 +1308,151 @@ public final class OverlayManagerService extends SystemService { } } } + + // Helper methods to update other parts of the system or read/write + // settings: these methods should never call into each other! + + private void broadcastActionOverlayChanged(@NonNull final Collection packageNames, + final int userId) { + for (final String packageName : packageNames) { + broadcastActionOverlayChanged(packageName, userId); + } + } + + private void broadcastActionOverlayChanged(@NonNull final String targetPackageName, + final int userId) { + final Intent intent = new Intent(ACTION_OVERLAY_CHANGED, + Uri.fromParts("package", targetPackageName, null)); + intent.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); + try { + ActivityManager.getService().broadcastIntent(null, intent, null, null, 0, null, null, + null, android.app.AppOpsManager.OP_NONE, null, false, false, userId); + } catch (RemoteException e) { + // Intentionally left empty. + } + } + + /** + * Tell the activity manager to tell a set of packages to reload their + * resources. + */ + private void updateActivityManager(List targetPackageNames, final int userId) { + final IActivityManager am = ActivityManager.getService(); + try { + am.scheduleApplicationInfoChanged(targetPackageNames, userId); + } catch (RemoteException e) { + // Intentionally left empty. + } + } + + private ArrayList updatePackageManager(String targetPackageNames, final int userId) { + return updatePackageManager(Collections.singletonList(targetPackageNames), userId); + } + + /** + * Updates the target packages' set of enabled overlays in PackageManager. + * @return the package names of affected targets (a superset of + * targetPackageNames: the target themserlves and shared libraries) + */ + private ArrayList updatePackageManager(@NonNull Collection targetPackageNames, + final int userId) { + try { + traceBegin(TRACE_TAG_RRO, "OMS#updatePackageManager " + targetPackageNames); + if (DEBUG) { + Slog.d(TAG, "Update package manager about changed overlays"); + } + final PackageManagerInternal pm = + LocalServices.getService(PackageManagerInternal.class); + final boolean updateFrameworkRes = targetPackageNames.contains("android"); + if (updateFrameworkRes) { + targetPackageNames = pm.getTargetPackageNames(userId); + } + + final Map> pendingChanges = + new ArrayMap<>(targetPackageNames.size()); + synchronized (mLock) { + final List frameworkOverlays = + mImpl.getEnabledOverlayPackageNames("android", userId); + for (final String targetPackageName : targetPackageNames) { + List list = new ArrayList<>(); + if (!"android".equals(targetPackageName)) { + list.addAll(frameworkOverlays); + } + list.addAll(mImpl.getEnabledOverlayPackageNames(targetPackageName, userId)); + pendingChanges.put(targetPackageName, list); + } + } + + final HashSet updatedPackages = new HashSet<>(); + for (final String targetPackageName : targetPackageNames) { + if (DEBUG) { + Slog.d(TAG, "-> Updating overlay: target=" + targetPackageName + " overlays=[" + + TextUtils.join(",", pendingChanges.get(targetPackageName)) + + "] userId=" + userId); + } + + if (!pm.setEnabledOverlayPackages( + userId, targetPackageName, pendingChanges.get(targetPackageName), + updatedPackages)) { + Slog.e(TAG, String.format("Failed to change enabled overlays for %s user %d", + targetPackageName, userId)); + } + } + return new ArrayList<>(updatedPackages); + } finally { + traceEnd(TRACE_TAG_RRO); + } + } + + private void persistSettings() { + if (DEBUG) { + Slog.d(TAG, "Writing overlay settings"); + } + synchronized (mLock) { + FileOutputStream stream = null; + try { + stream = mSettingsFile.startWrite(); + mSettings.persist(stream); + mSettingsFile.finishWrite(stream); + } catch (IOException | XmlPullParserException e) { + mSettingsFile.failWrite(stream); + Slog.e(TAG, "failed to persist overlay state", e); + } + } + } + + private void restoreSettings() { + try { + traceBegin(TRACE_TAG_RRO, "OMS#restoreSettings"); + synchronized (mLock) { + if (!mSettingsFile.getBaseFile().exists()) { + return; + } + try (FileInputStream stream = mSettingsFile.openRead()) { + mSettings.restore(stream); + + // We might have data for dying users if the device was + // restarted before we received USER_REMOVED. Remove data for + // users that will not exist after the system is ready. + + final List liveUsers = mUserManager.getUsers(true /*excludeDying*/); + final int[] liveUserIds = new int[liveUsers.size()]; + for (int i = 0; i < liveUsers.size(); i++) { + liveUserIds[i] = liveUsers.get(i).getUserHandle().getIdentifier(); + } + Arrays.sort(liveUserIds); + + for (int userId : mSettings.getUsers()) { + if (Arrays.binarySearch(liveUserIds, userId) < 0) { + mSettings.removeUser(userId); + } + } + } catch (IOException | XmlPullParserException e) { + Slog.e(TAG, "failed to restore overlay state", e); + } + } + } finally { + traceEnd(TRACE_TAG_RRO); + } + } } From 2b1b3b282f620c15025fb5c449ed35a0f1db304b Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Mon, 28 Dec 2020 11:41:43 -0800 Subject: [PATCH 5/6] Push OverlayDeviceTests onto /system Since OverlayDeviceTests needs to use the OverlayManager API directly, push the test APk onto the system image and reboot the device before running tests. Bug: 119916381 Test: atest OverlayDeviceTests Change-Id: I8f54e1532902b984e32da68ed8aa32bf6c5b9272 --- core/tests/overlaytests/device/Android.bp | 6 +++++- core/tests/overlaytests/device/AndroidManifest.xml | 2 ++ core/tests/overlaytests/device/AndroidTest.xml | 13 ++++++++++++- .../device/test-apps/AppOverlayOne/Android.bp | 2 +- .../device/test-apps/AppOverlayTwo/Android.bp | 2 +- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/core/tests/overlaytests/device/Android.bp b/core/tests/overlaytests/device/Android.bp index 12a2b08150501..f86ac9ce37e11 100644 --- a/core/tests/overlaytests/device/Android.bp +++ b/core/tests/overlaytests/device/Android.bp @@ -16,7 +16,11 @@ android_test { name: "OverlayDeviceTests", srcs: ["src/**/*.java"], platform_apis: true, - static_libs: ["androidx.test.rules"], + certificate: "platform", + static_libs: [ + "androidx.test.rules", + "testng", + ], test_suites: ["device-tests"], data: [ ":OverlayDeviceTests_AppOverlayOne", diff --git a/core/tests/overlaytests/device/AndroidManifest.xml b/core/tests/overlaytests/device/AndroidManifest.xml index 4881636c70956..a69911f8d827d 100644 --- a/core/tests/overlaytests/device/AndroidManifest.xml +++ b/core/tests/overlaytests/device/AndroidManifest.xml @@ -19,6 +19,8 @@ + + diff --git a/core/tests/overlaytests/device/AndroidTest.xml b/core/tests/overlaytests/device/AndroidTest.xml index 6507839a42889..db45750ff6dc6 100644 --- a/core/tests/overlaytests/device/AndroidTest.xml +++ b/core/tests/overlaytests/device/AndroidTest.xml @@ -19,9 +19,20 @@