Don't perform layout while adjusting displays/stacks state.

When we detach the stack from the display we are in an inconsistent
state. We need to finish that operation, before we start laying out
things again. Otherwise we will encounter subtle bugs, where stack is
partially closed, but still used during the layout.

Display detachment was already doing the right thing and scheduling a
layout after it finishes the display detach. However, removing the
stack's windows was triggering immediate relayout and causing a crash.

This CL also adds some missing synchronization around
WindowManagerService.mStackIdToStack, which is in most cases protected by
mWindowMap.

Bug: 22191609
Bug: 23615329
Change-Id: I1e2fc42e1a5b673be808acdec473f85f138d7062

2 files changed, 20 insertions, 7 deletions

diff --git a/services/core/java/com/android/server/wm/TaskStack.java b/services/core/java/com/android/server/wm/TaskStack.java
index 794b49c8cb19..985bbfbbf1e9 100644
--- a/services/core/java/com/android/server/wm/TaskStack.java
+++ b/services/core/java/com/android/server/wm/TaskStack.java
@@ -375,7 +375,10 @@ public class TaskStack {
             for (int appNdx = appWindowTokens.size() - 1; appNdx >= 0; --appNdx) {
                 final WindowList appWindows = appWindowTokens.get(appNdx).allAppWindows;
                 for (int winNdx = appWindows.size() - 1; winNdx >= 0; --winNdx) {
-                    mService.removeWindowInnerLocked(appWindows.get(winNdx));
+                    // We are in the middle of changing the state of displays/stacks/tasks. We need
+                    // to finish that, before we let layout interfere with it.
+                    mService.removeWindowInnerLocked(appWindows.get(winNdx),
+                            false /* performLayout */);
                     doAnotherLayoutPass = true;
                 }
             }
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index bc5ef36fc347..03c10d4d3abc 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -2793,6 +2793,10 @@ public class WindowManagerService extends IWindowManager.Stub
     }
 
     void removeWindowInnerLocked(WindowState win) {
+        removeWindowInnerLocked(win, true);
+    }
+
+    void removeWindowInnerLocked(WindowState win, boolean performLayout) {
         if (win.mRemoved) {
             // Nothing to do.
             return;
@@ -2890,7 +2894,9 @@ public class WindowManagerService extends IWindowManager.Stub
                 if (displayContent != null) {
                     displayContent.layoutNeeded = true;
                 }
-                performLayoutAndPlaceSurfacesLocked();
+                if (performLayout) {
+                    performLayoutAndPlaceSurfacesLocked();
+                }
                 if (win.mAppToken != null) {
                     win.mAppToken.updateReportedVisibilityLocked();
                 }
@@ -5193,7 +5199,9 @@ public class WindowManagerService extends IWindowManager.Stub
     }
 
     public void removeStack(int stackId) {
-        mStackIdToStack.remove(stackId);
+        synchronized (mWindowMap) {
+            mStackIdToStack.remove(stackId);
+        }
     }
 
     public void removeTask(int taskId) {
@@ -5267,10 +5275,12 @@ public class WindowManagerService extends IWindowManager.Stub
     }
 
     public void getStackBounds(int stackId, Rect bounds) {
-        final TaskStack stack = mStackIdToStack.get(stackId);
-        if (stack != null) {
-            stack.getBounds(bounds);
-            return;
+        synchronized (mWindowMap) {
+            final TaskStack stack = mStackIdToStack.get(stackId);
+            if (stack != null) {
+                stack.getBounds(bounds);
+                return;
+            }
         }
         bounds.setEmpty();
     }