From 3895dba9476547f431efc41b38caa0ea526e2866 Mon Sep 17 00:00:00 2001 From: Cliff Wu Date: Tue, 22 Feb 2022 16:44:38 +0800 Subject: [PATCH] Fixed that the camera will not be blocked on the virtual display in special cases - Root cause: When an app uses the system default camera app to open the camera on the phone, and another app uses an Intent (MediaStore.ACTION_IMAGE_CAPTURE) (the system default camera app) to open the camera on the virtual display, because they will use the same thread in the camera service, the camera will only reconfigure the stream without creating it again. Therefore, the onCameraOpened callback will not be triggered, causing the issue that the camera on the virtual display is not blocked. - Solution: The camera mechanism is that only one camera can run at a time. If two apps open the camera at the same time, it will first open and close the camera of one of two apps, and then open the camera of another app. So we identify above issue according to this rule: track the app that opens the camera on the phone (main display). When it is detected that this app also appears on the virtual display, it means that the app appears on both sides at the same time, which belongs to the above issue, and the camera of this app is set to a blocked state. Bug: 210795569 Test: Manual Change-Id: Idabee8a38085c8aae1f47f3579cead1adde385ea --- .../virtual/CameraAccessController.java | 52 ++++++++++++++++++- .../GenericWindowPolicyController.java | 24 +++++---- .../companion/virtual/VirtualDeviceImpl.java | 30 ++++++++--- .../virtual/VirtualDeviceManagerService.java | 5 +- .../virtual/audio/VirtualAudioController.java | 5 +- .../virtual/CameraAccessControllerTest.java | 36 +++++++++++-- .../VirtualDeviceManagerServiceTest.java | 5 +- 7 files changed, 133 insertions(+), 24 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 adc8459de6588..ec0da490adcf8 100644 --- a/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java +++ b/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java @@ -30,6 +30,8 @@ import android.util.Slog; import com.android.internal.annotations.GuardedBy; +import java.util.Set; + /** * Handles blocking access to the camera for apps running on virtual devices. */ @@ -50,11 +52,23 @@ class CameraAccessController extends CameraManager.AvailabilityCallback implemen @GuardedBy("mLock") private ArrayMap mPackageToSessionData = new ArrayMap<>(); + /** + * Mapping from camera ID to open camera app associations. Key is the camera id, value is the + * information of the app's uid and package name. + */ + @GuardedBy("mLock") + private ArrayMap mAppsToBlockOnVirtualDevice = new ArrayMap<>(); + static class InjectionSessionData { public int appUid; public ArrayMap cameraIdToSession = new ArrayMap<>(); } + static class OpenCameraInfo { + public String packageName; + public int packageUid; + } + interface CameraAccessBlockedCallback { /** * Called whenever an app was blocked from accessing a camera. @@ -98,6 +112,33 @@ class CameraAccessController extends CameraManager.AvailabilityCallback implemen } } + /** + * Need to block camera access for applications running on virtual displays. + *

+ * Apps that open the camera on the main display will need to block camera access if moved to a + * virtual display. + * + * @param runningUids uids of the application running on the virtual display + */ + public void blockCameraAccessIfNeeded(Set runningUids) { + synchronized (mLock) { + for (int i = 0; i < mAppsToBlockOnVirtualDevice.size(); i++) { + final String cameraId = mAppsToBlockOnVirtualDevice.keyAt(i); + final OpenCameraInfo openCameraInfo = mAppsToBlockOnVirtualDevice.get(cameraId); + int packageUid = openCameraInfo.packageUid; + if (runningUids.contains(packageUid)) { + final String packageName = openCameraInfo.packageName; + InjectionSessionData data = mPackageToSessionData.get(packageName); + if (data == null) { + data = new InjectionSessionData(); + data.appUid = packageUid; + mPackageToSessionData.put(packageName, data); + } + startBlocking(packageName, cameraId); + } + } + } + } @Override public void close() { @@ -115,10 +156,13 @@ class CameraAccessController extends CameraManager.AvailabilityCallback implemen public void onCameraOpened(@NonNull String cameraId, @NonNull String packageName) { synchronized (mLock) { try { - final ApplicationInfo ainfo = - mPackageManager.getApplicationInfo(packageName, 0); + final ApplicationInfo ainfo = mPackageManager.getApplicationInfo(packageName, 0); InjectionSessionData data = mPackageToSessionData.get(packageName); if (!mVirtualDeviceManagerInternal.isAppRunningOnAnyVirtualDevice(ainfo.uid)) { + OpenCameraInfo openCameraInfo = new OpenCameraInfo(); + openCameraInfo.packageName = packageName; + openCameraInfo.packageUid = ainfo.uid; + mAppsToBlockOnVirtualDevice.put(cameraId, openCameraInfo); CameraInjectionSession existingSession = (data != null) ? data.cameraIdToSession.get(cameraId) : null; if (existingSession != null) { @@ -149,6 +193,7 @@ class CameraAccessController extends CameraManager.AvailabilityCallback implemen @Override public void onCameraClosed(@NonNull String cameraId) { synchronized (mLock) { + mAppsToBlockOnVirtualDevice.remove(cameraId); for (int i = mPackageToSessionData.size() - 1; i >= 0; i--) { InjectionSessionData data = mPackageToSessionData.valueAt(i); CameraInjectionSession session = data.cameraIdToSession.get(cameraId); @@ -168,6 +213,9 @@ class CameraAccessController extends CameraManager.AvailabilityCallback implemen */ private void startBlocking(String packageName, String cameraId) { try { + Slog.d( + TAG, + "startBlocking() cameraId: " + cameraId + " packageName: " + packageName); mCameraManager.injectCamera(packageName, cameraId, /* externalCamId */ "", mContext.getMainExecutor(), new CameraInjectionSession.InjectionStatusCallback() { diff --git a/services/companion/java/com/android/server/companion/virtual/GenericWindowPolicyController.java b/services/companion/java/com/android/server/companion/virtual/GenericWindowPolicyController.java index 27de8cdc0421d..0b437446826a0 100644 --- a/services/companion/java/com/android/server/companion/virtual/GenericWindowPolicyController.java +++ b/services/companion/java/com/android/server/companion/virtual/GenericWindowPolicyController.java @@ -93,9 +93,8 @@ public class GenericWindowPolicyController extends DisplayWindowPolicyController final ArraySet mRunningUids = new ArraySet<>(); @Nullable private final ActivityListener mActivityListener; private final Handler mHandler = new Handler(Looper.getMainLooper()); - - @Nullable - private RunningAppsChangedListener mRunningAppsChangedListener; + private final ArraySet mRunningAppsChangedListener = + new ArraySet<>(); /** * Creates a window policy controller that is generic to the different use cases of virtual @@ -142,9 +141,14 @@ public class GenericWindowPolicyController extends DisplayWindowPolicyController mActivityListener = activityListener; } - /** Sets listener for running applications change. */ - public void setRunningAppsChangedListener(@Nullable RunningAppsChangedListener listener) { - mRunningAppsChangedListener = listener; + /** Register a listener for running applications changes. */ + public void registerRunningAppsChangedListener(@NonNull RunningAppsChangedListener listener) { + mRunningAppsChangedListener.add(listener); + } + + /** Unregister a listener for running applications changes. */ + public void unregisterRunningAppsChangedListener(@NonNull RunningAppsChangedListener listener) { + mRunningAppsChangedListener.remove(listener); } @Override @@ -237,9 +241,11 @@ public class GenericWindowPolicyController extends DisplayWindowPolicyController mHandler.post(() -> mActivityListener.onDisplayEmpty(Display.INVALID_DISPLAY)); } } - if (mRunningAppsChangedListener != null) { - mRunningAppsChangedListener.onRunningAppsChanged(runningUids); - } + mHandler.post(() -> { + for (RunningAppsChangedListener listener : mRunningAppsChangedListener) { + listener.onRunningAppsChanged(runningUids); + } + }); } /** 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 90c879aee90a8..de14ef61a075a 100644 --- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java +++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java @@ -68,16 +68,18 @@ import android.window.DisplayWindowPolicyController; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.BlockedAppStreamingActivity; +import com.android.server.companion.virtual.GenericWindowPolicyController.RunningAppsChangedListener; import com.android.server.companion.virtual.audio.VirtualAudioController; import java.io.FileDescriptor; import java.io.PrintWriter; import java.util.Map; import java.util.Set; +import java.util.function.Consumer; final class VirtualDeviceImpl extends IVirtualDevice.Stub - implements IBinder.DeathRecipient { + implements IBinder.DeathRecipient, RunningAppsChangedListener { private static final String TAG = "VirtualDeviceImpl"; @@ -101,6 +103,8 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub private final VirtualDeviceParams mParams; private final Map mPerDisplayWakelocks = new ArrayMap<>(); private final IVirtualDeviceActivityListener mActivityListener; + @NonNull + private Consumer> mRunningAppsChangedCallback; // The default setting for showing the pointer on new displays. @GuardedBy("mVirtualDeviceLock") private boolean mDefaultShowPointerIcon = true; @@ -139,21 +143,25 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub IBinder token, int ownerUid, OnDeviceCloseListener listener, PendingTrampolineCallback pendingTrampolineCallback, IVirtualDeviceActivityListener activityListener, + Consumer> runningAppsChangedCallback, VirtualDeviceParams params) { this(context, associationInfo, token, ownerUid, /* inputController= */ null, listener, - pendingTrampolineCallback, activityListener, params); + pendingTrampolineCallback, activityListener, runningAppsChangedCallback, params); } @VisibleForTesting VirtualDeviceImpl(Context context, AssociationInfo associationInfo, IBinder token, int ownerUid, InputController inputController, OnDeviceCloseListener listener, PendingTrampolineCallback pendingTrampolineCallback, - IVirtualDeviceActivityListener activityListener, VirtualDeviceParams params) { + IVirtualDeviceActivityListener activityListener, + Consumer> runningAppsChangedCallback, + VirtualDeviceParams params) { UserHandle ownerUserHandle = UserHandle.getUserHandleForUid(ownerUid); mContext = context.createContextAsUser(ownerUserHandle, 0); mAssociationInfo = associationInfo; mPendingTrampolineCallback = pendingTrampolineCallback; mActivityListener = activityListener; + mRunningAppsChangedCallback = runningAppsChangedCallback; mOwnerUid = ownerUid; mAppToken = token; mParams = params; @@ -278,6 +286,11 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub close(); } + @Override + public void onRunningAppsChanged(ArraySet runningUids) { + mRunningAppsChangedCallback.accept(runningUids); + } + @VisibleForTesting VirtualAudioController getVirtualAudioControllerForTesting() { return mVirtualAudioController; @@ -529,7 +542,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub // reentrancy problems. mContext.getMainThreadHandler().post(() -> addWakeLockForDisplay(displayId)); - final GenericWindowPolicyController dwpc = + final GenericWindowPolicyController gwpc = new GenericWindowPolicyController(FLAG_SECURE, SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS, getAllowedUserHandles(), @@ -540,8 +553,9 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub mParams.getDefaultActivityPolicy(), createListenerAdapter(displayId), activityInfo -> onActivityBlocked(displayId, activityInfo)); - mWindowPolicyControllers.put(displayId, dwpc); - return dwpc; + gwpc.registerRunningAppsChangedListener(/* listener= */ this); + mWindowPolicyControllers.put(displayId, gwpc); + return gwpc; } } @@ -599,6 +613,10 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub wakeLock.release(); mPerDisplayWakelocks.remove(displayId); } + GenericWindowPolicyController gwpc = mWindowPolicyControllers.get(displayId); + if (gwpc != null) { + gwpc.unregisterRunningAppsChangedListener(/* listener= */ this); + } mVirtualDisplayIds.remove(displayId); mWindowPolicyControllers.remove(displayId); } 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 9acca8025920c..6398b2142e373 100644 --- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java @@ -251,7 +251,10 @@ public class VirtualDeviceManagerService extends SystemService { } } }, - this, activityListener, params); + this, activityListener, + runningUids -> cameraAccessController.blockCameraAccessIfNeeded( + runningUids), + params); if (cameraAccessController != null) { cameraAccessController.startObservingIfNeeded(); } else { diff --git a/services/companion/java/com/android/server/companion/virtual/audio/VirtualAudioController.java b/services/companion/java/com/android/server/companion/virtual/audio/VirtualAudioController.java index 13a47d6817295..c91877aad47e2 100644 --- a/services/companion/java/com/android/server/companion/virtual/audio/VirtualAudioController.java +++ b/services/companion/java/com/android/server/companion/virtual/audio/VirtualAudioController.java @@ -85,7 +85,7 @@ public final class VirtualAudioController implements AudioPlaybackCallback, @NonNull IAudioRoutingCallback routingCallback, @Nullable IAudioConfigChangedCallback configChangedCallback) { mGenericWindowPolicyController = genericWindowPolicyController; - mGenericWindowPolicyController.setRunningAppsChangedListener(/* listener= */ this); + mGenericWindowPolicyController.registerRunningAppsChangedListener(/* listener= */ this); synchronized (mCallbackLock) { mRoutingCallback = routingCallback; mConfigChangedCallback = configChangedCallback; @@ -111,7 +111,8 @@ public final class VirtualAudioController implements AudioPlaybackCallback, mAudioPlaybackDetector.unregister(); mAudioRecordingDetector.unregister(); if (mGenericWindowPolicyController != null) { - mGenericWindowPolicyController.setRunningAppsChangedListener(/* listener= */ null); + mGenericWindowPolicyController.unregisterRunningAppsChangedListener( + /* listener= */ this); mGenericWindowPolicyController = null; } synchronized (mCallbackLock) { 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 1b9cb28dc8b2b..c4c3abc1388ee 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 @@ -24,6 +24,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -34,6 +35,7 @@ import android.hardware.camera2.CameraInjectionSession; import android.hardware.camera2.CameraManager; import android.os.Process; import android.testing.TestableContext; +import android.util.ArraySet; import androidx.test.InstrumentationRegistry; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -73,11 +75,11 @@ public class CameraAccessControllerTest { private ApplicationInfo mTestAppInfo = new ApplicationInfo(); private ApplicationInfo mOtherAppInfo = new ApplicationInfo(); + private ArraySet mRunningUids = new ArraySet<>(); @Captor ArgumentCaptor mInjectionCallbackCaptor; - @Before public void setUp() throws PackageManager.NameNotFoundException { MockitoAnnotations.initMocks(this); @@ -89,6 +91,7 @@ public class CameraAccessControllerTest { mBlockedCallback); mTestAppInfo.uid = Process.FIRST_APPLICATION_UID; mOtherAppInfo.uid = Process.FIRST_APPLICATION_UID + 1; + mRunningUids.add(Process.FIRST_APPLICATION_UID); when(mPackageManager.getApplicationInfo(eq(TEST_APP_PACKAGE), anyInt())).thenReturn( mTestAppInfo); when(mPackageManager.getApplicationInfo(eq(OTHER_APP_PACKAGE), anyInt())).thenReturn( @@ -104,7 +107,6 @@ public class CameraAccessControllerTest { verify(mCameraManager, never()).injectCamera(any(), any(), any(), any(), any()); } - @Test public void onCameraOpened_uidRunning_cameraBlocked() throws CameraAccessException { when(mDeviceManagerInternal.isAppRunningOnAnyVirtualDevice( @@ -128,7 +130,6 @@ public class CameraAccessControllerTest { verify(session).close(); } - @Test public void onCameraClosed_otherCameraClosed_cameraNotUnblocked() throws CameraAccessException { when(mDeviceManagerInternal.isAppRunningOnAnyVirtualDevice( @@ -197,4 +198,33 @@ public class CameraAccessControllerTest { mInjectionCallbackCaptor.getValue().onInjectionError(ERROR_INJECTION_UNSUPPORTED); verify(mBlockedCallback).onCameraAccessBlocked(eq(mTestAppInfo.uid)); } + + @Test + public void twoCameraAccessesBySameUid_secondOnVirtualDisplay_noCallbackButCameraCanBlocked() + throws CameraAccessException { + when(mDeviceManagerInternal.isAppRunningOnAnyVirtualDevice( + eq(mTestAppInfo.uid))).thenReturn(false); + mController.onCameraOpened(FRONT_CAMERA, TEST_APP_PACKAGE); + mController.blockCameraAccessIfNeeded(mRunningUids); + + verify(mCameraManager).injectCamera(eq(TEST_APP_PACKAGE), eq(FRONT_CAMERA), anyString(), + any(), mInjectionCallbackCaptor.capture()); + CameraInjectionSession session = mock(CameraInjectionSession.class); + mInjectionCallbackCaptor.getValue().onInjectionSucceeded(session); + mInjectionCallbackCaptor.getValue().onInjectionError(ERROR_INJECTION_UNSUPPORTED); + verify(mBlockedCallback).onCameraAccessBlocked(eq(mTestAppInfo.uid)); + } + + @Test + public void twoCameraAccessesBySameUid_secondOnVirtualDisplay_firstCloseThenOpenCameraUnblock() + throws CameraAccessException { + when(mDeviceManagerInternal.isAppRunningOnAnyVirtualDevice( + eq(mTestAppInfo.uid))).thenReturn(false); + mController.onCameraOpened(FRONT_CAMERA, TEST_APP_PACKAGE); + mController.blockCameraAccessIfNeeded(mRunningUids); + mController.onCameraClosed(FRONT_CAMERA); + mController.onCameraOpened(FRONT_CAMERA, TEST_APP_PACKAGE); + + verify(mCameraManager, times(1)).injectCamera(any(), any(), any(), any(), any()); + } } 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 d1b015674b3a9..808f8c2cc6267 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 @@ -92,6 +92,7 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.Arrays; +import java.util.function.Consumer; @Presubmit @RunWith(AndroidTestingRunner.class) @@ -133,6 +134,8 @@ public class VirtualDeviceManagerServiceTest { @Mock private IVirtualDeviceActivityListener mActivityListener; @Mock + private Consumer> mRunningAppsChangedCallback; + @Mock IPowerManager mIPowerManagerMock; @Mock IThermalService mIThermalServiceMock; @@ -207,7 +210,7 @@ public class VirtualDeviceManagerServiceTest { mDeviceImpl = new VirtualDeviceImpl(mContext, mAssociationInfo, new Binder(), /* uid */ 0, mInputController, (int associationId) -> { - }, mPendingTrampolineCallback, mActivityListener, + }, mPendingTrampolineCallback, mActivityListener, mRunningAppsChangedCallback, params); }