Fix dot/icon overlap in notification shelf

This bug was caused by an off-by-one error where
	isOverflowing = iconX > overflowX
should have been
	isOverflowing = iconX >= overflowX

As a result, the icon that should have been hidden
by the dot was considered "not overflowing"
(iconX=overflowX) so it remained visible.

This change corrects the definition of isOverflowing;
refactors isOverflowing and forceOverflow out of
calculateIconXTranslations; adds tests.

Bug: 232056690
Test: NotificationIconContainer

Test: add notifs to short shelf until overflow
      swipe notifs away until short shelf empty
      (no dot/icon overlap)

Test: swipe down on ls notif then let go
      swipe down on ls notif to go to full shade
      add notifs/swipe away notifs in full shade
      scroll in full shade with long shelf
      (no regressions)

Change-Id: Ie270c55f6c9d27b5d0fee73c22fedb5335760179

2 files changed, 125 insertions, 10 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java
index a2140c6ab6b7..7b8c5fc998fa 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java
@@ -423,6 +423,21 @@ public class NotificationIconContainer extends ViewGroup {
                 + getActualPaddingEnd();
     }
 
+    @VisibleForTesting
+    boolean shouldForceOverflow(int i, int speedBumpIndex, float iconAppearAmount,
+            int maxVisibleIcons) {
+        return speedBumpIndex != -1 && i >= speedBumpIndex
+                && iconAppearAmount > 0.0f || i >= maxVisibleIcons;
+    }
+
+    @VisibleForTesting
+    boolean isOverflowing(boolean isLastChild, float translationX, float layoutEnd,
+            float iconSize) {
+        // Layout end, as used here, does not include padding end.
+        final float overflowX = isLastChild ? layoutEnd : layoutEnd - iconSize;
+        return translationX >= overflowX;
+    }
+
     /**
      * Calculate the horizontal translations for each notification based on how much the icons
      * are inserted into the notification container.
@@ -448,26 +463,26 @@ public class NotificationIconContainer extends ViewGroup {
             if (mFirstVisibleIconState == null) {
                 mFirstVisibleIconState = iconState;
             }
-            boolean forceOverflow = mSpeedBumpIndex != -1 && i >= mSpeedBumpIndex
-                    && iconState.iconAppearAmount > 0.0f || i >= maxVisibleIcons;
-            boolean isLastChild = i == childCount - 1;
-            float drawingScale = mOnLockScreen && view instanceof StatusBarIconView
-                    ? ((StatusBarIconView) view).getIconScaleIncreased()
-                    : 1f;
             iconState.visibleState = iconState.hidden
                     ? StatusBarIconView.STATE_HIDDEN
                     : StatusBarIconView.STATE_ICON;
 
-            final float overflowDotX = layoutEnd - mIconSize;
-            boolean isOverflowing = translationX > overflowDotX;
+            final boolean forceOverflow = shouldForceOverflow(i, mSpeedBumpIndex,
+                    iconState.iconAppearAmount, maxVisibleIcons);
+            final boolean isOverflowing = forceOverflow || isOverflowing(
+                    /* isLastChild= */ i == childCount - 1, translationX, layoutEnd, mIconSize);
 
-            if (firstOverflowIndex == -1 && (forceOverflow || isOverflowing)) {
-                firstOverflowIndex = isLastChild && !forceOverflow ? i - 1 : i;
+            // First icon to overflow.
+            if (firstOverflowIndex == -1 && isOverflowing) {
+                firstOverflowIndex = i;
                 mVisualOverflowStart = layoutEnd - mIconSize;
                 if (forceOverflow || mIsStaticLayout) {
                     mVisualOverflowStart = Math.min(translationX, mVisualOverflowStart);
                 }
             }
+            final float drawingScale = mOnLockScreen && view instanceof StatusBarIconView
+                    ? ((StatusBarIconView) view).getIconScaleIncreased()
+                    : 1f;
             translationX += iconState.iconAppearAmount * view.getWidth() * drawingScale;
         }
         mNumDots = 0;
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt
index 2ff6dd43e84e..086e5df50f3f 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt
@@ -153,6 +153,106 @@ class NotificationIconContainerTest : SysuiTestCase() {
         assertTrue(iconContainer.hasOverflow())
     }
 
+    @Test
+    fun shouldForceOverflow_appearingAboveSpeedBump_true() {
+        val forceOverflow = iconContainer.shouldForceOverflow(
+                /* i= */ 1,
+                /* speedBumpIndex= */ 0,
+                /* iconAppearAmount= */ 1f,
+                /* maxVisibleIcons= */ 5
+        )
+        assertTrue(forceOverflow);
+    }
+
+    @Test
+    fun shouldForceOverflow_moreThanMaxVisible_true() {
+        val forceOverflow = iconContainer.shouldForceOverflow(
+                /* i= */ 10,
+                /* speedBumpIndex= */ 11,
+                /* iconAppearAmount= */ 0f,
+                /* maxVisibleIcons= */ 5
+        )
+        assertTrue(forceOverflow);
+    }
+
+    @Test
+    fun shouldForceOverflow_belowSpeedBumpAndLessThanMaxVisible_false() {
+        val forceOverflow = iconContainer.shouldForceOverflow(
+                /* i= */ 0,
+                /* speedBumpIndex= */ 11,
+                /* iconAppearAmount= */ 0f,
+                /* maxVisibleIcons= */ 5
+        )
+        assertFalse(forceOverflow);
+    }
+
+    @Test
+    fun isOverflowing_lastChildXLessThanLayoutEnd_false() {
+        val isOverflowing = iconContainer.isOverflowing(
+                /* isLastChild= */ true,
+                /* translationX= */ 0f,
+                /* layoutEnd= */ 10f,
+                /* iconSize= */ 2f,
+        )
+        assertFalse(isOverflowing)
+    }
+
+
+    @Test
+    fun isOverflowing_lastChildXEqualToLayoutEnd_true() {
+        val isOverflowing = iconContainer.isOverflowing(
+                /* isLastChild= */ true,
+                /* translationX= */ 10f,
+                /* layoutEnd= */ 10f,
+                /* iconSize= */ 2f,
+        )
+        assertTrue(isOverflowing)
+    }
+
+    @Test
+    fun isOverflowing_lastChildXGreaterThanLayoutEnd_true() {
+        val isOverflowing = iconContainer.isOverflowing(
+                /* isLastChild= */ true,
+                /* translationX= */ 20f,
+                /* layoutEnd= */ 10f,
+                /* iconSize= */ 2f,
+        )
+        assertTrue(isOverflowing)
+    }
+
+    @Test
+    fun isOverflowing_notLastChildXLessThanDotX_false() {
+        val isOverflowing = iconContainer.isOverflowing(
+                /* isLastChild= */ false,
+                /* translationX= */ 0f,
+                /* layoutEnd= */ 10f,
+                /* iconSize= */ 2f,
+        )
+        assertFalse(isOverflowing)
+    }
+
+    @Test
+    fun isOverflowing_notLastChildXGreaterThanDotX_true() {
+        val isOverflowing = iconContainer.isOverflowing(
+                /* isLastChild= */ false,
+                /* translationX= */ 20f,
+                /* layoutEnd= */ 10f,
+                /* iconSize= */ 2f,
+        )
+        assertTrue(isOverflowing)
+    }
+
+    @Test
+    fun isOverflowing_notLastChildXEqualToDotX_true() {
+        val isOverflowing = iconContainer.isOverflowing(
+                /* isLastChild= */ false,
+                /* translationX= */ 8f,
+                /* layoutEnd= */ 10f,
+                /* iconSize= */ 2f,
+        )
+        assertTrue(isOverflowing)
+    }
+
     private fun mockStatusBarIcon() : StatusBarIconView {
         val iconView = mock(StatusBarIconView::class.java)
         whenever(iconView.width).thenReturn(10)