Use immutable shouldBeAnimated during Unfold Animation

Context:
We decide to run animations by using shouldBeAnimated lambda function. It represents mutable state which is combination of the
StatusBar and some external business logic.

It shows that shouldBeAnimated value may change for some reasons after the animation is fired.
This may cause some visual issues like wrong translationX values in the view(animation started but not completed properly).

Solution:
We make shouldBeAnimated immutable variable instead of lambda func in ViewToTranslate and use it during the animation.
We already map `ViewIdToTranslate` to `ViewToTranslate` on each animation start.  It also verifies that we invalidate the preserved value on each animation start.

Test: atest UnfoldConstantTranslateAnimatorTest
Bug: 264454435

Change-Id: I75f84a668ab3395193901cff569ca56e55456316

3 files changed, 49 insertions, 25 deletions

diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt b/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt
index 12e0b9ab835a..c5979cc50c3c 100644
--- a/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt
+++ b/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt
@@ -65,35 +65,40 @@ class UnfoldConstantTranslateAnimator(
             } else {
                 1
             }
-        viewsToTranslate.forEach { (view, direction, shouldBeAnimated) ->
-            if (shouldBeAnimated()) {
-                view.get()?.translationX = xTrans * direction.multiplier * rtlMultiplier
-            }
+        viewsToTranslate.forEach { (view, direction) ->
+            view.get()?.translationX = xTrans * direction.multiplier * rtlMultiplier
         }
     }
 
     /** Finds in [parent] all views specified by [ids] and register them for the animation. */
     private fun registerViewsForAnimation(parent: ViewGroup, ids: Set<ViewIdToTranslate>) {
         viewsToTranslate =
-            ids.mapNotNull { (id, dir, pred) ->
-                parent.findViewById<View>(id)?.let { view ->
-                    ViewToTranslate(WeakReference(view), dir, pred)
+            ids.asSequence()
+                .filter { it.shouldBeAnimated() }
+                .mapNotNull {
+                    parent.findViewById<View>(it.viewId)?.let { view ->
+                        ViewToTranslate(WeakReference(view), it.direction)
+                    }
                 }
-            }
+                .toList()
     }
 
-    /** Represents a view to animate. [rootView] should contain a view with [viewId] inside. */
+    /**
+     * Represents a view to animate. [rootView] should contain a view with [viewId] inside.
+     * [shouldBeAnimated] is only evaluated when the viewsToTranslate is registered in
+     * [registerViewsForAnimation].
+     */
     data class ViewIdToTranslate(
         val viewId: Int,
         val direction: Direction,
         val shouldBeAnimated: () -> Boolean = { true }
     )
 
-    private data class ViewToTranslate(
-        val view: WeakReference<View>,
-        val direction: Direction,
-        val shouldBeAnimated: () -> Boolean
-    )
+    /**
+     * Represents a view whose animation process is in-progress. It should be immutable because the
+     * started animation should be completed.
+     */
+    private data class ViewToTranslate(val view: WeakReference<View>, val direction: Direction)
 
     /** Direction of the animation. */
     enum class Direction(val multiplier: Float) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java
index 6b4a4d1389ff..be791f9e2242 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java
@@ -2211,10 +2211,10 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces {
             pw.println("Current Status Bar state:");
             pw.println("  mExpandedVisible=" + mShadeController.isExpandedVisible());
             pw.println("  mDisplayMetrics=" + mDisplayMetrics);
-            pw.println("  mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller));
-            pw.println("  mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller)
-                    + " scroll " + mStackScroller.getScrollX()
+            pw.print("  mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller));
+            pw.print(" scroll " + mStackScroller.getScrollX()
                     + "," + mStackScroller.getScrollY());
+            pw.println(" translationX " + mStackScroller.getTranslationX());
         }
 
         pw.print("  mInteractingWindows="); pw.println(mInteractingWindows);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt
index 7a74b1229c5c..56fc0c74dafa 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt
@@ -36,21 +36,26 @@ class UnfoldConstantTranslateAnimatorTest : SysuiTestCase() {
 
     private val progressProvider = TestUnfoldTransitionProvider()
 
-    @Mock private lateinit var parent: ViewGroup
+    @Mock
+    private lateinit var parent: ViewGroup
+
+    @Mock
+    private lateinit var shouldBeAnimated: () -> Boolean
 
     private lateinit var animator: UnfoldConstantTranslateAnimator
 
-    private val viewsIdToRegister =
-        setOf(
-            ViewIdToTranslate(START_VIEW_ID, Direction.START),
-            ViewIdToTranslate(END_VIEW_ID, Direction.END))
+    private val viewsIdToRegister
+        get() =
+            setOf(
+                    ViewIdToTranslate(START_VIEW_ID, Direction.START, shouldBeAnimated),
+                    ViewIdToTranslate(END_VIEW_ID, Direction.END, shouldBeAnimated)
+            )
 
     @Before
     fun setup() {
         MockitoAnnotations.initMocks(this)
-
-        animator =
-            UnfoldConstantTranslateAnimator(viewsIdToRegister, progressProvider)
+        whenever(shouldBeAnimated.invoke()).thenReturn(true)
+        animator = UnfoldConstantTranslateAnimator(viewsIdToRegister, progressProvider)
 
         animator.init(parent, MAX_TRANSLATION)
     }
@@ -96,6 +101,20 @@ class UnfoldConstantTranslateAnimatorTest : SysuiTestCase() {
         moveAndValidate(listOf(leftView to START, rightView to END), View.LAYOUT_DIRECTION_LTR)
     }
 
+    @Test
+    fun onTransition_completeStartedTranslation() {
+        // GIVEN
+        val leftView = View(context)
+        whenever(parent.findViewById<View>(START_VIEW_ID)).thenReturn(leftView)
+        // To start animation, shouldBeAnimated should return true.
+        // There is a possibility for shouldBeAnimated to return false during the animation.
+        whenever(shouldBeAnimated.invoke()).thenReturn(true).thenReturn(false)
+
+        // shouldBeAnimated state may change during the animation.
+        // However, started animation should be completed.
+        moveAndValidate(listOf(leftView to START), View.LAYOUT_DIRECTION_LTR)
+    }
+
     private fun moveAndValidate(list: List<Pair<View, Int>>, layoutDirection: Int) {
         // Compare values as ints because -0f != 0f