[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
This commit is contained in:
Caitlin Shkuratov
2023-01-25 17:56:05 +00:00
parent ba0a06d8f5
commit 971d26fca3
5 changed files with 180 additions and 29 deletions

View File

@@ -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,

View File

@@ -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

View File

@@ -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"

View File

@@ -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,

View File

@@ -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