Make sure cleanup is always done when task is removed

When Stack#removeImmediately() was called it also recursively
called Task#removeImmediately(), which remove tasks from stack.
This lead to Task#mStack reference being nullified, but task
dim layer user was still registered in DimLayerController.
Therefore there was a crash when dim layer animation occured.

This CL moves most of the logic from Task#removeIfPossible() to
Task#removeImmediately() to make sure that cleanup is performed
every time when task is removed.

Change-Id: Id8d72dc8c66b9eeefbf5c918cf0a0df4ea027fde
Fixes: 34052466
Test: bit FrameworksServicesTests:com.android.server.wm.TaskStackTests
Test: #testStackRemoveImmediately

diff --git a/services/core/java/com/android/server/wm/DimLayerController.java b/services/core/java/com/android/server/wm/DimLayerController.java
index da2c6a7..04ae72f 100644 (file)
--- a/services/core/java/com/android/server/wm/DimLayerController.java
+++ b/services/core/java/com/android/server/wm/DimLayerController.java
@@ -11,6 +11,7 @@ import android.util.ArrayMap;
 import android.util.Slog;
 import android.util.TypedValue;
 
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.wm.DimLayer.DimLayerUser;
 
 import java.io.PrintWriter;
@@ -310,6 +311,11 @@ class DimLayerController {
         }
     }
 
+    @VisibleForTesting
+    boolean hasDimLayerUser(DimLayer.DimLayerUser dimLayerUser) {
+        return mState.containsKey(dimLayerUser);
+    }
+
     void applyDimBehind(DimLayer.DimLayerUser dimLayerUser, WindowStateAnimator animator) {
         applyDim(dimLayerUser, animator, false /* aboveApp */);
     }

diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java
index 6005a99..a468598 100644 (file)
--- a/services/core/java/com/android/server/wm/Task.java
+++ b/services/core/java/com/android/server/wm/Task.java
@@ -144,14 +144,22 @@ class Task extends WindowContainer<AppWindowToken> implements DimLayer.DimLayerU
             mDeferRemoval = true;
             return;
         }
+        removeImmediately();
+    }
+
+    @Override
+    void removeImmediately() {
         if (DEBUG_STACK) Slog.i(TAG, "removeTask: removing taskId=" + mTaskId);
         EventLog.writeEvent(EventLogTags.WM_TASK_REMOVED, mTaskId, "removeTask");
         mDeferRemoval = false;
+
+        // Make sure to remove dim layer user first before removing task its from parent.
         DisplayContent content = getDisplayContent();
         if (content != null) {
             content.mDimLayerController.removeDimLayerUser(this);
         }
-        removeImmediately();
+
+        super.removeImmediately();
         mService.mTaskIdToTask.delete(mTaskId);
     }
 
@@ -170,7 +178,12 @@ class Task extends WindowContainer<AppWindowToken> implements DimLayer.DimLayerU
     /** @see com.android.server.am.ActivityManagerService#positionTaskInStack(int, int, int). */
     void positionTaskInStack(TaskStack stack, int position, Rect bounds,
             Configuration overrideConfig) {
-        if (mStack != null && stack != mStack) {
+        if (mStack == null) {
+            // There is an assumption that task already has a stack at this point, so lets make
+            // sure we comply with it.
+            throw new IllegalStateException("Trying to position task that has no parent.");
+        }
+        if (stack != mStack) {
             // Task is already attached to a different stack. First we need to remove it from there
             // and add to top of the target stack. We will move it proper position afterwards.
             if (DEBUG_STACK) Slog.i(TAG, "positionTaskInStack: removing taskId=" + mTaskId

diff --git a/services/tests/servicestests/src/com/android/server/wm/TaskStackTests.java b/services/tests/servicestests/src/com/android/server/wm/TaskStackTests.java
index eca2500..1c69033 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/wm/TaskStackTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/TaskStackTests.java
@@ -56,4 +56,20 @@ public class TaskStackTests extends WindowTestsBase {
         assertEquals(stack.mChildren.get(0), task2);
         assertEquals(stack.mChildren.get(1), task1);
     }
+
+    @Test
+    public void testStackRemoveImmediately() throws Exception {
+        final TaskStack stack = createTaskStackOnDisplay(sDisplayContent);
+        final Task task = createTaskInStack(stack, 0 /* userId */);
+        assertEquals(stack, task.mStack);
+        assertTrue(sDisplayContent.mDimLayerController.hasDimLayerUser(stack));
+        assertTrue(sDisplayContent.mDimLayerController.hasDimLayerUser(task));
+
+        // Remove stack and check if its child is also removed.
+        stack.removeImmediately();
+        assertNull(stack.getDisplayContent());
+        assertNull(task.mStack);
+        assertFalse(sDisplayContent.mDimLayerController.hasDimLayerUser(stack));
+        assertFalse(sDisplayContent.mDimLayerController.hasDimLayerUser(task));
+    }
 }