Enforce contigious but unordered buckets

Fixes: 204453772
Test: ShadeListBuilderTest NodeSpecBuilderTest
Change-Id: Ifd2eae4e521d59e1949cdf21a7096671a2b13f48

3 files changed, 48 insertions, 18 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java
index 1b5e52d7f8fb..df2fe4e8511f 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java
@@ -93,6 +93,7 @@ public class ShadeListBuilder implements Dumpable {
     private final NotificationInteractionTracker mInteractionTracker;
     private final DumpManager mDumpManager;
     // used exclusivly by ShadeListBuilder#notifySectionEntriesUpdated
+    // TODO replace temp with collection pool for readability
     private final ArrayList<ListEntry> mTempSectionMembers = new ArrayList<>();
     private final boolean mAlwaysLogList;
 
@@ -230,13 +231,7 @@ public class ShadeListBuilder implements Dumpable {
         mPipelineState.requireState(STATE_IDLE);
 
         mNotifSections.clear();
-        NotifSectioner lastSection = null;
         for (NotifSectioner sectioner : sectioners) {
-            if (lastSection != null && lastSection.getBucket() > sectioner.getBucket()) {
-                throw new IllegalArgumentException("setSectioners with non contiguous sections "
-                        + lastSection.getName() + " - " + lastSection.getBucket() + " & "
-                        + sectioner.getName() + " - " + sectioner.getBucket());
-            }
             final NotifSection section = new NotifSection(sectioner, mNotifSections.size());
             final NotifComparator sectionComparator = section.getComparator();
             mNotifSections.add(section);
@@ -244,10 +239,23 @@ public class ShadeListBuilder implements Dumpable {
             if (sectionComparator != null) {
                 sectionComparator.setInvalidationListener(this::onNotifComparatorInvalidated);
             }
-            lastSection = sectioner;
         }
 
         mNotifSections.add(new NotifSection(DEFAULT_SECTIONER, mNotifSections.size()));
+
+        // validate sections
+        final ArraySet<Integer> seenBuckets = new ArraySet<>();
+        int lastBucket = mNotifSections.size() > 0
+                ? mNotifSections.get(0).getBucket()
+                : 0;
+        for (NotifSection section : mNotifSections) {
+            if (lastBucket != section.getBucket() && seenBuckets.contains(section.getBucket())) {
+                throw new IllegalStateException("setSectioners with non contiguous sections "
+                        + section.getLabel() + " has an already seen bucket");
+            }
+            lastBucket = section.getBucket();
+            seenBuckets.add(lastBucket);
+        }
     }
 
     void setNotifStabilityManager(@NonNull NotifStabilityManager notifStabilityManager) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt
index 6db544c77f87..8be710c8842c 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NodeSpecBuilder.kt
@@ -57,7 +57,6 @@ class NodeSpecBuilder(
 
         var currentSection: NotifSection? = null
         val prevSections = mutableSetOf<NotifSection?>()
-        var lastSection: NotifSection? = null
         val showHeaders = sectionHeaderVisibilityProvider.sectionHeadersVisible
         val sectionOrder = mutableListOf<NotifSection?>()
         val sectionHeaders = mutableMapOf<NotifSection?, NodeController?>()
@@ -65,15 +64,6 @@ class NodeSpecBuilder(
 
         for (entry in notifList) {
             val section = entry.section!!
-
-            lastSection?.let {
-                if (it.bucket > section.bucket) {
-                    throw IllegalStateException("buildNodeSpec with non contiguous section " +
-                            "buckets ${it.sectioner.name} - ${it.bucket} & " +
-                            "${it.sectioner.name} - ${it.bucket}")
-                }
-            }
-            lastSection = section
             if (prevSections.contains(section)) {
                 throw java.lang.RuntimeException("Section ${section.label} has been duplicated")
             }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java
index 9ea1813377a0..4e7e79f2cb26 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java
@@ -1528,6 +1528,34 @@ public class ShadeListBuilderTest extends SysuiTestCase {
     }
 
     @Test
+    public void testContiguousSections() {
+        mListBuilder.setSectioners(List.of(
+                new PackageSectioner("pkg", 1),
+                new PackageSectioner("pkg", 1),
+                new PackageSectioner("pkg", 3),
+                new PackageSectioner("pkg", 2)
+        ));
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testNonContiguousSections() {
+        mListBuilder.setSectioners(List.of(
+                new PackageSectioner("pkg", 1),
+                new PackageSectioner("pkg", 1),
+                new PackageSectioner("pkg", 3),
+                new PackageSectioner("pkg", 1)
+        ));
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testBucketZeroNotAllowed() {
+        mListBuilder.setSectioners(List.of(
+                new PackageSectioner("pkg", 0),
+                new PackageSectioner("pkg", 1)
+        ));
+    }
+
+    @Test
     public void testStabilizeGroupsDelayedSummaryRendersAllNotifsTopLevel() {
         // GIVEN group children posted without a summary
         addGroupChild(0, PACKAGE_1, GROUP_1);
@@ -2189,7 +2217,11 @@ public class ShadeListBuilderTest extends SysuiTestCase {
         }
 
         PackageSectioner(String pkg) {
-            super("PackageSection_" + pkg, 0);
+            this(pkg, 0);
+        }
+
+        PackageSectioner(String pkg, int bucket) {
+            super("PackageSection_" + pkg, bucket);
             mPackages = List.of(pkg);
             mComparator = null;
         }