From 3934796dce7abfca27dcf1da6be95f979bbe4298 Mon Sep 17 00:00:00 2001 From: Vladimir Komsiyski Date: Wed, 14 Dec 2022 15:40:34 +0100 Subject: [PATCH] Small changes in VDMS w.r.t. caching and camera controller. Only get the CDM associations on demand when creating a virtual device, instead of caching them whenever a user is starting. The asscociations are only used during VD creation, so it's unnecessary to cache them. Remove the camera access controller maps from VDMS, and keep the controllers in VirtualDeviceImpl instead, to be a bit more consistent with input and sensor handling. Since the camera access controllers are per user, provide a mechanism to reuse them from existing VDs and only create a new one if no VD exists for the current user. They keep track of the current observers, and automatically close when it is 0. A couple of other very minor clean-ups as well. Bug: 263231559 Test: atest CameraAccessControllerTest Test: atest VirtualDeviceManagerServiceTest Test: atest android.virtualdevice.cts Change-Id: I3b007fc75ea8e36d826b9034f7574bc4e3207293 --- .../virtual/CameraAccessController.java | 18 ++ .../companion/virtual/VirtualDeviceImpl.java | 13 ++ .../virtual/VirtualDeviceManagerService.java | 204 +++++++----------- .../virtual/CameraAccessControllerTest.java | 7 + .../VirtualDeviceManagerServiceTest.java | 39 +++- 5 files changed, 140 insertions(+), 141 deletions(-) diff --git a/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java b/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java index 2904f28fca011..312ab548c7467 100644 --- a/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java +++ b/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java @@ -19,6 +19,7 @@ package com.android.server.companion.virtual; import static android.hardware.camera2.CameraInjectionSession.InjectionStatusCallback.ERROR_INJECTION_UNSUPPORTED; import android.annotation.NonNull; +import android.annotation.UserIdInt; import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; @@ -94,6 +95,23 @@ class CameraAccessController extends CameraManager.AvailabilityCallback implemen mUserManager = mContext.getSystemService(UserManager.class); } + /** + * Returns the userId for which the camera access should be blocked. + */ + @UserIdInt + public int getUserId() { + return mContext.getUserId(); + } + + /** + * Returns the number of observers currently relying on this controller. + */ + public int getObserverCount() { + synchronized (mLock) { + return mObserverCount; + } + } + /** * Starts watching for camera access by uids running on a virtual device, if we were not * already doing so. diff --git a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java index cdd54719e15f0..db163dcae3957 100644 --- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java +++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java @@ -110,6 +110,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub private final int mDeviceId; private final InputController mInputController; private final SensorController mSensorController; + private final CameraAccessController mCameraAccessController; private VirtualAudioController mVirtualAudioController; @VisibleForTesting final Set mVirtualDisplayIds = new ArraySet<>(); @@ -165,6 +166,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub IBinder token, int ownerUid, int deviceId, + CameraAccessController cameraAccessController, OnDeviceCloseListener onDeviceCloseListener, PendingTrampolineCallback pendingTrampolineCallback, IVirtualDeviceActivityListener activityListener, @@ -178,6 +180,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub deviceId, /* inputController= */ null, /* sensorController= */ null, + cameraAccessController, onDeviceCloseListener, pendingTrampolineCallback, activityListener, @@ -194,6 +197,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub int deviceId, InputController inputController, SensorController sensorController, + CameraAccessController cameraAccessController, OnDeviceCloseListener onDeviceCloseListener, PendingTrampolineCallback pendingTrampolineCallback, IVirtualDeviceActivityListener activityListener, @@ -223,6 +227,8 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub } else { mSensorController = sensorController; } + mCameraAccessController = cameraAccessController; + mCameraAccessController.startObservingIfNeeded(); mOnDeviceCloseListener = onDeviceCloseListener; try { token.linkToDeath(this, 0); @@ -243,6 +249,11 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub return flags; } + /** Returns the camera access controller of this device. */ + CameraAccessController getCameraAccessController() { + return mCameraAccessController; + } + /** Returns the device display name. */ CharSequence getDisplayName() { return mAssociationInfo.getDisplayName(); @@ -359,6 +370,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub } mOnDeviceCloseListener.onClose(mDeviceId); mAppToken.unlinkToDeath(this, 0); + mCameraAccessController.stopObservingIfNeeded(); final long ident = Binder.clearCallingIdentity(); try { @@ -376,6 +388,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub @Override public void onRunningAppsChanged(ArraySet runningUids) { + mCameraAccessController.blockCameraAccessIfNeeded(runningUids); mRunningAppsChangedCallback.accept(runningUids); } diff --git a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java index d31729872071b..758345f716c36 100644 --- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java @@ -27,7 +27,6 @@ import android.annotation.SuppressLint; import android.app.ActivityOptions; import android.companion.AssociationInfo; import android.companion.CompanionDeviceManager; -import android.companion.CompanionDeviceManager.OnAssociationsChangedListener; import android.companion.virtual.IVirtualDevice; import android.companion.virtual.IVirtualDeviceActivityListener; import android.companion.virtual.IVirtualDeviceManager; @@ -70,6 +69,7 @@ import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; @SuppressLint("LongLogTag") @@ -86,20 +86,6 @@ public class VirtualDeviceManagerService extends SystemService { private static AtomicInteger sNextUniqueIndex = new AtomicInteger( VirtualDeviceManager.DEVICE_ID_DEFAULT + 1); - /** - * Mapping from user IDs to CameraAccessControllers. - */ - @GuardedBy("mVirtualDeviceManagerLock") - private final SparseArray mCameraAccessControllers = - new SparseArray<>(); - - /** - * Mapping from device IDs to CameraAccessControllers. - */ - @GuardedBy("mVirtualDeviceManagerLock") - private final SparseArray mCameraAccessControllersByDeviceId = - new SparseArray<>(); - /** * Mapping from device IDs to virtual devices. */ @@ -112,21 +98,6 @@ public class VirtualDeviceManagerService extends SystemService { @GuardedBy("mVirtualDeviceManagerLock") private final SparseArray> mAppsOnVirtualDevices = new SparseArray<>(); - /** - * Mapping from user ID to CDM associations. The associations come from - * {@link CompanionDeviceManager#getAllAssociations()}, which contains associations across all - * packages. - */ - private final ConcurrentHashMap> mAllAssociations = - new ConcurrentHashMap<>(); - - /** - * Mapping from user ID to its change listener. The listeners are added when the user is - * started and removed when the user stops. - */ - private final SparseArray mOnAssociationsChangedListeners = - new SparseArray<>(); - public VirtualDeviceManagerService(Context context) { super(context); mImpl = new VirtualDeviceManagerImpl(); @@ -177,54 +148,9 @@ public class VirtualDeviceManagerService extends SystemService { } } - @Override - public void onUserStarting(@NonNull TargetUser user) { - super.onUserStarting(user); - Context userContext = getContext().createContextAsUser(user.getUserHandle(), 0); - synchronized (mVirtualDeviceManagerLock) { - final CompanionDeviceManager cdm = - userContext.getSystemService(CompanionDeviceManager.class); - final int userId = user.getUserIdentifier(); - mAllAssociations.put(userId, cdm.getAllAssociations()); - OnAssociationsChangedListener listener = - associations -> mAllAssociations.put(userId, associations); - mOnAssociationsChangedListeners.put(userId, listener); - cdm.addOnAssociationsChangedListener(Runnable::run, listener); - CameraAccessController cameraAccessController = new CameraAccessController( - userContext, mLocalService, this::onCameraAccessBlocked); - mCameraAccessControllers.put(user.getUserIdentifier(), cameraAccessController); - } - } - - @Override - public void onUserStopping(@NonNull TargetUser user) { - super.onUserStopping(user); - synchronized (mVirtualDeviceManagerLock) { - int userId = user.getUserIdentifier(); - mAllAssociations.remove(userId); - final CompanionDeviceManager cdm = getContext().createContextAsUser( - user.getUserHandle(), 0) - .getSystemService(CompanionDeviceManager.class); - OnAssociationsChangedListener listener = mOnAssociationsChangedListeners.get(userId); - if (listener != null) { - cdm.removeOnAssociationsChangedListener(listener); - mOnAssociationsChangedListeners.remove(userId); - } - CameraAccessController cameraAccessController = mCameraAccessControllers.get( - user.getUserIdentifier()); - if (cameraAccessController != null) { - cameraAccessController.close(); - mCameraAccessControllers.remove(user.getUserIdentifier()); - } else { - Slog.w(TAG, "Cannot unregister cameraAccessController for user " + user); - } - } - } - void onCameraAccessBlocked(int appUid) { synchronized (mVirtualDeviceManagerLock) { - int size = mVirtualDevices.size(); - for (int i = 0; i < size; i++) { + for (int i = 0; i < mVirtualDevices.size(); i++) { CharSequence deviceName = mVirtualDevices.valueAt(i).getDisplayName(); mVirtualDevices.valueAt(i).showToastWhereUidIsRunning(appUid, getContext().getString( @@ -235,6 +161,21 @@ public class VirtualDeviceManagerService extends SystemService { } } + CameraAccessController getCameraAccessController(UserHandle userHandle) { + int userId = userHandle.getIdentifier(); + synchronized (mVirtualDeviceManagerLock) { + for (int i = 0; i < mVirtualDevices.size(); i++) { + final CameraAccessController cameraAccessController = + mVirtualDevices.valueAt(i).getCameraAccessController(); + if (cameraAccessController.getUserId() == userId) { + return cameraAccessController; + } + } + } + Context userContext = getContext().createContextAsUser(userHandle, 0); + return new CameraAccessController(userContext, mLocalService, this::onCameraAccessBlocked); + } + @VisibleForTesting VirtualDeviceManagerInternal getLocalServiceInstance() { return mLocalService; @@ -255,8 +196,34 @@ public class VirtualDeviceManagerService extends SystemService { } } - class VirtualDeviceManagerImpl extends IVirtualDeviceManager.Stub implements - VirtualDeviceImpl.PendingTrampolineCallback { + @VisibleForTesting + void removeVirtualDevice(int deviceId) { + synchronized (mVirtualDeviceManagerLock) { + mAppsOnVirtualDevices.remove(deviceId); + mVirtualDevices.remove(deviceId); + } + } + + class VirtualDeviceManagerImpl extends IVirtualDeviceManager.Stub { + + private final VirtualDeviceImpl.PendingTrampolineCallback mPendingTrampolineCallback = + new VirtualDeviceImpl.PendingTrampolineCallback() { + @Override + public void startWaitingForPendingTrampoline(PendingTrampoline pendingTrampoline) { + PendingTrampoline existing = mPendingTrampolines.put( + pendingTrampoline.mPendingIntent.getCreatorPackage(), + pendingTrampoline); + if (existing != null) { + existing.mResultReceiver.send( + VirtualDeviceManager.LAUNCH_FAILURE_NO_ACTIVITY, null); + } + } + + @Override + public void stopWaitingForPendingTrampoline(PendingTrampoline pendingTrampoline) { + mPendingTrampolines.remove(pendingTrampoline.mPendingIntent.getCreatorPackage()); + } + }; @Override // Binder call public IVirtualDevice createVirtualDevice( @@ -279,25 +246,16 @@ public class VirtualDeviceManagerService extends SystemService { throw new IllegalArgumentException("No association with ID " + associationId); } synchronized (mVirtualDeviceManagerLock) { - final int userId = UserHandle.getUserId(callingUid); + final UserHandle userHandle = getCallingUserHandle(); final CameraAccessController cameraAccessController = - mCameraAccessControllers.get(userId); + getCameraAccessController(userHandle); final int deviceId = sNextUniqueIndex.getAndIncrement(); + final Consumer> runningAppsChangedCallback = + runningUids -> notifyRunningAppsChanged(deviceId, runningUids); VirtualDeviceImpl virtualDevice = new VirtualDeviceImpl(getContext(), - associationInfo, token, callingUid, deviceId, - /* onDeviceCloseListener= */ this::onDeviceClosed, - this, activityListener, - runningUids -> { - cameraAccessController.blockCameraAccessIfNeeded(runningUids); - notifyRunningAppsChanged(deviceId, runningUids); - }, - params); - if (cameraAccessController != null) { - cameraAccessController.startObservingIfNeeded(); - mCameraAccessControllersByDeviceId.put(deviceId, cameraAccessController); - } else { - Slog.w(TAG, "cameraAccessController not found for user " + userId); - } + associationInfo, token, callingUid, deviceId, cameraAccessController, + this::onDeviceClosed, mPendingTrampolineCallback, activityListener, + runningAppsChangedCallback, params); mVirtualDevices.put(deviceId, virtualDevice); return virtualDevice; } @@ -409,8 +367,18 @@ public class VirtualDeviceManagerService extends SystemService { @Nullable private AssociationInfo getAssociationInfo(String packageName, int associationId) { - final int callingUserId = getCallingUserHandle().getIdentifier(); - final List associations = mAllAssociations.get(callingUserId); + final UserHandle userHandle = getCallingUserHandle(); + final CompanionDeviceManager cdm = + getContext().createContextAsUser(userHandle, 0) + .getSystemService(CompanionDeviceManager.class); + List associations; + final long identity = Binder.clearCallingIdentity(); + try { + associations = cdm.getAllAssociations(); + } finally { + Binder.restoreCallingIdentity(identity); + } + final int callingUserId = userHandle.getIdentifier(); if (associations != null) { final int associationSize = associations.size(); for (int i = 0; i < associationSize; i++) { @@ -427,25 +395,15 @@ public class VirtualDeviceManagerService extends SystemService { } private void onDeviceClosed(int deviceId) { - synchronized (mVirtualDeviceManagerLock) { - mVirtualDevices.remove(deviceId); - Intent i = new Intent(VirtualDeviceManager.ACTION_VIRTUAL_DEVICE_REMOVED); - i.putExtra(VirtualDeviceManager.EXTRA_VIRTUAL_DEVICE_ID, deviceId); - i.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY); - final long identity = Binder.clearCallingIdentity(); - try { - getContext().sendBroadcastAsUser(i, UserHandle.ALL); - } finally { - Binder.restoreCallingIdentity(identity); - } - mAppsOnVirtualDevices.remove(deviceId); - final CameraAccessController cameraAccessController = - mCameraAccessControllersByDeviceId.removeReturnOld(deviceId); - if (cameraAccessController != null) { - cameraAccessController.stopObservingIfNeeded(); - } else { - Slog.w(TAG, "cameraAccessController not found for device Id " + deviceId); - } + removeVirtualDevice(deviceId); + Intent i = new Intent(VirtualDeviceManager.ACTION_VIRTUAL_DEVICE_REMOVED); + i.putExtra(VirtualDeviceManager.EXTRA_VIRTUAL_DEVICE_ID, deviceId); + i.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY); + final long identity = Binder.clearCallingIdentity(); + try { + getContext().sendBroadcastAsUser(i, UserHandle.ALL); + } finally { + Binder.restoreCallingIdentity(identity); } } @@ -474,22 +432,6 @@ public class VirtualDeviceManagerService extends SystemService { } } } - - @Override - public void startWaitingForPendingTrampoline(PendingTrampoline pendingTrampoline) { - PendingTrampoline existing = mPendingTrampolines.put( - pendingTrampoline.mPendingIntent.getCreatorPackage(), - pendingTrampoline); - if (existing != null) { - existing.mResultReceiver.send( - VirtualDeviceManager.LAUNCH_FAILURE_NO_ACTIVITY, null); - } - } - - @Override - public void stopWaitingForPendingTrampoline(PendingTrampoline pendingTrampoline) { - mPendingTrampolines.remove(pendingTrampoline.mPendingIntent.getCreatorPackage()); - } } private final class LocalService extends VirtualDeviceManagerInternal { diff --git a/services/tests/mockingservicestests/src/com/android/server/companion/virtual/CameraAccessControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/companion/virtual/CameraAccessControllerTest.java index 145e66c92f148..757d27b6b5697 100644 --- a/services/tests/mockingservicestests/src/com/android/server/companion/virtual/CameraAccessControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/companion/virtual/CameraAccessControllerTest.java @@ -18,6 +18,8 @@ package com.android.server.companion.virtual; import static android.hardware.camera2.CameraInjectionSession.InjectionStatusCallback.ERROR_INJECTION_UNSUPPORTED; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; @@ -115,6 +117,11 @@ public class CameraAccessControllerTest { mController.startObservingIfNeeded(); } + @Test + public void getUserId_returnsCorrectId() { + assertThat(mController.getUserId()).isEqualTo(mContext.getUserId()); + } + @Test public void onCameraOpened_uidNotRunning_noCameraBlocking() throws CameraAccessException { when(mDeviceManagerInternal.isAppRunningOnAnyVirtualDevice( diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java index 89eaa2c3d85ad..759b0497044f2 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java @@ -203,6 +203,7 @@ public class VirtualDeviceManagerServiceTest { private VirtualDeviceImpl mDeviceImpl; private InputController mInputController; private SensorController mSensorController; + private CameraAccessController mCameraAccessController; private AssociationInfo mAssociationInfo; private VirtualDeviceManagerService mVdms; private VirtualDeviceManagerInternal mLocalService; @@ -237,6 +238,8 @@ public class VirtualDeviceManagerServiceTest { @Mock private IAudioConfigChangedCallback mConfigChangedCallback; @Mock + private CameraAccessController.CameraAccessBlockedCallback mCameraAccessBlockedCallback; + @Mock private ApplicationInfo mApplicationInfoMock; @Mock IInputManager mIInputManagerMock; @@ -325,6 +328,8 @@ public class VirtualDeviceManagerServiceTest { new Handler(TestableLooper.get(this).getLooper()), mContext.getSystemService(WindowManager.class), threadVerifier); mSensorController = new SensorController(new Object(), VIRTUAL_DEVICE_ID_1); + mCameraAccessController = + new CameraAccessController(mContext, mLocalService, mCameraAccessBlockedCallback); mAssociationInfo = new AssociationInfo(/* associationId= */ 1, 0, null, MacAddress.BROADCAST_ADDRESS, "", null, null, true, false, false, 0, 0); @@ -394,12 +399,7 @@ public class VirtualDeviceManagerServiceTest { .setBlockedActivities(getBlockedActivities()) .setDevicePolicy(POLICY_TYPE_SENSORS, DEVICE_POLICY_CUSTOM) .build(); - mDeviceImpl = new VirtualDeviceImpl(mContext, - mAssociationInfo, new Binder(), /* ownerUid */ 0, VIRTUAL_DEVICE_ID_1, - mInputController, mSensorController, - /* onDeviceCloseListener= */ (int deviceId) -> {}, - mPendingTrampolineCallback, mActivityListener, mRunningAppsChangedCallback, params); - mVdms.addVirtualDevice(mDeviceImpl); + mDeviceImpl = createVirtualDevice(VIRTUAL_DEVICE_ID_1, DEVICE_OWNER_UID_1, params); assertThat(mVdm.getDevicePolicy(mDeviceImpl.getDeviceId(), POLICY_TYPE_SENSORS)) .isEqualTo(DEVICE_POLICY_CUSTOM); @@ -557,6 +557,21 @@ public class VirtualDeviceManagerServiceTest { LocaleList.forLanguageTags(firstKeyboardConfig.getLanguageTag())); } + @Test + public void cameraAccessController_observerCountUpdated() { + assertThat(mCameraAccessController.getObserverCount()).isEqualTo(1); + + VirtualDeviceImpl secondDevice = + createVirtualDevice(VIRTUAL_DEVICE_ID_2, DEVICE_OWNER_UID_2); + assertThat(mCameraAccessController.getObserverCount()).isEqualTo(2); + + mDeviceImpl.close(); + assertThat(mCameraAccessController.getObserverCount()).isEqualTo(1); + + secondDevice.close(); + assertThat(mCameraAccessController.getObserverCount()).isEqualTo(0); + } + @Test public void onVirtualDisplayRemovedLocked_doesNotThrowException() { mDeviceImpl.onVirtualDisplayCreatedLocked( @@ -1543,14 +1558,18 @@ public class VirtualDeviceManagerServiceTest { } private VirtualDeviceImpl createVirtualDevice(int virtualDeviceId, int ownerUid) { - VirtualDeviceParams params = new VirtualDeviceParams - .Builder() + VirtualDeviceParams params = new VirtualDeviceParams.Builder() .setBlockedActivities(getBlockedActivities()) .build(); + return createVirtualDevice(virtualDeviceId, ownerUid, params); + } + + private VirtualDeviceImpl createVirtualDevice(int virtualDeviceId, int ownerUid, + VirtualDeviceParams params) { VirtualDeviceImpl virtualDeviceImpl = new VirtualDeviceImpl(mContext, mAssociationInfo, new Binder(), ownerUid, virtualDeviceId, - mInputController, mSensorController, - /* onDeviceCloseListener= */ (int deviceId) -> {}, + mInputController, mSensorController, mCameraAccessController, + /* onDeviceCloseListener= */ deviceId -> mVdms.removeVirtualDevice(deviceId), mPendingTrampolineCallback, mActivityListener, mRunningAppsChangedCallback, params); mVdms.addVirtualDevice(virtualDeviceImpl); return virtualDeviceImpl;