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

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

Address destroying activities multiple times. 

There are various paths which an activity can be destroyed. One
possibility is if we have already initiated destroying an activity
and then finish all activities in response to a display being
removed. In this case, a request will be made to destroy an activity
that could already be destroying. Additionally, the state is always
set to finishing, regardless of the current state.

With the activity lifecycler, multiple destroy requests are not
tolerated. The second request will lead to an exception due to not
being able to find the activity. Previously, redundant requests were
suppressed client side by simply ignoring them. The state change
leads to inconsistency as the Activity can come back from destroyed.

This changelist addresses these issues by not finishing a
destroying/destroyed activity. Additionally, the changelist now
enforces that state must move forward. Lastly, redundant destroy
requests are not ignored.

Bug: 71506345
Test: atest FrameworksServicesTests:com.android.server.am.ActivityRecordTests#testSetInvalidState
Test: atest FrameworksServicesTests:com.android.server.am.ActivityStackTests#testSuppressMultipleDestroy
Change-Id: I235fa8002f73ad20bb101551fe6ebd5bcc22fa6d
parent 9055c1b6

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

+5 −1
Original line number Diff line number Diff line
@@ -685,7 +685,7 @@ public class ActivityManagerService extends IActivityManager.Stub 














    private final ActivityStartController mActivityStartController;



























    final ClientLifecycleManager mLifecycleManager;













    private final ClientLifecycleManager mLifecycleManager;





























    final TaskChangeNotificationController mTaskChangeNotificationController;
























@@ -12303,6 +12303,10 @@ public class ActivityManagerService extends IActivityManager.Stub













        return mActivityStartController;















    }



























    ClientLifecycleManager getLifecycleManager() {













        return mLifecycleManager;













    }



























    PackageManagerInternal getPackageManagerInternalLocked() {















        if (mPackageManagerInt == null) {















            mPackageManagerInt = LocalServices.getService(PackageManagerInternal.class);

























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



+27
−17




























Original line number
Diff line number
Diff line








@@ -665,7 +665,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo













                    "Reporting activity moved to display" + ", activityRecord=" + this















                            + ", displayId=" + displayId + ", config=" + config);





























            service.mLifecycleManager.scheduleTransaction(app.thread, appToken,













            service.getLifecycleManager().scheduleTransaction(app.thread, appToken,















                    MoveToDisplayItem.obtain(displayId, config));















        } catch (RemoteException e) {















            // If process died, whatever.









@@ -683,7 +683,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo













            if (DEBUG_CONFIGURATION) Slog.v(TAG, "Sending new config to " + this + ", config: "















                    + config);





























            service.mLifecycleManager.scheduleTransaction(app.thread, appToken,













            service.getLifecycleManager().scheduleTransaction(app.thread, appToken,















                    ActivityConfigurationChangeItem.obtain(config));















        } catch (RemoteException e) {















            // If process died, whatever.









@@ -710,7 +710,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo





























    private void scheduleMultiWindowModeChanged(Configuration overrideConfig) {















        try {













            service.mLifecycleManager.scheduleTransaction(app.thread, appToken,













            service.getLifecycleManager().scheduleTransaction(app.thread, appToken,















                    MultiWindowModeChangeItem.obtain(mLastReportedMultiWindowMode,















                            overrideConfig));















        } catch (Exception e) {










@@ -738,7 +738,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo





























    private void schedulePictureInPictureModeChanged(Configuration overrideConfig) {















        try {













            service.mLifecycleManager.scheduleTransaction(app.thread, appToken,













            service.getLifecycleManager().scheduleTransaction(app.thread, appToken,















                    PipModeChangeItem.obtain(mLastReportedPictureInPictureMode,















                            overrideConfig));















        } catch (Exception e) {










@@ -1428,7 +1428,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo













            try {















                ArrayList<ReferrerIntent> ar = new ArrayList<>(1);















                ar.add(rintent);













                service.mLifecycleManager.scheduleTransaction(app.thread, appToken,













                service.getLifecycleManager().scheduleTransaction(app.thread, appToken,















                        NewIntentItem.obtain(ar, mState == PAUSED));















                unsent = false;















            } catch (RemoteException e) {










@@ -1615,20 +1615,30 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo













        if (DEBUG_STATES) Slog.v(TAG_STATES, "State movement: " + this + " from:" + getState()















                        + " to:" + state + " reason:" + reason);





























        final ActivityState prev = mState;













        final boolean stateChanged = prev != state;













        if (state == mState) {













            // No need to do anything if state doesn't change.













            if (DEBUG_STATES) Slog.v(TAG_STATES, "State unchanged from:" + state);













            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;





























        if (stateChanged) {















        if (mRecentTransitions.size() == MAX_STORED_STATE_TRANSITIONS) {















            mRecentTransitions.remove(0);















        }































        mRecentTransitions.add(new StateTransition(prev, state, reason));













        }





























        if (stateChanged && isState(DESTROYING, DESTROYED)) {













        mState = state;



























        if (isState(DESTROYING, DESTROYED)) {















            makeFinishingLocked();































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










@@ -1726,7 +1736,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo













            setVisible(true);















            sleeping = false;















            app.pendingUiClean = true;













            service.mLifecycleManager.scheduleTransaction(app.thread, appToken,













            service.getLifecycleManager().scheduleTransaction(app.thread, appToken,















                    WindowVisibilityItem.obtain(true /* showWindow */));















            // The activity may be waiting for stop, but that is no longer appropriate for it.















            mStackSupervisor.mStoppingActivities.remove(this);









@@ -1744,7 +1754,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo













                // An activity must be in the {@link PAUSING} state for the system to validate















                // the move to {@link PAUSED}.















                setState(PAUSING, "makeVisibleIfNeeded");













                service.mLifecycleManager.scheduleTransaction(app.thread, appToken,













                service.getLifecycleManager().scheduleTransaction(app.thread, appToken,















                        PauseActivityItem.obtain(finishing, false /* userLeaving */,















                                configChangeFlags, false /* dontReport */)















                                .setDescription(reason));










@@ -2712,7 +2722,7 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo













            final ClientTransaction transaction = ClientTransaction.obtain(app.thread, appToken);















            transaction.addCallback(callbackItem);















            transaction.setLifecycleStateRequest(lifecycleItem);













            service.mLifecycleManager.scheduleTransaction(transaction);













            service.getLifecycleManager().scheduleTransaction(transaction);















            // Note: don't need to call pauseIfSleepingLocked() here, because the caller will only















            // request resume if this activity is currently resumed, which implies we aren't















            // sleeping.

































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



+22
−7




























Original line number
Diff line number
Diff line








@@ -1472,7 +1472,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai













                        prev.shortComponentName, "userLeaving=" + userLeaving);















                mService.updateUsageStats(prev, false);





























                mService.mLifecycleManager.scheduleTransaction(prev.app.thread, prev.appToken,













                mService.getLifecycleManager().scheduleTransaction(prev.app.thread, prev.appToken,















                        PauseActivityItem.obtain(prev.finishing, userLeaving,















                                prev.configChangeFlags, pauseImmediately).setDescription(















                                        prev.getLifecycleDescription("startPausingLocked")));










@@ -2109,7 +2109,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai













                    if (r.app != null && r.app.thread != null) {















                        if (DEBUG_VISIBILITY) Slog.v(TAG_VISIBILITY,















                                "Scheduling invisibility: " + r);













                        mService.mLifecycleManager.scheduleTransaction(r.app.thread, r.appToken,













                        mService.getLifecycleManager().scheduleTransaction(r.app.thread, r.appToken,















                                WindowVisibilityItem.obtain(false /* showWindow */));















                    }


























@@ -2661,13 +2661,12 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai













                    next.app.pendingUiClean = true;















                    next.app.forceProcessStateUpTo(mService.mTopProcessState);















                    next.clearOptionsLocked();





























                    transaction.setLifecycleStateRequest(















                            ResumeActivityItem.obtain(next.app.repProcState,















                                    mService.isNextTransitionForward())















                                    .setDescription(next.getLifecycleDescription(















                                            "resumeTopActivityInnerLocked")));













                    mService.mLifecycleManager.scheduleTransaction(transaction);













                    mService.getLifecycleManager().scheduleTransaction(transaction);































                    if (DEBUG_STATES) Slog.d(TAG_STATES, "resumeTopActivityLocked: Resumed "















                            + next);










@@ -3317,7 +3316,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai













                ArrayList<ResultInfo> list = new ArrayList<ResultInfo>();















                list.add(new ResultInfo(resultWho, requestCode,















                        resultCode, data));













                mService.mLifecycleManager.scheduleTransaction(r.app.thread, r.appToken,













                mService.getLifecycleManager().scheduleTransaction(r.app.thread, r.appToken,















                        ActivityResultItem.obtain(list));















                return;















            } catch (Exception e) {










@@ -3446,7 +3445,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai













                }















                EventLogTags.writeAmStopActivity(















                        r.userId, System.identityHashCode(r), r.shortComponentName);













                mService.mLifecycleManager.scheduleTransaction(r.app.thread, r.appToken,













                mService.getLifecycleManager().scheduleTransaction(r.app.thread, r.appToken,















                        StopActivityItem.obtain(r.visible, r.configChangeFlags)















                                .setDescription(r.getLifecycleDescription("stopActivityLocked")));















                if (shouldSleepOrShutDownActivities()) {










@@ -3782,6 +3781,15 @@ 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()










@@ -4232,6 +4240,13 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai













        if (DEBUG_SWITCH || DEBUG_CLEANUP) Slog.v(TAG_SWITCH,















                "Removing activity from " + reason + ": token=" + r















                        + ", app=" + (r.app != null ? r.app.processName : "(null)"));



























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













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













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













            return false;













        }





























        EventLog.writeEvent(EventLogTags.AM_DESTROY_ACTIVITY,















                r.userId, System.identityHashCode(r),















                r.getTask().taskId, r.shortComponentName, reason);










@@ -4265,7 +4280,7 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai





























            try {















                if (DEBUG_SWITCH) Slog.i(TAG_SWITCH, "Destroying: " + r);













                mService.mLifecycleManager.scheduleTransaction(r.app.thread, r.appToken,













                mService.getLifecycleManager().scheduleTransaction(r.app.thread, r.appToken,















                        DestroyActivityItem.obtain(r.finishing, r.configChangeFlags)















                            .setDescription(















                                    r.getLifecycleDescription("destroyActivityLocked:" + reason)));

































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



+1
−1




























Original line number
Diff line number
Diff line








@@ -1459,7 +1459,7 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D













                clientTransaction.setLifecycleStateRequest(lifecycleItem);































                // Schedule transaction.













                mService.mLifecycleManager.scheduleTransaction(clientTransaction);













                mService.getLifecycleManager().scheduleTransaction(clientTransaction);















































                if ((app.info.privateFlags & ApplicationInfo.PRIVATE_FLAG_CANT_SAVE_STATE) != 0) {

































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



+16
−0




























Original line number
Diff line number
Diff line








@@ -24,6 +24,7 @@ import static android.view.Display.DEFAULT_DISPLAY;





























import static com.android.server.am.ActivityStack.ActivityState.DESTROYED;















import static com.android.server.am.ActivityStack.ActivityState.DESTROYING;













import static com.android.server.am.ActivityStack.ActivityState.FINISHING;















import static com.android.server.am.ActivityStack.ActivityState.INITIALIZING;















import static com.android.server.am.ActivityStack.ActivityState.PAUSED;















import static com.android.server.am.ActivityStack.ActivityState.PAUSING;










@@ -229,4 +230,19 @@ public class ActivityRecordTests extends ActivityTestsBase {













        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);













    }















}