From 9ba08918e95f4594f8ad53bcc4a771cb45fe6c77 Mon Sep 17 00:00:00 2001 From: Evan Chen Date: Wed, 4 May 2022 15:54:57 -0700 Subject: [PATCH] Delay role revoke when the app is not visible to the user This cl fixes the problem that CDM calls RoleManager.removeRoleHolderAsUser() will kill the application's process that lead to the poor user experience. In order to fix the problem: 1. Introduce a new OnUidImportanceListener which watches the importance of the packages. In this class, we only interested in the importance of the running process is greater than IMPORTANCE_VISIBLE. 2. Introduce a new field in AssociationInfo class: 'mRevoked' to indicate the associaiton has been reovked and pending for role holder removal. So that the revoked association can store in the same place as the active associaiton -> easy maintenance. 3. Finally remove the role holder for the packages are not visible to the users. Bug: 189250370 Test: atest CtsCompanionDeviceManagerCoreTestCases atest CtsCompanionDeviceManagerUiAutomationTestCases atest CtsOsTestCases:CompanionDeviceManagerTest Change-Id: Ib7fb08b0d800c62a9ee708ddd52cd174de1829d9 Merged-In: Ib7fb08b0d800c62a9ee708ddd52cd174de1829d9 --- .../android/companion/AssociationInfo.java | 46 ++- .../companion/AssociationStoreImpl.java | 16 + .../CompanionDeviceManagerService.java | 366 ++++++++++++++++-- .../server/companion/PersistentDataStore.java | 24 +- .../android/server/companion/RolesUtils.java | 2 + 5 files changed, 415 insertions(+), 39 deletions(-) diff --git a/core/java/android/companion/AssociationInfo.java b/core/java/android/companion/AssociationInfo.java index f7f0235cd5088..93748f81ffa16 100644 --- a/core/java/android/companion/AssociationInfo.java +++ b/core/java/android/companion/AssociationInfo.java @@ -55,6 +55,14 @@ public final class AssociationInfo implements Parcelable { private final boolean mSelfManaged; private final boolean mNotifyOnDeviceNearby; + + /** + * Indicates that the association has been revoked (removed), but we keep the association + * record for final clean up (e.g. removing the app from the list of the role holders). + * + * @see CompanionDeviceManager#disassociate(int) + */ + private final boolean mRevoked; private final long mTimeApprovedMs; /** * A long value indicates the last time connected reported by selfManaged devices @@ -71,7 +79,7 @@ public final class AssociationInfo implements Parcelable { public AssociationInfo(int id, @UserIdInt int userId, @NonNull String packageName, @Nullable MacAddress macAddress, @Nullable CharSequence displayName, @Nullable String deviceProfile, boolean selfManaged, boolean notifyOnDeviceNearby, - long timeApprovedMs, long lastTimeConnectedMs) { + boolean revoked, long timeApprovedMs, long lastTimeConnectedMs) { if (id <= 0) { throw new IllegalArgumentException("Association ID should be greater than 0"); } @@ -91,6 +99,7 @@ public final class AssociationInfo implements Parcelable { mSelfManaged = selfManaged; mNotifyOnDeviceNearby = notifyOnDeviceNearby; + mRevoked = revoked; mTimeApprovedMs = timeApprovedMs; mLastTimeConnectedMs = lastTimeConnectedMs; } @@ -175,6 +184,14 @@ public final class AssociationInfo implements Parcelable { return mUserId == userId && Objects.equals(mPackageName, packageName); } + /** + * @return if the association has been revoked (removed). + * @hide + */ + public boolean isRevoked() { + return mRevoked; + } + /** * @return the last time self reported disconnected for selfManaged only. * @hide @@ -244,6 +261,7 @@ public final class AssociationInfo implements Parcelable { + ", mDeviceProfile='" + mDeviceProfile + '\'' + ", mSelfManaged=" + mSelfManaged + ", mNotifyOnDeviceNearby=" + mNotifyOnDeviceNearby + + ", mRevoked=" + mRevoked + ", mTimeApprovedMs=" + new Date(mTimeApprovedMs) + ", mLastTimeConnectedMs=" + ( mLastTimeConnectedMs == Long.MAX_VALUE @@ -260,6 +278,7 @@ public final class AssociationInfo implements Parcelable { && mUserId == that.mUserId && mSelfManaged == that.mSelfManaged && mNotifyOnDeviceNearby == that.mNotifyOnDeviceNearby + && mRevoked == that.mRevoked && mTimeApprovedMs == that.mTimeApprovedMs && mLastTimeConnectedMs == that.mLastTimeConnectedMs && Objects.equals(mPackageName, that.mPackageName) @@ -271,7 +290,7 @@ public final class AssociationInfo implements Parcelable { @Override public int hashCode() { return Objects.hash(mId, mUserId, mPackageName, mDeviceMacAddress, mDisplayName, - mDeviceProfile, mSelfManaged, mNotifyOnDeviceNearby, mTimeApprovedMs, + mDeviceProfile, mSelfManaged, mNotifyOnDeviceNearby, mRevoked, mTimeApprovedMs, mLastTimeConnectedMs); } @@ -293,6 +312,7 @@ public final class AssociationInfo implements Parcelable { dest.writeBoolean(mSelfManaged); dest.writeBoolean(mNotifyOnDeviceNearby); + dest.writeBoolean(mRevoked); dest.writeLong(mTimeApprovedMs); dest.writeLong(mLastTimeConnectedMs); } @@ -309,6 +329,7 @@ public final class AssociationInfo implements Parcelable { mSelfManaged = in.readBoolean(); mNotifyOnDeviceNearby = in.readBoolean(); + mRevoked = in.readBoolean(); mTimeApprovedMs = in.readLong(); mLastTimeConnectedMs = in.readLong(); } @@ -352,11 +373,13 @@ public final class AssociationInfo implements Parcelable { @NonNull private final AssociationInfo mOriginalInfo; private boolean mNotifyOnDeviceNearby; + private boolean mRevoked; private long mLastTimeConnectedMs; private Builder(@NonNull AssociationInfo info) { mOriginalInfo = info; mNotifyOnDeviceNearby = info.mNotifyOnDeviceNearby; + mRevoked = info.mRevoked; mLastTimeConnectedMs = info.mLastTimeConnectedMs; } @@ -387,6 +410,17 @@ public final class AssociationInfo implements Parcelable { return this; } + /** + * Should only be used by the CompanionDeviceManagerService. + * @hide + */ + @Override + @NonNull + public Builder setRevoked(boolean revoked) { + mRevoked = revoked; + return this; + } + /** * @hide */ @@ -401,6 +435,7 @@ public final class AssociationInfo implements Parcelable { mOriginalInfo.mDeviceProfile, mOriginalInfo.mSelfManaged, mNotifyOnDeviceNearby, + mRevoked, mOriginalInfo.mTimeApprovedMs, mLastTimeConnectedMs ); @@ -433,5 +468,12 @@ public final class AssociationInfo implements Parcelable { */ @NonNull Builder setLastTimeConnected(long lastTimeConnectedMs); + + /** + * Should only be used by the CompanionDeviceManagerService. + * @hide + */ + @NonNull + Builder setRevoked(boolean revoked); } } diff --git a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java index 229799a8457d3..d5991d3930a86 100644 --- a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java +++ b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java @@ -73,6 +73,9 @@ class AssociationStoreImpl implements AssociationStore { private final Set mListeners = new LinkedHashSet<>(); void addAssociation(@NonNull AssociationInfo association) { + // Validity check first. + checkNotRevoked(association); + final int id = association.getId(); if (DEBUG) { @@ -99,6 +102,9 @@ class AssociationStoreImpl implements AssociationStore { } void updateAssociation(@NonNull AssociationInfo updated) { + // Validity check first. + checkNotRevoked(updated); + final int id = updated.getId(); if (DEBUG) { @@ -292,6 +298,9 @@ class AssociationStoreImpl implements AssociationStore { } void setAssociations(Collection allAssociations) { + // Validity check first. + allAssociations.forEach(AssociationStoreImpl::checkNotRevoked); + if (DEBUG) { Log.i(TAG, "setAssociations() n=" + allAssociations.size()); final StringJoiner stringJoiner = new StringJoiner(", "); @@ -324,4 +333,11 @@ class AssociationStoreImpl implements AssociationStore { mAddressMap.clear(); mCachedPerUser.clear(); } + + private static void checkNotRevoked(@NonNull AssociationInfo association) { + if (association.isRevoked()) { + throw new IllegalArgumentException( + "Revoked (removed) associations MUST NOT appear in the AssociationStore"); + } + } } diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index beea374bacc79..73cf953b0f61c 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -18,6 +18,7 @@ package com.android.server.companion; import static android.Manifest.permission.MANAGE_COMPANION_DEVICES; +import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_VISIBLE; import static android.content.pm.PackageManager.CERT_INPUT_SHA256; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.os.Process.SYSTEM_UID; @@ -48,6 +49,8 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.SuppressLint; import android.annotation.UserIdInt; +import android.app.ActivityManager; +import android.app.ActivityManager.RunningAppProcessInfo; import android.app.ActivityManagerInternal; import android.app.AppOpsManager; import android.app.NotificationManager; @@ -92,6 +95,7 @@ import android.util.SparseBooleanArray; import com.android.internal.annotations.GuardedBy; import com.android.internal.app.IAppOpsService; import com.android.internal.content.PackageMonitor; +import com.android.internal.infra.PerUser; import com.android.internal.notification.NotificationAccessConfirmationActivityContract; import com.android.internal.os.BackgroundThread; import com.android.internal.util.ArrayUtils; @@ -109,6 +113,7 @@ import com.android.server.pm.UserManagerInternal; import java.io.File; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -132,6 +137,9 @@ public class CompanionDeviceManagerService extends SystemService { private static final long ASSOCIATION_REMOVAL_TIME_WINDOW_DEFAULT = DAYS.toMillis(90); + private final ActivityManager mActivityManager; + private final OnPackageVisibilityChangeListener mOnPackageVisibilityChangeListener; + private PersistentDataStore mPersistentStore; private final PersistUserStateHandler mUserPersistenceHandler; @@ -159,12 +167,40 @@ public class CompanionDeviceManagerService extends SystemService { @GuardedBy("mPreviouslyUsedIds") private final SparseArray>> mPreviouslyUsedIds = new SparseArray<>(); + /** + * A structure that consists of a set of revoked associations that pending for role holder + * removal per each user. + * + * @see #maybeRemoveRoleHolderForAssociation(AssociationInfo) + * @see #addToPendingRoleHolderRemoval(AssociationInfo) + * @see #removeFromPendingRoleHolderRemoval(AssociationInfo) + * @see #getPendingRoleHolderRemovalAssociationsForUser(int) + */ + @GuardedBy("mRevokedAssociationsPendingRoleHolderRemoval") + private final PerUserAssociationSet mRevokedAssociationsPendingRoleHolderRemoval = + new PerUserAssociationSet(); + /** + * Contains uid-s of packages pending to be removed from the role holder list (after + * revocation of an association), which will happen one the package is no longer visible to the + * user. + * For quicker uid -> (userId, packageName) look-up this is not a {@code Set} but + * a {@code Map} which maps uid-s to packageName-s (userId-s can be derived + * from uid-s using {@link UserHandle#getUserId(int)}). + * + * @see #maybeRemoveRoleHolderForAssociation(AssociationInfo) + * @see #addToPendingRoleHolderRemoval(AssociationInfo) + * @see #removeFromPendingRoleHolderRemoval(AssociationInfo) + */ + @GuardedBy("mRevokedAssociationsPendingRoleHolderRemoval") + private final Map mUidsPendingRoleHolderRemoval = new HashMap<>(); + private final RemoteCallbackList mListeners = new RemoteCallbackList<>(); public CompanionDeviceManagerService(Context context) { super(context); + mActivityManager = context.getSystemService(ActivityManager.class); mPowerWhitelistManager = context.getSystemService(PowerWhitelistManager.class); mAppOpsManager = IAppOpsService.Stub.asInterface( ServiceManager.getService(Context.APP_OPS_SERVICE)); @@ -175,6 +211,9 @@ public class CompanionDeviceManagerService extends SystemService { mUserPersistenceHandler = new PersistUserStateHandler(); mAssociationStore = new AssociationStoreImpl(); mSystemDataTransferRequestStore = new SystemDataTransferRequestStore(); + + mOnPackageVisibilityChangeListener = + new OnPackageVisibilityChangeListener(mActivityManager); } @Override @@ -216,7 +255,33 @@ public class CompanionDeviceManagerService extends SystemService { mUserManager.getAliveUsers(), allAssociations, mPreviouslyUsedIds); } - mAssociationStore.setAssociations(allAssociations); + final Set activeAssociations = + new ArraySet<>(/* capacity */ allAssociations.size()); + // A set contains the userIds that need to persist state after remove the app + // from the list of role holders. + final Set usersToPersistStateFor = new ArraySet<>(); + + for (AssociationInfo association : allAssociations) { + if (!association.isRevoked()) { + activeAssociations.add(association); + } else if (maybeRemoveRoleHolderForAssociation(association)) { + // Nothing more to do here, but we'll need to persist all the associations to the + // disk afterwards. + usersToPersistStateFor.add(association.getUserId()); + } else { + addToPendingRoleHolderRemoval(association); + } + } + + mAssociationStore.setAssociations(activeAssociations); + + // IMPORTANT: only do this AFTER mAssociationStore.setAssociations(), because + // persistStateForUser() queries AssociationStore. + // (If persistStateForUser() is invoked before mAssociationStore.setAssociations() it + // would effectively just clear-out all the persisted associations). + for (int userId : usersToPersistStateFor) { + persistStateForUser(userId); + } } @Override @@ -366,10 +431,18 @@ public class CompanionDeviceManagerService extends SystemService { } private void persistStateForUser(@UserIdInt int userId) { - final List updatedAssociations = - mAssociationStore.getAssociationsForUser(userId); + // We want to store both active associations and the revoked (removed) association that we + // are keeping around for the final clean-up (delayed role holder removal). + final List allAssociations; + // Start with the active associations - these we can get from the AssociationStore. + allAssociations = new ArrayList<>( + mAssociationStore.getAssociationsForUser(userId)); + // ... and add the revoked (removed) association, that are yet to be permanently removed. + allAssociations.addAll(getPendingRoleHolderRemovalAssociationsForUser(userId)); + final Map> usedIdsForUser = getPreviouslyUsedIdsForUser(userId); - mPersistentStore.persistStateForUser(userId, updatedAssociations, usedIdsForUser); + + mPersistentStore.persistStateForUser(userId, allAssociations, usedIdsForUser); } private void notifyListeners( @@ -437,13 +510,17 @@ public class CompanionDeviceManagerService extends SystemService { removalWindow = ASSOCIATION_REMOVAL_TIME_WINDOW_DEFAULT; } - for (AssociationInfo ai : mAssociationStore.getAssociations()) { - if (!ai.isSelfManaged()) continue; - final boolean isInactive = currentTime - ai.getLastTimeConnectedMs() >= removalWindow; - if (isInactive) { - Slog.i(TAG, "Removing inactive self-managed association: " + ai.getId()); - disassociateInternal(ai.getId()); - } + for (AssociationInfo association : mAssociationStore.getAssociations()) { + if (!association.isSelfManaged()) continue; + + final boolean isInactive = + currentTime - association.getLastTimeConnectedMs() >= removalWindow; + if (!isInactive) continue; + + final int id = association.getId(); + + Slog.i(TAG, "Removing inactive self-managed association id=" + id); + disassociateInternal(id); } } @@ -708,7 +785,7 @@ public class CompanionDeviceManagerService extends SystemService { enforceCallerIsSystemOr(userId, packageName); AssociationInfo association = mAssociationStore.getAssociationsForPackageWithAddress( - userId, packageName, deviceAddress); + userId, packageName, deviceAddress); if (association == null) { throw new RemoteException(new DeviceNotAssociatedException("App " + packageName @@ -768,7 +845,7 @@ public class CompanionDeviceManagerService extends SystemService { enforceUsesCompanionDeviceFeature(getContext(), userId, callingPackage); checkState(!ArrayUtils.isEmpty( - mAssociationStore.getAssociationsForPackage(userId, callingPackage)), + mAssociationStore.getAssociationsForPackage(userId, callingPackage)), "App must have an association before calling this API"); } @@ -828,8 +905,8 @@ public class CompanionDeviceManagerService extends SystemService { final long timestamp = System.currentTimeMillis(); final AssociationInfo association = new AssociationInfo(id, userId, packageName, - macAddress, displayName, deviceProfile, selfManaged, false, timestamp, - Long.MAX_VALUE); + macAddress, displayName, deviceProfile, selfManaged, + /* notifyOnDeviceNearby */ false, /* revoked */ false, timestamp, Long.MAX_VALUE); Slog.i(TAG, "New CDM association created=" + association); mAssociationStore.addAssociation(association); @@ -841,6 +918,11 @@ public class CompanionDeviceManagerService extends SystemService { updateSpecialAccessPermissionForAssociatedPackage(association); logCreateAssociation(deviceProfile); + + // Don't need to update the mRevokedAssociationsPendingRoleHolderRemoval since + // maybeRemoveRoleHolderForAssociation in PackageInactivityListener will handle the case + // that there are other devices with the same profile, so the role holder won't be removed. + return association; } @@ -921,39 +1003,187 @@ public class CompanionDeviceManagerService extends SystemService { final String packageName = association.getPackageName(); final String deviceProfile = association.getDeviceProfile(); + if (!maybeRemoveRoleHolderForAssociation(association)) { + // Need to remove the app from list of the role holders, but will have to do it later + // (the app is in foreground at the moment). + addToPendingRoleHolderRemoval(association); + } + + // Need to check if device still present now because CompanionDevicePresenceMonitor will + // remove current connected device after mAssociationStore.removeAssociation final boolean wasPresent = mDevicePresenceMonitor.isDevicePresent(associationId); // Removing the association. mAssociationStore.removeAssociation(associationId); + // Do not need to persistUserState since CompanionDeviceManagerService will get callback + // from #onAssociationChanged, and it will handle the persistUserState which including + // active and revoked association. logRemoveAssociation(deviceProfile); // Remove all the system data transfer requests for the association. mSystemDataTransferRequestStore.removeRequestsByAssociationId(userId, associationId); - final List otherAssociations = - mAssociationStore.getAssociationsForPackage(userId, packageName); - - // Check if the package is associated with other devices with the same profile. - // If not: take away the role. - if (deviceProfile != null) { - final boolean shouldKeepTheRole = any(otherAssociations, - it -> deviceProfile.equals(it.getDeviceProfile())); - if (!shouldKeepTheRole) { - Binder.withCleanCallingIdentity(() -> - removeRoleHolderForAssociation(getContext(), association)); - } - } - if (!wasPresent || !association.isNotifyOnDeviceNearby()) return; // The device was connected and the app was notified: check if we need to unbind the app // now. - final boolean shouldStayBound = any(otherAssociations, + final boolean shouldStayBound = any( + mAssociationStore.getAssociationsForPackage(userId, packageName), it -> it.isNotifyOnDeviceNearby() && mDevicePresenceMonitor.isDevicePresent(it.getId())); if (shouldStayBound) return; mCompanionAppController.unbindCompanionApplication(userId, packageName); } + /** + * First, checks if the companion application should be removed from the list role holders when + * upon association's removal, i.e.: association's profile (matches the role) is not null, + * the application does not have other associations with the same profile, etc. + * + *

+ * Then, if establishes that the application indeed has to be removed from the list of the role + * holders, checks if it could be done right now - + * {@link android.app.role.RoleManager#removeRoleHolderAsUser(String, String, int, UserHandle, java.util.concurrent.Executor, java.util.function.Consumer) RoleManager#removeRoleHolderAsUser()} + * will kill the application's process, which leads poor user experience if the application was + * in foreground when this happened, to avoid this CDMS delays invoking + * {@code RoleManager.removeRoleHolderAsUser()} until the app is no longer in foreground. + * + * @return {@code true} if the application does NOT need be removed from the list of the role + * holders OR if the application was successfully removed from the list of role holders. + * I.e.: from the role-management perspective the association is done with. + * {@code false} if the application needs to be removed from the list of role the role + * holders, BUT it CDMS would prefer to do it later. + * I.e.: application is in the foreground at the moment, but invoking + * {@code RoleManager.removeRoleHolderAsUser()} will kill the application's process, + * which would lead to the poor UX, hence need to try later. + */ + + private boolean maybeRemoveRoleHolderForAssociation(@NonNull AssociationInfo association) { + if (DEBUG) Log.d(TAG, "maybeRemoveRoleHolderForAssociation() association=" + association); + + final String deviceProfile = association.getDeviceProfile(); + if (deviceProfile == null) { + // No role was granted to for this association, there is nothing else we need to here. + return true; + } + + // Check if the applications is associated with another devices with the profile. If so, + // it should remain the role holder. + final int id = association.getId(); + final int userId = association.getUserId(); + final String packageName = association.getPackageName(); + final boolean roleStillInUse = any( + mAssociationStore.getAssociationsForPackage(userId, packageName), + it -> deviceProfile.equals(it.getDeviceProfile()) && id != it.getId()); + if (roleStillInUse) { + // Application should remain a role holder, there is nothing else we need to here. + return true; + } + + final int packageProcessImportance = getPackageProcessImportance(userId, packageName); + if (packageProcessImportance <= IMPORTANCE_VISIBLE) { + // Need to remove the app from the list of role holders, but the process is visible to + // the user at the moment, so we'll need to it later: log and return false. + Slog.i(TAG, "Cannot remove role holder for the removed association id=" + id + + " now - process is visible."); + return false; + } + + removeRoleHolderForAssociation(getContext(), association); + return true; + } + + private int getPackageProcessImportance(@UserIdInt int userId, @NonNull String packageName) { + return Binder.withCleanCallingIdentity(() -> { + final int uid = + mPackageManagerInternal.getPackageUid(packageName, /* flags */0, userId); + return mActivityManager.getUidImportance(uid); + }); + } + + /** + * Set revoked flag for active association and add the revoked association and the uid into + * the caches. + * + * @see #mRevokedAssociationsPendingRoleHolderRemoval + * @see #mUidsPendingRoleHolderRemoval + * @see OnPackageVisibilityChangeListener + */ + private void addToPendingRoleHolderRemoval(@NonNull AssociationInfo association) { + // First: set revoked flag. + association = AssociationInfo.builder(association) + .setRevoked(true) + .build(); + + final String packageName = association.getPackageName(); + final int userId = association.getUserId(); + final int uid = mPackageManagerInternal.getPackageUid(packageName, /* flags */0, userId); + + // Second: add to the set. + synchronized (mRevokedAssociationsPendingRoleHolderRemoval) { + mRevokedAssociationsPendingRoleHolderRemoval.forUser(association.getUserId()) + .add(association); + if (!mUidsPendingRoleHolderRemoval.containsKey(uid)) { + mUidsPendingRoleHolderRemoval.put(uid, packageName); + + if (mUidsPendingRoleHolderRemoval.size() == 1) { + // Just added first uid: start the listener + mOnPackageVisibilityChangeListener.startListening(); + } + } + } + } + + /** + * Remove the revoked association form the cache and also remove the uid form the map if + * there are other associations with the same package still pending for role holder removal. + * + * @see #mRevokedAssociationsPendingRoleHolderRemoval + * @see #mUidsPendingRoleHolderRemoval + * @see OnPackageVisibilityChangeListener + */ + private void removeFromPendingRoleHolderRemoval(@NonNull AssociationInfo association) { + final String packageName = association.getPackageName(); + final int userId = association.getUserId(); + final int uid = mPackageManagerInternal.getPackageUid(packageName, /* flags */0, userId); + + synchronized (mRevokedAssociationsPendingRoleHolderRemoval) { + mRevokedAssociationsPendingRoleHolderRemoval.forUser(userId) + .remove(association); + + final boolean shouldKeepUidForRemoval = any( + getPendingRoleHolderRemovalAssociationsForUser(userId), + ai -> packageName.equals(ai.getPackageName())); + // Do not remove the uid form the map since other associations with + // the same packageName still pending for role holder removal. + if (!shouldKeepUidForRemoval) { + mUidsPendingRoleHolderRemoval.remove(uid); + } + + if (mUidsPendingRoleHolderRemoval.isEmpty()) { + // The set is empty now - can "turn off" the listener. + mOnPackageVisibilityChangeListener.stopListening(); + } + } + } + + /** + * @return a copy of the revoked associations set (safeguarding against + * {@code ConcurrentModificationException}-s). + */ + private @NonNull Set getPendingRoleHolderRemovalAssociationsForUser( + @UserIdInt int userId) { + synchronized (mRevokedAssociationsPendingRoleHolderRemoval) { + // Return a copy. + return new ArraySet<>(mRevokedAssociationsPendingRoleHolderRemoval.forUser(userId)); + } + } + + private String getPackageNameByUid(int uid) { + synchronized (mRevokedAssociationsPendingRoleHolderRemoval) { + return mUidsPendingRoleHolderRemoval.get(uid); + } + } + private void updateSpecialAccessPermissionForAssociatedPackage(AssociationInfo association) { final PackageInfo packageInfo = getPackageInfo(getContext(), association.getUserId(), association.getPackageName()); @@ -1171,4 +1401,80 @@ public class CompanionDeviceManagerService extends SystemService { persistStateForUser(userId); } } + + /** + * An OnUidImportanceListener class which watches the importance of the packages. + * In this class, we ONLY interested in the importance of the running process is greater than + * {@link RunningAppProcessInfo.IMPORTANCE_VISIBLE} for the uids have been added into the + * {@link mUidsPendingRoleHolderRemoval}. Lastly remove the role holder for the revoked + * associations for the same packages. + * + * @see #maybeRemoveRoleHolderForAssociation(AssociationInfo) + * @see #removeFromPendingRoleHolderRemoval(AssociationInfo) + * @see #getPendingRoleHolderRemovalAssociationsForUser(int) + */ + private class OnPackageVisibilityChangeListener implements + ActivityManager.OnUidImportanceListener { + final @NonNull ActivityManager mAm; + + OnPackageVisibilityChangeListener(@NonNull ActivityManager am) { + this.mAm = am; + } + + void startListening() { + Binder.withCleanCallingIdentity( + () -> mAm.addOnUidImportanceListener( + /* listener */ OnPackageVisibilityChangeListener.this, + RunningAppProcessInfo.IMPORTANCE_VISIBLE)); + } + + void stopListening() { + Binder.withCleanCallingIdentity( + () -> mAm.removeOnUidImportanceListener( + /* listener */ OnPackageVisibilityChangeListener.this)); + } + + @Override + public void onUidImportance(int uid, int importance) { + if (importance <= RunningAppProcessInfo.IMPORTANCE_VISIBLE) { + // The lower the importance value the more "important" the process is. + // We are only interested when the process ceases to be visible. + return; + } + + final String packageName = getPackageNameByUid(uid); + if (packageName == null) { + // Not interested in this uid. + return; + } + + final int userId = UserHandle.getUserId(uid); + + boolean needToPersistStateForUser = false; + + for (AssociationInfo association : + getPendingRoleHolderRemovalAssociationsForUser(userId)) { + if (!packageName.equals(association.getPackageName())) continue; + + if (!maybeRemoveRoleHolderForAssociation(association)) { + // Did not remove the role holder, will have to try again later. + continue; + } + + removeFromPendingRoleHolderRemoval(association); + needToPersistStateForUser = true; + } + + if (needToPersistStateForUser) { + mUserPersistenceHandler.postPersistUserState(userId); + } + } + } + + private static class PerUserAssociationSet extends PerUser> { + @Override + protected @NonNull Set create(int userId) { + return new ArraySet<>(); + } + } } diff --git a/services/companion/java/com/android/server/companion/PersistentDataStore.java b/services/companion/java/com/android/server/companion/PersistentDataStore.java index 4d42838fff50b..4b56c1b280362 100644 --- a/services/companion/java/com/android/server/companion/PersistentDataStore.java +++ b/services/companion/java/com/android/server/companion/PersistentDataStore.java @@ -103,7 +103,7 @@ import java.util.concurrent.ConcurrentMap; * Since Android T the data is stored to "companion_device_manager.xml" file in * {@link Environment#getDataSystemDeDirectory(int) /data/system_de/}. * - * See {@link #getStorageFileForUser(int)} + * See {@link DataStoreUtils#getBaseStorageFileForUser(int, String)} * *

* Since Android T the data is stored using the v1 schema. @@ -120,7 +120,7 @@ import java.util.concurrent.ConcurrentMap; *

  • {@link #readPreviouslyUsedIdsV1(TypedXmlPullParser, Map) readPreviouslyUsedIdsV1()} * * - * The following snippet is a sample of a file that is using v0 schema. + * The following snippet is a sample of a file that is using v1 schema. * 
    {@code
  * 
  *     
@@ -130,6 +130,8 @@ import java.util.concurrent.ConcurrentMap;
  *             mac_address="AA:BB:CC:DD:EE:00"
  *             self_managed="false"
  *             notify_device_nearby="false"
+ *             revoked="false"
+ *             last_time_connected="1634641160229"
  *             time_approved="1634389553216"/>
  *
  *         
  *     
  *
@@ -178,6 +182,7 @@ final class PersistentDataStore {
     private static final String XML_ATTR_PROFILE = "profile";
     private static final String XML_ATTR_SELF_MANAGED = "self_managed";
     private static final String XML_ATTR_NOTIFY_DEVICE_NEARBY = "notify_device_nearby";
+    private static final String XML_ATTR_REVOKED = "revoked";
     private static final String XML_ATTR_TIME_APPROVED = "time_approved";
     private static final String XML_ATTR_LAST_TIME_CONNECTED = "last_time_connected";
 
@@ -415,7 +420,8 @@ final class PersistentDataStore {
 
         out.add(new AssociationInfo(associationId, userId, appPackage,
                 MacAddress.fromString(deviceAddress), null, profile,
-                /* managedByCompanionApp */false, notify, timeApproved, Long.MAX_VALUE));
+                /* managedByCompanionApp */ false, notify, /* revoked */ false, timeApproved,
+                Long.MAX_VALUE));
     }
 
     private static void readAssociationsV1(@NonNull TypedXmlPullParser parser,
@@ -444,13 +450,14 @@ final class PersistentDataStore {
         final String displayName = readStringAttribute(parser, XML_ATTR_DISPLAY_NAME);
         final boolean selfManaged = readBooleanAttribute(parser, XML_ATTR_SELF_MANAGED);
         final boolean notify = readBooleanAttribute(parser, XML_ATTR_NOTIFY_DEVICE_NEARBY);
+        final boolean revoked = readBooleanAttribute(parser, XML_ATTR_REVOKED, false);
         final long timeApproved = readLongAttribute(parser, XML_ATTR_TIME_APPROVED, 0L);
         final long lastTimeConnected = readLongAttribute(
                 parser, XML_ATTR_LAST_TIME_CONNECTED, Long.MAX_VALUE);
 
         final AssociationInfo associationInfo = createAssociationInfoNoThrow(associationId, userId,
-                appPackage, macAddress, displayName, profile, selfManaged, notify, timeApproved,
-                lastTimeConnected);
+                appPackage, macAddress, displayName, profile, selfManaged, notify, revoked,
+                timeApproved, lastTimeConnected);
         if (associationInfo != null) {
             out.add(associationInfo);
         }
@@ -503,6 +510,8 @@ final class PersistentDataStore {
         writeBooleanAttribute(serializer, XML_ATTR_SELF_MANAGED, a.isSelfManaged());
         writeBooleanAttribute(
                 serializer, XML_ATTR_NOTIFY_DEVICE_NEARBY, a.isNotifyOnDeviceNearby());
+        writeBooleanAttribute(
+                serializer, XML_ATTR_REVOKED, a.isRevoked());
         writeLongAttribute(serializer, XML_ATTR_TIME_APPROVED, a.getTimeApprovedMs());
         writeLongAttribute(
                 serializer, XML_ATTR_LAST_TIME_CONNECTED, a.getLastTimeConnectedMs());
@@ -544,11 +553,12 @@ final class PersistentDataStore {
     private static AssociationInfo createAssociationInfoNoThrow(int associationId,
             @UserIdInt int userId, @NonNull String appPackage, @Nullable MacAddress macAddress,
             @Nullable CharSequence displayName, @Nullable String profile, boolean selfManaged,
-            boolean notify, long timeApproved, long lastTimeConnected) {
+            boolean notify, boolean revoked, long timeApproved, long lastTimeConnected) {
         AssociationInfo associationInfo = null;
         try {
             associationInfo = new AssociationInfo(associationId, userId, appPackage, macAddress,
-                    displayName, profile, selfManaged, notify, timeApproved, lastTimeConnected);
+                    displayName, profile, selfManaged, notify, revoked, timeApproved,
+                    lastTimeConnected);
         } catch (Exception e) {
             if (DEBUG) Log.w(TAG, "Could not create AssociationInfo", e);
         }
diff --git a/services/companion/java/com/android/server/companion/RolesUtils.java b/services/companion/java/com/android/server/companion/RolesUtils.java
index 35488a80b78b9..0fff3f4885625 100644
--- a/services/companion/java/com/android/server/companion/RolesUtils.java
+++ b/services/companion/java/com/android/server/companion/RolesUtils.java
@@ -85,6 +85,8 @@ final class RolesUtils {
         final int userId = associationInfo.getUserId();
         final UserHandle userHandle = UserHandle.of(userId);
 
+        Slog.i(TAG, "Removing CDM role holder, role=" + deviceProfile
+                + ", package=u" + userId + "\\" + packageName);
         roleManager.removeRoleHolderAsUser(deviceProfile, packageName,
                 MANAGE_HOLDERS_FLAG_DONT_KILL_APP, userHandle, context.getMainExecutor(),
                 success -> {