Have better separation between adding, positioning, and reparenting task

Several methods in activity manager and window manager performed adding,
positioning, and reparenting a task operation and sometimes failed silently
when things don't work due the callers using the methods for a particular
operation, but getting a different operation due to programmer error.
This CL better separate the methods responsible for adding, positioning, and
reparenting a task and also fails hard when there is an error.

Test: bit FrameworksServicesTests:com.android.server.wm.TaskWindowContainerControllerTests
Test: Manual testing existing PiP doesn't leave the device in a bad state.
Bug: 34260633
Change-Id: Id64367da30fc6214eb6f95b2bd5e58ed0e953a88

12 files changed, 345 insertions, 176 deletions

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index c87412e3f05d..7fd91cb91d48 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -9572,7 +9572,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                 boolean preserveWindow = (resizeMode & RESIZE_MODE_PRESERVE_WINDOW) != 0;
                 if (stackId != task.getStackId()) {
                     mStackSupervisor.moveTaskToStackUncheckedLocked(task, stackId, ON_TOP,
-                            !FORCE_FOCUS, "resizeTask", true /* allowStackOnTop */);
+                            !FORCE_FOCUS, "resizeTask");
                     preserveWindow = false;
                 }
 
@@ -10020,7 +10020,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                 // Defer the resume so resume/pausing while moving stacks is dangerous.
                 mStackSupervisor.moveTaskToStackLocked(topTask.taskId, DOCKED_STACK_ID,
                         false /* toTop */, !FORCE_FOCUS, "swapDockedAndFullscreenStack",
-                        ANIMATE, true /* deferResume */, true /* allowStackOnTop */);
+                        ANIMATE, true /* deferResume */);
                 final int size = tasks.size();
                 for (int i = 0; i < size; i++) {
                     final int id = tasks.get(i).taskId;
@@ -10029,8 +10029,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                     }
                     mStackSupervisor.moveTaskToStackLocked(id,
                             FULLSCREEN_WORKSPACE_STACK_ID, true /* toTop */, !FORCE_FOCUS,
-                            "swapDockedAndFullscreenStack", ANIMATE, true /* deferResume */,
-                            true /* allowStackOnTop */);
+                            "swapDockedAndFullscreenStack", ANIMATE, true /* deferResume */);
                 }
 
                 // Because we deferred the resume, to avoid conflicts with stack switches while
@@ -10071,7 +10070,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                 mWindowManager.setDockedStackCreateState(createMode, initialBounds);
                 final boolean moved = mStackSupervisor.moveTaskToStackLocked(
                         taskId, DOCKED_STACK_ID, toTop, !FORCE_FOCUS, "moveTaskToDockedStack",
-                        animate, DEFER_RESUME, true /* allowStackOnTop */);
+                        animate, DEFER_RESUME);
                 if (moved) {
                     if (moveHomeStackFront) {
                         mStackSupervisor.moveHomeStackToFront("moveTaskToDockedStack");
@@ -10187,10 +10186,24 @@ public class ActivityManagerService extends IActivityManager.Stub
             try {
                 if (DEBUG_STACK) Slog.d(TAG_STACK, "positionTaskInStack: positioning task="
                         + taskId + " in stackId=" + stackId + " at position=" + position);
+                final TaskRecord task = mStackSupervisor.anyTaskForIdLocked(taskId);
+                if (task == null) {
+                    throw new IllegalArgumentException("positionTaskInStack: no task for id="
+                            + taskId);
+                }
+
                 final ActivityStack stack = mStackSupervisor.getStack(stackId, CREATE_IF_NEEDED,
                         !ON_TOP);
 
-                stack.positionChildAt(taskId, position);
+                // TODO: Have the callers of this API call a separate reparent method if that is
+                // what they intended to do vs. having this method also do reparenting.
+                if (task.getStack() == stack) {
+                    // Change position in current stack.
+                    stack.positionChildAt(task, position);
+                } else {
+                    // Reparent to new stack.
+                    task.reparent(stackId, position, "positionTaskInStack");
+                }
             } finally {
                 Binder.restoreCallingIdentity(ident);
             }
diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java
index abcaa249744f..4df0cb1d8e62 100644
--- a/services/core/java/com/android/server/am/ActivityStack.java
+++ b/services/core/java/com/android/server/am/ActivityStack.java
@@ -72,6 +72,8 @@ import static com.android.server.wm.AppTransition.TRANSIT_TASK_OPEN;
 import static com.android.server.wm.AppTransition.TRANSIT_TASK_OPEN_BEHIND;
 import static com.android.server.wm.AppTransition.TRANSIT_TASK_TO_BACK;
 import static com.android.server.wm.AppTransition.TRANSIT_TASK_TO_FRONT;
+import static java.lang.Integer.MAX_VALUE;
+import static java.lang.Integer.MIN_VALUE;
 
 import android.app.Activity;
 import android.app.ActivityManager;
@@ -2534,6 +2536,30 @@ final class ActivityStack extends ConfigurationContainer {
         return null;
     }
 
+    /** Returns the position the input task should be placed in this stack. */
+    int getAdjustedPositionForTask(TaskRecord task, int suggestedPosition,
+            ActivityRecord starting) {
+
+        int maxPosition = mTaskHistory.size();
+        if ((starting != null && starting.okToShowLocked())
+                || (starting == null && task.okToShowLocked())) {
+            // If the task or starting activity can be shown, then whatever position is okay.
+            return Math.min(suggestedPosition, maxPosition);
+        }
+
+        // The task can't be shown, put non-current user tasks below current user tasks.
+        while (maxPosition > 0) {
+            final TaskRecord tmpTask = mTaskHistory.get(maxPosition - 1);
+            if (!mStackSupervisor.isCurrentProfileLocked(tmpTask.userId)
+                    || tmpTask.topRunningActivityLocked() == null) {
+                break;
+            }
+            maxPosition--;
+        }
+
+        return  Math.min(suggestedPosition, maxPosition);
+    }
+
     /**
      * Used from {@link ActivityStack#positionTask(TaskRecord, int)}.
      * @see ActivityManagerService#positionTaskInStack(int, int, int).
@@ -2543,32 +2569,25 @@ final class ActivityStack extends ConfigurationContainer {
             insertTaskAtTop(task, null);
             return;
         }
-        // Calculate maximum possible position for this task.
-        int maxPosition = mTaskHistory.size();
-        if (!task.okToShowLocked()) {
-            // Put non-current user tasks below current user tasks.
-            while (maxPosition > 0) {
-                final TaskRecord tmpTask = mTaskHistory.get(maxPosition - 1);
-                if (!mStackSupervisor.isCurrentProfileLocked(tmpTask.userId)
-                        || tmpTask.topRunningActivityLocked() == null) {
-                    break;
-                }
-                maxPosition--;
-            }
-        }
-        position = Math.min(position, maxPosition);
+        position = getAdjustedPositionForTask(task, position, null /* starting */);
         mTaskHistory.remove(task);
         mTaskHistory.add(position, task);
-        task.positionWindowContainerAt(mStackId, position);
+        task.positionWindowContainerAt(position);
         updateTaskMovement(task, true);
     }
 
-    private void insertTaskAtTop(TaskRecord task, ActivityRecord newActivity) {
-        insertTaskAtTop(task, newActivity, true /* allowStackOnTop */);
+    private void insertTaskAtTop(TaskRecord task, ActivityRecord starting) {
+        updateTaskReturnToForTopInsertion(task);
+        // TODO: Better place to put all the code below...may be addTask...
+        mTaskHistory.remove(task);
+        // Now put task at top.
+        final int position = getAdjustedPositionForTask(task, mTaskHistory.size(), starting);
+        mTaskHistory.add(position, task);
+        updateTaskMovement(task, true);
+        task.moveWindowContainerToTop(true /* includingParents */);
     }
 
-    private void insertTaskAtTop(TaskRecord task, ActivityRecord newActivity,
-            boolean allowStackOnTop) {
+    private void updateTaskReturnToForTopInsertion(TaskRecord task) {
         boolean isLastTaskOverHome = false;
         // If the moving task is over home stack, transfer its return type to next task
         if (task.isOverHomeStack()) {
@@ -2585,49 +2604,27 @@ final class ActivityStack extends ConfigurationContainer {
         if (isOnHomeDisplay()) {
             ActivityStack lastStack = mStackSupervisor.getLastStack();
             final boolean fromHomeOrRecents = lastStack.isHomeOrRecentsStack();
-            final boolean fromOnTopLauncher = lastStack.topTask() != null &&
-                    lastStack.topTask().isOnTopLauncher();
+            final TaskRecord topTask = lastStack.topTask();
+            final boolean fromOnTopLauncher = topTask != null && topTask.isOnTopLauncher();
             if (fromOnTopLauncher) {
                 // Since an on-top launcher will is moved to back when tasks are launched from it,
                 // those tasks should first try to return to a non-home activity.
                 // This also makes sure that non-home activities are visible under a transparent
                 // non-home activity.
                 task.setTaskToReturnTo(APPLICATION_ACTIVITY_TYPE);
-            } else if (!isHomeOrRecentsStack() && (fromHomeOrRecents || topTask() != task)) {
+            } else if (!isHomeOrRecentsStack() && (fromHomeOrRecents || topTask != task)) {
                 // If it's a last task over home - we default to keep its return to type not to
                 // make underlying task focused when this one will be finished.
                 int returnToType = isLastTaskOverHome
                         ? task.getTaskToReturnTo() : APPLICATION_ACTIVITY_TYPE;
                 if (fromHomeOrRecents && StackId.allowTopTaskToReturnHome(mStackId)) {
-                    returnToType = lastStack.topTask() == null
-                            ? HOME_ACTIVITY_TYPE : lastStack.topTask().taskType;
+                    returnToType = topTask == null ? HOME_ACTIVITY_TYPE : topTask.taskType;
                 }
                 task.setTaskToReturnTo(returnToType);
             }
         } else {
             task.setTaskToReturnTo(APPLICATION_ACTIVITY_TYPE);
         }
-
-        mTaskHistory.remove(task);
-        // Now put task at top.
-        int taskNdx = mTaskHistory.size();
-        final boolean notShownWhenLocked =
-                (newActivity != null && !newActivity.okToShowLocked())
-                || (newActivity == null && !task.okToShowLocked());
-        if (notShownWhenLocked) {
-            // Put non-current user tasks below current user tasks.
-            while (--taskNdx >= 0) {
-                final TaskRecord tmpTask = mTaskHistory.get(taskNdx);
-                if (!mStackSupervisor.isCurrentProfileLocked(tmpTask.userId)
-                        || tmpTask.topRunningActivityLocked() == null) {
-                    break;
-                }
-            }
-            ++taskNdx;
-        }
-        mTaskHistory.add(taskNdx, task);
-        updateTaskMovement(task, true);
-        task.moveWindowContainerToTop(allowStackOnTop /* includingParents */);
     }
 
     final void startActivityLocked(ActivityRecord r, ActivityRecord focusedTopActivity,
@@ -4891,38 +4888,49 @@ final class ActivityStack extends ConfigurationContainer {
     }
 
     void addTask(final TaskRecord task, final boolean toTop, String reason) {
-        addTask(task, toTop, reason, true /* allowStackOnTop */);
+        addTask(task, toTop ? MAX_VALUE : 0, reason);
+        if (toTop) {
+            // TODO: figure-out a way to remove this call.
+            task.moveWindowContainerToTop(true /* includingParents */);
+        }
     }
 
-    void addTask(final TaskRecord task, final boolean toTop, String reason,
-            boolean allowStackOnTop) {
+    // TODO: This shouldn't allow automatic reparenting. Remove the call to preAddTask and deal
+    // with the fall-out...
+    void addTask(final TaskRecord task, int position, String reason) {
+        // TODO: Is this remove really needed? Need to look into the call path for the other addTask
+        mTaskHistory.remove(task);
+        position = getAdjustedPositionForTask(task, position, null /* starting */);
+        final boolean toTop = position >= mTaskHistory.size();
         final ActivityStack prevStack = preAddTask(task, reason, toTop);
 
         task.setStack(this);
+
         if (toTop) {
-            insertTaskAtTop(task, null, allowStackOnTop);
-        } else {
-            mTaskHistory.add(0, task);
-            updateTaskMovement(task, false);
+            updateTaskReturnToForTopInsertion(task);
         }
+
+        mTaskHistory.add(position, task);
+        updateTaskMovement(task, toTop);
+
         postAddTask(task, prevStack);
     }
 
-    void positionChildAt(int taskId, int index) {
-        final TaskRecord task = mStackSupervisor.anyTaskForIdLocked(taskId);
-        if (task == null) {
-            Slog.w(TAG, "positionTaskInStackLocked: no task for id=" + taskId);
-            return;
+    void positionChildAt(TaskRecord task, int index) {
+
+        if (task.getStack() != this) {
+            throw new IllegalArgumentException("AS.positionChildAt: task=" + task
+                    + " is not a child of stack=" + this + " current parent=" + task.getStack());
         }
 
         task.updateOverrideConfigurationForStack(this);
 
         final ActivityRecord topRunningActivity = task.topRunningActivityLocked();
         final boolean wasResumed = topRunningActivity == task.getStack().mResumedActivity;
-        final ActivityStack prevStack = preAddTask(task, "positionTask", !ON_TOP);
         task.setStack(this);
         insertTaskAtPosition(task, index);
-        postAddTask(task, prevStack);
+        postAddTask(task, null /* prevStack */);
+
         if (wasResumed) {
             if (mResumedActivity != null) {
                 Log.wtf(TAG, "mResumedActivity was already set when moving mResumedActivity from"
diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
index 8cf0708e22aa..527e91830ccf 100644
--- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
@@ -1933,7 +1933,7 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
                 }
                 if (stackId != currentStack.mStackId) {
                     currentStack = moveTaskToStackUncheckedLocked(task, stackId, ON_TOP,
-                            !FORCE_FOCUS, reason, true /* allowStackOnTop */);
+                            !FORCE_FOCUS, reason);
                     stackId = currentStack.mStackId;
                     // moveTaskToStackUncheckedLocked() should already placed the task on top,
                     // still need moveTaskToFrontLocked() below for any transition settings.
@@ -2238,18 +2238,16 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
                     }
                     moveTaskToStackLocked(tasks.get(i).taskId,
                             FULLSCREEN_WORKSPACE_STACK_ID, onTop, onTop /*forceFocus*/,
-                            "moveTasksToFullscreenStack - onTop", ANIMATE, DEFER_RESUME,
-                            true /* allowStackOnTop */);
+                            "moveTasksToFullscreenStack - onTop", ANIMATE, DEFER_RESUME);
                 }
 
                 ensureActivitiesVisibleLocked(null, 0, PRESERVE_WINDOWS);
                 resumeFocusedStackTopActivityLocked();
             } else {
                 for (int i = 0; i < size; i++) {
-                    moveTaskToStackLocked(tasks.get(i).taskId, FULLSCREEN_WORKSPACE_STACK_ID,
-                            true /* onTop */, false /* forceFocus */,
-                            "moveTasksToFullscreenStack - NOT_onTop", !ANIMATE, DEFER_RESUME,
-                            false /* allowStackOnTop */);
+                    final TaskRecord task = tasks.get(i);
+                    task.reparent(FULLSCREEN_WORKSPACE_STACK_ID, MAX_VALUE,
+                            "moveTasksToFullscreenStack - NOT_onTop");
                 }
             }
         } finally {
@@ -2385,7 +2383,8 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
                     final int insertPosition = isFullscreenStackVisible
                             ? Math.max(0, fullscreenStack.getChildCount() - 1)
                             : fullscreenStack.getChildCount();
-                    fullscreenStack.positionChildAt(tasks.get(i).taskId, insertPosition);
+                    final TaskRecord task = tasks.get(i);
+                    task.reparent(FULLSCREEN_WORKSPACE_STACK_ID, insertPosition, "removeStack");
                 }
                 ensureActivitiesVisibleLocked(null, 0, !PRESERVE_WINDOWS);
                 resumeFocusedStackTopActivityLocked();
@@ -2605,13 +2604,10 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
      * @param toTop True if the task should be placed at the top of the stack.
      * @param forceFocus if focus should be moved to the new stack
      * @param reason Reason the task is been moved.
-     * @param allowStackOnTop If stack movement should be moved to the top due to the addition of
-     *                        the task to the stack. E.g. Moving the stack to the front because it
-     *                        should be focused because it now contains the focused activity.
      * @return The stack the task was moved to.
      */
     ActivityStack moveTaskToStackUncheckedLocked(TaskRecord task, int stackId, boolean toTop,
-            boolean forceFocus, String reason, boolean allowStackOnTop) {
+            boolean forceFocus, String reason) {
 
         if (StackId.isMultiWindowStack(stackId) && !mService.mSupportsMultiWindow) {
             throw new IllegalStateException("moveTaskToStackUncheckedLocked: Device doesn't "
@@ -2642,15 +2638,14 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
         // if a docked stack is created below which will lead to the stack we are moving from and
         // its resizeable tasks being resized.
         task.mTemporarilyUnresizable = true;
-        final ActivityStack stack = getStack(stackId, CREATE_IF_NEEDED, toTop && allowStackOnTop);
+        final ActivityStack stack = getStack(stackId, CREATE_IF_NEEDED, toTop);
         task.mTemporarilyUnresizable = false;
-        task.reparentWindowContainer(stack.mStackId, toTop ? MAX_VALUE : MIN_VALUE);
-        stack.addTask(task, toTop, reason, allowStackOnTop);
+        task.reparent(stack.mStackId, toTop ? MAX_VALUE : 0, reason);
 
         // If the task had focus before (or we're requested to move focus),
         // move focus to the new stack by moving the stack to the front.
         stack.moveToFrontAndResumeStateIfNeeded(
-                r, allowStackOnTop && (forceFocus || wasFocused || wasFront), wasResumed, reason);
+                r, forceFocus || wasFocused || wasFront, wasResumed, reason);
 
         return stack;
     }
@@ -2658,11 +2653,11 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
     boolean moveTaskToStackLocked(int taskId, int stackId, boolean toTop, boolean forceFocus,
             String reason, boolean animate) {
         return moveTaskToStackLocked(taskId, stackId, toTop, forceFocus, reason, animate,
-                false /* deferResume */, true /* allowStackOnTop */);
+                false /* deferResume */);
     }
 
     boolean moveTaskToStackLocked(int taskId, int stackId, boolean toTop, boolean forceFocus,
-            String reason, boolean animate, boolean deferResume, boolean allowStackOnTop) {
+            String reason, boolean animate, boolean deferResume) {
         final TaskRecord task = anyTaskForIdLocked(taskId);
         if (task == null) {
             Slog.w(TAG, "moveTaskToStack: no task for id=" + taskId);
@@ -2702,7 +2697,7 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
         boolean kept = true;
         try {
             final ActivityStack stack = moveTaskToStackUncheckedLocked(
-                    task, stackId, toTop, forceFocus, reason + " moveTaskToStack", allowStackOnTop);
+                    task, stackId, toTop, forceFocus, reason + " moveTaskToStack");
             stackId = stack.mStackId;
 
             if (!animate) {
diff --git a/services/core/java/com/android/server/am/ActivityStarter.java b/services/core/java/com/android/server/am/ActivityStarter.java
index 547e0b22d958..81a5d3beefbd 100644
--- a/services/core/java/com/android/server/am/ActivityStarter.java
+++ b/services/core/java/com/android/server/am/ActivityStarter.java
@@ -1777,7 +1777,7 @@ class ActivityStarter {
             int stackId = mInTask.getLaunchStackId();
             if (stackId != mInTask.getStackId()) {
                 final ActivityStack stack = mSupervisor.moveTaskToStackUncheckedLocked(mInTask,
-                        stackId, ON_TOP, !FORCE_FOCUS, "inTaskToFront", true /* allowStackOnTop */);
+                        stackId, ON_TOP, !FORCE_FOCUS, "inTaskToFront");
                 stackId = stack.mStackId;
             }
             if (StackId.resizeStackWithLaunchBounds(stackId)) {
diff --git a/services/core/java/com/android/server/am/TaskRecord.java b/services/core/java/com/android/server/am/TaskRecord.java
index 9e22c505f9bf..4c4c4443b8ca 100644
--- a/services/core/java/com/android/server/am/TaskRecord.java
+++ b/services/core/java/com/android/server/am/TaskRecord.java
@@ -101,6 +101,8 @@ import static com.android.server.am.ActivityRecord.APPLICATION_ACTIVITY_TYPE;
 import static com.android.server.am.ActivityRecord.HOME_ACTIVITY_TYPE;
 import static com.android.server.am.ActivityRecord.RECENTS_ACTIVITY_TYPE;
 import static com.android.server.am.ActivityRecord.STARTING_WINDOW_SHOWN;
+import static com.android.server.am.ActivityStack.REMOVE_TASK_MODE_MOVING;
+import static com.android.server.am.ActivityStackSupervisor.CREATE_IF_NEEDED;
 import static com.android.server.am.ActivityStackSupervisor.PRESERVE_WINDOWS;
 
 final class TaskRecord extends ConfigurationContainer {
@@ -512,8 +514,8 @@ final class TaskRecord extends ConfigurationContainer {
     }
 
     // TODO: Remove once we have a stack controller.
-    void positionWindowContainerAt(int stackId, int index) {
-        mWindowContainerController.positionAt(stackId, index, mBounds, getOverrideConfiguration());
+    void positionWindowContainerAt(int position) {
+        mWindowContainerController.positionAt(position, mBounds, getOverrideConfiguration());
     }
 
     // TODO: Replace with moveChildToTop?
@@ -534,9 +536,36 @@ final class TaskRecord extends ConfigurationContainer {
         mWindowContainerController.getBounds(bounds);
     }
 
-    // TODO: make this part of adding it to the stack?
-    void reparentWindowContainer(int stackId, int position) {
-        mWindowContainerController.reparent(stackId, position);
+    // TODO: Should we be doing all the stuff in ASS.moveTaskToStackLocked?
+    void reparent(int stackId, int position, String reason) {
+        mService.mWindowManager.deferSurfaceLayout();
+
+        try {
+            final ActivityStackSupervisor supervisor = mService.mStackSupervisor;
+            final ActivityStack newStack = supervisor.getStack(stackId,
+                    CREATE_IF_NEEDED, false /* toTop */);
+            // Adjust the position for the new parent stack as needed.
+            position = newStack.getAdjustedPositionForTask(this, position, null /* starting */);
+
+            // Must reparent first in window manager to avoid a situation where AM can delete the
+            // we are coming from in WM before we reparent because it became empty.
+            mWindowContainerController.reparent(stackId, position);
+
+            final ActivityStack prevStack = mStack;
+            prevStack.removeTask(this, reason, REMOVE_TASK_MODE_MOVING);
+            newStack.addTask(this, position, reason);
+
+            supervisor.scheduleReportPictureInPictureModeChangedIfNeeded(this, prevStack);
+
+            if (voiceSession != null) {
+                try {
+                    voiceSession.taskStarted(intent, taskId);
+                } catch (RemoteException e) {
+                }
+            }
+        } finally {
+            mService.mWindowManager.continueSurfaceLayout();
+        }
     }
 
     void cancelWindowTransition() {
@@ -978,6 +1007,8 @@ final class TaskRecord extends ConfigurationContainer {
         addActivityAtIndex(mActivities.size(), r);
     }
 
+    // TODO: Figure-out if any of the call points expect the window container to be reparented and
+    // correct them to use the right method.
     void addActivityAtIndex(int index, ActivityRecord r) {
         // Remove r first, and if it wasn't already in the list and it's fullscreen, count it.
         if (!mActivities.remove(r) && r.fullscreen) {
diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java
index b1b7542f62ce..2d50e3a502b4 100644
--- a/services/core/java/com/android/server/wm/Task.java
+++ b/services/core/java/com/android/server/wm/Task.java
@@ -38,6 +38,7 @@ import android.util.Slog;
 import android.view.DisplayInfo;
 import android.view.Surface;
 
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.EventLogTags;
 
 import java.io.PrintWriter;
@@ -104,32 +105,40 @@ class Task extends WindowContainer<AppWindowToken> implements DimLayer.DimLayerU
     }
 
     DisplayContent getDisplayContent() {
-        return mStack.getDisplayContent();
+        return mStack != null ? mStack.getDisplayContent() : null;
     }
 
-    @Override
-    void addChild(AppWindowToken wtoken, int addPos) {
-        final int lastPos = mChildren.size();
-        if (addPos >= lastPos) {
-            addPos = lastPos;
-        } else {
-            for (int pos = 0; pos < lastPos && pos < addPos; ++pos) {
-                if (mChildren.get(pos).removed) {
-                    // addPos assumes removed tokens are actually gone.
-                    ++addPos;
-                }
-            }
+    int getAdjustedAddPosition(int suggestedPosition) {
+        final int size = mChildren.size();
+        if (suggestedPosition >= size) {
+            return Math.min(size, suggestedPosition);
         }
 
-        final WindowContainer parent = wtoken.getParent();
-        if (parent != null) {
-            parent.removeChild(wtoken);
+        for (int pos = 0; pos < size && pos < suggestedPosition; ++pos) {
+            // TODO: Confirm that this is the behavior we want long term.
+            if (mChildren.get(pos).removed) {
+                // suggestedPosition assumes removed tokens are actually gone.
+                ++suggestedPosition;
+            }
         }
-        super.addChild(wtoken, addPos);
+        return Math.min(size, suggestedPosition);
+    }
+
+    @Override
+    void addChild(AppWindowToken wtoken, int position) {
+        position = getAdjustedAddPosition(position);
+        super.addChild(wtoken, position);
         wtoken.mTask = this;
         mDeferRemoval = false;
     }
 
+    @Override
+    void positionChildAt(int position, AppWindowToken child, boolean includingParents) {
+        position = getAdjustedAddPosition(position);
+        super.positionChildAt(position, child, includingParents);
+        mDeferRemoval = false;
+    }
+
     private boolean hasWindowsAlive() {
         for (int i = mChildren.size() - 1; i >= 0; i--) {
             if (mChildren.get(i).hasWindowsAlive()) {
@@ -139,9 +148,14 @@ class Task extends WindowContainer<AppWindowToken> implements DimLayer.DimLayerU
         return false;
     }
 
+    @VisibleForTesting
+    boolean shouldDeferRemoval() {
+        return hasWindowsAlive() && mStack.isAnimating();
+    }
+
     @Override
     void removeIfPossible() {
-        if (hasWindowsAlive() && mStack.isAnimating()) {
+        if (shouldDeferRemoval()) {
             if (DEBUG_STACK) Slog.i(TAG, "removeTask: deferring removing taskId=" + mTaskId);
             mDeferRemoval = true;
             return;
@@ -166,7 +180,8 @@ class Task extends WindowContainer<AppWindowToken> implements DimLayer.DimLayerU
 
     void reparent(TaskStack stack, int position) {
         if (stack == mStack) {
-            return;
+            throw new IllegalArgumentException(
+                    "task=" + this + " already child of stack=" + mStack);
         }
         if (DEBUG_STACK) Slog.i(TAG, "reParentTask: removing taskId=" + mTaskId
                 + " from stack=" + mStack);
@@ -176,25 +191,8 @@ 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) {
-            // 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
-                    + " from stack=" + mStack);
-            EventLog.writeEvent(WM_TASK_REMOVED, mTaskId, "positionTaskInStack");
-            mStack.removeChild(this);
-            stack.addTask(this, position);
-        } else {
-            stack.positionChildAt(position, this, true /* includingParents */);
-        }
-
+    void positionAt(int position, Rect bounds, Configuration overrideConfig) {
+        mStack.positionChildAt(position, this, false /* includingParents */);
         resizeLocked(bounds, overrideConfig, false /* force */);
 
         for (int activityNdx = mChildren.size() - 1; activityNdx >= 0; --activityNdx) {
diff --git a/services/core/java/com/android/server/wm/TaskWindowContainerController.java b/services/core/java/com/android/server/wm/TaskWindowContainerController.java
index 0e4d048ee15e..bbc9ed2ef332 100644
--- a/services/core/java/com/android/server/wm/TaskWindowContainerController.java
+++ b/services/core/java/com/android/server/wm/TaskWindowContainerController.java
@@ -18,10 +18,10 @@ package com.android.server.wm;
 
 import android.app.ActivityManager.TaskSnapshot;
 import android.content.res.Configuration;
-import android.graphics.GraphicBuffer;
 import android.graphics.Rect;
 import android.util.EventLog;
 import android.util.Slog;
+import com.android.internal.annotations.VisibleForTesting;
 
 import static com.android.server.EventLogTags.WM_TASK_CREATED;
 import static com.android.server.wm.DragResizeMode.DRAG_RESIZE_MODE_DOCKED_DIVIDER;
@@ -59,13 +59,21 @@ public class TaskWindowContainerController
                         + stackId);
             }
             EventLog.writeEvent(WM_TASK_CREATED, taskId, stackId);
-            final Task task = new Task(taskId, stack, userId, mService, bounds, overrideConfig,
-                    isOnTopLauncher, resizeMode, homeTask, this);
+            final Task task = createTask(taskId, stack, userId, bounds, overrideConfig, resizeMode,
+                    homeTask, isOnTopLauncher);
             final int position = toTop ? POSITION_TOP : POSITION_BOTTOM;
             stack.addTask(task, position, showForAllUsers, true /* moveParents */);
         }
     }
 
+    @VisibleForTesting
+    Task createTask(int taskId, TaskStack stack, int userId, Rect bounds,
+            Configuration overrideConfig, int resizeMode, boolean homeTask,
+            boolean isOnTopLauncher) {
+        return new Task(taskId, stack, userId, mService, bounds, overrideConfig, isOnTopLauncher,
+                resizeMode, homeTask, this);
+    }
+
     @Override
     public void removeContainer() {
         synchronized(mWindowMap) {
@@ -78,7 +86,7 @@ public class TaskWindowContainerController
         }
     }
 
-    public void positionChildAt(AppWindowContainerController childController, int index) {
+    public void positionChildAt(AppWindowContainerController childController, int position) {
         synchronized(mService.mWindowMap) {
             final AppWindowToken aToken = childController.mContainer;
             if (aToken == null) {
@@ -91,7 +99,7 @@ public class TaskWindowContainerController
             if (task == null) {
                 throw new IllegalArgumentException("positionChildAt: invalid task=" + this);
             }
-            task.addChild(aToken, index);
+            task.positionChildAt(position, aToken, false /* includeParents */);
         }
     }
 
@@ -106,9 +114,7 @@ public class TaskWindowContainerController
             }
             final TaskStack stack = mService.mStackIdToStack.get(stackId);
             if (stack == null) {
-                if (DEBUG_STACK) Slog.i(TAG_WM,
-                        "reparent: could not find stackId=" + stackId);
-                return;
+                throw new IllegalArgumentException("reparent: could not find stackId=" + stackId);
             }
             mContainer.reparent(stack, position);
             final DisplayContent displayContent = stack.getDisplayContent();
@@ -140,22 +146,22 @@ public class TaskWindowContainerController
     }
 
     // TODO: Move to positionChildAt() in stack controller once we have a stack controller.
-    public void positionAt(int stackId, int index, Rect bounds, Configuration overrideConfig) {
+    public void positionAt(int position, Rect bounds, Configuration overrideConfig) {
         synchronized (mWindowMap) {
             if (DEBUG_STACK) Slog.i(TAG_WM, "positionChildAt: positioning taskId=" + mTaskId
-                    + " in stackId=" + stackId + " at " + index);
+                    + " at " + position);
             if (mContainer == null) {
                 if (DEBUG_STACK) Slog.i(TAG_WM,
-                        "positionTaskInStack: could not find taskId=" + mTaskId);
+                        "positionAt: could not find taskId=" + mTaskId);
                 return;
             }
-            final TaskStack stack = mService.mStackIdToStack.get(stackId);
+            final TaskStack stack = mContainer.mStack;
             if (stack == null) {
                 if (DEBUG_STACK) Slog.i(TAG_WM,
-                        "positionTaskInStack: could not find stackId=" + stackId);
+                        "positionAt: could not find stack for task=" + mContainer);
                 return;
             }
-            mContainer.positionTaskInStack(stack, index, bounds, overrideConfig);
+            mContainer.positionAt(position, bounds, overrideConfig);
             final DisplayContent displayContent = stack.getDisplayContent();
             displayContent.setLayoutNeeded();
             mService.mWindowPlacerLocked.performSurfacePlacement();
diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java
index fd7ea6d6726a..e231da8a7264 100644
--- a/services/core/java/com/android/server/wm/WindowContainer.java
+++ b/services/core/java/com/android/server/wm/WindowContainer.java
@@ -234,13 +234,20 @@ class WindowContainer<E extends WindowContainer> implements Comparable<WindowCon
      */
     @CallSuper
     void positionChildAt(int position, E child, boolean includingParents) {
+
+        if (child.getParent() != this) {
+            throw new IllegalArgumentException("removeChild: container=" + child.getName()
+                    + " is not a child of container=" + getName()
+                    + " current parent=" + child.getParent());
+        }
+
         if ((position < 0 && position != POSITION_BOTTOM)
                 || (position > mChildren.size() && position != POSITION_TOP)) {
             throw new IllegalArgumentException("positionAt: invalid position=" + position
                     + ", children number=" + mChildren.size());
         }
 
-        if (position == mChildren.size() - 1) {
+        if (position >= mChildren.size() - 1) {
             position = POSITION_TOP;
         } else if (position == 0) {
             position = POSITION_BOTTOM;
diff --git a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java
index 7f1c273300da..26accc3bcd41 100644
--- a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java
@@ -77,35 +77,8 @@ public class AppWindowContainerControllerTests extends WindowTestsBase {
     }
 
     private TestAppWindowContainerController createAppWindowController() {
-        final TaskStack stack = createTaskStackOnDisplay(sDisplayContent);
         final TestTaskWindowContainerController taskController =
-                new TestTaskWindowContainerController(stack.mStackId);
-        final IApplicationToken token = new TestIApplicationToken();
-        return new TestAppWindowContainerController(taskController, token);
-    }
-
-    private class TestAppWindowContainerController extends AppWindowContainerController {
-
-        final IApplicationToken mToken;
-
-        TestAppWindowContainerController(TestTaskWindowContainerController taskController,
-                IApplicationToken token) {
-            super(taskController, token, null /* listener */, 0 /* index */,
-                    SCREEN_ORIENTATION_UNSPECIFIED, true /* fullscreen */,
-                    true /* showForAllUsers */, 0 /* configChanges */, false /* voiceInteraction */,
-                    false /* launchTaskBehind */, false /* alwaysFocusable */,
-                    0 /* targetSdkVersion */, 0 /* rotationAnimationHint */,
-                    0 /* inputDispatchingTimeoutNanos */, sWm);
-            mToken = token;
-        }
-    }
-
-    private class TestIApplicationToken implements IApplicationToken {
-
-        private final Binder mBinder = new Binder();
-        @Override
-        public IBinder asBinder() {
-            return mBinder;
-        }
+                new TestTaskWindowContainerController();
+        return new TestAppWindowContainerController(taskController);
     }
 }
diff --git a/services/tests/servicestests/src/com/android/server/wm/TaskWindowContainerControllerTests.java b/services/tests/servicestests/src/com/android/server/wm/TaskWindowContainerControllerTests.java
index 0dd31c36e207..f84bf607204b 100644
--- a/services/tests/servicestests/src/com/android/server/wm/TaskWindowContainerControllerTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/TaskWindowContainerControllerTests.java
@@ -22,6 +22,11 @@ import android.platform.test.annotations.Presubmit;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
 /**
  * Test class for {@link TaskWindowContainerController}.
  *
@@ -32,7 +37,73 @@ import android.support.test.runner.AndroidJUnit4;
 @Presubmit
 @org.junit.runner.RunWith(AndroidJUnit4.class)
 public class TaskWindowContainerControllerTests extends WindowTestsBase {
-// TODO Add tests once StackWindowContainerController is created.
+
     @Test
-    public void dummyTest() throws Exception {}
+    public void testRemoveContainer() throws Exception {
+        final TestTaskWindowContainerController taskController =
+                new TestTaskWindowContainerController();
+        final TestAppWindowContainerController appController =
+                new TestAppWindowContainerController(taskController);
+
+        taskController.removeContainer();
+        // Assert that the container was removed.
+        assertNull(taskController.mContainer);
+        assertNull(appController.mContainer);
+    }
+
+    @Test
+    public void testRemoveContainer_DeferRemoval() throws Exception {
+        final TestTaskWindowContainerController taskController =
+                new TestTaskWindowContainerController();
+        final TestAppWindowContainerController appController =
+                new TestAppWindowContainerController(taskController);
+
+        final TestTask task = (TestTask) taskController.mContainer;
+        final AppWindowToken app = appController.mContainer;
+        task.mShouldDeferRemoval = true;
+
+        taskController.removeContainer();
+        // For the case of deferred removal the task controller will no longer be connected to the
+        // container, but the app controller will still be connected to the its container until
+        // the task window container is removed.
+        assertNull(taskController.mContainer);
+        assertNull(task.getController());
+        assertNotNull(appController.mContainer);
+        assertNotNull(app.getController());
+
+        task.removeImmediately();
+        assertNull(appController.mContainer);
+        assertNull(app.getController());
+    }
+
+    @Test
+    public void testReparent() throws Exception {
+        final TaskStack stack1 = createTaskStackOnDisplay(sDisplayContent);
+        final TestTaskWindowContainerController taskController =
+                new TestTaskWindowContainerController(stack1.mStackId);
+        final TaskStack stack2 = createTaskStackOnDisplay(sDisplayContent);
+        final TestTaskWindowContainerController taskController2 =
+                new TestTaskWindowContainerController(stack2.mStackId);
+
+        boolean gotException = false;
+        try {
+            taskController.reparent(stack1.mStackId, 0);
+        } catch (IllegalArgumentException e) {
+            gotException = true;
+        }
+        assertTrue("Should not be able to reparent to the same parent", gotException);
+
+        gotException = false;
+        try {
+            taskController.reparent(sNextStackId + 1, 0);
+        } catch (IllegalArgumentException e) {
+            gotException = true;
+        }
+        assertTrue("Should not be able to reparent to a stackId that doesn't exist", gotException);
+
+        taskController.reparent(stack2.mStackId, 0);
+        assertEquals(stack2, taskController.mContainer.getParent());
+        assertEquals(0, ((TestTask) taskController.mContainer).positionInParent());
+        assertEquals(1, ((TestTask) taskController2.mContainer).positionInParent());
+    }
 }
diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java
index b57329c5c701..4f740ac46646 100644
--- a/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java
+++ b/services/tests/servicestests/src/com/android/server/wm/WindowContainerTests.java
@@ -315,7 +315,7 @@ public class WindowContainerTests extends WindowTestsBase {
         gotException = false;
         try {
             // Check response to position that's bigger than child number.
-            root.positionChildAt(2, child1, false /* includingParents */);
+            root.positionChildAt(3, child1, false /* includingParents */);
         } catch (IllegalArgumentException e) {
             gotException = true;
         }
diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java
index 466bd6700f16..53e5a2a5995a 100644
--- a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java
+++ b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java
@@ -16,6 +16,10 @@
 
 package com.android.server.wm;
 
+import android.content.res.Configuration;
+import android.graphics.Rect;
+import android.os.Binder;
+import android.view.IApplicationToken;
 import org.junit.Assert;
 import org.junit.Before;
 
@@ -28,6 +32,7 @@ import android.view.WindowManager;
 import static android.app.ActivityManager.StackId.FIRST_DYNAMIC_STACK_ID;
 import static android.app.AppOpsManager.OP_NONE;
 import static android.content.pm.ActivityInfo.RESIZE_MODE_UNRESIZEABLE;
+import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;
 import static android.content.res.Configuration.EMPTY;
 import static android.view.WindowManager.LayoutParams.FIRST_APPLICATION_WINDOW;
 import static android.view.WindowManager.LayoutParams.LAST_APPLICATION_WINDOW;
@@ -50,7 +55,7 @@ class WindowTestsBase {
     static WindowManagerService sWm = null;
     private final IWindow mIWindow = new TestIWindow();
     private final Session mMockSession = mock(Session.class);
-    private static int sNextStackId = FIRST_DYNAMIC_STACK_ID;
+    static int sNextStackId = FIRST_DYNAMIC_STACK_ID;
     private static int sNextTaskId = 0;
 
     private static boolean sOneTimeSetupDone = false;
@@ -201,16 +206,78 @@ class WindowTestsBase {
         }
     }
 
+    /* Used so we can gain access to some protected members of the {@link Task} class */
+    class TestTask extends Task {
+
+        boolean mShouldDeferRemoval = false;
+
+        TestTask(int taskId, TaskStack stack, int userId, WindowManagerService service, Rect bounds,
+                Configuration overrideConfig, boolean isOnTopLauncher, int resizeMode,
+                boolean homeTask, TaskWindowContainerController controller) {
+            super(taskId, stack, userId, service, bounds, overrideConfig, isOnTopLauncher,
+                    resizeMode, homeTask, controller);
+        }
+
+        boolean shouldDeferRemoval() {
+            return mShouldDeferRemoval;
+        }
+
+        int positionInParent() {
+            return getParent().mChildren.indexOf(this);
+        }
+    }
+
     /**
      * Used so we can gain access to some protected members of {@link TaskWindowContainerController}
      * class.
      */
     class TestTaskWindowContainerController extends TaskWindowContainerController {
 
+        TestTaskWindowContainerController() {
+            this(createTaskStackOnDisplay(sDisplayContent).mStackId);
+        }
+
         TestTaskWindowContainerController(int stackId) {
             super(sNextTaskId++, stackId, 0 /* userId */, null /* bounds */,
                     EMPTY /* overrideConfig*/, RESIZE_MODE_UNRESIZEABLE, false /* homeTask*/,
                     false /* isOnTopLauncher */, true /* toTop*/, true /* showForAllUsers */);
         }
+
+        @Override
+        TestTask createTask(int taskId, TaskStack stack, int userId, Rect bounds,
+                Configuration overrideConfig, int resizeMode, boolean homeTask,
+                boolean isOnTopLauncher) {
+            return new TestTask(taskId, stack, userId, mService, bounds, overrideConfig,
+                    isOnTopLauncher, resizeMode, homeTask, this);
+        }
+    }
+
+    class TestAppWindowContainerController extends AppWindowContainerController {
+
+        final IApplicationToken mToken;
+
+        TestAppWindowContainerController(TestTaskWindowContainerController taskController) {
+            this(taskController, new TestIApplicationToken());
+        }
+
+        TestAppWindowContainerController(TestTaskWindowContainerController taskController,
+                IApplicationToken token) {
+            super(taskController, token, null /* listener */, 0 /* index */,
+                    SCREEN_ORIENTATION_UNSPECIFIED, true /* fullscreen */,
+                    true /* showForAllUsers */, 0 /* configChanges */, false /* voiceInteraction */,
+                    false /* launchTaskBehind */, false /* alwaysFocusable */,
+                    0 /* targetSdkVersion */, 0 /* rotationAnimationHint */,
+                    0 /* inputDispatchingTimeoutNanos */, sWm);
+            mToken = token;
+        }
+    }
+
+    class TestIApplicationToken implements IApplicationToken {
+
+        private final Binder mBinder = new Binder();
+        @Override
+        public IBinder asBinder() {
+            return mBinder;
+        }
     }
 }