Cleanup ActivityStack#requestFinishActivityLocked
In an attempt to reduce the number of different finish* methods in
ActivityTaskManager codebase this CL consolidates
ActivityStack#requestFinishActivityLocked() into
ActivityRecord#finishActivityLocked().
It was used before to initiate activity finishing and the return
value indicated if the request was successful. It was different from
the return value of ActivityRecord#finishActivityLocked(), which only
returned 'true' if the activity record was removed from history
immediately. In order to accomodate both, this CL updates the finish
method in ActivityRecord to return an integer status value, so it can
now be used in both cases.
Bug: 137329632
Test: atest WmTests:ActivityRecordTests
Change-Id: I30ebc306637dea5e8b28ca4b4dfaab8df31d2be3
diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index 26b1f7b..015464e 100644
--- a/services/core/java/com/android/server/wm/ActivityRecord.java
+++ b/services/core/java/com/android/server/wm/ActivityRecord.java
@@ -163,6 +163,7 @@
import static org.xmlpull.v1.XmlPullParser.END_TAG;
import static org.xmlpull.v1.XmlPullParser.START_TAG;
+import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.Activity;
@@ -1630,22 +1631,52 @@
setSavedState(null /* savedState */);
}
+ /** Activity finish request was not executed. */
+ static final int FINISH_RESULT_CANCELLED = 0;
+ /** Activity finish was requested, activity will be fully removed later. */
+ static final int FINISH_RESULT_REQUESTED = 1;
+ /** Activity finish was requested, activity was removed from history. */
+ static final int FINISH_RESULT_REMOVED = 2;
+
+ /** Definition of possible results for activity finish request. */
+ @IntDef(prefix = { "FINISH_RESULT_" }, value = {
+ FINISH_RESULT_CANCELLED,
+ FINISH_RESULT_REQUESTED,
+ FINISH_RESULT_REMOVED,
+ })
+ @interface FinishRequest {}
+
/**
* See {@link #finishActivityLocked(int, Intent, String, boolean, boolean)}
*/
- boolean finishActivityLocked(int resultCode, Intent resultData, String reason, boolean oomAdj) {
+ @FinishRequest int finishActivityLocked(int resultCode, Intent resultData, String reason,
+ boolean oomAdj) {
return finishActivityLocked(resultCode, resultData, reason, oomAdj, !PAUSE_IMMEDIATELY);
}
/**
- * @return Returns true if this activity has been removed from the history
- * list, or false if it is still in the list and will be removed later.
+ * @return One of {@link FinishRequest} values:
+ * {@link #FINISH_RESULT_REMOVED} if this activity has been removed from the history list.
+ * {@link #FINISH_RESULT_REQUESTED} if removal process was started, but it is still in the list
+ * and will be removed from history later.
+ * {@link #FINISH_RESULT_CANCELLED} if activity is already finishing or in invalid state and the
+ * request to finish it was not ignored.
*/
- boolean finishActivityLocked(int resultCode, Intent resultData, String reason, boolean oomAdj,
- boolean pauseImmediately) {
+ @FinishRequest int finishActivityLocked(int resultCode, Intent resultData, String reason,
+ boolean oomAdj, boolean pauseImmediately) {
+ if (DEBUG_RESULTS || DEBUG_STATES) {
+ Slog.v(TAG_STATES, "Finishing activity r=" + this + ", result=" + resultCode
+ + ", data=" + resultData + ", reason=" + reason);
+ }
+
if (finishing) {
- Slog.w(TAG, "Duplicate finish request for " + this);
- return false;
+ Slog.w(TAG, "Duplicate finish request for r=" + this);
+ return FINISH_RESULT_CANCELLED;
+ }
+
+ if (!isInStackLocked()) {
+ Slog.w(TAG, "Finish request when not in stack for r=" + this);
+ return FINISH_RESULT_CANCELLED;
}
mAtmService.mWindowManager.deferSurfaceLayout();
@@ -1737,12 +1768,12 @@
taskOverlay.prepareActivityHideTransitionAnimation(transit);
}
}
- return removedActivity;
+ return removedActivity ? FINISH_RESULT_REMOVED : FINISH_RESULT_REQUESTED;
} else {
if (DEBUG_PAUSE) Slog.v(TAG_PAUSE, "Finish waiting for pause of: " + this);
}
- return false;
+ return FINISH_RESULT_REQUESTED;
} finally {
mAtmService.mWindowManager.continueSurfaceLayout();
}
diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java
index 81d8fa9..17536e4 100644
--- a/services/core/java/com/android/server/wm/ActivityStack.java
+++ b/services/core/java/com/android/server/wm/ActivityStack.java
@@ -58,6 +58,8 @@
import static com.android.server.wm.ActivityDisplay.POSITION_TOP;
import static com.android.server.wm.ActivityRecord.FINISH_AFTER_VISIBLE;
import static com.android.server.wm.ActivityRecord.FINISH_IMMEDIATELY;
+import static com.android.server.wm.ActivityRecord.FINISH_RESULT_CANCELLED;
+import static com.android.server.wm.ActivityRecord.FINISH_RESULT_REMOVED;
import static com.android.server.wm.ActivityStack.ActivityState.DESTROYED;
import static com.android.server.wm.ActivityStack.ActivityState.DESTROYING;
import static com.android.server.wm.ActivityStack.ActivityState.PAUSED;
@@ -1153,13 +1155,6 @@
return null;
}
- private TaskRecord bottomTask() {
- if (mTaskHistory.isEmpty()) {
- return null;
- }
- return mTaskHistory.get(0);
- }
-
TaskRecord taskForIdLocked(int id) {
for (int taskNdx = mTaskHistory.size() - 1; taskNdx >= 0; --taskNdx) {
final TaskRecord task = mTaskHistory.get(taskNdx);
@@ -2787,8 +2782,8 @@
!mLastNoHistoryActivity.finishing) {
if (DEBUG_STATES) Slog.d(TAG_STATES,
"no-history finish of " + mLastNoHistoryActivity + " on new resume");
- requestFinishActivityLocked(mLastNoHistoryActivity.appToken, Activity.RESULT_CANCELED,
- null, "resume-no-history", false);
+ mLastNoHistoryActivity.finishActivityLocked(Activity.RESULT_CANCELED,
+ null /* resultData */, "resume-no-history", false /* oomAdj */);
mLastNoHistoryActivity = null;
}
@@ -3023,8 +3018,8 @@
// If any exception gets thrown, toss away this
// activity and try the next one.
Slog.w(TAG, "Exception thrown during resume of " + next, e);
- requestFinishActivityLocked(next.appToken, Activity.RESULT_CANCELED, null,
- "resume-exception", true);
+ next.finishActivityLocked(Activity.RESULT_CANCELED, null /* resultData */,
+ "resume-exception", true /* oomAdj */);
return true;
}
} else {
@@ -3452,7 +3447,7 @@
if (DEBUG_TASKS) Slog.w(TAG_TASKS,
"resetTaskIntendedTask: calling finishActivity on " + p);
if (p.finishActivityLocked(Activity.RESULT_CANCELED, null /* resultData */,
- "reset-task", false /* oomAdj */)) {
+ "reset-task", false /* oomAdj */) == FINISH_RESULT_REMOVED) {
end--;
srcPos--;
}
@@ -3760,10 +3755,9 @@
if (!r.finishing) {
if (!shouldSleepActivities()) {
if (DEBUG_STATES) Slog.d(TAG_STATES, "no-history finish of " + r);
- if (requestFinishActivityLocked(r.appToken, Activity.RESULT_CANCELED, null,
- "stop-no-history", false)) {
- // If {@link requestFinishActivityLocked} returns {@code true},
- // {@link adjustFocusedActivityStack} would have been already called.
+ if (r.finishActivityLocked(Activity.RESULT_CANCELED, null /* resultData */,
+ "stop-no-history", false /* oomAdj */) != FINISH_RESULT_CANCELLED) {
+ // {@link adjustFocusedActivityStack} must have been already called.
r.resumeKeyDispatchingLocked();
return;
}
@@ -3812,25 +3806,7 @@
}
}
- /**
- * @return Returns true if the activity is being finished, false if for
- * some reason it is being left as-is.
- */
- final boolean requestFinishActivityLocked(IBinder token, int resultCode,
- Intent resultData, String reason, boolean oomAdj) {
- ActivityRecord r = isInStackLocked(token);
- if (DEBUG_RESULTS || DEBUG_STATES) Slog.v(TAG_STATES,
- "Finishing activity token=" + token + " r="
- + ", result=" + resultCode + ", data=" + resultData
- + ", reason=" + reason);
- if (r == null) {
- return false;
- }
-
- r.finishActivityLocked(resultCode, resultData, reason, oomAdj);
- return true;
- }
-
+ /** Finish all activities that were started for result from the specified activity. */
final void finishSubActivityLocked(ActivityRecord self, String resultWho, int requestCode) {
for (int taskNdx = mTaskHistory.size() - 1; taskNdx >= 0; --taskNdx) {
ArrayList<ActivityRecord> activities = mTaskHistory.get(taskNdx).mActivities;
@@ -4052,8 +4028,8 @@
}
final long origId = Binder.clearCallingIdentity();
for (int i = start; i > finishTo; i--) {
- ActivityRecord r = activities.get(i);
- requestFinishActivityLocked(r.appToken, resultCode, resultData, "navigate-up", true);
+ final ActivityRecord r = activities.get(i);
+ r.finishActivityLocked(resultCode, resultData, "navigate-up", true /* oomAdj */);
// Only return the supplied result for the first activity finished
resultCode = Activity.RESULT_CANCELED;
resultData = null;
@@ -4090,8 +4066,8 @@
} catch (RemoteException e) {
foundParentInTask = false;
}
- requestFinishActivityLocked(parent.appToken, resultCode,
- resultData, "navigate-top", true);
+ parent.finishActivityLocked(resultCode, resultData, "navigate-top",
+ true /* oomAdj */);
}
}
Binder.restoreCallingIdentity(origId);
diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
index e49e991..beff5fb 100644
--- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java
@@ -890,8 +890,8 @@
Slog.e(TAG, "Second failure launching "
+ r.intent.getComponent().flattenToShortString() + ", giving up", e);
proc.appDied();
- stack.requestFinishActivityLocked(r.appToken, Activity.RESULT_CANCELED, null,
- "2nd-crash", false);
+ r.finishActivityLocked(Activity.RESULT_CANCELED, null /* resultData */,
+ "2nd-crash", false /* oomAdj */);
return false;
}
diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
index fb393e6..7e741a0 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
@@ -84,6 +84,7 @@
import static com.android.server.am.ActivityManagerServiceDumpProcessesProto.SCREEN_COMPAT_PACKAGES;
import static com.android.server.am.ActivityManagerServiceDumpProcessesProto.ScreenCompatPackage.MODE;
import static com.android.server.am.ActivityManagerServiceDumpProcessesProto.ScreenCompatPackage.PACKAGE;
+import static com.android.server.wm.ActivityRecord.FINISH_RESULT_CANCELLED;
import static com.android.server.wm.ActivityStack.REMOVE_TASK_MODE_DESTROYING;
import static com.android.server.wm.ActivityStackSupervisor.DEFER_RESUME;
import static com.android.server.wm.ActivityStackSupervisor.ON_TOP;
@@ -1560,13 +1561,13 @@
}
synchronized (mGlobalLock) {
- ActivityRecord r = ActivityRecord.isInStackLocked(token);
+ final ActivityRecord r = ActivityRecord.isInStackLocked(token);
if (r == null) {
return true;
}
// Keep track of the root activity of the task before we finish it
final TaskRecord tr = r.getTaskRecord();
- ActivityRecord rootR = tr.getRootActivity();
+ final ActivityRecord rootR = tr.getRootActivity();
if (rootR == null) {
Slog.w(TAG, "Finishing task with all activities already finished");
}
@@ -1616,7 +1617,7 @@
// because we don't support returning them across task boundaries. Also, to
// keep backwards compatibility we remove the task from recents when finishing
// task with root activity.
- res = mStackSupervisor.removeTaskByIdLocked(tr.taskId, false,
+ res = mStackSupervisor.removeTaskByIdLocked(tr.taskId, false /* killProcess */,
finishWithRootActivity, "finish-activity");
if (!res) {
Slog.i(TAG, "Removing task failed to finish activity");
@@ -1624,8 +1625,8 @@
// Explicitly dismissing the activity so reset its relaunch flag.
r.mRelaunchReason = RELAUNCH_REASON_NONE;
} else {
- res = tr.getStack().requestFinishActivityLocked(token, resultCode,
- resultData, "app-request", true);
+ res = r.finishActivityLocked(resultCode, resultData, "app-request",
+ true /* oomAdj */) != FINISH_RESULT_CANCELLED;
if (!res) {
Slog.i(TAG, "Failed to finish by app-request");
}
@@ -1649,7 +1650,6 @@
// Do not allow task to finish if last task in lockTask mode. Launchable priv-apps
// can finish.
- final TaskRecord task = r.getTaskRecord();
if (getLockTaskController().activityBlockedFromFinish(r)) {
return false;
}
diff --git a/services/core/java/com/android/server/wm/TaskRecord.java b/services/core/java/com/android/server/wm/TaskRecord.java
index 23515d8..882f411 100644
--- a/services/core/java/com/android/server/wm/TaskRecord.java
+++ b/services/core/java/com/android/server/wm/TaskRecord.java
@@ -66,6 +66,7 @@
import static com.android.server.am.TaskRecordProto.REAL_ACTIVITY;
import static com.android.server.am.TaskRecordProto.RESIZE_MODE;
import static com.android.server.am.TaskRecordProto.STACK_ID;
+import static com.android.server.wm.ActivityRecord.FINISH_RESULT_REMOVED;
import static com.android.server.wm.ActivityRecord.STARTING_WINDOW_SHOWN;
import static com.android.server.wm.ActivityStack.REMOVE_TASK_MODE_MOVING;
import static com.android.server.wm.ActivityStack.REMOVE_TASK_MODE_MOVING_TO_TOP;
@@ -1419,7 +1420,7 @@
--activityNdx;
--numActivities;
} else if (r.finishActivityLocked(Activity.RESULT_CANCELED, null,
- reason, false /* oomAdj */, pauseImmediately)) {
+ reason, false /* oomAdj */, pauseImmediately) == FINISH_RESULT_REMOVED) {
--activityNdx;
--numActivities;
}
@@ -1474,7 +1475,7 @@
ret.updateOptionsLocked(opts);
}
if (r.finishActivityLocked(Activity.RESULT_CANCELED, null /* resultData */,
- "clear-task-stack", false /* oomAdj */)) {
+ "clear-task-stack", false /* oomAdj */) == FINISH_RESULT_REMOVED) {
--activityNdx;
--numActivities;
}
diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
index ef11a43..7b252cb 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java
@@ -31,6 +31,9 @@
import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.when;
+import static com.android.server.wm.ActivityRecord.FINISH_RESULT_CANCELLED;
+import static com.android.server.wm.ActivityRecord.FINISH_RESULT_REMOVED;
+import static com.android.server.wm.ActivityRecord.FINISH_RESULT_REQUESTED;
import static com.android.server.wm.ActivityStack.ActivityState.INITIALIZING;
import static com.android.server.wm.ActivityStack.ActivityState.PAUSING;
import static com.android.server.wm.ActivityStack.ActivityState.RESUMED;
@@ -681,6 +684,73 @@
assertEquals(persistentSavedState, mActivity.getPersistentSavedState());
}
+ /**
+ * Verify that activity finish request is not performed if activity is finishing or is in
+ * incorrect state.
+ */
+ @Test
+ public void testFinishActivityLocked_cancelled() {
+ // Mark activity as finishing
+ mActivity.finishing = true;
+ assertEquals("Duplicate finish request must be ignored", FINISH_RESULT_CANCELLED,
+ mActivity.finishActivityLocked(0 /* resultCode */, null /* resultData */, "test",
+ false /* oomAdj */));
+ assertTrue(mActivity.finishing);
+ assertTrue(mActivity.isInStackLocked());
+
+ // Remove activity from task
+ mActivity.finishing = false;
+ mActivity.setTask(null);
+ assertEquals("Activity outside of task/stack cannot be finished", FINISH_RESULT_CANCELLED,
+ mActivity.finishActivityLocked(0 /* resultCode */, null /* resultData */, "test",
+ false /* oomAdj */));
+ assertFalse(mActivity.finishing);
+ }
+
+ /**
+ * Verify that activity finish request is requested, but not executed immediately if activity is
+ * not ready yet.
+ */
+ @Test
+ public void testFinishActivityLocked_requested() {
+ mActivity.finishing = false;
+ assertEquals("Currently resumed activity be paused removal", FINISH_RESULT_REQUESTED,
+ mActivity.finishActivityLocked(0 /* resultCode */, null /* resultData */, "test",
+ false /* oomAdj */));
+ assertTrue(mActivity.finishing);
+ assertTrue(mActivity.isInStackLocked());
+
+ // First request to finish activity must schedule a "destroy" request to the client.
+ // Activity must be removed from history after the client reports back or after timeout.
+ mActivity.finishing = false;
+ mActivity.setState(STOPPED, "test");
+ assertEquals("Activity outside of task/stack cannot be finished", FINISH_RESULT_REQUESTED,
+ mActivity.finishActivityLocked(0 /* resultCode */, null /* resultData */, "test",
+ false /* oomAdj */));
+ assertTrue(mActivity.finishing);
+ assertTrue(mActivity.isInStackLocked());
+ }
+
+ /**
+ * Verify that activity finish request removes activity immediately if it's ready.
+ */
+ @Test
+ public void testFinishActivityLocked_removed() {
+ // Prepare the activity record to be ready for immediate removal. It should be invisible and
+ // have no process. Otherwise, request to finish it will send a message to client first.
+ mActivity.setState(STOPPED, "test");
+ mActivity.visible = false;
+ mActivity.nowVisible = false;
+ // Set process to 'null' to allow immediate removal, but don't call mActivity.setProcess() -
+ // this will cause NPE when updating task's process.
+ mActivity.app = null;
+ assertEquals("Activity outside of task/stack cannot be finished", FINISH_RESULT_REMOVED,
+ mActivity.finishActivityLocked(0 /* resultCode */, null /* resultData */, "test",
+ false /* oomAdj */));
+ assertTrue(mActivity.finishing);
+ assertFalse(mActivity.isInStackLocked());
+ }
+
/** Setup {@link #mActivity} as a size-compat-mode-able activity without fixed orientation. */
private void prepareFixedAspectRatioUnresizableActivity() {
setupDisplayContentForCompatDisplayInsets();