From e8f9151270ec7f8995f6437410ae5580f9c09a3b Mon Sep 17 00:00:00 2001
From: Shan Huang <shanh@google.com>
Date: Mon, 18 Apr 2022 19:55:02 +0000
Subject: [PATCH] Fix broken animation on two consecutive swipes.

The bug is caused by allowing a second swipe to restart animation in
BackAnimationController. This CL adds a flag to reject incoming gestures
when an uninterruptable transition (e.g. commit transition, cancel
transition) is already in progress.

To prevent an unfinished transition from failing all future swipes, this CL also introduces a timeout mechanism to start accepting gesture again after a fixed period.

Bug: 221394367
Test: Do two quick consecutive swipes and observe the back to home
animation.

Merged-In: Ib969afb72cc15bd268613546f3495008b6bc9125
Change-Id: Ib969afb72cc15bd268613546f3495008b6bc9125
---
 .../shell/back/BackAnimationController.java   | 41 ++++++++++++++++-
 .../back/BackAnimationControllerTest.java     | 45 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 3 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 8fa9f564fb222..a16841cb870ac 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
@@ -67,8 +67,12 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
                     SETTING_VALUE_ON) != SETTING_VALUE_OFF;
     private static final int PROGRESS_THRESHOLD = SystemProperties
             .getInt(PREDICTIVE_BACK_PROGRESS_THRESHOLD_PROP, -1);
-
     private final AtomicBoolean mEnableAnimations = new AtomicBoolean(false);
+    /**
+     * Max duration to wait for a transition to finish before accepting another gesture start
+     * request.
+     */
+    private static final long MAX_TRANSITION_DURATION = 2000;
 
     /**
      * Location of the initial touch event of the back gesture.
@@ -84,6 +88,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
     /** True when a back gesture is ongoing */
     private boolean mBackGestureStarted = false;
 
+    /** Tracks if an uninterruptible transition is in progress */
+    private boolean mTransitionInProgress = false;
     /** @see #setTriggerBack(boolean) */
     private boolean mTriggerBack;
 
@@ -96,6 +102,10 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
     private IOnBackInvokedCallback mBackToLauncherCallback;
     private float mTriggerThreshold;
     private float mProgressThreshold;
+    private final Runnable mResetTransitionRunnable = () -> {
+        finishAnimation();
+        mTransitionInProgress = false;
+    };
 
     public BackAnimationController(
             @NonNull @ShellMainThread ShellExecutor shellExecutor,
@@ -229,7 +239,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
         mBackToLauncherCallback = null;
     }
 
-    private void onBackToLauncherAnimationFinished() {
+    @VisibleForTesting
+    void onBackToLauncherAnimationFinished() {
         if (mBackNavigationInfo != null) {
             IOnBackInvokedCallback callback = mBackNavigationInfo.getOnBackInvokedCallback();
             if (mTriggerBack) {
@@ -246,6 +257,9 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
      * {@link BackAnimationController}
      */
     public void onMotionEvent(MotionEvent event, int action, @BackEvent.SwipeEdge int swipeEdge) {
+        if (mTransitionInProgress) {
+            return;
+        }
         if (action == MotionEvent.ACTION_MOVE) {
             if (!mBackGestureStarted) {
                 // Let the animation initialized here to make sure the onPointerDownOutsideFocus
@@ -370,6 +384,9 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
         IOnBackInvokedCallback targetCallback = shouldDispatchToLauncher
                 ? mBackToLauncherCallback
                 : mBackNavigationInfo.getOnBackInvokedCallback();
+        if (shouldDispatchToLauncher) {
+            startTransition();
+        }
         if (mTriggerBack) {
             dispatchOnBackInvoked(targetCallback);
         } else {
@@ -436,6 +453,9 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
      * Sets to true when the back gesture has passed the triggering threshold, false otherwise.
      */
     public void setTriggerBack(boolean triggerBack) {
+        if (mTransitionInProgress) {
+            return;
+        }
         mTriggerBack = triggerBack;
     }
 
@@ -467,6 +487,23 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
             mTransaction.remove(screenshotSurface);
         }
         mTransaction.apply();
+        stopTransition();
         backNavigationInfo.onBackNavigationFinished(triggerBack);
     }
+
+    private void startTransition() {
+        if (mTransitionInProgress) {
+            return;
+        }
+        mTransitionInProgress = true;
+        mShellExecutor.executeDelayed(mResetTransitionRunnable, MAX_TRANSITION_DURATION);
+    }
+
+    private void stopTransition() {
+        if (!mTransitionInProgress) {
+            return;
+        }
+        mShellExecutor.removeCallbacks(mResetTransitionRunnable);
+        mTransitionInProgress = false;
+    }
 }
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 42b1014396640..c3369e4bb90b3 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
@@ -26,8 +26,10 @@ import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import android.app.IActivityTaskManager;
 import android.app.WindowConfiguration;
@@ -180,7 +182,8 @@ public class BackAnimationControllerTest {
         // b/207481538, we check that the surface is not moved for now, we can re-enable this once
         // we implement the animation
         verify(mTransaction, never()).setScale(eq(screenshotSurface), anyInt(), anyInt());
-        verify(mTransaction, never()).setPosition(animationTarget.leash, 100, 100);
+        verify(mTransaction, never()).setPosition(
+                animationTarget.leash, 100, 100);
         verify(mTransaction, atLeastOnce()).apply();
     }
 
@@ -251,6 +254,46 @@ public class BackAnimationControllerTest {
         verify(mIOnBackInvokedCallback, never()).onBackInvoked();
     }
 
+    public void ignoresGesture_transitionInProgress() throws RemoteException {
+        mController.setBackToLauncherCallback(mIOnBackInvokedCallback);
+        RemoteAnimationTarget animationTarget = createAnimationTarget();
+        createNavigationInfo(animationTarget, null, null,
+                BackNavigationInfo.TYPE_RETURN_TO_HOME, null);
+
+        triggerBackGesture();
+        // Check that back invocation is dispatched.
+        verify(mIOnBackInvokedCallback).onBackInvoked();
+
+        reset(mIOnBackInvokedCallback);
+        // Verify that we prevent animation from restarting if another gestures happens before
+        // the previous transition is finished.
+        doMotionEvent(MotionEvent.ACTION_DOWN, 0);
+        verifyNoMoreInteractions(mIOnBackInvokedCallback);
+
+        // Verify that we start accepting gestures again once transition finishes.
+        mController.onBackToLauncherAnimationFinished();
+        doMotionEvent(MotionEvent.ACTION_DOWN, 0);
+        doMotionEvent(MotionEvent.ACTION_MOVE, 100);
+        verify(mIOnBackInvokedCallback).onBackStarted();
+    }
+
+    @Test
+    public void acceptsGesture_transitionTimeout() throws RemoteException {
+        mController.setBackToLauncherCallback(mIOnBackInvokedCallback);
+        RemoteAnimationTarget animationTarget = createAnimationTarget();
+        createNavigationInfo(animationTarget, null, null,
+                BackNavigationInfo.TYPE_RETURN_TO_HOME, null);
+
+        triggerBackGesture();
+        reset(mIOnBackInvokedCallback);
+
+        // Simulate transition timeout.
+        mShellExecutor.flushAll();
+        doMotionEvent(MotionEvent.ACTION_DOWN, 0);
+        doMotionEvent(MotionEvent.ACTION_MOVE, 100);
+        verify(mIOnBackInvokedCallback).onBackStarted();
+    }
+
     private void doMotionEvent(int actionDown, int coordinate) {
         mController.onMotionEvent(
                 MotionEvent.obtain(0, mEventTime, actionDown, coordinate, coordinate, 0),