Merge "Fix a newer race condition in QSIconViewImpl" into main

MinatiScape · Feb 5, 2024 · d00dead · d00dead
2 parents 94c6fc7 + 0942153
commit d00dead
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 5 deletions.
diff --git 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 class QSIconViewImpl extends QSIconView {
 
     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 @@ public class QSIconViewImpl extends QSIconView {
     @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 @@ public void setIcon(State state, boolean allowAnimations) {
     }
 
     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 @@ protected void setIcon(ImageView iv, QSTile.State state, boolean allowAnimations
             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 @@ protected final void layout(View child, int left, int top) {
         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
@@ -34,7 +34,7 @@ import kotlin.test.Test
 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 @@ class QSIconViewImplTest_311121830 : SysuiTestCase() {
         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) {