Merge "[Chipbar] Force the views' alphas to 1 if the animate in fails." into tm-qpr-dev am: b68a869107

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21098625

Change-Id: Ieff2a31c1a27af36520de9dcfadd1999f84c110b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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,

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

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 =
+        val animatedIn = chipbarAnimator.animateViewIn(view.getInnerView(), onAnimationEnd)
-            ViewHierarchyAnimator.animateAddition(
+
-                view.getInnerView(),
+        // If the view doesn't get animated, the [onAnimationEnd] runnable won't get run and the
-                ViewHierarchyAnimator.Hotspot.TOP,
+        // views would remain un-displayed. So, just force-set/run those items immediately.
-                Interpolators.EMPHASIZED_DECELERATE,
+        if (!animatedIn) {
-                duration = ANIMATION_IN_DURATION,
+            logger.logAnimateInFailure()
-                includeMargins = true,
+            chipbarAnimator.forceDisplayView(view.getInnerView())
-                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) {
             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 =
+        val removed = chipbarAnimator.animateViewOut(innerView, onAnimationEnd)
-            ViewHierarchyAnimator.animateRemoval(
-                innerView,
-                ViewHierarchyAnimator.Hotspot.TOP,
-                Interpolators.EMPHASIZED_ACCELERATE,
-                ANIMATION_OUT_DURATION,
-                includeMargins = true,
-                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"

packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt

@@ -45,6 +45,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.ChipbarLogger
 import com.android.systemui.temporarydisplay.chipbar.SwipeChipbarAwayGestureHandler
@@ -145,6 +146,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
                 configurationController,
                 dumpManager,
                 powerManager,
+                ChipbarAnimator(),
                 falsingManager,
                 falsingCollector,
                 swipeHandler,

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 =
+    private fun ViewGroup.getChipTextView() = this.requireViewById<TextView>(R.id.text)
-        (this.requireViewById<TextView>(R.id.text)).text as String
+
+    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