commit 770597c76174184a05410e007ac992d905e27538
author Charles Chen <charlesccchen@google.com> Mon May 18 19:58:23 2020 +0800
committer Charles Chen <charlesccchen@google.com> Mon May 25 17:39:59 2020 +0800
tree bf1a4d36d24e44299818a0bdbd4ba6bc514312f6
parent 43c04e1457bbdd44aeb5526f6fe33eeceafc35e8

Fix recursive calls on DC#ensureActivitiesVisible

Previously CL[1] used display#ensureActivitiesVisible instead of
stack#ensureActivitiesVisible, and it caused recursive calls on
ensureActivitiesVisible in a very hidden way. A possible scenario
is when an activity paused and AM tried to resume another activity
which did not support multi-resume.

[1]: Id8253eb97581a9dd52ab748117d9cd4cc63a206d

fixes: 156189923
Test: atest DisplayContentTests#testEnsureActivitiesVisibleNotRecursive
Change-Id: Iba2a052e4a1d91edfc6fa80954a730ad5ca3ff0d

services/core/java/com/android/server/wm/DisplayContent.java

diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java
index d93e976..eb85db6 100644
--- a/services/core/java/com/android/server/wm/DisplayContent.java
+++ b/services/core/java/com/android/server/wm/DisplayContent.java
@@ -625,6 +625,12 @@
     // Used in performing layout
     private boolean mTmpWindowsBehindIme;
 
+    /**
+     * Used to prevent recursions when calling
+     * {@link #ensureActivitiesVisible(ActivityRecord, int, boolean, boolean)}
+     */
+    private boolean mInEnsureActivitiesVisible = false;
+
     private final Consumer<WindowState> mUpdateWindowsForAnimator = w -> {
         WindowStateAnimator winAnimator = w.mWinAnimator;
         final ActivityRecord activity = w.mActivityRecord;
@@ -5442,9 +5448,18 @@
 
     void ensureActivitiesVisible(ActivityRecord starting, int configChanges,
             boolean preserveWindows, boolean notifyClients) {
-        for (int i = getTaskDisplayAreaCount() - 1; i >= 0; --i) {
-            getTaskDisplayAreaAt(i).ensureActivitiesVisible(starting, configChanges,
-                    preserveWindows, notifyClients);
+        if (mInEnsureActivitiesVisible) {
+            // Don't do recursive work.
+            return;
+        }
+        mInEnsureActivitiesVisible = true;
+        try {
+            for (int i = getTaskDisplayAreaCount() - 1; i >= 0; --i) {
+                getTaskDisplayAreaAt(i).ensureActivitiesVisible(starting, configChanges,
+                        preserveWindows, notifyClients);
+            }
+        } finally {
+            mInEnsureActivitiesVisible = false;
         }
     }
 

services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java

diff --git a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java
index ac95a81..4e82ceb 100644
--- a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java
@@ -72,6 +72,7 @@
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 
 import android.annotation.SuppressLint;
 import android.app.ActivityTaskManager;
@@ -1142,7 +1143,7 @@
         Mockito.doReturn(ROTATION_90).when(dr).rotationForOrientation(anyInt(), anyInt());
         final boolean[] continued = new boolean[1];
         // TODO(display-merge): Remove cast
-        Mockito.doAnswer(
+        doAnswer(
                 invocation -> {
                     continued[0] = true;
                     return true;
@@ -1248,6 +1249,22 @@
         assertEquals(window, result);
     }
 
+    @Test
+    public void testEnsureActivitiesVisibleNotRecursive() {
+        final TaskDisplayArea mockTda = mock(TaskDisplayArea.class);
+        doReturn(mockTda).when(mDisplayContent).getTaskDisplayAreaAt(anyInt());
+        final boolean[] called = { false };
+        doAnswer(invocation -> {
+            // The assertion will fail if DisplayArea#ensureActivitiesVisible is called twice.
+            assertFalse(called[0]);
+            called[0] = true;
+            mDisplayContent.ensureActivitiesVisible(null, 0, false, false);
+            return null;
+        }).when(mockTda).ensureActivitiesVisible(any(), anyInt(), anyBoolean(), anyBoolean());
+
+        mDisplayContent.ensureActivitiesVisible(null, 0, false, false);
+    }
+
     private boolean isOptionsPanelAtRight(int displayId) {
         return (mWm.getPreferredOptionsPanelGravity(displayId) & Gravity.RIGHT) == Gravity.RIGHT;
     }