Do not always destroy activity when cleaning up.

There are some invocations of ActivityStack#cleanUpActivityLocked
where the activity is not removed from history. In this case, we
want to keep the ActivityRecord around, but clear the process.
Previously, we were also setting the state to destroy. This is
inconsistent with the client side state, which is still in the
stopped state. With recent changes, this state change now causes
the server side record to be destroyed, removing it from history
and causing it to not return on back traversal.

This changelist separates clearing the app process from setting the
activity state, allowing for the record to remain when appropriate.

Bug: 71506345
Test: atest CtsActivityManagerDeviceTestCases:ActivityLifecycleTests#testRestoreFromKill
Test: atest FrameworksServicesTests:com.android.server.am.ActivityRecordTests#testFinishingAfterDestroying
Test: atest FrameworksServicesTests:com.android.server.am.ActivityRecordTests#testFinishingAfterDestroyed
Change-Id: I98366a1b51d8d4d002536683c6335ee57c22129c

diff --git a/services/core/java/com/android/server/am/ActivityRecord.java b/services/core/java/com/android/server/am/ActivityRecord.java
index ae98ca0..519024a 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityRecord.java
+++ b/services/core/java/com/android/server/am/ActivityRecord.java
@@ -1614,16 +1614,36 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo
     void setState(ActivityState state, String reason) {
         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;
+
         mState = state;
 
-        if (mState != prev) {
+        if (stateChanged) {
             if (mRecentTransitions.size() == MAX_STORED_STATE_TRANSITIONS) {
                 mRecentTransitions.remove(0);
             }
 
             mRecentTransitions.add(new StateTransition(prev, state, reason));
         }
+
+        if (stateChanged && 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() {

diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java
index 728c07d..c9a1304 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityStack.java
+++ b/services/core/java/com/android/server/am/ActivityStack.java
@@ -3981,16 +3981,26 @@ 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 setState) {
+    private void cleanUpActivityLocked(ActivityRecord r, boolean cleanServices, boolean destroy,
+            boolean clearProcess) {
         onActivityRemovedFromStack(r);
 
         r.deferRelaunchUntilPaused = false;
         r.frozenBeforeDestroy = false;
 
-        if (setState) {
+        if (destroy) {
             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;
         }
@@ -4209,7 +4219,8 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai
 
         boolean removedFromHistory = false;
 
-        cleanUpActivityLocked(r, false, false);
+        cleanUpActivityLocked(r, false /* cleanServices */, false /* destroy */,
+                false /*clearProcess*/);
 
         final boolean hadApp = r.app != null;
 
@@ -4315,7 +4326,8 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai
 
         if (isInStackLocked(record) != null) {
             if (record.isState(DESTROYING, DESTROYED)) {
-                cleanUpActivityLocked(record, true, false);
+                cleanUpActivityLocked(record, true /* cleanServices */, false /* destroy */,
+                        false /*clearProcess*/);
                 removeActivityFromHistoryLocked(record, reason);
             }
         }
@@ -4420,7 +4432,8 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai
                             r.icicle = null;
                         }
                     }
-                    cleanUpActivityLocked(r, true, true);
+                    cleanUpActivityLocked(r, true /* cleanServices */, remove /* destroy */,
+                            true /*clearProcess*/);
                     if (remove) {
                         removeActivityFromHistoryLocked(r, "appDied");
                     }

diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java b/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java
index a527e17..f2f1b10 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java
+++ b/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java
@@ -202,4 +202,20 @@ 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);
+    }
 }