From b1888aa5271e6dc83db538089c1ec97cc61d051c Mon Sep 17 00:00:00 2001 From: Andrii Kulian Date: Fri, 16 Feb 2018 14:40:42 -0800 Subject: [PATCH] Don't resume until keyguard is gone If we try to resume top activity immediately after we remove the sleep token and something else in the system will call ActivityStackSupervisor#ensureActivitiesVisibleLocked(), then the activity will be immediately stopped. This not only introduces an extra unnecessary cycle, but also leads to other with activities ending in a wrong state or cycling through states indefinitely. Bug: 73003134 Bug: 73062280 Bug: 71582913 Test: Launch an app or go to launcher, lock and unlock, observe lifecycle logs. Change-Id: Ic0117a55e27c8a67de4ce24ca349bc842d356093 --- .../server/am/ActivityManagerDebugConfig.java | 2 +- .../com/android/server/am/ActivityRecord.java | 14 -------------- .../java/com/android/server/am/ActivityStack.java | 5 +++++ .../server/am/ActivityStackSupervisor.java | 6 +++++- .../com/android/server/am/KeyguardController.java | 15 +++++++++++++-- .../android/server/am/ActivityRecordTests.java | 3 ++- 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerDebugConfig.java b/services/core/java/com/android/server/am/ActivityManagerDebugConfig.java index 8367916497484..4901192adbcaf 100644 --- a/services/core/java/com/android/server/am/ActivityManagerDebugConfig.java +++ b/services/core/java/com/android/server/am/ActivityManagerDebugConfig.java @@ -65,7 +65,7 @@ class ActivityManagerDebugConfig { static final boolean DEBUG_NETWORK = DEBUG_ALL || false; static final boolean DEBUG_OOM_ADJ = DEBUG_ALL || false; static final boolean DEBUG_OOM_ADJ_REASON = DEBUG_ALL || false; - static final boolean DEBUG_PAUSE = DEBUG_ALL || true; + static final boolean DEBUG_PAUSE = DEBUG_ALL || false; static final boolean DEBUG_POWER = DEBUG_ALL || false; static final boolean DEBUG_POWER_QUICK = DEBUG_POWER || false; static final boolean DEBUG_PROCESS_OBSERVERS = DEBUG_ALL || false; diff --git a/services/core/java/com/android/server/am/ActivityRecord.java b/services/core/java/com/android/server/am/ActivityRecord.java index ddba349dbe86d..40021eb588379 100644 --- a/services/core/java/com/android/server/am/ActivityRecord.java +++ b/services/core/java/com/android/server/am/ActivityRecord.java @@ -1625,20 +1625,6 @@ final class ActivityRecord extends ConfigurationContainer implements AppWindowCo // The activity may be waiting for stop, but that is no longer appropriate for it. mStackSupervisor.mStoppingActivities.remove(this); mStackSupervisor.mGoingToSleepActivities.remove(this); - - // If the activity is stopped or stopping, cycle to the paused state. - if (state == STOPPED || state == STOPPING) { - // Capture reason before state change - final String reason = getLifecycleDescription("makeVisibleIfNeeded"); - - // An activity must be in the {@link PAUSING} state for the system to validate - // the move to {@link PAUSED}. - state = PAUSING; - service.mLifecycleManager.scheduleTransaction(app.thread, appToken, - PauseActivityItem.obtain(finishing, false /* userLeaving */, - configChangeFlags, false /* dontReport */) - .setDescription(reason)); - } } catch (Exception e) { // Just skip on any failure; we'll make it visible when it next restarts. Slog.w(TAG, "Exception thrown making visibile: " + intent.getComponent(), e); diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java index 812de88729de3..fe10670966095 100644 --- a/services/core/java/com/android/server/am/ActivityStack.java +++ b/services/core/java/com/android/server/am/ActivityStack.java @@ -1419,6 +1419,11 @@ class ActivityStack extends ConfigurationContai return false; } + if (prev == resuming) { + Slog.wtf(TAG, "Trying to pause activity that is in process of being resumed"); + return false; + } + if (DEBUG_STATES) Slog.v(TAG_STATES, "Moving to PAUSING: " + prev); else if (DEBUG_PAUSE) Slog.v(TAG_PAUSE, "Start pausing: " + prev); mResumedActivity = null; diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java index a82facd174a3b..9a3b102991554 100644 --- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java @@ -3341,7 +3341,11 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D stack.goToSleepIfPossible(false /* shuttingDown */); } else { stack.awakeFromSleepingLocked(); - if (isFocusedStack(stack)) { + if (isFocusedStack(stack) + && !mKeyguardController.isKeyguardActive(display.mDisplayId)) { + // If there is no keyguard on this display - resume immediately. Otherwise + // we'll wait for keyguard visibility callback and resume while ensuring + // activities visibility resumeFocusedStackTopActivityLocked(); } } diff --git a/services/core/java/com/android/server/am/KeyguardController.java b/services/core/java/com/android/server/am/KeyguardController.java index 05305f300e611..e361a7028539d 100644 --- a/services/core/java/com/android/server/am/KeyguardController.java +++ b/services/core/java/com/android/server/am/KeyguardController.java @@ -86,8 +86,16 @@ class KeyguardController { * display, false otherwise */ boolean isKeyguardShowing(int displayId) { - return mKeyguardShowing && !mKeyguardGoingAway && - (displayId == DEFAULT_DISPLAY ? !mOccluded : displayId == mSecondaryDisplayShowing); + return isKeyguardActive(displayId) && !mKeyguardGoingAway; + } + + /** + * @return true if Keyguard is showing and not occluded. We ignore whether it is going away or + * not here. + */ + boolean isKeyguardActive(int displayId) { + return mKeyguardShowing && (displayId == DEFAULT_DISPLAY ? !mOccluded + : displayId == mSecondaryDisplayShowing); } /** @@ -114,6 +122,9 @@ class KeyguardController { mDismissalRequested = false; } } + if (!showing) { + mStackSupervisor.resumeFocusedStackTopActivityLocked(); + } mStackSupervisor.ensureActivitiesVisibleLocked(null, 0, !PRESERVE_WINDOWS); updateKeyguardSleepToken(); } 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 9923fa86758b0..d3df9241283ac 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java @@ -108,7 +108,8 @@ public class ActivityRecordTests extends ActivityTestsBase { assertEquals(mStack.onActivityRemovedFromStackInvocationCount(), 0); } - @Test + // TODO: b/71582913 + //@Test public void testPausingWhenVisibleFromStopped() throws Exception { final MutableBoolean pauseFound = new MutableBoolean(false); doAnswer((InvocationOnMock invocationOnMock) -> {