Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Do not trim tasks when updating activity visibilities

Browse Source 
When updating visibility of all activities, the state of activity
may be changed to resumed, and it will also be added to recents
task. If the max amount of task is reached, the inactive tasks will
be trimmed. So it is possible that the index of visibility-update
loop becomes out of bounds.

Though it can also be solved by checking the index every time, that
seems to be a bit unintuitive and waste. And skip trimming should
also reduce the side effect of visibility-update to focus on its
major functionality.

Fixes: 154809437
Fixes: 142627724
Test: RecentTasksTest#testAddTasksInVisibilityUpdate_expectNoTrim

Change-Id: Iba66f2e6ce0beae3ef1187549318ef8779154a3b
This commit is contained in:
Riddle Hsu
2020-04-29 19:15:33 +08:00
parent 33cf9d091c
commit f2ff25fbb1
6 changed files with 66 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
services/core/java/com/android/server/wm/ActivityStack.java
View File

@@ -1388,7 +1388,7 @@ class ActivityStack extends Task {
boolean preserveWindows, boolean notifyClients) {
mTopActivityOccludesKeyguard = false;
mTopDismissingKeyguardActivity = null;
mStackSupervisor.getKeyguardController().beginActivityVisibilityUpdate();
mStackSupervisor.beginActivityVisibilityUpdate();
try {
mEnsureActivitiesVisibleHelper.process(
starting, configChanges, preserveWindows, notifyClients);
@@ -1399,7 +1399,7 @@ class ActivityStack extends Task {
notifyActivityDrawnLocked(null);
}
} finally {
mStackSupervisor.getKeyguardController().endActivityVisibilityUpdate();
mStackSupervisor.endActivityVisibilityUpdate();
}
}

26
services/core/java/com/android/server/wm/ActivityStackSupervisor.java
View File

@@ -332,11 +332,10 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks {
PowerManager.WakeLock mGoingToSleepWakeLock;
/**
* Temporary rect used during docked stack resize calculation so we don't need to create a new
* object each time.
* Used to keep {@link RootWindowContainer#ensureActivitiesVisible} from being entered
* recursively. And only update keyguard states once the nested updates are done.
*/
private final Rect tempRect = new Rect();
private final ActivityOptions mTmpOptions = ActivityOptions.makeBasic();
private int mVisibilityTransactionDepth;
private ActivityMetricsLogger mActivityMetricsLogger;
@@ -1923,6 +1922,7 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks {
pw.print(prefix);
pw.println("mCurTaskIdForUser=" + mCurTaskIdForUser);
pw.println(prefix + "mUserStackInFront=" + mRootWindowContainer.mUserStackInFront);
pw.println(prefix + "mVisibilityTransactionDepth=" + mVisibilityTransactionDepth);
if (!mWaitingForActivityVisible.isEmpty()) {
pw.println(prefix + "mWaitingForActivityVisible=");
for (int i = 0; i < mWaitingForActivityVisible.size(); ++i) {
@@ -2314,6 +2314,24 @@ public class ActivityStackSupervisor implements RecentTasks.Callbacks {
"android.server.am:TURN_ON:" + reason);
}
/** Starts a batch of visibility updates. */
void beginActivityVisibilityUpdate() {
mVisibilityTransactionDepth++;
}
/** Ends a batch of visibility updates. */
void endActivityVisibilityUpdate() {
mVisibilityTransactionDepth--;
if (mVisibilityTransactionDepth == 0) {
getKeyguardController().visibilitiesUpdated();
}
}
/** Returns {@code true} if the caller is on the path to update visibility. */
boolean inActivityVisibilityUpdate() {
return mVisibilityTransactionDepth > 0;
}
/**
* Begin deferring resume to avoid duplicate resumes in one pass.
*/

26
services/core/java/com/android/server/wm/KeyguardController.java
View File

@@ -70,7 +70,6 @@ class KeyguardController {
private boolean mKeyguardGoingAway;
private boolean mDismissalRequested;
private int mBeforeUnoccludeTransit;
private int mVisibilityTransactionDepth;
private final SparseArray<KeyguardDisplayState> mDisplayStates = new SparseArray<>();
private final ActivityTaskManagerService mService;
private RootWindowContainer mRootWindowContainer;
@@ -251,24 +250,6 @@ class KeyguardController {
return result;
}
/**
* Starts a batch of visibility updates.
*/
void beginActivityVisibilityUpdate() {
mVisibilityTransactionDepth++;
}
/**
* Ends a batch of visibility updates. After all batches are done, this method makes sure to
* update lockscreen occluded/dismiss state if needed.
*/
void endActivityVisibilityUpdate() {
mVisibilityTransactionDepth--;
if (mVisibilityTransactionDepth == 0) {
visibilitiesUpdated();
}
}
/**
* @return True if we may show an activity while Keyguard is showing because we are in the
* process of dismissing it anyways, false otherwise.
@@ -292,7 +273,11 @@ class KeyguardController {
&& !mWindowManager.isKeyguardSecure(mService.getCurrentUserId());
}
private void visibilitiesUpdated() {
/**
* Makes sure to update lockscreen occluded/dismiss state if needed after completing all
* visibility updates ({@link ActivityStackSupervisor#endActivityVisibilityUpdate}).
*/
void visibilitiesUpdated() {
boolean requestDismissKeyguard = false;
for (int displayNdx = mRootWindowContainer.getChildCount() - 1;
displayNdx >= 0; displayNdx--) {
@@ -568,7 +553,6 @@ class KeyguardController {
pw.println(prefix + " mKeyguardGoingAway=" + mKeyguardGoingAway);
dumpDisplayStates(pw, prefix);
pw.println(prefix + " mDismissalRequested=" + mDismissalRequested);
pw.println(prefix + " mVisibilityTransactionDepth=" + mVisibilityTransactionDepth);
pw.println();
}

10
services/core/java/com/android/server/wm/RecentTasks.java
View File

@@ -1031,9 +1031,13 @@ class RecentTasks {
void add(Task task) {
if (DEBUG_RECENTS_TRIM_TASKS) Slog.d(TAG, "add: task=" + task);
// Only allow trimming task if it is not updating visibility for activities, so the caller
// doesn't need to handle unexpected size and index when looping task containers.
final boolean canTrimTask = !mSupervisor.inActivityVisibilityUpdate();
// Clean up the hidden tasks when going to home because the user may not be unable to return
// to the task from recents.
if (!mHiddenTasks.isEmpty() && task.isActivityTypeHome()) {
if (canTrimTask && !mHiddenTasks.isEmpty() && task.isActivityTypeHome()) {
removeUnreachableHiddenTasks(task.getWindowingMode());
}
@@ -1155,7 +1159,9 @@ class RecentTasks {
}
// Trim the set of tasks to the active set
trimInactiveRecentTasks();
if (canTrimTask) {
trimInactiveRecentTasks();
}
notifyTaskPersisterLocked(task, false /* flush */);
}

11
services/core/java/com/android/server/wm/RootWindowContainer.java
View File

@@ -262,9 +262,6 @@ class RootWindowContainer extends WindowContainer<DisplayContent>
/** Set when a power hint has started, but not ended. */
private boolean mPowerHintSent;
/** Used to keep ensureActivitiesVisible() from being entered recursively. */
private boolean mInEnsureActivitiesVisible = false;
// The default minimal size that will be used if the activity doesn't specify its minimal size.
// It will be calculated when the default display gets added.
int mDefaultMinSizeOfResizeableTaskDp = -1;
@@ -1993,14 +1990,13 @@ class RootWindowContainer extends WindowContainer<DisplayContent>
*/
void ensureActivitiesVisible(ActivityRecord starting, int configChanges,
boolean preserveWindows, boolean notifyClients) {
if (mInEnsureActivitiesVisible) {
if (mStackSupervisor.inActivityVisibilityUpdate()) {
// Don't do recursive work.
return;
}
mInEnsureActivitiesVisible = true;
try {
mStackSupervisor.getKeyguardController().beginActivityVisibilityUpdate();
mStackSupervisor.beginActivityVisibilityUpdate();
// First the front stacks. In case any are not fullscreen and are in front of home.
for (int displayNdx = getChildCount() - 1; displayNdx >= 0; --displayNdx) {
final DisplayContent display = getChildAt(displayNdx);
@@ -2008,8 +2004,7 @@ class RootWindowContainer extends WindowContainer<DisplayContent>
notifyClients);
}
} finally {
mStackSupervisor.getKeyguardController().endActivityVisibilityUpdate();
mInEnsureActivitiesVisible = false;
mStackSupervisor.endActivityVisibilityUpdate();
}
}

26
services/tests/wmtests/src/com/android/server/wm/RecentTasksTest.java
View File

@@ -46,6 +46,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -275,6 +276,31 @@ public class RecentTasksTest extends ActivityTestsBase {
assertThat(mCallbacksRecorder.mRemoved).isEmpty();
}
@Test
public void testAddTasksInVisibilityUpdate_expectNoTrim() {
mRecentTasks.setOnlyTestVisibleRange();
mRecentTasks.setParameters(-1 /* min */, 1 /* max */, -1 /* ms */);
mRecentTasks.add(mTasks.get(0));
doAnswer(invocation -> {
assertTrue(mSupervisor.inActivityVisibilityUpdate());
// Simulate an activity is resumed by EnsureActivitiesVisibleHelper. If its state is
// change to RESUMED, it will also be added to recents.
mRecentTasks.add(mTasks.get(1));
invocation.callRealMethod();
return null;
}).when(mSupervisor).endActivityVisibilityUpdate();
mTaskContainer.ensureActivitiesVisible(null /* starting */, 0 /* configChanges */,
false /* preserveWindows */, false /* notifyClients */);
assertFalse(mSupervisor.inActivityVisibilityUpdate());
assertThat(mCallbacksRecorder.mAdded).hasSize(2);
// Expect nothing is trimmed because we don't want the loop of ensure-visibility to be
// impacted by the arbitrary number of task removals.
assertNoTasksTrimmed();
}
@Test
public void testAddTasksMultipleTasks_expectRemovedNoTrim() {
// Add multiple same-affinity non-document tasks, ensure that it removes the other task,