diff options
4 files changed, 152 insertions, 54 deletions
diff --git a/services/core/java/com/android/server/wm/CameraCompatFreeformPolicy.java b/services/core/java/com/android/server/wm/CameraCompatFreeformPolicy.java index 506477f67bfc..cb95b3655c61 100644 --- a/services/core/java/com/android/server/wm/CameraCompatFreeformPolicy.java +++ b/services/core/java/com/android/server/wm/CameraCompatFreeformPolicy.java @@ -65,6 +65,9 @@ final class CameraCompatFreeformPolicy implements CameraStateMonitor.CameraCompa @NonNull private final CameraStateMonitor mCameraStateMonitor; + // TODO(b/380840084): Consider moving this to the CameraStateMonitor, and keeping track of + // all current camera activities, especially when the camera access is switching from one app to + // another. @Nullable private Task mCameraTask; @@ -123,8 +126,7 @@ final class CameraCompatFreeformPolicy implements CameraStateMonitor.CameraCompa } @Override - public void onCameraOpened(@NonNull ActivityRecord cameraActivity, - @NonNull String cameraId) { + public void onCameraOpened(@NonNull ActivityRecord cameraActivity) { // Do not check orientation outside of the config recompute, as the app's orientation intent // might be obscured by a fullscreen override. Especially for apps which have a camera // functionality which is not the main focus of the app: while most of the app might work @@ -136,18 +138,15 @@ final class CameraCompatFreeformPolicy implements CameraStateMonitor.CameraCompa return; } - cameraActivity.recomputeConfiguration(); - cameraActivity.getTask().dispatchTaskInfoChangedIfNeeded(/* force= */ true); - cameraActivity.ensureActivityConfiguration(/* ignoreVisibility= */ false); + mCameraTask = cameraActivity.getTask(); + updateAndDispatchCameraConfiguration(); } @Override - public boolean onCameraClosed(@NonNull String cameraId) { + public boolean canCameraBeClosed(@NonNull String cameraId) { // Top activity in the same task as the camera activity, or `null` if the task is // closed. - final ActivityRecord topActivity = mCameraTask != null - ? mCameraTask.getTopActivity(/* isFinishing */ false, /* includeOverlays */ false) - : null; + final ActivityRecord topActivity = getTopActivityFromCameraTask(); if (topActivity != null) { if (isActivityForCameraIdRefreshing(topActivity, cameraId)) { ProtoLog.v(WmProtoLogGroups.WM_DEBUG_STATES, @@ -157,10 +156,36 @@ final class CameraCompatFreeformPolicy implements CameraStateMonitor.CameraCompa return false; } } - mCameraTask = null; return true; } + @Override + public void onCameraClosed() { + // Top activity in the same task as the camera activity, or `null` if the task is + // closed. + final ActivityRecord topActivity = getTopActivityFromCameraTask(); + // Only clean up if the camera is not running - this close signal could be from switching + // cameras (e.g. back to front camera, and vice versa). + if (topActivity == null || !mCameraStateMonitor.isCameraRunningForActivity(topActivity)) { + updateAndDispatchCameraConfiguration(); + mCameraTask = null; + } + } + + private void updateAndDispatchCameraConfiguration() { + if (mCameraTask == null) { + return; + } + final ActivityRecord activity = getTopActivityFromCameraTask(); + if (activity != null) { + activity.recomputeConfiguration(); + mCameraTask.dispatchTaskInfoChangedIfNeeded(/* force= */ true); + activity.ensureActivityConfiguration(/* ignoreVisibility= */ true); + } else { + mCameraTask.dispatchTaskInfoChangedIfNeeded(/* force= */ true); + } + } + boolean shouldCameraCompatControlOrientation(@NonNull ActivityRecord activity) { return isCameraRunningAndWindowingModeEligible(activity); } @@ -262,10 +287,17 @@ final class CameraCompatFreeformPolicy implements CameraStateMonitor.CameraCompa && !activity.isEmbedded(); } + @Nullable + private ActivityRecord getTopActivityFromCameraTask() { + return mCameraTask != null + ? mCameraTask.getTopActivity(/* isFinishing */ false, /* includeOverlays */ false) + : null; + } + private boolean isActivityForCameraIdRefreshing(@NonNull ActivityRecord topActivity, @NonNull String cameraId) { if (!isTreatmentEnabledForActivity(topActivity, /* checkOrientation= */ true) - || mCameraStateMonitor.isCameraWithIdRunningForActivity(topActivity, cameraId)) { + || !mCameraStateMonitor.isCameraWithIdRunningForActivity(topActivity, cameraId)) { return false; } return topActivity.mAppCompatController.getAppCompatCameraOverrides().isRefreshRequested(); diff --git a/services/core/java/com/android/server/wm/CameraStateMonitor.java b/services/core/java/com/android/server/wm/CameraStateMonitor.java index 3b6e30ab2a6d..3aa355869d85 100644 --- a/services/core/java/com/android/server/wm/CameraStateMonitor.java +++ b/services/core/java/com/android/server/wm/CameraStateMonitor.java @@ -67,6 +67,10 @@ class CameraStateMonitor { // when camera connection is closed and we need to clean up our records. private final CameraIdPackageNameBiMapping mCameraIdPackageBiMapping = new CameraIdPackageNameBiMapping(); + // TODO(b/380840084): Consider making this a set of CameraId/PackageName pairs. This is to + // keep track of camera-closed signals when apps are switching camera access, so that the policy + // can restore app configuration when an app closes camera (e.g. loses camera access due to + // another app). private final Set<String> mScheduledToBeRemovedCameraIdSet = new ArraySet<>(); // TODO(b/336474959): should/can this go in the compat listeners? @@ -163,15 +167,14 @@ class CameraStateMonitor { if (cameraActivity == null || cameraActivity.getTask() == null) { return; } - notifyListenersCameraOpened(cameraActivity, cameraId); + notifyListenersCameraOpened(cameraActivity); } } - private void notifyListenersCameraOpened(@NonNull ActivityRecord cameraActivity, - @NonNull String cameraId) { + private void notifyListenersCameraOpened(@NonNull ActivityRecord cameraActivity) { for (int i = 0; i < mCameraStateListeners.size(); i++) { CameraCompatStateListener listener = mCameraStateListeners.get(i); - listener.onCameraOpened(cameraActivity, cameraId); + listener.onCameraOpened(cameraActivity); } } @@ -224,11 +227,11 @@ class CameraStateMonitor { // Already reconnected to this camera, no need to clean up. return; } - - final boolean closeSuccessfulForAllListeners = notifyListenersCameraClosed(cameraId); - if (closeSuccessfulForAllListeners) { + final boolean canClose = checkCanCloseForAllListeners(cameraId); + if (canClose) { // Finish cleaning up. mCameraIdPackageBiMapping.removeCameraId(cameraId); + notifyListenersCameraClosed(); } else { // Not ready to process closure yet - the camera activity might be refreshing. // Try again later. @@ -238,15 +241,21 @@ class CameraStateMonitor { } /** - * @return {@code false} if any listeners have reported issues processing the close. + * @return {@code false} if any listener has reported that they cannot process camera close now. */ - private boolean notifyListenersCameraClosed(@NonNull String cameraId) { - boolean closeSuccessfulForAllListeners = true; + private boolean checkCanCloseForAllListeners(@NonNull String cameraId) { for (int i = 0; i < mCameraStateListeners.size(); i++) { - closeSuccessfulForAllListeners &= mCameraStateListeners.get(i).onCameraClosed(cameraId); + if (!mCameraStateListeners.get(i).canCameraBeClosed(cameraId)) { + return false; + } } + return true; + } - return closeSuccessfulForAllListeners; + private void notifyListenersCameraClosed() { + for (int i = 0; i < mCameraStateListeners.size(); i++) { + mCameraStateListeners.get(i).onCameraClosed(); + } } // TODO(b/335165310): verify that this works in multi instance and permission dialogs. @@ -297,14 +306,18 @@ class CameraStateMonitor { /** * Notifies the compat listener that an activity has opened camera. */ - // TODO(b/336474959): try to decouple `cameraId` from the listeners. - void onCameraOpened(@NonNull ActivityRecord cameraActivity, @NonNull String cameraId); + void onCameraOpened(@NonNull ActivityRecord cameraActivity); /** - * Notifies the compat listener that camera is closed. + * Checks whether a listener is ready to do a cleanup when camera is closed. * - * @return true if cleanup has been successful - the notifier might try again if false. + * <p>The notifier might try again if false is returned. */ // TODO(b/336474959): try to decouple `cameraId` from the listeners. - boolean onCameraClosed(@NonNull String cameraId); + boolean canCameraBeClosed(@NonNull String cameraId); + + /** + * Notifies the compat listener that camera is closed. + */ + void onCameraClosed(); } } diff --git a/services/core/java/com/android/server/wm/DisplayRotationCompatPolicy.java b/services/core/java/com/android/server/wm/DisplayRotationCompatPolicy.java index 0ccc0fe80b52..3c199dba565b 100644 --- a/services/core/java/com/android/server/wm/DisplayRotationCompatPolicy.java +++ b/services/core/java/com/android/server/wm/DisplayRotationCompatPolicy.java @@ -71,6 +71,9 @@ final class DisplayRotationCompatPolicy implements CameraStateMonitor.CameraComp @NonNull private final ActivityRefresher mActivityRefresher; + // TODO(b/380840084): Consider moving this to the CameraStateMonitor, and keeping track of + // all current camera activities, especially when the camera access is switching from one app to + // another. @Nullable private Task mCameraTask; @@ -327,8 +330,7 @@ final class DisplayRotationCompatPolicy implements CameraStateMonitor.CameraComp } @Override - public void onCameraOpened(@NonNull ActivityRecord cameraActivity, - @NonNull String cameraId) { + public void onCameraOpened(@NonNull ActivityRecord cameraActivity) { mCameraTask = cameraActivity.getTask(); // Checking whether an activity in fullscreen rather than the task as this camera // compat treatment doesn't cover activity embedding. @@ -374,16 +376,9 @@ final class DisplayRotationCompatPolicy implements CameraStateMonitor.CameraComp } @Override - public boolean onCameraClosed(@NonNull String cameraId) { - final ActivityRecord topActivity; - if (Flags.cameraCompatFullscreenPickSameTaskActivity()) { - topActivity = mCameraTask != null ? mCameraTask.getTopActivity( - /* includeFinishing= */ true, /* includeOverlays= */ false) : null; - } else { - topActivity = mDisplayContent.topRunningActivity(/* considerKeyguardState= */ true); - } + public boolean canCameraBeClosed(@NonNull String cameraId) { + final ActivityRecord topActivity = getTopActivity(); - mCameraTask = null; if (topActivity == null) { return true; } @@ -399,6 +394,23 @@ final class DisplayRotationCompatPolicy implements CameraStateMonitor.CameraComp return false; } } + return true; + } + + @Override + public void onCameraClosed() { + final ActivityRecord topActivity = getTopActivity(); + + // Only clean up if the camera is not running - this close signal could be from switching + // cameras (e.g. back to front camera, and vice versa). + if (topActivity == null || !mCameraStateMonitor.isCameraRunningForActivity(topActivity)) { + // Call after getTopActivity(), as that method might use the activity from mCameraTask. + mCameraTask = null; + } + + if (topActivity == null) { + return; + } ProtoLog.v(WM_DEBUG_ORIENTATION, "Display id=%d is notified that Camera is closed, updating rotation.", @@ -406,11 +418,10 @@ final class DisplayRotationCompatPolicy implements CameraStateMonitor.CameraComp // Checking whether an activity in fullscreen rather than the task as this camera compat // treatment doesn't cover activity embedding. if (topActivity.getWindowingMode() != WINDOWING_MODE_FULLSCREEN) { - return true; + return; } recomputeConfigurationForCameraCompatIfNeeded(topActivity); mDisplayContent.updateOrientation(); - return true; } // TODO(b/336474959): Do we need cameraId here? @@ -430,6 +441,16 @@ final class DisplayRotationCompatPolicy implements CameraStateMonitor.CameraComp } } + @Nullable + private ActivityRecord getTopActivity() { + if (Flags.cameraCompatFullscreenPickSameTaskActivity()) { + return mCameraTask != null ? mCameraTask.getTopActivity( + /* includeFinishing= */ true, /* includeOverlays= */ false) : null; + } else { + return mDisplayContent.topRunningActivity(/* considerKeyguardState= */ true); + } + } + /** * @return {@code true} if the configuration needs to be recomputed after a camera state update. */ diff --git a/services/tests/wmtests/src/com/android/server/wm/CameraStateMonitorTests.java b/services/tests/wmtests/src/com/android/server/wm/CameraStateMonitorTests.java index ad80f82c8ea8..4810c7fc32d2 100644 --- a/services/tests/wmtests/src/com/android/server/wm/CameraStateMonitorTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/CameraStateMonitorTests.java @@ -22,6 +22,8 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.when; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; @@ -137,6 +139,14 @@ public final class CameraStateMonitorTests extends WindowTestsBase { } @Test + public void testOnCameraOpened_listenerAdded_cameraRegistersAsOpenedDuringTheCallback() { + mCameraStateMonitor.addCameraStateListener(mListener); + mCameraAvailabilityCallback.onCameraOpened(CAMERA_ID_1, TEST_PACKAGE_1); + + assertTrue(mListener.mIsCameraOpened); + } + + @Test public void testOnCameraOpened_cameraClosed_notifyCameraClosed() { mCameraStateMonitor.addCameraStateListener(mListener); // Listener returns true on `onCameraOpened`. @@ -144,10 +154,21 @@ public final class CameraStateMonitorTests extends WindowTestsBase { mCameraAvailabilityCallback.onCameraClosed(CAMERA_ID_1); + assertEquals(1, mListener.mCheckCanCloseCounter); assertEquals(1, mListener.mOnCameraClosedCounter); } @Test + public void testOnCameraOpenedAndClosed_cameraRegistersAsClosedDuringTheCallback() { + mCameraStateMonitor.addCameraStateListener(mListener); + // Listener returns true on `onCameraOpened`. + mCameraAvailabilityCallback.onCameraOpened(CAMERA_ID_1, TEST_PACKAGE_1); + + mCameraAvailabilityCallback.onCameraClosed(CAMERA_ID_1); + assertFalse(mListener.mIsCameraOpened); + } + + @Test public void testOnCameraOpened_listenerCannotCloseYet_notifyCameraClosedAgain() { mCameraStateMonitor.addCameraStateListener(mListenerCannotClose); // Listener returns true on `onCameraOpened`. @@ -155,7 +176,8 @@ public final class CameraStateMonitorTests extends WindowTestsBase { mCameraAvailabilityCallback.onCameraClosed(CAMERA_ID_1); - assertEquals(2, mListenerCannotClose.mOnCameraClosedCounter); + assertEquals(2, mListenerCannotClose.mCheckCanCloseCounter); + assertEquals(1, mListenerCannotClose.mOnCameraClosedCounter); } @Test @@ -197,39 +219,49 @@ public final class CameraStateMonitorTests extends WindowTestsBase { CameraStateMonitor.CameraCompatStateListener { int mOnCameraOpenedCounter = 0; + int mCheckCanCloseCounter = 0; int mOnCameraClosedCounter = 0; - private boolean mOnCameraClosedReturnValue = true; + boolean mIsCameraOpened; + + private boolean mCheckCanCloseReturnValue = true; /** - * @param simulateUnsuccessfulCloseOnce When false, returns `true` on every - * `onCameraClosed`. When true, returns `false` on the - * first `onCameraClosed` callback, and `true on the + * @param simulateCannotCloseOnce When false, returns `true` on every + * `checkCanClose`. When true, returns `false` on the + * first `checkCanClose` callback, and `true on the * subsequent calls. This fake implementation tests the * retry mechanism in {@link CameraStateMonitor}. */ - FakeCameraCompatStateListener(boolean simulateUnsuccessfulCloseOnce) { - mOnCameraClosedReturnValue = !simulateUnsuccessfulCloseOnce; + FakeCameraCompatStateListener(boolean simulateCannotCloseOnce) { + mCheckCanCloseReturnValue = !simulateCannotCloseOnce; } @Override - public void onCameraOpened(@NonNull ActivityRecord cameraActivity, - @NonNull String cameraId) { + public void onCameraOpened(@NonNull ActivityRecord cameraActivity) { mOnCameraOpenedCounter++; + mIsCameraOpened = mCameraStateMonitor.isCameraRunningForActivity(cameraActivity); } @Override - public boolean onCameraClosed(@NonNull String cameraId) { - mOnCameraClosedCounter++; - boolean returnValue = mOnCameraClosedReturnValue; + public boolean canCameraBeClosed(@NonNull String cameraId) { + mCheckCanCloseCounter++; + final boolean returnValue = mCheckCanCloseReturnValue; // If false, return false only the first time, so it doesn't fall in the infinite retry // loop. - mOnCameraClosedReturnValue = true; + mCheckCanCloseReturnValue = true; return returnValue; } + @Override + public void onCameraClosed() { + mOnCameraClosedCounter++; + mIsCameraOpened = mCameraStateMonitor.isCameraRunningForActivity(mActivity); + } + void resetCounters() { mOnCameraOpenedCounter = 0; + mCheckCanCloseCounter = 0; mOnCameraClosedCounter = 0; } } |