Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9ba08918 authored by Evan Chen's avatar Evan Chen
Browse files

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
parent c9bdb0e7

core/java/android/companion/AssociationInfo.java

+44 −2
Original line number Original line Diff line number Diff line
@@ -55,6 +55,14 @@ public final class AssociationInfo implements Parcelable { 




    private final boolean mSelfManaged;

    private final boolean mSelfManaged;

    private final boolean mNotifyOnDeviceNearby;

    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;

    private final long mTimeApprovedMs;

    /**

    /**

     * A long value indicates the last time connected reported by selfManaged devices

     * 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,

    public AssociationInfo(int id, @UserIdInt int userId, @NonNull String packageName,

            @Nullable MacAddress macAddress, @Nullable CharSequence displayName,

            @Nullable MacAddress macAddress, @Nullable CharSequence displayName,

            @Nullable String deviceProfile, boolean selfManaged, boolean notifyOnDeviceNearby,

            @Nullable String deviceProfile, boolean selfManaged, boolean notifyOnDeviceNearby,

            long timeApprovedMs, long lastTimeConnectedMs) {

            boolean revoked, long timeApprovedMs, long lastTimeConnectedMs) {

        if (id <= 0) {

        if (id <= 0) {

            throw new IllegalArgumentException("Association ID should be greater than 0");

            throw new IllegalArgumentException("Association ID should be greater than 0");

        }

        }
@@ -91,6 +99,7 @@ public final class AssociationInfo implements Parcelable { 




        mSelfManaged = selfManaged;

        mSelfManaged = selfManaged;

        mNotifyOnDeviceNearby = notifyOnDeviceNearby;

        mNotifyOnDeviceNearby = notifyOnDeviceNearby;

        mRevoked = revoked;

        mTimeApprovedMs = timeApprovedMs;

        mTimeApprovedMs = timeApprovedMs;

        mLastTimeConnectedMs = lastTimeConnectedMs;

        mLastTimeConnectedMs = lastTimeConnectedMs;

    }

    }
@@ -175,6 +184,14 @@ public final class AssociationInfo implements Parcelable { 
        return mUserId == userId && Objects.equals(mPackageName, packageName);

        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.

     * @return the last time self reported disconnected for selfManaged only.

     * @hide

     * @hide
@@ -244,6 +261,7 @@ public final class AssociationInfo implements Parcelable { 
                + ", mDeviceProfile='" + mDeviceProfile + '\''

                + ", mDeviceProfile='" + mDeviceProfile + '\''

                + ", mSelfManaged=" + mSelfManaged

                + ", mSelfManaged=" + mSelfManaged

                + ", mNotifyOnDeviceNearby=" + mNotifyOnDeviceNearby

                + ", mNotifyOnDeviceNearby=" + mNotifyOnDeviceNearby

                + ", mRevoked=" + mRevoked

                + ", mTimeApprovedMs=" + new Date(mTimeApprovedMs)

                + ", mTimeApprovedMs=" + new Date(mTimeApprovedMs)

                + ", mLastTimeConnectedMs=" + (

                + ", mLastTimeConnectedMs=" + (

                    mLastTimeConnectedMs == Long.MAX_VALUE

                    mLastTimeConnectedMs == Long.MAX_VALUE
@@ -260,6 +278,7 @@ public final class AssociationInfo implements Parcelable { 
                && mUserId == that.mUserId

                && mUserId == that.mUserId

                && mSelfManaged == that.mSelfManaged

                && mSelfManaged == that.mSelfManaged

                && mNotifyOnDeviceNearby == that.mNotifyOnDeviceNearby

                && mNotifyOnDeviceNearby == that.mNotifyOnDeviceNearby

                && mRevoked == that.mRevoked

                && mTimeApprovedMs == that.mTimeApprovedMs

                && mTimeApprovedMs == that.mTimeApprovedMs

                && mLastTimeConnectedMs == that.mLastTimeConnectedMs

                && mLastTimeConnectedMs == that.mLastTimeConnectedMs

                && Objects.equals(mPackageName, that.mPackageName)

                && Objects.equals(mPackageName, that.mPackageName)
@@ -271,7 +290,7 @@ public final class AssociationInfo implements Parcelable { 
    @Override

    @Override

    public int hashCode() {

    public int hashCode() {

        return Objects.hash(mId, mUserId, mPackageName, mDeviceMacAddress, mDisplayName,

        return Objects.hash(mId, mUserId, mPackageName, mDeviceMacAddress, mDisplayName,

                mDeviceProfile, mSelfManaged, mNotifyOnDeviceNearby, mTimeApprovedMs,

                mDeviceProfile, mSelfManaged, mNotifyOnDeviceNearby, mRevoked, mTimeApprovedMs,

                mLastTimeConnectedMs);

                mLastTimeConnectedMs);

    }

    }
@@ -293,6 +312,7 @@ public final class AssociationInfo implements Parcelable { 




        dest.writeBoolean(mSelfManaged);

        dest.writeBoolean(mSelfManaged);

        dest.writeBoolean(mNotifyOnDeviceNearby);

        dest.writeBoolean(mNotifyOnDeviceNearby);

        dest.writeBoolean(mRevoked);

        dest.writeLong(mTimeApprovedMs);

        dest.writeLong(mTimeApprovedMs);

        dest.writeLong(mLastTimeConnectedMs);

        dest.writeLong(mLastTimeConnectedMs);

    }

    }
@@ -309,6 +329,7 @@ public final class AssociationInfo implements Parcelable { 




        mSelfManaged = in.readBoolean();

        mSelfManaged = in.readBoolean();

        mNotifyOnDeviceNearby = in.readBoolean();

        mNotifyOnDeviceNearby = in.readBoolean();

        mRevoked = in.readBoolean();

        mTimeApprovedMs = in.readLong();

        mTimeApprovedMs = in.readLong();

        mLastTimeConnectedMs = in.readLong();

        mLastTimeConnectedMs = in.readLong();

    }

    }
@@ -352,11 +373,13 @@ public final class AssociationInfo implements Parcelable { 
        @NonNull

        @NonNull

        private final AssociationInfo mOriginalInfo;

        private final AssociationInfo mOriginalInfo;

        private boolean mNotifyOnDeviceNearby;

        private boolean mNotifyOnDeviceNearby;

        private boolean mRevoked;

        private long mLastTimeConnectedMs;

        private long mLastTimeConnectedMs;





        private Builder(@NonNull AssociationInfo info) {

        private Builder(@NonNull AssociationInfo info) {

            mOriginalInfo = info;

            mOriginalInfo = info;

            mNotifyOnDeviceNearby = info.mNotifyOnDeviceNearby;

            mNotifyOnDeviceNearby = info.mNotifyOnDeviceNearby;

            mRevoked = info.mRevoked;

            mLastTimeConnectedMs = info.mLastTimeConnectedMs;

            mLastTimeConnectedMs = info.mLastTimeConnectedMs;

        }

        }
@@ -387,6 +410,17 @@ public final class AssociationInfo implements Parcelable { 
            return this;

            return this;

        }

        }





        /**

         * Should only be used by the CompanionDeviceManagerService.

         * @hide

         */

        @Override

        @NonNull

        public Builder setRevoked(boolean revoked) {

            mRevoked = revoked;

            return this;

        }



        /**

        /**

         * @hide

         * @hide

         */

         */
@@ -401,6 +435,7 @@ public final class AssociationInfo implements Parcelable { 
                    mOriginalInfo.mDeviceProfile,

                    mOriginalInfo.mDeviceProfile,

                    mOriginalInfo.mSelfManaged,

                    mOriginalInfo.mSelfManaged,

                    mNotifyOnDeviceNearby,

                    mNotifyOnDeviceNearby,

                    mRevoked,

                    mOriginalInfo.mTimeApprovedMs,

                    mOriginalInfo.mTimeApprovedMs,

                    mLastTimeConnectedMs

                    mLastTimeConnectedMs

            );

            );
@@ -433,5 +468,12 @@ public final class AssociationInfo implements Parcelable { 
         */

         */

        @NonNull

        @NonNull

        Builder setLastTimeConnected(long lastTimeConnectedMs);

        Builder setLastTimeConnected(long lastTimeConnectedMs);



        /**

         * Should only be used by the CompanionDeviceManagerService.

         * @hide

         */

        @NonNull

        Builder setRevoked(boolean revoked);

    }

    }

}
 
}

services/companion/java/com/android/server/companion/AssociationStoreImpl.java

+16 −0
Original line number Original line Diff line number Diff line
@@ -73,6 +73,9 @@ class AssociationStoreImpl implements AssociationStore { 
    private final Set<OnChangeListener> mListeners = new LinkedHashSet<>();

    private final Set<OnChangeListener> mListeners = new LinkedHashSet<>();





    void addAssociation(@NonNull AssociationInfo association) {

    void addAssociation(@NonNull AssociationInfo association) {

        // Validity check first.

        checkNotRevoked(association);



        final int id = association.getId();

        final int id = association.getId();





        if (DEBUG) {

        if (DEBUG) {
@@ -99,6 +102,9 @@ class AssociationStoreImpl implements AssociationStore { 
    }

    }





    void updateAssociation(@NonNull AssociationInfo updated) {

    void updateAssociation(@NonNull AssociationInfo updated) {

        // Validity check first.

        checkNotRevoked(updated);



        final int id = updated.getId();

        final int id = updated.getId();





        if (DEBUG) {

        if (DEBUG) {
@@ -292,6 +298,9 @@ class AssociationStoreImpl implements AssociationStore { 
    }

    }





    void setAssociations(Collection<AssociationInfo> allAssociations) {

    void setAssociations(Collection<AssociationInfo> allAssociations) {

        // Validity check first.

        allAssociations.forEach(AssociationStoreImpl::checkNotRevoked);



        if (DEBUG) {

        if (DEBUG) {

            Log.i(TAG, "setAssociations() n=" + allAssociations.size());

            Log.i(TAG, "setAssociations() n=" + allAssociations.size());

            final StringJoiner stringJoiner = new StringJoiner(", ");

            final StringJoiner stringJoiner = new StringJoiner(", ");
@@ -324,4 +333,11 @@ class AssociationStoreImpl implements AssociationStore { 
        mAddressMap.clear();

        mAddressMap.clear();

        mCachedPerUser.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");

        }

    }

}
 
}

services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java

+336 −30

File changed.

Preview size limit exceeded, changes collapsed.

services/companion/java/com/android/server/companion/PersistentDataStore.java

+17 −7
Original line number Original line Diff line number Diff line
@@ -103,7 +103,7 @@ import java.util.concurrent.ConcurrentMap; 
 * Since Android T the data is stored to "companion_device_manager.xml" file in

 * Since Android T the data is stored to "companion_device_manager.xml" file in

 * {@link Environment#getDataSystemDeDirectory(int) /data/system_de/}.

 * {@link Environment#getDataSystemDeDirectory(int) /data/system_de/}.

 *

 *

 * See {@link #getStorageFileForUser(int)}

 * See {@link DataStoreUtils#getBaseStorageFileForUser(int, String)}

 *

 *

 * <p>

 * <p>

 * Since Android T the data is stored using the v1 schema.

 * Since Android T the data is stored using the v1 schema.
@@ -120,7 +120,7 @@ import java.util.concurrent.ConcurrentMap; 
 * <li> {@link #readPreviouslyUsedIdsV1(TypedXmlPullParser, Map) readPreviouslyUsedIdsV1()}

 * <li> {@link #readPreviouslyUsedIdsV1(TypedXmlPullParser, Map) readPreviouslyUsedIdsV1()}

 * </ul>

 * </ul>

 *

 *

 * 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.

 * <pre>{@code

 * <pre>{@code

 * <state persistence-version="1">

 * <state persistence-version="1">

 *     <associations>

 *     <associations>
@@ -130,6 +130,8 @@ import java.util.concurrent.ConcurrentMap; 
 *             mac_address="AA:BB:CC:DD:EE:00"

 *             mac_address="AA:BB:CC:DD:EE:00"

 *             self_managed="false"

 *             self_managed="false"

 *             notify_device_nearby="false"

 *             notify_device_nearby="false"

 *             revoked="false"

 *             last_time_connected="1634641160229"

 *             time_approved="1634389553216"/>

 *             time_approved="1634389553216"/>

 *

 *

 *         <association

 *         <association
@@ -139,6 +141,8 @@ import java.util.concurrent.ConcurrentMap; 
 *             display_name="Jhon's Chromebook"

 *             display_name="Jhon's Chromebook"

 *             self_managed="true"

 *             self_managed="true"

 *             notify_device_nearby="false"

 *             notify_device_nearby="false"

 *             revoked="false"

 *             last_time_connected="1634641160229"

 *             time_approved="1634641160229"/>

 *             time_approved="1634641160229"/>

 *     </associations>

 *     </associations>

 *

 *
@@ -178,6 +182,7 @@ final class PersistentDataStore { 
    private static final String XML_ATTR_PROFILE = "profile";

    private static final String XML_ATTR_PROFILE = "profile";

    private static final String XML_ATTR_SELF_MANAGED = "self_managed";

    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_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_TIME_APPROVED = "time_approved";

    private static final String XML_ATTR_LAST_TIME_CONNECTED = "last_time_connected";

    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,

        out.add(new AssociationInfo(associationId, userId, appPackage,

                MacAddress.fromString(deviceAddress), null, profile,

                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,

    private static void readAssociationsV1(@NonNull TypedXmlPullParser parser,
@@ -444,13 +450,14 @@ final class PersistentDataStore { 
        final String displayName = readStringAttribute(parser, XML_ATTR_DISPLAY_NAME);

        final String displayName = readStringAttribute(parser, XML_ATTR_DISPLAY_NAME);

        final boolean selfManaged = readBooleanAttribute(parser, XML_ATTR_SELF_MANAGED);

        final boolean selfManaged = readBooleanAttribute(parser, XML_ATTR_SELF_MANAGED);

        final boolean notify = readBooleanAttribute(parser, XML_ATTR_NOTIFY_DEVICE_NEARBY);

        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 timeApproved = readLongAttribute(parser, XML_ATTR_TIME_APPROVED, 0L);

        final long lastTimeConnected = readLongAttribute(

        final long lastTimeConnected = readLongAttribute(

                parser, XML_ATTR_LAST_TIME_CONNECTED, Long.MAX_VALUE);

                parser, XML_ATTR_LAST_TIME_CONNECTED, Long.MAX_VALUE);





        final AssociationInfo associationInfo = createAssociationInfoNoThrow(associationId, userId,

        final AssociationInfo associationInfo = createAssociationInfoNoThrow(associationId, userId,

                appPackage, macAddress, displayName, profile, selfManaged, notify, timeApproved,

                appPackage, macAddress, displayName, profile, selfManaged, notify, revoked,

                lastTimeConnected);

                timeApproved, lastTimeConnected);

        if (associationInfo != null) {

        if (associationInfo != null) {

            out.add(associationInfo);

            out.add(associationInfo);

        }

        }
@@ -503,6 +510,8 @@ final class PersistentDataStore { 
        writeBooleanAttribute(serializer, XML_ATTR_SELF_MANAGED, a.isSelfManaged());

        writeBooleanAttribute(serializer, XML_ATTR_SELF_MANAGED, a.isSelfManaged());

        writeBooleanAttribute(

        writeBooleanAttribute(

                serializer, XML_ATTR_NOTIFY_DEVICE_NEARBY, a.isNotifyOnDeviceNearby());

                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_TIME_APPROVED, a.getTimeApprovedMs());

        writeLongAttribute(

        writeLongAttribute(

                serializer, XML_ATTR_LAST_TIME_CONNECTED, a.getLastTimeConnectedMs());

                serializer, XML_ATTR_LAST_TIME_CONNECTED, a.getLastTimeConnectedMs());
@@ -544,11 +553,12 @@ final class PersistentDataStore { 
    private static AssociationInfo createAssociationInfoNoThrow(int associationId,

    private static AssociationInfo createAssociationInfoNoThrow(int associationId,

            @UserIdInt int userId, @NonNull String appPackage, @Nullable MacAddress macAddress,

            @UserIdInt int userId, @NonNull String appPackage, @Nullable MacAddress macAddress,

            @Nullable CharSequence displayName, @Nullable String profile, boolean selfManaged,

            @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;

        AssociationInfo associationInfo = null;

        try {

        try {

            associationInfo = new AssociationInfo(associationId, userId, appPackage, macAddress,

            associationInfo = new AssociationInfo(associationId, userId, appPackage, macAddress,

                    displayName, profile, selfManaged, notify, timeApproved, lastTimeConnected);

                    displayName, profile, selfManaged, notify, revoked, timeApproved,

                    lastTimeConnected);

        } catch (Exception e) {

        } catch (Exception e) {

            if (DEBUG) Log.w(TAG, "Could not create AssociationInfo", e);

            if (DEBUG) Log.w(TAG, "Could not create AssociationInfo", e);

        }

        }

services/companion/java/com/android/server/companion/RolesUtils.java

+2 −0
Original line number Original line Diff line number Diff line
@@ -85,6 +85,8 @@ final class RolesUtils { 
        final int userId = associationInfo.getUserId();

        final int userId = associationInfo.getUserId();

        final UserHandle userHandle = UserHandle.of(userId);

        final UserHandle userHandle = UserHandle.of(userId);





        Slog.i(TAG, "Removing CDM role holder, role=" + deviceProfile

                + ", package=u" + userId + "\\" + packageName);

        roleManager.removeRoleHolderAsUser(deviceProfile, packageName,

        roleManager.removeRoleHolderAsUser(deviceProfile, packageName,

                MANAGE_HOLDERS_FLAG_DONT_KILL_APP, userHandle, context.getMainExecutor(),

                MANAGE_HOLDERS_FLAG_DONT_KILL_APP, userHandle, context.getMainExecutor(),

                success -> {

                success -> {