From 124b010ea9d2e70f60e5e87728f4a503a81a297b Mon Sep 17 00:00:00 2001 From: Schneider Victor-Tulias Date: Tue, 20 May 2025 11:29:39 -0400 Subject: [PATCH] Add recents animation start timeout handling to TaskAnimationManager In cases where a recents aniamtion transition was successfully requested but failed to start, we should clean up the gesture state to recover. Flag: EXEMPT bug fix Fixes: 415930274 Test: TaskAnimationManagerTest.testRecentsAnimationStartTimeout_cleansUpRecentsAnimation Change-Id: If1a24d6c7baac7338c49056531bc01665f082ddf --- .../quickstep/TaskAnimationManager.java | 28 +++++++++++- .../util/ActiveGestureErrorDetector.java | 28 ++++-------- .../util/ActiveGestureProtoLogProxy.java | 10 +++++ .../quickstep/TaskAnimationManagerTest.java | 44 +++++++++++++++++-- 4 files changed, 86 insertions(+), 24 deletions(-) diff --git a/quickstep/src/com/android/quickstep/TaskAnimationManager.java b/quickstep/src/com/android/quickstep/TaskAnimationManager.java index 9b7ff0089a..93c06cf60a 100644 --- a/quickstep/src/com/android/quickstep/TaskAnimationManager.java +++ b/quickstep/src/com/android/quickstep/TaskAnimationManager.java @@ -41,6 +41,7 @@ import android.window.TransitionInfo; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; +import androidx.annotation.VisibleForTesting; import com.android.app.displaylib.PerDisplayRepository; import com.android.internal.util.ArrayUtils; @@ -69,6 +70,11 @@ import java.util.HashMap; import java.util.Locale; public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAnimationListener { + + private static final String TAG = "TaskAnimationManager"; + + @VisibleForTesting + public static final long RECENTS_ANIMATION_START_TIMEOUT_MS = 5000L; public static final boolean SHELL_TRANSITIONS_ROTATION = SystemProperties.getBoolean("persist.wm.debug.shell_transit_rotate", false); private final Context mCtx; @@ -154,7 +160,7 @@ public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAn if (FeatureFlags.IS_STUDIO_BUILD) { throw new IllegalArgumentException(msg); } else { - Log.e("TaskAnimationManager", msg, new Exception()); + Log.e(TAG, msg, new Exception()); } } // Notify if recents animation is still running @@ -163,7 +169,7 @@ public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAn if (FeatureFlags.IS_STUDIO_BUILD) { throw new IllegalArgumentException(msg); } else { - Log.e("TaskAnimationManager", msg, new Exception()); + Log.e(TAG, msg, new Exception()); } } // But force-finish it anyways @@ -183,6 +189,14 @@ public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAn containerInterface.getCreatedContainer()); mCallbacks = newCallbacks; mLauncherDestroyCallbackSet = false; + final Runnable recentsAnimationStartTimeoutCallback = () -> { + if (!mRecentsAnimationStartPending) return; + mRecentsAnimationStartPending = false; + Log.wtf(TAG, "Recents animation start has been pending for over " + + RECENTS_ANIMATION_START_TIMEOUT_MS + "ms"); + ActiveGestureProtoLogProxy.logRecentsAnimationStartTimedOut(); + cleanUpRecentsAnimation(newCallbacks); + }; mCallbacks.addListener(new RecentsAnimationCallbacks.RecentsAnimationListener() { @Override public void onRecentsAnimationStart(RecentsAnimationController controller, @@ -191,6 +205,8 @@ public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAn ActiveGestureProtoLogProxy.logStartRecentsAnimationCallback( "onRecentsAnimationStart"); mRecentsAnimationStartPending = false; + MAIN_EXECUTOR.getHandler().removeCallbacks( + recentsAnimationStartTimeoutCallback); } if (mCallbacks == null) { // It's possible for the recents animation to have finished and be cleaned up @@ -236,6 +252,8 @@ public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAn ActiveGestureProtoLogProxy.logStartRecentsAnimationCallback( "onRecentsAnimationCanceled"); mRecentsAnimationStartPending = false; + MAIN_EXECUTOR.getHandler().removeCallbacks( + recentsAnimationStartTimeoutCallback); } cleanUpRecentsAnimation(newCallbacks); } @@ -246,6 +264,8 @@ public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAn ActiveGestureProtoLogProxy.logStartRecentsAnimationCallback( "onRecentsAnimationFinished"); mRecentsAnimationStartPending = false; + MAIN_EXECUTOR.getHandler().removeCallbacks( + recentsAnimationStartTimeoutCallback); } cleanUpRecentsAnimation(newCallbacks); } @@ -353,6 +373,10 @@ public class TaskAnimationManager implements RecentsAnimationCallbacks.RecentsAn options, mCallbacks, false /* useSyntheticRecentsTransition */, null, mDisplayId); } + if (mRecentsAnimationStartPending) { + MAIN_EXECUTOR.getHandler().postDelayed( + recentsAnimationStartTimeoutCallback, RECENTS_ANIMATION_START_TIMEOUT_MS); + } ActiveGestureProtoLogProxy.logSettingRecentsAnimationStartPending( mRecentsAnimationStartPending); gestureState.setState(STATE_RECENTS_ANIMATION_INITIALIZED); diff --git a/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureErrorDetector.java b/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureErrorDetector.java index ab10979927..799ba77a36 100644 --- a/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureErrorDetector.java +++ b/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureErrorDetector.java @@ -36,11 +36,12 @@ public class ActiveGestureErrorDetector { SET_END_TARGET_NEW_TASK, SET_END_TARGET_ALL_APPS, ON_SETTLED_ON_END_TARGET, ON_START_RECENTS_ANIMATION, ON_FINISH_RECENTS_ANIMATION, ON_CANCEL_RECENTS_ANIMATION, START_RECENTS_ANIMATION, FINISH_RECENTS_ANIMATION, CANCEL_RECENTS_ANIMATION, - SET_ON_PAGE_TRANSITION_END_CALLBACK, CANCEL_CURRENT_ANIMATION, CLEANUP_SCREENSHOT, + SET_ON_PAGE_TRANSITION_END_CALLBACK, CANCEL_CURRENT_ANIMATION, SCROLLER_ANIMATION_ABORTED, TASK_APPEARED, EXPECTING_TASK_APPEARED, FLAG_USING_OTHER_ACTIVITY_INPUT_CONSUMER, LAUNCHER_DESTROYED, RECENT_TASKS_MISSING, INVALID_VELOCITY_ON_SWIPE_UP, RECENTS_ANIMATION_START_PENDING, QUICK_SWITCH_FROM_HOME_FALLBACK, QUICK_SWITCH_FROM_HOME_FAILED, NAVIGATION_MODE_SWITCHED, + RECENTS_ANIMATION_START_TIMEOUT, /** * These GestureEvents are specifically associated to state flags that get set in @@ -126,15 +127,6 @@ public class ActiveGestureErrorDetector { + "before/without startRecentsAnimation.", writer); break; - case CLEANUP_SCREENSHOT: - errorDetected |= printErrorIfTrue( - !encounteredEvents.contains(GestureEvent.STATE_SCREENSHOT_CAPTURED), - prefix, - /* errorMessage= */ "recents activity screenshot was " - + "cleaned up before/without STATE_SCREENSHOT_CAPTURED " - + "being set.", - writer); - break; case SCROLLER_ANIMATION_ABORTED: errorDetected |= printErrorIfTrue( encounteredEvents.contains(GestureEvent.SET_END_TARGET_HOME) @@ -306,6 +298,13 @@ public class ActiveGestureErrorDetector { /* errorMessage= */ "Navigation mode switched mid-gesture.", writer); break; + case RECENTS_ANIMATION_START_TIMEOUT: + errorDetected |= printErrorIfTrue( + true, + prefix, + /* errorMessage= */ "Recents animation start timed out.", + writer); + break; case EXPECTING_TASK_APPEARED: case MOTION_DOWN: case SET_END_TARGET: @@ -420,15 +419,6 @@ public class ActiveGestureErrorDetector { + "wasn't called and STATE_HANDLER_INVALIDATED wasn't set.", writer); - errorDetected |= printErrorIfTrue( - /* condition= */ encounteredEvents.contains( - GestureEvent.STATE_RECENTS_ANIMATION_CANCELED) - && !encounteredEvents.contains(GestureEvent.CLEANUP_SCREENSHOT), - prefix, - /* errorMessage= */ "STATE_RECENTS_ANIMATION_CANCELED was set but " - + "the task screenshot wasn't cleaned up.", - writer); - errorDetected |= printErrorIfTrue( /* condition= */ encounteredEvents.contains( GestureEvent.EXPECTING_TASK_APPEARED) diff --git a/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureProtoLogProxy.java b/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureProtoLogProxy.java index cf1eb87df1..9b26b39bb4 100644 --- a/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureProtoLogProxy.java +++ b/quickstep/src_protolog/com/android/quickstep/util/ActiveGestureProtoLogProxy.java @@ -32,6 +32,7 @@ import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent.QUICK_SWITCH_FROM_HOME_FAILED; import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent.QUICK_SWITCH_FROM_HOME_FALLBACK; import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent.RECENTS_ANIMATION_START_PENDING; +import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent.RECENTS_ANIMATION_START_TIMEOUT; import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent.RECENT_TASKS_MISSING; import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent.SET_END_TARGET; import static com.android.quickstep.util.ActiveGestureErrorDetector.GestureEvent.START_RECENTS_ANIMATION; @@ -587,4 +588,13 @@ public class ActiveGestureProtoLogProxy { ProtoLog.d(ACTIVE_GESTURE_LOG, "Launcher destroyed while mRecentsAnimationStartPending ==" + " true, queuing a callback to clean the pending animation up on start"); } + + public static void logRecentsAnimationStartTimedOut() { + ActiveGestureLog.INSTANCE.addLog("Recents animation start has timed out; forcefully " + + "cleaning up the recents animation.", + /* gestureEvent= */ RECENTS_ANIMATION_START_TIMEOUT); + if (!isProtoLogInitialized()) return; + ProtoLog.d(ACTIVE_GESTURE_LOG, "Recents animation start has timed out; forcefully " + + "cleaning up the recents animation."); + } } diff --git a/quickstep/tests/multivalentTests/src/com/android/quickstep/TaskAnimationManagerTest.java b/quickstep/tests/multivalentTests/src/com/android/quickstep/TaskAnimationManagerTest.java index 5cecde479b..a831e82e67 100644 --- a/quickstep/tests/multivalentTests/src/com/android/quickstep/TaskAnimationManagerTest.java +++ b/quickstep/tests/multivalentTests/src/com/android/quickstep/TaskAnimationManagerTest.java @@ -16,7 +16,11 @@ package com.android.quickstep; +import static com.android.quickstep.TaskAnimationManager.RECENTS_ANIMATION_START_TIMEOUT_MS; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -34,6 +38,7 @@ import android.content.Intent; import android.content.res.Configuration; import android.graphics.Rect; import android.os.Bundle; +import android.os.SystemClock; import android.platform.test.annotations.DisableFlags; import android.platform.test.annotations.EnableFlags; import android.platform.test.flag.junit.SetFlagsRule; @@ -111,7 +116,7 @@ public class TaskAnimationManagerTest { @Test public void testLauncherDestroyed_whileRecentsAnimationStartPending_finishesAnimation() { - final GestureState gestureState = mock(GestureState.class); + final GestureState gestureState = buildMockGestureState(); final ArgumentCaptor listenerCaptor = ArgumentCaptor.forClass(RecentsAnimationCallbacks.class); final RecentsAnimationControllerCompat controllerCompat = @@ -134,12 +139,10 @@ public class TaskAnimationManagerTest { /* taskInfo= */ new ActivityManager.RunningTaskInfo(), /* allowEnterPip= */ false); - doReturn(mock(LauncherActivityInterface.class)).when(gestureState).getContainerInterface(); when(mSystemUiProxy .startRecentsActivity(any(), any(), listenerCaptor.capture(), anyBoolean(), any(), anyInt())) .thenReturn(true); - when(gestureState.getRunningTaskIds(anyBoolean())).thenReturn(new int[0]); runOnMainSync(() -> { mTaskAnimationManager.startRecentsAnimation( @@ -166,10 +169,45 @@ public class TaskAnimationManagerTest { .finish(/* toHome= */ eq(false), anyBoolean(), any())); } + @Test + public void testRecentsAnimationStartTimeout_cleansUpRecentsAnimation() { + final GestureState gestureState = buildMockGestureState(); + when(mSystemUiProxy + .startRecentsActivity(any(), any(), any(), anyBoolean(), any(), anyInt())) + .thenReturn(true); + + runOnMainSync(() -> { + assertNull("Recents animation was started prematurely:", + mTaskAnimationManager.getCurrentCallbacks()); + + mTaskAnimationManager.startRecentsAnimation( + gestureState, + new Intent(), + mock(RecentsAnimationCallbacks.RecentsAnimationListener.class)); + + assertNotNull("TaskAnimationManager was cleaned up prematurely:", + mTaskAnimationManager.getCurrentCallbacks()); + }); + + SystemClock.sleep(RECENTS_ANIMATION_START_TIMEOUT_MS); + + runOnMainSync(() -> assertNull("TaskAnimationManager was not cleaned up after the timeout:", + mTaskAnimationManager.getCurrentCallbacks())); + } + protected static void runOnMainSync(Runnable runnable) { InstrumentationRegistry.getInstrumentation().runOnMainSync(runnable); } + private GestureState buildMockGestureState() { + final GestureState gestureState = mock(GestureState.class); + + doReturn(mock(LauncherActivityInterface.class)).when(gestureState).getContainerInterface(); + when(gestureState.getRunningTaskIds(anyBoolean())).thenReturn(new int[0]); + + return gestureState; + } + /** * Invokes maybeStartHomeAction on the given TaskAnimationManager and verifies whether the * provided Runnable was invoked, based on the expectedResult.