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

Commit c940660b authored by Bryce Lee's avatar Bryce Lee
Browse files

Ignore activity lifecycle requests without matching client records. 

Previously, changes were made to finish destroyed activities. This
prevented the code sending lifecycle requests to a non-existent
client record. However, there are scenarios where an activity can
be destroyed without being finished. Finishing these activities will
lead to side effects, such as the activity not being in history and
therefore not restored when navigated back to.

This changelist unlinks finishing from setting an activity's state to
destroyed. For now, we will suppress lifecycle requests client side
without matching records.

Fixes: 74403650
Fixes: 74409828
Bug: 71506345
Test: enable don't keep activities. navigate back and forth
Test: atest CtsActivityManagerDeviceTestCases:ActivityLifecycleTests#testRestoreFromKill
Test: atest CtsActivityManagerDeviceTestCases:ActivityLifecycleTests#testPausedWhenRecreatedFromInNonFocusedStack
Change-Id: I1b76a4758c3be27dc30ecac5ee56949a5b173754
parent a8cb5872

core/java/android/app/servertransaction/TransactionExecutor.java

+4 −1
Original line number Diff line number Diff line
@@ -147,7 +147,10 @@ public class TransactionExecutor { 
            pw.println("Executor:");

            dump(pw, prefix);



            Slog.wtf(TAG, stringWriter.toString());

            Slog.w(TAG, stringWriter.toString());



            // Ignore requests for non-existent client records for now.

            return;

        }



        // Cycle to the state right before the final requested state.

services/core/java/com/android/server/am/ActivityRecord.java

+0 −23
Original line number Diff line number Diff line
@@ -1621,12 +1621,6 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo 
            return;

        }



        if (isState(DESTROYED) || (state != DESTROYED && isState(DESTROYING))) {

            // We cannot move backwards from destroyed and destroying states.

            throw new IllegalArgumentException("cannot move back states once destroying"

                    + "current:" + mState + " requested:" + state);

        }



        final ActivityState prev = mState;

        mState = state;
@@ -1641,23 +1635,6 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo 
        if (parent != null) {

            parent.onActivityStateChanged(this, state, reason);

        }



        if (isState(DESTROYING, DESTROYED)) {

            makeFinishingLocked();



            // When moving to the destroyed state, immediately destroy the activity in the

            // associated stack. Most paths for finishing an activity will handle an activity's path

            // to destroy through mechanisms such as ActivityStackSupervisor#mFinishingActivities.

            // However, moving to the destroyed state directly (as in the case of an app dying) and

            // marking it as finished will lead to cleanup steps that will prevent later handling

            // from happening.

            if (isState(DESTROYED)) {

                final ActivityStack stack = getStack();

                if (stack != null) {

                    stack.activityDestroyedLocked(this, reason);

                }

            }

        }

    }



    ActivityState getState() {

services/core/java/com/android/server/am/ActivityStack.java

+10 −27
Original line number Diff line number Diff line
@@ -3811,14 +3811,6 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai 
        final ActivityState prevState = r.getState();

        if (DEBUG_STATES) Slog.v(TAG_STATES, "Moving to FINISHING: " + r);



        // We are already destroying / have already destroyed the activity. Do not continue to

        // modify it. Note that we do not use ActivityRecord#finishing here as finishing is not

        // indicative of destruction (though destruction is indicative of finishing) as finishing

        // can be delayed below.

        if (r.isState(DESTROYING, DESTROYED)) {

            return null;

        }



        r.setState(FINISHING, "finishCurrentActivityLocked");

        final boolean finishingActivityInNonFocusedStack

                = r.getStack() != mStackSupervisor.getFocusedStack()
@@ -4037,26 +4029,16 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai 
     * state to destroy so only the cleanup here is needed.

     *

     * Note: Call before #removeActivityFromHistoryLocked.

     *

     * @param r             The {@link ActivityRecord} to cleanup.

     * @param cleanServices Whether services bound to the {@link ActivityRecord} should also be

     *                      cleaned up.

     * @param destroy       Whether the {@link ActivityRecord} should be destroyed.

     * @param clearProcess  Whether the client process should be cleared.

     */

    private void cleanUpActivityLocked(ActivityRecord r, boolean cleanServices, boolean destroy,

            boolean clearProcess) {

    private void cleanUpActivityLocked(ActivityRecord r, boolean cleanServices, boolean setState) {

        onActivityRemovedFromStack(r);



        r.deferRelaunchUntilPaused = false;

        r.frozenBeforeDestroy = false;



        if (destroy) {

        if (setState) {

            if (DEBUG_STATES) Slog.v(TAG_STATES, "Moving to DESTROYED: " + r + " (cleaning up)");

            r.setState(DESTROYED, "cleanupActivityLocked");

        }



        if (clearProcess) {

            if (DEBUG_APP) Slog.v(TAG_APP, "Clearing app during cleanUp for activity " + r);

            r.app = null;

        }
@@ -4271,7 +4253,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai 
                        + ", app=" + (r.app != null ? r.app.processName : "(null)"));



        if (r.isState(DESTROYING, DESTROYED)) {

            if (DEBUG_STATES) Slog.v(TAG_STATES, "activity " + r + " already finishing."

            if (DEBUG_STATES) Slog.v(TAG_STATES, "activity " + r + " already destroying."

                    + "skipping request with reason:" + reason);

            return false;

        }
@@ -4282,8 +4264,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai 


        boolean removedFromHistory = false;



        cleanUpActivityLocked(r, false /* cleanServices */, false /* destroy */,

                false /*clearProcess*/);

        cleanUpActivityLocked(r, false, false);



        final boolean hadApp = r.app != null;
@@ -4380,6 +4361,10 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai 
        }

    }



    /**

     * This method is to only be called from the client via binder when the activity is destroyed

     * AND finished.

     */

    final void activityDestroyedLocked(ActivityRecord record, String reason) {

        if (record != null) {

            mHandler.removeMessages(DESTROY_TIMEOUT_MSG, record);
@@ -4389,8 +4374,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai 


        if (isInStackLocked(record) != null) {

            if (record.isState(DESTROYING, DESTROYED)) {

                cleanUpActivityLocked(record, true /* cleanServices */, false /* destroy */,

                        false /*clearProcess*/);

                cleanUpActivityLocked(record, true, false);

                removeActivityFromHistoryLocked(record, reason);

            }

        }
@@ -4499,8 +4483,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai 
                            r.icicle = null;

                        }

                    }

                    cleanUpActivityLocked(r, true /* cleanServices */, remove /* destroy */,

                            true /*clearProcess*/);

                    cleanUpActivityLocked(r, true, true);

                    if (remove) {

                        removeActivityFromHistoryLocked(r, "appDied");

                    }

services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java

+0 −31
Original line number Diff line number Diff line
@@ -214,35 +214,4 @@ public class ActivityRecordTests extends ActivityTestsBase { 
        verify(mService.mStackSupervisor, times(1)).canPlaceEntityOnDisplay(anyInt(), eq(expected),

                anyInt(), anyInt(), eq(record.info));

    }



    @Test

    public void testFinishingAfterDestroying() throws Exception {

        assertFalse(mActivity.finishing);

        mActivity.setState(DESTROYING, "testFinishingAfterDestroying");

        assertTrue(mActivity.isState(DESTROYING));

        assertTrue(mActivity.finishing);

    }



    @Test

    public void testFinishingAfterDestroyed() throws Exception {

        assertFalse(mActivity.finishing);

        mActivity.setState(DESTROYED, "testFinishingAfterDestroyed");

        assertTrue(mActivity.isState(DESTROYED));

        assertTrue(mActivity.finishing);

    }



    @Test

    public void testSetInvalidState() throws Exception {

        mActivity.setState(DESTROYED, "testInvalidState");



        boolean exceptionEncountered = false;



        try {

            mActivity.setState(FINISHING, "testInvalidState");

        } catch (IllegalArgumentException e) {

            exceptionEncountered = true;

        }



        assertTrue(exceptionEncountered);

    }

}

services/tests/servicestests/src/com/android/server/am/ActivityStackTests.java

+0 −22
Original line number Diff line number Diff line
@@ -478,28 +478,6 @@ public class ActivityStackTests extends ActivityTestsBase { 
        return stack;

    }



    @Test

    public void testSuppressMultipleDestroy() throws Exception {

        final ActivityRecord r = new ActivityBuilder(mService).setTask(mTask).build();

        final ClientLifecycleManager lifecycleManager = mock(ClientLifecycleManager.class);

        final ProcessRecord app = r.app;



        // The mocked lifecycle manager must be set on the ActivityStackSupervisor's reference to

        // the service rather than mService as mService is a spy and setting the value will not

        // propagate as ActivityManagerService hands its own reference to the

        // ActivityStackSupervisor during construction.

        ((TestActivityManagerService) mSupervisor.mService).setLifecycleManager(lifecycleManager);



        mStack.destroyActivityLocked(r, true, "first invocation");

        verify(lifecycleManager, times(1)).scheduleTransaction(eq(app.thread),

                eq(r.appToken), any(DestroyActivityItem.class));

        assertTrue(r.isState(DESTROYED, DESTROYING));



        mStack.destroyActivityLocked(r, true, "second invocation");

        verify(lifecycleManager, times(1)).scheduleTransaction(eq(app.thread),

                eq(r.appToken), any(DestroyActivityItem.class));

    }



    @Test

    public void testFinishDisabledPackageActivities() throws Exception {

        final ActivityRecord firstActivity = new ActivityBuilder(mService).setTask(mTask).build();