Merge "Do not trim tasks when updating activity visibilities" into rvc-dev

6 files changed, 66 insertions, 37 deletions

diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java
index 2413386db268..1cb4684b718e 100644
--- a/services/core/java/com/android/server/wm/ActivityStack.java
+++ b/services/core/java/com/android/server/wm/ActivityStack.java
@@ -1389,7 +1389,7 @@ class ActivityStack extends Task {
             boolean preserveWindows, boolean notifyClients) {
         mTopActivityOccludesKeyguard = false;
         mTopDismissingKeyguardActivity = null;
-        mStackSupervisor.getKeyguardController().beginActivityVisibilityUpdate();
+        mStackSupervisor.beginActivityVisibilityUpdate();
         try {
             mEnsureActivitiesVisibleHelper.process(
                     starting, configChanges, preserveWindows, notifyClients);
@@ -1400,7 +1400,7 @@ class ActivityStack extends Task {
                 notifyActivityDrawnLocked(null);
             }
         } finally {
-            mStackSupervisor.getKeyguardController().endActivityVisibilityUpdate();
+            mStackSupervisor.endActivityVisibilityUpdate();
         }
     }
 
diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
index 105b673c3871..0c3293030758 100644
--- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
@@ -332,11 +332,10 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks {
     PowerManager.WakeLock mGoingToSleepWakeLock;
 
     /**
-     * Temporary rect used during docked stack resize calculation so we don't need to create a new
-     * object each time.
+     * Used to keep {@link RootWindowContainer#ensureActivitiesVisible} from being entered
+     * recursively. And only update keyguard states once the nested updates are done.
      */
-    private final Rect tempRect = new Rect();
-    private final ActivityOptions mTmpOptions = ActivityOptions.makeBasic();
+    private int mVisibilityTransactionDepth;
 
     private ActivityMetricsLogger mActivityMetricsLogger;
 
@@ -1925,6 +1924,7 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks {
         pw.print(prefix);
         pw.println("mCurTaskIdForUser=" + mCurTaskIdForUser);
         pw.println(prefix + "mUserStackInFront=" + mRootWindowContainer.mUserStackInFront);
+        pw.println(prefix + "mVisibilityTransactionDepth=" + mVisibilityTransactionDepth);
         if (!mWaitingForActivityVisible.isEmpty()) {
             pw.println(prefix + "mWaitingForActivityVisible=");
             for (int i = 0; i < mWaitingForActivityVisible.size(); ++i) {
@@ -2313,6 +2313,24 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks {
                 "android.server.am:TURN_ON:" + reason);
     }
 
+    /** Starts a batch of visibility updates. */
+    void beginActivityVisibilityUpdate() {
+        mVisibilityTransactionDepth++;
+    }
+
+    /** Ends a batch of visibility updates. */
+    void endActivityVisibilityUpdate() {
+        mVisibilityTransactionDepth--;
+        if (mVisibilityTransactionDepth == 0) {
+            getKeyguardController().visibilitiesUpdated();
+        }
+    }
+
+    /** Returns {@code true} if the caller is on the path to update visibility. */
+    boolean inActivityVisibilityUpdate() {
+        return mVisibilityTransactionDepth > 0;
+    }
+
     /**
      * Begin deferring resume to avoid duplicate resumes in one pass.
      */
diff --git a/services/core/java/com/android/server/wm/KeyguardController.java b/services/core/java/com/android/server/wm/KeyguardController.java
index f672394251f2..4c10d5819c10 100644
--- a/services/core/java/com/android/server/wm/KeyguardController.java
+++ b/services/core/java/com/android/server/wm/KeyguardController.java
@@ -70,7 +70,6 @@ class KeyguardController {
     private boolean mKeyguardGoingAway;
     private boolean mDismissalRequested;
     private int mBeforeUnoccludeTransit;
-    private int mVisibilityTransactionDepth;
     private final SparseArray<KeyguardDisplayState> mDisplayStates = new SparseArray<>();
     private final ActivityTaskManagerService mService;
     private RootWindowContainer mRootWindowContainer;
@@ -252,24 +251,6 @@ class KeyguardController {
     }
 
     /**
-     * Starts a batch of visibility updates.
-     */
-    void beginActivityVisibilityUpdate() {
-        mVisibilityTransactionDepth++;
-    }
-
-    /**
-     * Ends a batch of visibility updates. After all batches are done, this method makes sure to
-     * update lockscreen occluded/dismiss state if needed.
-     */
-    void endActivityVisibilityUpdate() {
-        mVisibilityTransactionDepth--;
-        if (mVisibilityTransactionDepth == 0) {
-            visibilitiesUpdated();
-        }
-    }
-
-    /**
      * @return True if we may show an activity while Keyguard is showing because we are in the
      *         process of dismissing it anyways, false otherwise.
      */
@@ -292,7 +273,11 @@ class KeyguardController {
                 && !mWindowManager.isKeyguardSecure(mService.getCurrentUserId());
     }
 
-    private void visibilitiesUpdated() {
+    /**
+     * Makes sure to update lockscreen occluded/dismiss state if needed after completing all
+     * visibility updates ({@link ActivityStackSupervisor#endActivityVisibilityUpdate}).
+     */
+    void visibilitiesUpdated() {
         boolean requestDismissKeyguard = false;
         for (int displayNdx = mRootWindowContainer.getChildCount() - 1;
              displayNdx >= 0; displayNdx--) {
@@ -568,7 +553,6 @@ class KeyguardController {
         pw.println(prefix + "  mKeyguardGoingAway=" + mKeyguardGoingAway);
         dumpDisplayStates(pw, prefix);
         pw.println(prefix + "  mDismissalRequested=" + mDismissalRequested);
-        pw.println(prefix + "  mVisibilityTransactionDepth=" + mVisibilityTransactionDepth);
         pw.println();
     }
 
diff --git a/services/core/java/com/android/server/wm/RecentTasks.java b/services/core/java/com/android/server/wm/RecentTasks.java
index ccd51de28c88..d8a4ecbbc650 100644
--- a/services/core/java/com/android/server/wm/RecentTasks.java
+++ b/services/core/java/com/android/server/wm/RecentTasks.java
@@ -1031,9 +1031,13 @@ class RecentTasks {
     void add(Task task) {
         if (DEBUG_RECENTS_TRIM_TASKS) Slog.d(TAG, "add: task=" + task);
 
+        // Only allow trimming task if it is not updating visibility for activities, so the caller
+        // doesn't need to handle unexpected size and index when looping task containers.
+        final boolean canTrimTask = !mSupervisor.inActivityVisibilityUpdate();
+
         // Clean up the hidden tasks when going to home because the user may not be unable to return
         // to the task from recents.
-        if (!mHiddenTasks.isEmpty() && task.isActivityTypeHome()) {
+        if (canTrimTask && !mHiddenTasks.isEmpty() && task.isActivityTypeHome()) {
             removeUnreachableHiddenTasks(task.getWindowingMode());
         }
 
@@ -1155,7 +1159,9 @@ class RecentTasks {
         }
 
         // Trim the set of tasks to the active set
-        trimInactiveRecentTasks();
+        if (canTrimTask) {
+            trimInactiveRecentTasks();
+        }
         notifyTaskPersisterLocked(task, false /* flush */);
     }
 
diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java
index a691736e754a..8b9e9fe132b7 100644
--- a/services/core/java/com/android/server/wm/RootWindowContainer.java
+++ b/services/core/java/com/android/server/wm/RootWindowContainer.java
@@ -262,9 +262,6 @@ class RootWindowContainer extends WindowContainer<DisplayContent>
     /** Set when a power hint has started, but not ended. */
     private boolean mPowerHintSent;
 
-    /** Used to keep ensureActivitiesVisible() from being entered recursively. */
-    private boolean mInEnsureActivitiesVisible = false;
-
     // The default minimal size that will be used if the activity doesn't specify its minimal size.
     // It will be calculated when the default display gets added.
     int mDefaultMinSizeOfResizeableTaskDp = -1;
@@ -1993,14 +1990,13 @@ class RootWindowContainer extends WindowContainer<DisplayContent>
      */
     void ensureActivitiesVisible(ActivityRecord starting, int configChanges,
             boolean preserveWindows, boolean notifyClients) {
-        if (mInEnsureActivitiesVisible) {
+        if (mStackSupervisor.inActivityVisibilityUpdate()) {
             // Don't do recursive work.
             return;
         }
-        mInEnsureActivitiesVisible = true;
 
         try {
-            mStackSupervisor.getKeyguardController().beginActivityVisibilityUpdate();
+            mStackSupervisor.beginActivityVisibilityUpdate();
             // First the front stacks. In case any are not fullscreen and are in front of home.
             for (int displayNdx = getChildCount() - 1; displayNdx >= 0; --displayNdx) {
                 final DisplayContent display = getChildAt(displayNdx);
@@ -2008,8 +2004,7 @@ class RootWindowContainer extends WindowContainer<DisplayContent>
                         notifyClients);
             }
         } finally {
-            mStackSupervisor.getKeyguardController().endActivityVisibilityUpdate();
-            mInEnsureActivitiesVisible = false;
+            mStackSupervisor.endActivityVisibilityUpdate();
         }
     }
 
diff --git a/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java b/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java
index ea933dfe42dc..5005c07832ab 100644
--- a/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java
+++ b/services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java
@@ -46,6 +46,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -276,6 +277,31 @@ public class RecentTasksTest extends ActivityTestsBase {
     }
 
     @Test
+    public void testAddTasksInVisibilityUpdate_expectNoTrim() {
+        mRecentTasks.setOnlyTestVisibleRange();
+        mRecentTasks.setParameters(-1 /* min */, 1 /* max */, -1 /* ms */);
+        mRecentTasks.add(mTasks.get(0));
+
+        doAnswer(invocation -> {
+            assertTrue(mSupervisor.inActivityVisibilityUpdate());
+            // Simulate an activity is resumed by EnsureActivitiesVisibleHelper. If its state is
+            // change to RESUMED, it will also be added to recents.
+            mRecentTasks.add(mTasks.get(1));
+            invocation.callRealMethod();
+            return null;
+        }).when(mSupervisor).endActivityVisibilityUpdate();
+
+        mTaskContainer.ensureActivitiesVisible(null /* starting */, 0 /* configChanges */,
+                false /* preserveWindows */, false /* notifyClients */);
+
+        assertFalse(mSupervisor.inActivityVisibilityUpdate());
+        assertThat(mCallbacksRecorder.mAdded).hasSize(2);
+        // Expect nothing is trimmed because we don't want the loop of ensure-visibility to be
+        // impacted by the arbitrary number of task removals.
+        assertNoTasksTrimmed();
+    }
+
+    @Test
     public void testAddTasksMultipleTasks_expectRemovedNoTrim() {
         // Add multiple same-affinity non-document tasks, ensure that it removes the other task,
         // but that it does not trim it