From f322cbd5ccb43113dc4a0603b1692a59b3d91ff0 Mon Sep 17 00:00:00 2001 From: Arthur Hung Date: Thu, 13 Oct 2022 12:27:23 +0000 Subject: [PATCH] Rephrase BackAnimationController and BackAnimationRunner This CL did some refactors include: - Make the back animation will follow these two phases to better understand. 1. Phase 1 animation will start from gesture start, and end when gesture released. 2. Phase 2 animation aka post commit animation will start after gesture released. 3. When the animation is finished, make sure we could trigger the real back behavior and notify the core. Bug: 238475694 Test: atest BackAnimationControllerTest Change-Id: I251b9674a4aa9965c338ccf592efaeabdcecd5e8 --- .../shell/back/BackAnimationController.java | 295 +++++++++--------- .../wm/shell/back/BackAnimationRunner.java | 28 +- .../back/BackAnimationControllerTest.java | 150 +++++---- 3 files changed, 236 insertions(+), 237 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java index b6327e5d91f51..d9eaeeeaf45fd 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java @@ -38,7 +38,6 @@ import android.os.UserHandle; import android.provider.Settings.Global; import android.util.Log; import android.util.SparseArray; -import android.view.IRemoteAnimationFinishedCallback; import android.view.IRemoteAnimationRunner; import android.view.IWindowFocusObserver; import android.view.InputDevice; @@ -69,7 +68,7 @@ import java.util.concurrent.atomic.AtomicBoolean; * Controls the window animation run when a user initiates a back gesture. */ public class BackAnimationController implements RemoteCallable { - private static final String TAG = "BackAnimationController"; + private static final String TAG = "ShellBackPreview"; private static final int SETTING_VALUE_OFF = 0; private static final int SETTING_VALUE_ON = 1; public static final boolean IS_ENABLED = @@ -82,16 +81,15 @@ public class BackAnimationController implements RemoteCallable { - ProtoLog.w(WM_SHELL_BACK_PREVIEW, "Transition didn't finish in %d ms. Resetting...", - MAX_TRANSITION_DURATION); + private final Runnable mAnimationTimeoutRunnable = () -> { + ProtoLog.w(WM_SHELL_BACK_PREVIEW, "Animation didn't finish in %d ms. Resetting...", + MAX_ANIMATION_DURATION); onBackAnimationFinished(); }; @@ -119,6 +117,8 @@ public class BackAnimationController implements RemoteCallable mAnimationDefinition = new SparseArray<>(); + private IOnBackInvokedCallback mActiveCallback; + @VisibleForTesting final IWindowFocusObserver mFocusObserver = new IWindowFocusObserver.Stub() { @Override @@ -126,9 +126,9 @@ public class BackAnimationController implements RemoteCallable { - if (!mBackGestureStarted || mTransitionInProgress) { - // If an uninterruptible transition is already in progress, we should ignore - // this due to the transition may cause focus lost. (alpha = 0) + if (!mBackGestureStarted || mPostCommitAnimationInProgress) { + // If an uninterruptible animation is already in progress, we should ignore + // this due to it may cause focus lost. (alpha = 0) return; } ProtoLog.i(WM_SHELL_BACK_PREVIEW, "Target window lost focus."); @@ -180,26 +180,11 @@ public class BackAnimationController implements RemoteCallable controller.setBackToLauncherCallback(callback, runner)); + (controller) -> controller.registerAnimation( + BackNavigationInfo.TYPE_RETURN_TO_HOME, + new BackAnimationRunner(callback, runner))); } @Override @@ -294,44 +280,22 @@ public class BackAnimationController implements RemoteCallable= 1) { - final int backType = mBackNavigationInfo.getType(); - IOnBackInvokedCallback targetCallback = mAnimationDefinition.get(backType) - .getCallback(); dispatchOnBackStarted( - targetCallback, mTouchTracker.createStartEvent(apps[0])); + mActiveCallback, mTouchTracker.createStartEvent(apps[0])); } if (!mBackGestureStarted) { // if the down -> up gesture happened before animation start, we have to // trigger the uninterruptible transition to finish the back animation. - final BackEvent backFinish = mTouchTracker.createProgressEvent(1); - startTransition(); - runner.consumeIfGestureFinished(backFinish); + final BackEvent backFinish = mTouchTracker.createProgressEvent(); + dispatchOnBackProgressed(mActiveCallback, backFinish); + startPostCommitAnimation(); } }); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationRunner.java b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationRunner.java index c53fcfc99c9c0..d70b8f53a9115 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationRunner.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationRunner.java @@ -18,12 +18,12 @@ package com.android.wm.shell.back; import static android.view.WindowManager.TRANSIT_OLD_UNSET; +import android.annotation.NonNull; import android.os.RemoteException; import android.util.Log; import android.view.IRemoteAnimationFinishedCallback; import android.view.IRemoteAnimationRunner; import android.view.RemoteAnimationTarget; -import android.window.BackEvent; import android.window.IBackAnimationRunner; import android.window.IOnBackInvokedCallback; @@ -38,11 +38,11 @@ class BackAnimationRunner { private final IOnBackInvokedCallback mCallback; private final IRemoteAnimationRunner mRunner; - private boolean mTriggerBack; // Whether we are waiting to receive onAnimationStart private boolean mWaitingAnimation; - BackAnimationRunner(IOnBackInvokedCallback callback, IRemoteAnimationRunner runner) { + BackAnimationRunner(@NonNull IOnBackInvokedCallback callback, + @NonNull IRemoteAnimationRunner runner) { mCallback = callback; mRunner = runner; } @@ -83,25 +83,7 @@ class BackAnimationRunner { mWaitingAnimation = true; } - boolean onGestureFinished(boolean triggerBack) { - if (mWaitingAnimation) { - mTriggerBack = triggerBack; - return true; - } - return false; - } - - void consumeIfGestureFinished(final BackEvent backFinish) { - Log.d(TAG, "Start transition due to gesture is finished"); - try { - mCallback.onBackProgressed(backFinish); - if (mTriggerBack) { - mCallback.onBackInvoked(); - } else { - mCallback.onBackCancelled(); - } - } catch (RemoteException e) { - Log.e(TAG, "dispatch error: ", e); - } + boolean isWaitingAnimation() { + return mWaitingAnimation; } } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java index 7eccbf42e67dc..b603e0355e98d 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java @@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -93,7 +94,10 @@ public class BackAnimationControllerTest extends ShellTestCase { private IActivityTaskManager mActivityTaskManager; @Mock - private IOnBackInvokedCallback mIOnBackInvokedCallback; + private IOnBackInvokedCallback mAppCallback; + + @Mock + private IOnBackInvokedCallback mAnimatorCallback; @Mock private IBackAnimationFinishedCallback mBackAnimationFinishedCallback; @@ -105,8 +109,6 @@ public class BackAnimationControllerTest extends ShellTestCase { private ShellController mShellController; private BackAnimationController mController; - - private int mEventTime = 0; private TestableContentResolver mContentResolver; private TestableLooper mTestableLooper; @@ -126,16 +128,15 @@ public class BackAnimationControllerTest extends ShellTestCase { mContentResolver); mController.setEnableUAnimation(true); mShellInit.init(); - mEventTime = 0; mShellExecutor.flushAll(); } - private void createNavigationInfo(int backType, IOnBackInvokedCallback onBackInvokedCallback) { + private void createNavigationInfo(int backType, boolean enableAnimation) { BackNavigationInfo.Builder builder = new BackNavigationInfo.Builder() .setType(backType) .setOnBackNavigationDone(new RemoteCallback((bundle) -> {})) - .setOnBackInvokedCallback(onBackInvokedCallback) - .setPrepareRemoteAnimation(true); + .setOnBackInvokedCallback(mAppCallback) + .setPrepareRemoteAnimation(enableAnimation); createNavigationInfo(builder); } @@ -176,26 +177,47 @@ public class BackAnimationControllerTest extends ShellTestCase { } @Test - public void verifyAnimationFinishes() { - RemoteAnimationTarget animationTarget = createAnimationTarget(); - boolean[] backNavigationDone = new boolean[]{false}; - boolean[] triggerBack = new boolean[]{false}; - createNavigationInfo(new BackNavigationInfo.Builder() - .setType(BackNavigationInfo.TYPE_CROSS_ACTIVITY) - .setOnBackNavigationDone( - new RemoteCallback(result -> { - backNavigationDone[0] = true; - triggerBack[0] = result.getBoolean(KEY_TRIGGER_BACK); - }))); - triggerBackGesture(); - assertTrue("Navigation Done callback not called", backNavigationDone[0]); - assertTrue("TriggerBack should have been true", triggerBack[0]); + public void verifyNavigationFinishes() throws RemoteException { + final int[] testTypes = new int[] {BackNavigationInfo.TYPE_RETURN_TO_HOME, + BackNavigationInfo.TYPE_CROSS_TASK, + BackNavigationInfo.TYPE_CROSS_ACTIVITY, + BackNavigationInfo.TYPE_DIALOG_CLOSE, + BackNavigationInfo.TYPE_CALLBACK }; + + for (int type: testTypes) { + registerAnimation(type); + } + + for (int type: testTypes) { + boolean[] backNavigationDone = new boolean[]{false}; + boolean[] triggerBack = new boolean[]{false}; + + createNavigationInfo(new BackNavigationInfo.Builder() + .setType(type) + .setOnBackInvokedCallback(mAppCallback) + .setPrepareRemoteAnimation(true) + .setOnBackNavigationDone( + new RemoteCallback(result -> { + backNavigationDone[0] = true; + triggerBack[0] = result.getBoolean(KEY_TRIGGER_BACK); + }))); + triggerBackGesture(); + simulateRemoteAnimationStart(type); + simulateRemoteAnimationFinished(); + mShellExecutor.flushAll(); + + assertTrue("Navigation Done callback not called for " + + BackNavigationInfo.typeToString(type), backNavigationDone[0]); + assertTrue("TriggerBack should have been true", triggerBack[0]); + } } + + @Test public void backToHome_dispatchesEvents() throws RemoteException { - mController.setBackToLauncherCallback(mIOnBackInvokedCallback, mBackAnimationRunner); - createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, mIOnBackInvokedCallback); + registerAnimation(BackNavigationInfo.TYPE_RETURN_TO_HOME); + createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, true); doMotionEvent(MotionEvent.ACTION_DOWN, 0); @@ -203,14 +225,16 @@ public class BackAnimationControllerTest extends ShellTestCase { doMotionEvent(MotionEvent.ACTION_MOVE, 100); simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME); - verify(mIOnBackInvokedCallback).onBackStarted(any(BackEvent.class)); + + verify(mAnimatorCallback).onBackStarted(any(BackEvent.class)); verify(mBackAnimationRunner).onAnimationStart(anyInt(), any(), any(), any(), any()); - verify(mIOnBackInvokedCallback, atLeastOnce()).onBackProgressed(any(BackEvent.class)); + ArgumentCaptor backEventCaptor = ArgumentCaptor.forClass(BackEvent.class); + verify(mAnimatorCallback, atLeastOnce()).onBackProgressed(backEventCaptor.capture()); // Check that back invocation is dispatched. mController.setTriggerBack(true); // Fake trigger back doMotionEvent(MotionEvent.ACTION_UP, 0); - verify(mIOnBackInvokedCallback).onBackInvoked(); + verify(mAnimatorCallback).onBackInvoked(); } @Test @@ -223,97 +247,94 @@ public class BackAnimationControllerTest extends ShellTestCase { mActivityTaskManager, mContext, mContentResolver); shellInit.init(); - mController.setBackToLauncherCallback(mIOnBackInvokedCallback, mBackAnimationRunner); + registerAnimation(BackNavigationInfo.TYPE_RETURN_TO_HOME); - IOnBackInvokedCallback appCallback = mock(IOnBackInvokedCallback.class); ArgumentCaptor backEventCaptor = ArgumentCaptor.forClass(BackEvent.class); - createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, appCallback); + createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, false); triggerBackGesture(); - verify(appCallback, never()).onBackStarted(any(BackEvent.class)); - verify(appCallback, never()).onBackProgressed(backEventCaptor.capture()); - verify(appCallback, times(1)).onBackInvoked(); + verify(mAppCallback, never()).onBackStarted(any()); + verify(mAppCallback, never()).onBackProgressed(backEventCaptor.capture()); + verify(mAppCallback, times(1)).onBackInvoked(); - verify(mIOnBackInvokedCallback, never()).onBackStarted(any(BackEvent.class)); - verify(mIOnBackInvokedCallback, never()).onBackProgressed(backEventCaptor.capture()); - verify(mIOnBackInvokedCallback, never()).onBackInvoked(); + verify(mAnimatorCallback, never()).onBackStarted(any()); + verify(mAnimatorCallback, never()).onBackProgressed(backEventCaptor.capture()); + verify(mAnimatorCallback, never()).onBackInvoked(); verify(mBackAnimationRunner, never()).onAnimationStart( anyInt(), any(), any(), any(), any()); } @Test public void ignoresGesture_transitionInProgress() throws RemoteException { - mController.setBackToLauncherCallback(mIOnBackInvokedCallback, mBackAnimationRunner); - createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, null); + registerAnimation(BackNavigationInfo.TYPE_RETURN_TO_HOME); + createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, true); triggerBackGesture(); simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME); // Check that back invocation is dispatched. - verify(mIOnBackInvokedCallback).onBackInvoked(); + verify(mAnimatorCallback).onBackInvoked(); verify(mBackAnimationRunner).onAnimationStart(anyInt(), any(), any(), any(), any()); - reset(mIOnBackInvokedCallback); + reset(mAnimatorCallback); reset(mBackAnimationRunner); // Verify that we prevent animation from restarting if another gestures happens before // the previous transition is finished. doMotionEvent(MotionEvent.ACTION_DOWN, 0); - verifyNoMoreInteractions(mIOnBackInvokedCallback); - mController.onBackAnimationFinished(); - // Pretend the transition handler called finishAnimation. - mController.finishBackNavigation(); + verifyNoMoreInteractions(mAnimatorCallback); + + // Finish back navigation. + simulateRemoteAnimationFinished(); // Verify that more events from a rejected swipe cannot start animation. doMotionEvent(MotionEvent.ACTION_MOVE, 100); doMotionEvent(MotionEvent.ACTION_UP, 0); - verifyNoMoreInteractions(mIOnBackInvokedCallback); + verifyNoMoreInteractions(mAnimatorCallback); // Verify that we start accepting gestures again once transition finishes. doMotionEvent(MotionEvent.ACTION_DOWN, 0); doMotionEvent(MotionEvent.ACTION_MOVE, 100); simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME); - verify(mIOnBackInvokedCallback).onBackStarted(any(BackEvent.class)); + verify(mAnimatorCallback).onBackStarted(any()); verify(mBackAnimationRunner).onAnimationStart(anyInt(), any(), any(), any(), any()); } @Test public void acceptsGesture_transitionTimeout() throws RemoteException { - mController.setBackToLauncherCallback(mIOnBackInvokedCallback, mBackAnimationRunner); - createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, null); + registerAnimation(BackNavigationInfo.TYPE_RETURN_TO_HOME); + createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, true); + + // In case it is still running in animation. + doNothing().when(mAnimatorCallback).onBackInvoked(); triggerBackGesture(); simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME); - reset(mIOnBackInvokedCallback); - // Simulate transition timeout. mShellExecutor.flushAll(); - mController.onBackAnimationFinished(); - // Pretend the transition handler called finishAnimation. - mController.finishBackNavigation(); + reset(mAnimatorCallback); doMotionEvent(MotionEvent.ACTION_DOWN, 0); doMotionEvent(MotionEvent.ACTION_MOVE, 100); simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME); - verify(mIOnBackInvokedCallback).onBackStarted(any(BackEvent.class)); + verify(mAnimatorCallback).onBackStarted(any()); } - @Test public void cancelBackInvokeWhenLostFocus() throws RemoteException { - mController.setBackToLauncherCallback(mIOnBackInvokedCallback, mBackAnimationRunner); + registerAnimation(BackNavigationInfo.TYPE_RETURN_TO_HOME); - createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, null); + createNavigationInfo(BackNavigationInfo.TYPE_RETURN_TO_HOME, true); doMotionEvent(MotionEvent.ACTION_DOWN, 0); // Check that back start and progress is dispatched when first move. doMotionEvent(MotionEvent.ACTION_MOVE, 100); simulateRemoteAnimationStart(BackNavigationInfo.TYPE_RETURN_TO_HOME); - verify(mIOnBackInvokedCallback).onBackStarted(any(BackEvent.class)); + verify(mAnimatorCallback).onBackStarted(any()); verify(mBackAnimationRunner).onAnimationStart(anyInt(), any(), any(), any(), any()); // Check that back invocation is dispatched. @@ -323,11 +344,11 @@ public class BackAnimationControllerTest extends ShellTestCase { IBinder token = mock(IBinder.class); mController.mFocusObserver.focusLost(token); mShellExecutor.flushAll(); - verify(mIOnBackInvokedCallback).onBackCancelled(); + verify(mAnimatorCallback).onBackCancelled(); // No more back invoke. doMotionEvent(MotionEvent.ACTION_UP, 0); - verify(mIOnBackInvokedCallback, never()).onBackInvoked(); + verify(mAnimatorCallback, never()).onBackInvoked(); } private void doMotionEvent(int actionDown, int coordinate) { @@ -335,7 +356,6 @@ public class BackAnimationControllerTest extends ShellTestCase { coordinate, coordinate, actionDown, BackEvent.EDGE_LEFT); - mEventTime += 10; } private void simulateRemoteAnimationStart(int type) throws RemoteException { @@ -347,4 +367,14 @@ public class BackAnimationControllerTest extends ShellTestCase { mShellExecutor.flushAll(); } } + + private void simulateRemoteAnimationFinished() { + mController.onBackAnimationFinished(); + mController.finishBackNavigation(); + } + + private void registerAnimation(int type) { + mController.registerAnimation(type, + new BackAnimationRunner(mAnimatorCallback, mBackAnimationRunner)); + } }