[Chipbar] Force the views' alphas to 1 if the animate in fails.

Bug: 266119467
Test: manual: Set `ViewHierarchyAnimator.animateAddition` to not do
anything, and verify that the chipbar still shows up (see video in bug)
Test: atest ChipbarCoordinatorTest

Change-Id: I67301dc3ed0b1041c6f4d7cddd504ce38bbd97d9

5 files changed, 180 insertions, 29 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewLogger.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewLogger.kt
index ec6965a83b5a..899b0c2b0947 100644
--- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewLogger.kt
+++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewLogger.kt
@@ -80,6 +80,26 @@ open class TemporaryViewLogger<T : TemporaryViewInfo>(
         )
     }
 
+    /** Logs that there was a failure to animate the view in. */
+    fun logAnimateInFailure() {
+        buffer.log(
+            tag,
+            LogLevel.WARNING,
+            {},
+            { "View's appearance animation failed. Forcing view display manually." },
+        )
+    }
+
+    /** Logs that there was a failure to animate the view out. */
+    fun logAnimateOutFailure() {
+        buffer.log(
+            tag,
+            LogLevel.WARNING,
+            {},
+            { "View's disappearance animation failed." },
+        )
+    }
+
     fun logViewHidden(info: T) {
         buffer.log(
             tag,
diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarAnimator.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarAnimator.kt
new file mode 100644
index 000000000000..01a81deabc95
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarAnimator.kt
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.temporarydisplay.chipbar
+
+import android.view.View
+import android.view.ViewGroup
+import com.android.systemui.animation.Interpolators
+import com.android.systemui.animation.ViewHierarchyAnimator
+import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.util.children
+import javax.inject.Inject
+
+/**
+ * A class controlling chipbar animations. Typically delegates to [ViewHierarchyAnimator].
+ *
+ * Used so that animations can be mocked out in tests.
+ */
+@SysUISingleton
+open class ChipbarAnimator @Inject constructor() {
+    /**
+     * Animates [innerView] and its children into view.
+     *
+     * @return true if the animation was successfully started and false if the animation can't be
+     * run for any reason.
+     *
+     * See [ViewHierarchyAnimator.animateAddition].
+     */
+    open fun animateViewIn(innerView: ViewGroup, onAnimationEnd: Runnable): Boolean {
+        return ViewHierarchyAnimator.animateAddition(
+            innerView,
+            ViewHierarchyAnimator.Hotspot.TOP,
+            Interpolators.EMPHASIZED_DECELERATE,
+            duration = ANIMATION_IN_DURATION,
+            includeMargins = true,
+            includeFadeIn = true,
+            onAnimationEnd = onAnimationEnd,
+        )
+    }
+
+    /**
+     * Animates [innerView] and its children out of view.
+     *
+     * @return true if the animation was successfully started and false if the animation can't be
+     * run for any reason.
+     *
+     * See [ViewHierarchyAnimator.animateRemoval].
+     */
+    open fun animateViewOut(innerView: ViewGroup, onAnimationEnd: Runnable): Boolean {
+        return ViewHierarchyAnimator.animateRemoval(
+            innerView,
+            ViewHierarchyAnimator.Hotspot.TOP,
+            Interpolators.EMPHASIZED_ACCELERATE,
+            ANIMATION_OUT_DURATION,
+            includeMargins = true,
+            onAnimationEnd,
+        )
+    }
+
+    /** Force shows this view and all child views. Should be used in case [animateViewIn] fails. */
+    fun forceDisplayView(innerView: View) {
+        innerView.alpha = 1f
+        if (innerView is ViewGroup) {
+            innerView.children.forEach { forceDisplayView(it) }
+        }
+    }
+}
+
+private const val ANIMATION_IN_DURATION = 500L
+private const val ANIMATION_OUT_DURATION = 250L
diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt
index 46f13cc6b037..696134cde3c9 100644
--- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt
+++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt
@@ -32,8 +32,6 @@ import androidx.annotation.IdRes
 import com.android.internal.widget.CachingIconView
 import com.android.systemui.Gefingerpoken
 import com.android.systemui.R
-import com.android.systemui.animation.Interpolators
-import com.android.systemui.animation.ViewHierarchyAnimator
 import com.android.systemui.classifier.FalsingCollector
 import com.android.systemui.common.shared.model.ContentDescription.Companion.loadContentDescription
 import com.android.systemui.common.shared.model.Text.Companion.loadText
@@ -78,6 +76,7 @@ constructor(
     configurationController: ConfigurationController,
     dumpManager: DumpManager,
     powerManager: PowerManager,
+    private val chipbarAnimator: ChipbarAnimator,
     private val falsingManager: FalsingManager,
     private val falsingCollector: FalsingCollector,
     private val swipeChipbarAwayGestureHandler: SwipeChipbarAwayGestureHandler?,
@@ -206,23 +205,17 @@ constructor(
     }
 
     override fun animateViewIn(view: ViewGroup) {
+        // We can only request focus once the animation finishes.
         val onAnimationEnd = Runnable {
             maybeGetAccessibilityFocus(view.getTag(INFO_TAG) as ChipbarInfo?, view)
         }
-        val added =
-            ViewHierarchyAnimator.animateAddition(
-                view.getInnerView(),
-                ViewHierarchyAnimator.Hotspot.TOP,
-                Interpolators.EMPHASIZED_DECELERATE,
-                duration = ANIMATION_IN_DURATION,
-                includeMargins = true,
-                includeFadeIn = true,
-                // We can only request focus once the animation finishes.
-                onAnimationEnd = onAnimationEnd,
-            )
-        // If the view doesn't get animated, the [onAnimationEnd] runnable won't get run. So, just
-        // run it immediately.
-        if (!added) {
+        val animatedIn = chipbarAnimator.animateViewIn(view.getInnerView(), onAnimationEnd)
+
+        // If the view doesn't get animated, the [onAnimationEnd] runnable won't get run and the
+        // views would remain un-displayed. So, just force-set/run those items immediately.
+        if (!animatedIn) {
+            logger.logAnimateInFailure()
+            chipbarAnimator.forceDisplayView(view.getInnerView())
             onAnimationEnd.run()
         }
     }
@@ -230,18 +223,11 @@ constructor(
     override fun animateViewOut(view: ViewGroup, removalReason: String?, onAnimationEnd: Runnable) {
         val innerView = view.getInnerView()
         innerView.accessibilityLiveRegion = ACCESSIBILITY_LIVE_REGION_NONE
-        val removed =
-            ViewHierarchyAnimator.animateRemoval(
-                innerView,
-                ViewHierarchyAnimator.Hotspot.TOP,
-                Interpolators.EMPHASIZED_ACCELERATE,
-                ANIMATION_OUT_DURATION,
-                includeMargins = true,
-                onAnimationEnd,
-            )
+        val removed = chipbarAnimator.animateViewOut(innerView, onAnimationEnd)
         // If the view doesn't get animated, the [onAnimationEnd] runnable won't get run. So, just
         // run it immediately.
         if (!removed) {
+            logger.logAnimateOutFailure()
             onAnimationEnd.run()
         }
 
@@ -299,8 +285,6 @@ constructor(
     }
 }
 
-private const val ANIMATION_IN_DURATION = 500L
-private const val ANIMATION_OUT_DURATION = 250L
 @IdRes private val INFO_TAG = R.id.tag_chipbar_info
 private const val SWIPE_UP_GESTURE_REASON = "SWIPE_UP_GESTURE_DETECTED"
 private const val TAG = "ChipbarCoordinator"
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt
index c20589a28621..30bd2dceca7b 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt
@@ -46,6 +46,7 @@ import com.android.systemui.statusbar.CommandQueue
 import com.android.systemui.statusbar.VibratorHelper
 import com.android.systemui.statusbar.policy.ConfigurationController
 import com.android.systemui.temporarydisplay.TemporaryViewDisplayController
+import com.android.systemui.temporarydisplay.chipbar.ChipbarAnimator
 import com.android.systemui.temporarydisplay.chipbar.ChipbarCoordinator
 import com.android.systemui.temporarydisplay.chipbar.ChipbarInfo
 import com.android.systemui.temporarydisplay.chipbar.ChipbarLogger
@@ -147,6 +148,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
                 configurationController,
                 dumpManager,
                 powerManager,
+                ChipbarAnimator(),
                 falsingManager,
                 falsingCollector,
                 swipeHandler,
diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt
index dd04ac4d79f6..fc7436a6b273 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt
@@ -79,6 +79,7 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
     @Mock private lateinit var viewUtil: ViewUtil
     @Mock private lateinit var vibratorHelper: VibratorHelper
     @Mock private lateinit var swipeGestureHandler: SwipeChipbarAwayGestureHandler
+    private lateinit var chipbarAnimator: TestChipbarAnimator
     private lateinit var fakeWakeLockBuilder: WakeLockFake.Builder
     private lateinit var fakeWakeLock: WakeLockFake
     private lateinit var fakeClock: FakeSystemClock
@@ -98,6 +99,7 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
         fakeWakeLockBuilder.setWakeLock(fakeWakeLock)
 
         uiEventLoggerFake = UiEventLoggerFake()
+        chipbarAnimator = TestChipbarAnimator()
 
         underTest =
             ChipbarCoordinator(
@@ -109,6 +111,7 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
                 configurationController,
                 dumpManager,
                 powerManager,
+                chipbarAnimator,
                 falsingManager,
                 falsingCollector,
                 swipeGestureHandler,
@@ -371,6 +374,26 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
         verify(vibratorHelper).vibrate(VibrationEffect.get(VibrationEffect.EFFECT_DOUBLE_CLICK))
     }
 
+    /** Regression test for b/266119467. */
+    @Test
+    fun displayView_animationFailure_viewsStillBecomeVisible() {
+        chipbarAnimator.allowAnimation = false
+
+        underTest.displayView(
+            createChipbarInfo(
+                Icon.Resource(R.id.check_box, null),
+                Text.Loaded("text"),
+                endItem = ChipbarEndItem.Loading,
+            )
+        )
+
+        val view = getChipbarView()
+        assertThat(view.getInnerView().alpha).isEqualTo(1f)
+        assertThat(view.getStartIconView().alpha).isEqualTo(1f)
+        assertThat(view.getLoadingIcon().alpha).isEqualTo(1f)
+        assertThat(view.getChipTextView().alpha).isEqualTo(1f)
+    }
+
     @Test
     fun updateView_viewUpdated() {
         // First, display a view
@@ -453,6 +476,25 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
         verify(windowManager).removeView(chipbarView)
     }
 
+    /** Regression test for b/266209420. */
+    @Test
+    fun removeView_animationFailure_viewStillRemoved() {
+        chipbarAnimator.allowAnimation = false
+
+        underTest.displayView(
+            createChipbarInfo(
+                Icon.Resource(R.drawable.ic_cake, contentDescription = null),
+                Text.Loaded("title text"),
+                endItem = ChipbarEndItem.Error,
+            ),
+        )
+        val chipbarView = getChipbarView()
+
+        underTest.removeView(DEVICE_ID, "test reason")
+
+        verify(windowManager).removeView(chipbarView)
+    }
+
     @Test
     fun swipeToDismiss_false_neverListensForGesture() {
         underTest.displayView(
@@ -560,8 +602,9 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
 
     private fun ViewGroup.getStartIconView() = this.requireViewById<ImageView>(R.id.start_icon)
 
-    private fun ViewGroup.getChipText(): String =
-        (this.requireViewById<TextView>(R.id.text)).text as String
+    private fun ViewGroup.getChipTextView() = this.requireViewById<TextView>(R.id.text)
+
+    private fun ViewGroup.getChipText(): String = this.getChipTextView().text as String
 
     private fun ViewGroup.getLoadingIcon(): View = this.requireViewById(R.id.loading)
 
@@ -574,6 +617,25 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
         verify(windowManager).addView(viewCaptor.capture(), any())
         return viewCaptor.value as ViewGroup
     }
+
+    /** Test class that lets us disallow animations. */
+    inner class TestChipbarAnimator : ChipbarAnimator() {
+        var allowAnimation: Boolean = true
+
+        override fun animateViewIn(innerView: ViewGroup, onAnimationEnd: Runnable): Boolean {
+            if (!allowAnimation) {
+                return false
+            }
+            return super.animateViewIn(innerView, onAnimationEnd)
+        }
+
+        override fun animateViewOut(innerView: ViewGroup, onAnimationEnd: Runnable): Boolean {
+            if (!allowAnimation) {
+                return false
+            }
+            return super.animateViewOut(innerView, onAnimationEnd)
+        }
+    }
 }
 
 private const val TIMEOUT = 10000