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

Commit 06b8dcb5 authored by Andrii Kulian's avatar Andrii Kulian
Browse files

Encapsulate savedState updates in ActivityRecord 

ActivityRecord#icicle and ActivityRecord#haveState values should
always be in sync. Since they were package-private, this creates an
opportunity for making error by updating one without the other.

This CL adds getters and a setter (for updates from package) that
always updates both values simultaneously, and makes the fields
private. It also hides ActivityRecord#persistentState and exposes a
package-private getter instead.

Test: ActivityRecordTests
Change-Id: Ie6cf248bf58f8d447ab321bb4348a4a69d547b19
parent 39f27442

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

+47 −20
Original line number Diff line number Diff line
@@ -317,10 +317,15 @@ final class ActivityRecord extends ConfigurationContainer { 
    UriPermissionOwner uriPermissions; // current special URI access perms.

    WindowProcessController app;      // if non-null, hosting application

    private ActivityState mState;    // current state we are in

    Bundle  icicle;         // last saved activity state

    PersistableBundle persistentState; // last persistently saved activity state

    private Bundle mIcicle;         // last saved activity state

    private PersistableBundle mPersistentState; // last persistently saved activity state

    private boolean mHaveState = true; // Indicates whether the last saved state of activity is

                                       // preserved. This starts out 'true', since the initial state

                                       // of an activity is that we have everything, and we should

                                       // never consider it lacking in state to be removed if it

                                       // dies. After an activity is launched it follows the value

                                       // of #mIcicle.

    boolean launchFailed;   // set if a launched failed, to abort on 2nd try

    boolean haveState;      // have we gotten the last activity state?

    boolean stopped;        // is activity pause finished?

    boolean delayedResume;  // not yet resumed because of stopped app switches?

    boolean finishing;      // activity in pending finish list?
@@ -550,8 +555,8 @@ final class ActivityRecord extends ConfigurationContainer { 
                if (lastLaunchTime == 0) pw.print("0");

                else TimeUtils.formatDuration(lastLaunchTime, now, pw);

                pw.println();

        pw.print(prefix); pw.print("haveState="); pw.print(haveState);

                pw.print(" icicle="); pw.println(icicle);

        pw.print(prefix); pw.print("mHaveState="); pw.print(mHaveState);

                pw.print(" mIcicle="); pw.println(mIcicle);

        pw.print(prefix); pw.print("state="); pw.print(mState);

                pw.print(" stopped="); pw.print(stopped);

                pw.print(" delayedResume="); pw.print(delayedResume);
@@ -612,6 +617,34 @@ final class ActivityRecord extends ConfigurationContainer { 
        }

    }



    /** Update the saved state of an activity. */

    void setSavedState(@Nullable Bundle savedState) {

        mIcicle = savedState;

        mHaveState = mIcicle != null;

    }



    /**

     * Get the actual Bundle instance of the saved state.

     * @see #hasSavedState() for checking if the record has saved state.

     */

    @Nullable Bundle getSavedState() {

        return mIcicle;

    }



    /**

     * Check if the activity has saved state.

     * @return {@code true} if the client reported a non-empty saved state from last onStop(), or

     *         if this record was just created and the client is yet to be launched and resumed.

     */

    boolean hasSavedState() {

        return mHaveState;

    }



    /** @return The actual PersistableBundle instance of the saved persistent state. */

    @Nullable PersistableBundle getPersistentSavedState() {

        return mPersistentState;

    }



    void updateApplicationInfo(ApplicationInfo aInfo) {

        info.applicationInfo = aInfo;

    }
@@ -969,10 +1002,6 @@ final class ActivityRecord extends ConfigurationContainer { 
        hasBeenLaunched = false;

        mStackSupervisor = supervisor;



        // This starts out true, since the initial state of an activity is that we have everything,

        // and we shouldn't never consider it lacking in state to be removed if it dies.

        haveState = true;



        // If the class name in the intent doesn't match that of the target, this is

        // probably an alias. We have to create a new ComponentName object to keep track

        // of the real activity name, so that FLAG_ACTIVITY_CLEAR_TOP is handled properly.
@@ -2182,8 +2211,7 @@ final class ActivityRecord extends ConfigurationContainer { 
            // been removed (e.g. destroy timeout), so the token could be null.

            return;

        }

        r.icicle = null;

        r.haveState = false;

        r.setSavedState(null /* savedState */);



        final ActivityDisplay display = r.getDisplay();

        if (display != null) {
@@ -2256,19 +2284,18 @@ final class ActivityRecord extends ConfigurationContainer { 
            return;

        }

        if (newPersistentState != null) {

            persistentState = newPersistentState;

            mPersistentState = newPersistentState;

            mAtmService.notifyTaskPersisterLocked(task, false);

        }

        if (DEBUG_SAVED_STATE) Slog.i(TAG_SAVED_STATE, "Saving icicle of " + this + ": " + icicle);



        if (newIcicle != null) {

            // If icicle is null, this is happening due to a timeout, so we haven't really saved

            // the state.

            icicle = newIcicle;

            haveState = true;

            setSavedState(newIcicle);

            launchCount = 0;

            updateTaskDescription(description);

        }

        if (DEBUG_SAVED_STATE) Slog.i(TAG_SAVED_STATE, "Saving icicle of " + this + ": " + mIcicle);

        if (!stopped) {

            if (DEBUG_STATES) Slog.v(TAG_STATES, "Moving to STOPPED: " + this + " (stop complete)");

            stack.mHandler.removeMessages(STOP_TIMEOUT_MSG, this);
@@ -2588,7 +2615,7 @@ final class ActivityRecord extends ConfigurationContainer { 
        }

        final ActivityStack stack = getActivityStack();

        if (stack == null || this == stack.getResumedActivity() || this == stack.mPausingActivity

                || !haveState || !stopped) {

                || !mHaveState || !stopped) {

            // We're not ready for this kind of thing.

            return false;

        }
@@ -3523,7 +3550,7 @@ final class ActivityRecord extends ConfigurationContainer { 
        // The restarting state avoids removing this record when process is died.

        setState(RESTARTING_PROCESS, "restartActivityProcess");



        if (!visible || haveState) {

        if (!visible || mHaveState) {

            // Kill its process immediately because the activity should be in background.

            // The activity state will be update to {@link #DESTROYED} in

            // {@link ActivityStack#cleanUpActivityLocked} when handling process died.
@@ -3612,9 +3639,9 @@ final class ActivityRecord extends ConfigurationContainer { 
        intent.saveToXml(out);

        out.endTag(null, TAG_INTENT);



        if (isPersistable() && persistentState != null) {

        if (isPersistable() && mPersistentState != null) {

            out.startTag(null, TAG_PERSISTABLEBUNDLE);

            persistentState.saveToXml(out);

            mPersistentState.saveToXml(out);

            out.endTag(null, TAG_PERSISTABLEBUNDLE);

        }

    }
@@ -3695,7 +3722,7 @@ final class ActivityRecord extends ConfigurationContainer { 
                0 /* reqCode */, componentSpecified, false /* rootVoiceInteraction */,

                stackSupervisor, null /* options */, null /* sourceRecord */);



        r.persistentState = persistentState;

        r.mPersistentState = persistentState;

        r.taskDescription = taskDescription;

        r.createTime = createTime;

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

+3 −11
Original line number Diff line number Diff line
@@ -78,7 +78,6 @@ import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_CONTAIN 
import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_PAUSE;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_RELEASE;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_RESULTS;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_SAVED_STATE;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_STACK;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_STATES;

import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_SWITCH;
@@ -1499,8 +1498,6 @@ class ActivityStack extends ConfigurationContainer { 
                + " callers=" + Debug.getCallers(5));

        r.setState(RESUMED, "minimalResumeActivityLocked");

        r.completeResumeLocked();

        if (DEBUG_SAVED_STATE) Slog.i(TAG_SAVED_STATE,

                "Launch completed; removing icicle of " + r.icicle);

    }



    private void clearLaunchTime(ActivityRecord r) {
@@ -3983,7 +3980,7 @@ class ActivityStack extends ConfigurationContainer { 
        r.results = null;

        r.pendingResults = null;

        r.newIntents = null;

        r.icicle = null;

        r.setSavedState(null /* savedState */);

    }



    /**
@@ -4800,7 +4797,7 @@ class ActivityStack extends ConfigurationContainer { 
                        // it has failed more than twice. Skip activities that's already finishing

                        // cleanly by itself.

                        remove = false;

                    } else if ((!r.haveState && !r.stateNotNeeded

                    } else if ((!r.hasSavedState() && !r.stateNotNeeded

                            && !r.isState(ActivityState.RESTARTING_PROCESS)) || r.finishing) {

                        // Don't currently have state for the activity, or

                        // it is finishing -- always remove it.
@@ -4820,7 +4817,7 @@ class ActivityStack extends ConfigurationContainer { 
                    if (remove) {

                        if (DEBUG_ADD_REMOVE || DEBUG_CLEANUP) Slog.i(TAG_ADD_REMOVE,

                                "Removing activity " + r + " from stack at " + i

                                + ": haveState=" + r.haveState

                                + ": hasSavedState=" + r.hasSavedState()

                                + " stateNotNeeded=" + r.stateNotNeeded

                                + " finishing=" + r.finishing

                                + " state=" + r.getState() + " callers=" + Debug.getCallers(5));
@@ -4843,11 +4840,6 @@ class ActivityStack extends ConfigurationContainer { 
                        // This is needed when user later tap on the dead window, we need to stop

                        // other apps when user transfers focus to the restarted activity.

                        r.nowVisible = r.visible;

                        if (!r.haveState) {

                            if (DEBUG_SAVED_STATE) Slog.i(TAG_SAVED_STATE,

                                    "App died, clearing saved state of " + r);

                            r.icicle = null;

                        }

                    }

                    cleanUpActivityLocked(r, true, true);

                    if (remove) {

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

+5 −4
Original line number Diff line number Diff line
@@ -813,8 +813,9 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { 
                    newIntents = r.newIntents;

                }

                if (DEBUG_SWITCH) Slog.v(TAG_SWITCH,

                        "Launching: " + r + " icicle=" + r.icicle + " with results=" + results

                                + " newIntents=" + newIntents + " andResume=" + andResume);

                        "Launching: " + r + " savedState=" + r.getSavedState()

                                + " with results=" + results + " newIntents=" + newIntents

                                + " andResume=" + andResume);

                EventLog.writeEvent(EventLogTags.AM_RESTART_ACTIVITY, r.mUserId,

                        System.identityHashCode(r), task.taskId, r.shortComponentName);

                if (r.isActivityTypeHome()) {
@@ -836,7 +837,7 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { 
                        proc.getConfiguration(), r.getMergedOverrideConfiguration());

                r.setLastReportedConfiguration(mergedConfiguration);



                logIfTransactionTooLarge(r.intent, r.icicle);

                logIfTransactionTooLarge(r.intent, r.getSavedState());





                // Create activity launch transaction.
@@ -851,7 +852,7 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks { 
                        mergedConfiguration.getGlobalConfiguration(),

                        mergedConfiguration.getOverrideConfiguration(), r.compat,

                        r.launchedFromPackage, task.voiceInteractor, proc.getReportedProcState(),

                        r.icicle, r.persistentState, results, newIntents,

                        r.getSavedState(), r.getPersistentSavedState(), results, newIntents,

                        dc.isNextTransitionForward(), proc.createProfilerInfoIfNeeded(),

                                r.assistToken));

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

+1 −1
Original line number Diff line number Diff line
@@ -734,7 +734,7 @@ public class WindowProcessController extends ConfigurationContainer<Configuratio 
            }

            // Don't consider any activities that are currently not in a state where they

            // can be destroyed.

            if (r.visible || !r.stopped || !r.haveState

            if (r.visible || !r.stopped || !r.hasSavedState()

                    || r.isState(STARTED, RESUMED, PAUSING, PAUSED, STOPPING)) {

                if (DEBUG_RELEASE) Slog.d(TAG_RELEASE, "Not releasing in-use activity: " + r);

                continue;

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

+56 −1
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import static com.android.server.wm.ActivityStack.ActivityState.PAUSING; 
import static com.android.server.wm.ActivityStack.ActivityState.RESUMED;

import static com.android.server.wm.ActivityStack.ActivityState.STARTED;

import static com.android.server.wm.ActivityStack.ActivityState.STOPPED;

import static com.android.server.wm.ActivityStack.ActivityState.STOPPING;

import static com.android.server.wm.ActivityStack.REMOVE_TASK_MODE_MOVING;

import static com.android.server.wm.ActivityStack.STACK_VISIBILITY_INVISIBLE;

import static com.android.server.wm.ActivityStack.STACK_VISIBILITY_VISIBLE;
@@ -62,6 +63,8 @@ import android.content.pm.ActivityInfo; 
import android.content.res.Configuration;

import android.content.res.Resources;

import android.graphics.Rect;

import android.os.Bundle;

import android.os.PersistableBundle;

import android.platform.test.annotations.Presubmit;

import android.util.MergedConfiguration;

import android.util.MutableBoolean;
@@ -200,7 +203,7 @@ public class ActivityRecordTests extends ActivityTestsBase { 
    public void testRestartProcessIfVisible() {

        doNothing().when(mSupervisor).scheduleRestartTimeout(mActivity);

        mActivity.visible = true;

        mActivity.haveState = false;

        mActivity.setSavedState(null /* savedState */);

        mActivity.setState(ActivityStack.ActivityState.RESUMED, "testRestart");

        prepareFixedAspectRatioUnresizableActivity();
@@ -626,6 +629,58 @@ public class ActivityRecordTests extends ActivityTestsBase { 
        assertThat(mActivity.canLaunchHomeActivity(NOBODY_UID, chooserActivity)).isTrue();

    }



    /**

     * Verify that an {@link ActivityRecord} reports that it has saved state after creation, and

     * that it is cleared after activity is resumed.

     */

    @Test

    public void testHasSavedState() {

        assertTrue(mActivity.hasSavedState());



        ActivityRecord.activityResumedLocked(mActivity.appToken);

        assertFalse(mActivity.hasSavedState());

        assertNull(mActivity.getSavedState());

    }



    /** Verify the behavior of {@link ActivityRecord#setSavedState(Bundle)}. */

    @Test

    public void testUpdateSavedState() {

        mActivity.setSavedState(null /* savedState */);

        assertFalse(mActivity.hasSavedState());

        assertNull(mActivity.getSavedState());



        final Bundle savedState = new Bundle();

        savedState.putString("test", "string");

        mActivity.setSavedState(savedState);

        assertTrue(mActivity.hasSavedState());

        assertEquals(savedState, mActivity.getSavedState());

    }



    /** Verify the correct updates of saved state when activity client reports stop. */

    @Test

    public void testUpdateSavedState_activityStopped() {

        final Bundle savedState = new Bundle();

        savedState.putString("test", "string");

        final PersistableBundle persistentSavedState = new PersistableBundle();

        persistentSavedState.putString("persist", "string");



        // Set state to STOPPING, or ActivityRecord#activityStoppedLocked() call will be ignored.

        mActivity.setState(STOPPING, "test");

        mActivity.activityStoppedLocked(savedState, persistentSavedState, "desc");

        assertTrue(mActivity.hasSavedState());

        assertEquals(savedState, mActivity.getSavedState());

        assertEquals(persistentSavedState, mActivity.getPersistentSavedState());



        // Sending 'null' for saved state can only happen due to timeout, so previously stored saved

        // states should not be overridden.

        mActivity.setState(STOPPING, "test");

        mActivity.activityStoppedLocked(null /* savedState */, null /* persistentSavedState */,

                "desc");

        assertTrue(mActivity.hasSavedState());

        assertEquals(savedState, mActivity.getSavedState());

        assertEquals(persistentSavedState, mActivity.getPersistentSavedState());

    }



    /** Setup {@link #mActivity} as a size-compat-mode-able activity without fixed orientation. */

    private void prepareFixedAspectRatioUnresizableActivity() {

        setupDisplayContentForCompatDisplayInsets();