Mark camera as closed before notifying listeners.

This was the behaviour when only one camera policy existed. When
`CameraStateMonitor` was extracted, listeners were notified first.
There is an issue if the config update on camera close is synchronous:
it is possible to read camera compat values as if the camera compat mode
is still active, because `CameraStateMonitor` used to still provide
camera-running signal until listeners processed onCameraClosed.

Flag: com.android.window.flags.enable_camera_compat_for_desktop_windowing
Fixes: 380292051
Test: atest WmTests:CameraStateMonitorTests
Change-Id: I2acd2f7619dfafd24c5934377679b396130623e7

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