From aafc593f8801eb07ea8332e3ef9d3c418a0a6c1d Mon Sep 17 00:00:00 2001 From: Guojing Yuan Date: Fri, 4 Mar 2022 00:04:41 +0000 Subject: [PATCH] [CDM perm sync] Fix user consent flow Fix several issues in order to make the API CL clean. This CL doesn't contain any API changes, but has been tested with the API changes. 1. Renamed isPermissionSyncAllPackages() to getPermissionSyncAllPackages() due to API convension. 2. Changed return type in the request store from List to ArrayList to fix a runtime expection that AbstractList can't add elements. Bug: 213338076 Test: manually added and tested the user consent dialog flow in the CDM test app Change-Id: Ibabb02e797f0ccf5288de976acfcc73ef6f67896 --- .../companion/SystemDataTransferRequest.java | 10 +++---- .../AndroidManifest.xml | 8 ++++++ .../CompanionDeviceDataTransferActivity.java | 6 +---- .../SystemDataTransferProcessor.java | 9 ++++--- .../SystemDataTransferRequestStore.java | 26 +++++++++---------- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/core/java/android/companion/SystemDataTransferRequest.java b/core/java/android/companion/SystemDataTransferRequest.java index 461b4f5f27e6e..ccc720edbe33a 100644 --- a/core/java/android/companion/SystemDataTransferRequest.java +++ b/core/java/android/companion/SystemDataTransferRequest.java @@ -66,8 +66,7 @@ public final class SystemDataTransferRequest implements Parcelable { return mAssociationId; } - @NonNull - public boolean isPermissionSyncAllPackages() { + public boolean getPermissionSyncAllPackages() { return mPermissionSyncAllPackages; } @@ -103,6 +102,7 @@ public final class SystemDataTransferRequest implements Parcelable { + "associationId=" + mAssociationId + ", isPermissionSyncAllPackages=" + mPermissionSyncAllPackages + ", permissionSyncPackages=[" + String.join(",", mPermissionSyncPackages) + "]" + + ", userId=" + mUserId + ", isUserConsented=" + mUserConsented + ")"; } @@ -192,10 +192,10 @@ public final class SystemDataTransferRequest implements Parcelable { /** * Check if two requests have the same data type. */ - public boolean hasSameDataType(SystemDataTransferRequest request) { + public boolean hasSameDataType(@NonNull SystemDataTransferRequest request) { // Check if they are permission sync requests. - if (this.isPermissionSyncAllPackages() || !this.getPermissionSyncPackages().isEmpty()) { - return request.isPermissionSyncAllPackages() || !request.getPermissionSyncPackages() + if (this.getPermissionSyncAllPackages() || !this.getPermissionSyncPackages().isEmpty()) { + return request.getPermissionSyncAllPackages() || !request.getPermissionSyncPackages() .isEmpty(); } return false; diff --git a/packages/CompanionDeviceManager/AndroidManifest.xml b/packages/CompanionDeviceManager/AndroidManifest.xml index 8b5d214f7a105..c1fce7c3dd232 100644 --- a/packages/CompanionDeviceManager/AndroidManifest.xml +++ b/packages/CompanionDeviceManager/AndroidManifest.xml @@ -48,6 +48,14 @@ android:permission="android.permission.BIND_COMPANION_DEVICE_MANAGER_SERVICE" android:theme="@style/ChooserActivity"/> + + diff --git a/packages/CompanionDeviceManager/src/com/android/companiondevicemanager/CompanionDeviceDataTransferActivity.java b/packages/CompanionDeviceManager/src/com/android/companiondevicemanager/CompanionDeviceDataTransferActivity.java index 67efa03b645f4..036ea2e62aff2 100644 --- a/packages/CompanionDeviceManager/src/com/android/companiondevicemanager/CompanionDeviceDataTransferActivity.java +++ b/packages/CompanionDeviceManager/src/com/android/companiondevicemanager/CompanionDeviceDataTransferActivity.java @@ -27,9 +27,7 @@ import android.os.Bundle; import android.os.ResultReceiver; import android.text.Html; import android.util.Log; -import android.view.View; import android.widget.Button; -import android.widget.ListView; import android.widget.TextView; /** @@ -61,8 +59,6 @@ public class CompanionDeviceDataTransferActivity extends Activity { TextView titleView = findViewById(R.id.title); TextView summaryView = findViewById(R.id.summary); - ListView listView = findViewById(R.id.device_list); - listView.setVisibility(View.GONE); Button allowButton = findViewById(R.id.btn_positive); Button disallowButton = findViewById(R.id.btn_negative); @@ -73,7 +69,7 @@ public class CompanionDeviceDataTransferActivity extends Activity { requireNonNull(mRequest); requireNonNull(mCdmServiceReceiver); - if (mRequest.isPermissionSyncAllPackages() + if (mRequest.getPermissionSyncAllPackages() || !mRequest.getPermissionSyncPackages().isEmpty()) { titleView.setText(Html.fromHtml(getString( R.string.permission_sync_confirmation_title), 0)); diff --git a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java index a46b7857f6bc4..c3b8616895873 100644 --- a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java +++ b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java @@ -37,6 +37,7 @@ import android.content.Intent; import android.os.Binder; import android.os.Bundle; import android.os.Handler; +import android.os.RemoteException; import android.os.ResultReceiver; import android.util.Slog; @@ -68,7 +69,7 @@ public class SystemDataTransferProcessor { private final AssociationStore mAssociationStore; private final SystemDataTransferRequestStore mSystemDataTransferRequestStore; - SystemDataTransferProcessor(CompanionDeviceManagerService service, + public SystemDataTransferProcessor(CompanionDeviceManagerService service, AssociationStore associationStore, SystemDataTransferRequestStore systemDataTransferRequestStore) { mContext = service.getContext(); @@ -80,14 +81,14 @@ public class SystemDataTransferProcessor { * Build a PendingIntent of user consent dialog */ public PendingIntent buildSystemDataTransferConfirmationIntent(@UserIdInt int userId, - SystemDataTransferRequest request) throws DeviceNotAssociatedException { + SystemDataTransferRequest request) throws RemoteException { // The association must exist and either belong to the calling package, // or the calling package must hold REQUEST_SYSTEM_DATA_TRANSFER permission. int associationId = request.getAssociationId(); AssociationInfo association = mAssociationStore.getAssociationById(associationId); if (association == null) { - throw new DeviceNotAssociatedException( - "Association id: " + associationId + " doesn't exist."); + throw new RemoteException(new DeviceNotAssociatedException( + "Association id: " + associationId + " doesn't exist.")); } else { // If the package is not the companion app which owns the association, // it must hold REQUEST_SYSTEM_DATA_TRANSFER permission. diff --git a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferRequestStore.java b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferRequestStore.java index 1e29f56d190ec..204fb3df686fa 100644 --- a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferRequestStore.java +++ b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferRequestStore.java @@ -47,7 +47,6 @@ import java.io.FileInputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -103,7 +102,8 @@ public class SystemDataTransferRequestStore { private final Object mLock = new Object(); @GuardedBy("mLock") - private final SparseArray> mCachedPerUser = new SparseArray<>(); + private final SparseArray> mCachedPerUser = + new SparseArray<>(); public SystemDataTransferRequestStore() { mExecutor = Executors.newSingleThreadExecutor(); @@ -127,7 +127,7 @@ public class SystemDataTransferRequestStore { } void writeRequest(@UserIdInt int userId, SystemDataTransferRequest request) { - List cachedRequests; + ArrayList cachedRequests; synchronized (mLock) { // Write to cache cachedRequests = readRequestsFromCache(userId); @@ -139,10 +139,10 @@ public class SystemDataTransferRequestStore { } @GuardedBy("mLock") - private List readRequestsFromCache(@UserIdInt int userId) { - List cachedRequests = mCachedPerUser.get(userId); + private ArrayList readRequestsFromCache(@UserIdInt int userId) { + ArrayList cachedRequests = mCachedPerUser.get(userId); if (cachedRequests == null) { - Future> future = + Future> future = mExecutor.submit(() -> readRequestsFromStore(userId)); try { cachedRequests = future.get(READ_FROM_DISK_TIMEOUT, TimeUnit.SECONDS); @@ -167,7 +167,7 @@ public class SystemDataTransferRequestStore { * @return a list of SystemDataTransferRequest */ @NonNull - private List readRequestsFromStore(@UserIdInt int userId) { + private ArrayList readRequestsFromStore(@UserIdInt int userId) { final AtomicFile file = getStorageFileForUser(userId); Slog.i(LOG_TAG, "Reading SystemDataTransferRequests for user " + userId + " from " + "file=" + file.getBaseFile().getPath()); @@ -177,7 +177,7 @@ public class SystemDataTransferRequestStore { synchronized (file) { if (!file.getBaseFile().exists()) { Slog.d(LOG_TAG, "File does not exist -> Abort"); - return Collections.emptyList(); + return new ArrayList<>(); } try (FileInputStream in = file.openRead()) { final TypedXmlPullParser parser = Xml.resolvePullParser(in); @@ -186,19 +186,19 @@ public class SystemDataTransferRequestStore { return readRequestsFromXml(parser); } catch (XmlPullParserException | IOException e) { Slog.e(LOG_TAG, "Error while reading requests file", e); - return Collections.emptyList(); + return new ArrayList<>(); } } } @NonNull - private List readRequestsFromXml(@NonNull TypedXmlPullParser parser) - throws XmlPullParserException, IOException { + private ArrayList readRequestsFromXml( + @NonNull TypedXmlPullParser parser) throws XmlPullParserException, IOException { if (!isStartOfTag(parser, XML_TAG_REQUESTS)) { throw new XmlPullParserException("The XML doesn't have start tag: " + XML_TAG_REQUESTS); } - List requests = new ArrayList<>(); + ArrayList requests = new ArrayList<>(); while (true) { parser.nextTag(); @@ -286,7 +286,7 @@ public class SystemDataTransferRequestStore { writeIntAttribute(serializer, XML_ATTR_USER_ID, request.getUserId()); writeBooleanAttribute(serializer, XML_ATTR_IS_USER_CONSENTED, request.isUserConsented()); writeBooleanAttribute(serializer, XML_ATTR_IS_PERMISSION_SYNC_ALL_PACKAGES, - request.isPermissionSyncAllPackages()); + request.getPermissionSyncAllPackages()); try { writeListXml(request.getPermissionSyncPackages(), XML_ATTR_PERMISSION_SYNC_PACKAGES, serializer);