Fix a newer race condition in QSIconViewImpl
If multiple changes of state are pushed into QSIconViewImpl, they will
all schedule an icon change, and cancel the last animator, prompting the
previous icon to be displayed. However, because setting an icon would
prevent other scheduled icons to be applied, only the first one was
applied.
With this fix, we assign a transaction ID (long) to each icon that is
scheduled to be applied. We only apply it at the end (or cancel) of the
animation if no other icon has been applied or scheduled since.
Test: atest QSIconViewImplTest_311121830
Fixes: 323125376
Flag: NONE
Change-Id: I8fd5eef54b3bee034f8066bc5ce79117c6260008
diff --git a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSIconViewImpl.java b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSIconViewImpl.java
index 53f287b..720120b 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSIconViewImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSIconViewImpl.java
@@ -47,6 +47,8 @@
public static final long QS_ANIM_LENGTH = 350;
+ private static final long ICON_APPLIED_TRANSACTION_ID = -1;
+
protected final View mIcon;
protected int mIconSizePx;
private boolean mAnimationEnabled = true;
@@ -57,7 +59,8 @@
@VisibleForTesting
QSTile.Icon mLastIcon;
- private boolean mIconChangeScheduled;
+ private long mScheduledIconChangeTransactionId = ICON_APPLIED_TRANSACTION_ID;
+ private long mHighestScheduledIconChangeTransactionId = ICON_APPLIED_TRANSACTION_ID;
private ValueAnimator mColorAnimator = new ValueAnimator();
@@ -117,7 +120,7 @@
}
protected void updateIcon(ImageView iv, State state, boolean allowAnimations) {
- mIconChangeScheduled = false;
+ mScheduledIconChangeTransactionId = ICON_APPLIED_TRANSACTION_ID;
final QSTile.Icon icon = state.iconSupplier != null ? state.iconSupplier.get() : state.icon;
if (!Objects.equals(icon, iv.getTag(R.id.qs_icon_tag))) {
boolean shouldAnimate = allowAnimations && shouldAnimate(iv);
@@ -173,9 +176,10 @@
mState = state.state;
mDisabledByPolicy = state.disabledByPolicy;
if (mTint != 0 && allowAnimations && shouldAnimate(iv)) {
- mIconChangeScheduled = true;
+ final long iconTransactionId = getNextIconTransactionId();
+ mScheduledIconChangeTransactionId = iconTransactionId;
animateGrayScale(mTint, color, iv, () -> {
- if (mIconChangeScheduled) {
+ if (mScheduledIconChangeTransactionId == iconTransactionId) {
updateIcon(iv, state, allowAnimations);
}
});
@@ -237,6 +241,11 @@
child.layout(left, top, left + child.getMeasuredWidth(), top + child.getMeasuredHeight());
}
+ private long getNextIconTransactionId() {
+ mHighestScheduledIconChangeTransactionId++;
+ return mHighestScheduledIconChangeTransactionId;
+ }
+
/**
* Color to tint the tile icon based on state
*/
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSIconViewImplTest_311121830.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSIconViewImplTest_311121830.kt
index e8aa8f0..bbae0c9 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSIconViewImplTest_311121830.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSIconViewImplTest_311121830.kt
@@ -34,7 +34,7 @@
import org.junit.Rule
import org.junit.runner.RunWith
-/** Test for regression b/311121830 */
+/** Test for regression b/311121830 and b/323125376 */
@RunWith(AndroidTestingRunner::class)
@UiThreadTest
@SmallTest
@@ -82,6 +82,55 @@
assertThat(iconView.mLastIcon).isEqualTo(secondState.icon)
}
+ @Test
+ fun alwaysLastIcon_twoStateChanges() {
+ // Need to inflate with the correct theme so the colors can be retrieved and the animations
+ // are run
+ val iconView =
+ AnimateQSIconViewImpl(
+ ContextThemeWrapper(context, R.style.Theme_SystemUI_QuickSettings)
+ )
+
+ val initialState =
+ QSTile.State().apply {
+ state = Tile.STATE_ACTIVE
+ icon = QSTileImpl.ResourceIcon.get(WifiIcons.WIFI_FULL_ICONS[4])
+ }
+ val firstState =
+ QSTile.State().apply {
+ state = Tile.STATE_INACTIVE
+ icon = QSTileImpl.ResourceIcon.get(WifiIcons.WIFI_NO_INTERNET_ICONS[4])
+ }
+ val secondState =
+ QSTile.State().apply {
+ state = Tile.STATE_ACTIVE
+ icon = QSTileImpl.ResourceIcon.get(WifiIcons.WIFI_FULL_ICONS[3])
+ }
+ val thirdState =
+ QSTile.State().apply {
+ state = Tile.STATE_INACTIVE
+ icon = QSTileImpl.ResourceIcon.get(WifiIcons.WIFI_NO_NETWORK)
+ }
+
+ // Start with the initial state
+ iconView.setIcon(initialState, /* allowAnimations= */ false)
+
+ // Set the first state to animate, and advance time to one third of the animation
+ iconView.setIcon(firstState, /* allowAnimations= */ true)
+ animatorRule.advanceTimeBy(QSIconViewImpl.QS_ANIM_LENGTH / 3)
+
+ // Set the second state to animate and advance time by another third of animations length
+ iconView.setIcon(secondState, /* allowAnimations= */ true)
+ animatorRule.advanceTimeBy(QSIconViewImpl.QS_ANIM_LENGTH / 3)
+
+ // Set the third state to animate and advance time by two times the animation length
+ // to guarantee that all animations are done
+ iconView.setIcon(thirdState, /* allowAnimations= */ true)
+ animatorRule.advanceTimeBy(QSIconViewImpl.QS_ANIM_LENGTH * 2)
+
+ assertThat(iconView.mLastIcon).isEqualTo(thirdState.icon)
+ }
+
private class AnimateQSIconViewImpl(context: Context) : QSIconViewImpl(context) {
override fun createIcon(): View {
return object : ImageView(context) {