From a26de34cc496470115b1a614f759c500234860b0 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Tue, 3 Aug 2021 16:59:18 +0800 Subject: [PATCH] Check visibility for consecutive launch while device is sleeping Otherwise the launch event including trace will be finished until the device is no longer sleeping, e.g. unlock. That may cause unexpected long trace tag and launch time. Bug: 195368452 Test: ActivityMetricsLaunchObserverTests#testOnActivityLaunchWhileSleeping Change-Id: Ie82d1eb346f349513963d37ce2986ae1f8cce9f5 --- .../server/wm/ActivityMetricsLogger.java | 19 +++++---- .../ActivityMetricsLaunchObserverTests.java | 41 +++++++++++++++---- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityMetricsLogger.java b/services/core/java/com/android/server/wm/ActivityMetricsLogger.java index 005a62a208ebd..20958aa1c9b15 100644 --- a/services/core/java/com/android/server/wm/ActivityMetricsLogger.java +++ b/services/core/java/com/android/server/wm/ActivityMetricsLogger.java @@ -633,6 +633,7 @@ class ActivityMetricsLogger { if (crossPackage) { startLaunchTrace(info); } + scheduleCheckActivityToBeDrawnIfSleeping(launchedActivity); return; } @@ -654,13 +655,7 @@ class ActivityMetricsLogger { // As abort for no process switch. launchObserverNotifyIntentFailed(); } - if (launchedActivity.mDisplayContent.isSleeping()) { - // It is unknown whether the activity can be drawn or not, e.g. it depends on the - // keyguard states and the attributes or flags set by the activity. If the activity - // keeps invisible in the grace period, the tracker will be cancelled so it won't get - // a very long launch time that takes unlocking as the end of launch. - scheduleCheckActivityToBeDrawn(launchedActivity, UNKNOWN_VISIBILITY_CHECK_DELAY_MS); - } + scheduleCheckActivityToBeDrawnIfSleeping(launchedActivity); // If the previous transitions are no longer visible, abort them to avoid counting the // launch time when resuming from back stack. E.g. launch 2 independent tasks in a short @@ -675,6 +670,16 @@ class ActivityMetricsLogger { } } + private void scheduleCheckActivityToBeDrawnIfSleeping(@NonNull ActivityRecord r) { + if (r.mDisplayContent.isSleeping()) { + // It is unknown whether the activity can be drawn or not, e.g. it depends on the + // keyguard states and the attributes or flags set by the activity. If the activity + // keeps invisible in the grace period, the tracker will be cancelled so it won't get + // a very long launch time that takes unlocking as the end of launch. + scheduleCheckActivityToBeDrawn(r, UNKNOWN_VISIBILITY_CHECK_DELAY_MS); + } + } + /** * Notifies the tracker that all windows of the app have been drawn. * diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityMetricsLaunchObserverTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityMetricsLaunchObserverTests.java index fa1f4aca86428..b282cd7ce1cbc 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityMetricsLaunchObserverTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityMetricsLaunchObserverTests.java @@ -27,6 +27,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.verifyNoMor import static com.google.common.truth.Truth.assertWithMessage; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.argThat; @@ -70,6 +71,7 @@ import java.util.function.ToIntFunction; @Presubmit @RunWith(WindowTestRunner.class) public class ActivityMetricsLaunchObserverTests extends WindowTestsBase { + private static final long TIMEOUT_MS = TimeUnit.SECONDS.toMillis(5); private ActivityMetricsLogger mActivityMetricsLogger; private ActivityMetricsLogger.LaunchingState mLaunchingState; private ActivityMetricsLaunchObserver mLaunchObserver; @@ -137,7 +139,7 @@ public class ActivityMetricsLaunchObserverTests extends WindowTestsBase { // messages that are waiting for the lock. waitHandlerIdle(mAtm.mH); // AMLO callbacks happen on a separate thread than AML calls, so we need to use a timeout. - return verify(mock, timeout(TimeUnit.SECONDS.toMillis(5))); + return verify(mock, timeout(TIMEOUT_MS)); } private void verifyOnActivityLaunchFinished(ActivityRecord activity) { @@ -258,15 +260,40 @@ public class ActivityMetricsLaunchObserverTests extends WindowTestsBase { @Test public void testOnActivityLaunchWhileSleeping() { - notifyActivityLaunching(mTopActivity.intent); - notifyActivityLaunched(START_SUCCESS, mTopActivity); - doReturn(true).when(mTopActivity.mDisplayContent).isSleeping(); - mTopActivity.setState(ActivityRecord.State.RESUMED, "test"); - mTopActivity.setVisibility(false); + notifyActivityLaunching(mTrampolineActivity.intent); + notifyActivityLaunched(START_SUCCESS, mTrampolineActivity); + doReturn(true).when(mTrampolineActivity.mDisplayContent).isSleeping(); + mTrampolineActivity.setState(ActivityRecord.State.RESUMED, "test"); + mTrampolineActivity.setVisibility(false); waitHandlerIdle(mAtm.mH); // Not cancel immediately because in one of real cases, the keyguard may be going away or // occluded later, then the activity can be drawn. - verify(mLaunchObserver, never()).onActivityLaunchCancelled(eqProto(mTopActivity)); + verify(mLaunchObserver, never()).onActivityLaunchCancelled(eqProto(mTrampolineActivity)); + + clearInvocations(mLaunchObserver); + mLaunchTopByTrampoline = true; + mTopActivity.mVisibleRequested = false; + notifyActivityLaunching(mTopActivity.intent); + // It should schedule a message with UNKNOWN_VISIBILITY_CHECK_DELAY_MS to check whether + // the launch event is still valid. + notifyActivityLaunched(START_SUCCESS, mTopActivity); + + // The posted message will acquire wm lock, so the test needs to release the lock to verify. + final Throwable error = awaitInWmLock(() -> { + try { + // Though the aborting target should be eqProto(mTopActivity), use any() to avoid + // any changes in proto that may cause failure by different arguments. + verify(mLaunchObserver, timeout(TIMEOUT_MS)).onActivityLaunchCancelled(any()); + } catch (Throwable e) { + // Catch any errors including assertion because this runs in another thread. + return e; + } + return null; + }); + // The launch event must be cancelled because the activity keeps invisible. + if (error != null) { + throw new AssertionError(error); + } } @Test