Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit db6d13ce authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Fix incomplete removal of nested task 

For example:
T0 - T1 - A1 <- iterator
     T2 - A2
After calling A1.finishIfPossible(), the order may change to
T0 - T2 - A2
     T1 - A1 <- iterator
So A1 is finished twice (no-op) and T0 can not be removed because
A2 is still alive. This part is solved by using a list to collect
first. And change the cleanup order to from bottom to top, which
avoid some unnecessary focus adjusting.

Another problem is that when removing last task of a parent task,
it only checks !mCreatedByOrganizer, so the removal request is
ignored. This is fixed by checking shouldRemoveSelfOnLastChildRemoval.

Fixes: 222722020
Test: atest TaskTests#testRemoveContainer_multipleNestedTasks
Change-Id: I3a5cb17be57fae758f57e2d2c669cc3727346ff8
parent 1be9ef4f

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

+1 −1
Original line number Diff line number Diff line
@@ -1530,7 +1530,7 @@ class Task extends TaskFragment { 
                mTaskSupervisor.removeTask(this, false /* killProcess */,

                        !REMOVE_FROM_RECENTS, reason);

            }

        } else if (!mReuseTask && !mCreatedByOrganizer) {

        } else if (!mReuseTask && shouldRemoveSelfOnLastChildRemoval()) {

            // Remove entire task if it doesn't have any activity left and it isn't marked for reuse

            // or created by task organizer.

            if (!isRootTask()) {

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

+13 −4
Original line number Diff line number Diff line
@@ -2303,6 +2303,10 @@ class TaskFragment extends WindowContainer<WindowContainer> { 
        mMinHeight = minHeight;

    }



    boolean shouldRemoveSelfOnLastChildRemoval() {

        return !mCreatedByOrganizer || mIsRemovalRequested;

    }



    @Override

    void removeChild(WindowContainer child) {

        removeChild(child, true /* removeSelfIfPossible */);
@@ -2318,7 +2322,7 @@ class TaskFragment extends WindowContainer<WindowContainer> { 
                mBackScreenshots.remove(r.mActivityComponent.flattenToString());

            }

        }

        if (removeSelfIfPossible && (!mCreatedByOrganizer || mIsRemovalRequested) && !hasChild()) {

        if (removeSelfIfPossible && shouldRemoveSelfOnLastChildRemoval() && !hasChild()) {

            removeImmediately("removeLastChild " + child);

        }

    }
@@ -2336,13 +2340,18 @@ class TaskFragment extends WindowContainer<WindowContainer> { 
            return;

        }

        mIsRemovalRequested = true;

        forAllActivities(r -> {

            if (withTransition) {

        // The task order may be changed by finishIfPossible() for adjusting focus if there are

        // nested tasks, so add all activities into a list to avoid missed removals.

        final ArrayList<ActivityRecord> removingActivities = new ArrayList<>();

        forAllActivities((Consumer<ActivityRecord>) removingActivities::add);

        for (int i = removingActivities.size() - 1; i >= 0; --i) {

            final ActivityRecord r = removingActivities.get(i);

            if (withTransition && r.isVisible()) {

                r.finishIfPossible(reason, false /* oomAdj */);

            } else {

                r.destroyIfPossible(reason);

            }

        });

        }

    }



    void setDelayLastActivityRemoval(boolean delay) {

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

+21 −0
Original line number Diff line number Diff line
@@ -145,6 +145,27 @@ public class TaskTests extends WindowTestsBase { 
        verify(mAtm.getLockTaskController(), atLeast(1)).clearLockedTask(rootTask);

    }



    @Test

    public void testRemoveContainer_multipleNestedTasks() {

        final Task rootTask = createTask(mDisplayContent);

        rootTask.mCreatedByOrganizer = true;

        final Task task1 = new TaskBuilder(mSupervisor).setParentTaskFragment(rootTask).build();

        final Task task2 = new TaskBuilder(mSupervisor).setParentTaskFragment(rootTask).build();

        final ActivityRecord activity1 = createActivityRecord(task1);

        final ActivityRecord activity2 = createActivityRecord(task2);

        activity1.setVisible(false);



        // All activities under the root task should be finishing.

        rootTask.remove(true /* withTransition */, "test");

        assertTrue(activity1.finishing);

        assertTrue(activity2.finishing);



        // After all activities activities are destroyed, the root task should also be removed.

        activity1.removeImmediately();

        activity2.removeImmediately();

        assertFalse(rootTask.isAttached());

    }



    @Test

    public void testRemoveContainer_deferRemoval() {

        final Task rootTask = createTask(mDisplayContent);