From 2d35980e7201e042e253fadd0eb55e1866fc28d9 Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Thu, 8 Aug 2019 13:43:57 -0400 Subject: [PATCH] Ensure isTopBucket() gets set when there is just one notification My descendants will vilify this CL for generations to come. We'll clean it up for R, but this is our last, best hope for fixing things in Q. Bug: 138775282 Test: manual Change-Id: I615b2f7fddca30dae67dbaab0e5d54a824a4c441 --- .../collection/NotificationData.java | 9 ++- .../collection/NotificationDataTest.java | 66 ++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationData.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationData.java index 1ce493444e25b..00092929fd49c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationData.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationData.java @@ -420,7 +420,14 @@ public class NotificationData { } } - Collections.sort(mSortedAndFiltered, mRankingComparator); + if (mSortedAndFiltered.size() == 1) { + // HACK: We need the comparator to run on all children in order to set the + // isHighPriority field. If there is only one child, then the comparison won't be run, + // so we have to trigger it manually. Get rid of this code as soon as possible. + mRankingComparator.compare(mSortedAndFiltered.get(0), mSortedAndFiltered.get(0)); + } else { + Collections.sort(mSortedAndFiltered, mRankingComparator); + } } public void dump(PrintWriter pw, String indent) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java index 260555f712560..e2d8e5698daf3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationDataTest.java @@ -23,6 +23,7 @@ import static android.app.Notification.CATEGORY_CALL; import static android.app.Notification.CATEGORY_EVENT; import static android.app.Notification.CATEGORY_MESSAGE; import static android.app.Notification.CATEGORY_REMINDER; +import static android.app.NotificationManager.IMPORTANCE_DEFAULT; import static android.app.NotificationManager.IMPORTANCE_LOW; import static android.app.NotificationManager.IMPORTANCE_MIN; @@ -62,6 +63,8 @@ import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; import android.util.ArraySet; +import androidx.test.filters.SmallTest; + import com.android.systemui.Dependency; import com.android.systemui.ForegroundServiceController; import com.android.systemui.InitController; @@ -85,8 +88,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import androidx.test.filters.SmallTest; - @SmallTest @RunWith(AndroidTestingRunner.class) @RunWithLooper @@ -114,6 +115,7 @@ public class NotificationDataTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); when(mMockStatusBarNotification.getUid()).thenReturn(UID_NORMAL); when(mMockStatusBarNotification.cloneLight()).thenReturn(mMockStatusBarNotification); + when(mMockStatusBarNotification.getKey()).thenReturn("mock_key"); when(mMockPackageManager.checkUidPermission( eq(Manifest.permission.NOTIFICATION_DURING_SETUP), @@ -232,6 +234,7 @@ public class NotificationDataTest extends SysuiTestCase { Notification n = mMockStatusBarNotification.getNotification(); n.flags = Notification.FLAG_FOREGROUND_SERVICE; NotificationEntry entry = new NotificationEntry(mMockStatusBarNotification); + entry.setRow(mRow); mNotificationData.add(entry); Bundle override = new Bundle(); override.putInt(OVERRIDE_VIS_EFFECTS, 255); @@ -250,6 +253,7 @@ public class NotificationDataTest extends SysuiTestCase { n = nb.build(); when(mMockStatusBarNotification.getNotification()).thenReturn(n); NotificationEntry entry = new NotificationEntry(mMockStatusBarNotification); + entry.setRow(mRow); mNotificationData.add(entry); Bundle override = new Bundle(); override.putInt(OVERRIDE_VIS_EFFECTS, 255); @@ -263,6 +267,7 @@ public class NotificationDataTest extends SysuiTestCase { public void testIsExemptFromDndVisualSuppression_system() { initStatusBarNotification(false); NotificationEntry entry = new NotificationEntry(mMockStatusBarNotification); + entry.setRow(mRow); entry.mIsSystemNotification = true; mNotificationData.add(entry); Bundle override = new Bundle(); @@ -277,6 +282,7 @@ public class NotificationDataTest extends SysuiTestCase { public void testIsNotExemptFromDndVisualSuppression_hiddenCategories() { initStatusBarNotification(false); NotificationEntry entry = new NotificationEntry(mMockStatusBarNotification); + entry.setRow(mRow); entry.mIsSystemNotification = true; Bundle override = new Bundle(); override.putInt(OVERRIDE_VIS_EFFECTS, NotificationManager.Policy.SUPPRESSED_EFFECT_AMBIENT); @@ -529,6 +535,62 @@ public class NotificationDataTest extends SysuiTestCase { assertEquals(-1, mNotificationData.mRankingComparator.compare(a, b)); } + @Test + public void testSort_properlySetsIsTopBucket() { + + Notification notification = new Notification.Builder(mContext, "test") + .build(); + StatusBarNotification sbn = new StatusBarNotification( + "pkg", + "pkg", + 0, + "tag", + 0, + 0, + notification, + mContext.getUser(), + "", + 0); + + Bundle override = new Bundle(); + override.putInt(OVERRIDE_IMPORTANCE, IMPORTANCE_DEFAULT); + mNotificationData.rankingOverrides.put(sbn.getKey(), override); + + NotificationEntry entry = new NotificationEntry(sbn); + entry.setRow(mRow); + mNotificationData.add(entry); + + assertTrue(entry.isTopBucket()); + } + + @Test + public void testSort_properlySetsIsNotTopBucket() { + Notification notification = new Notification.Builder(mContext, "test") + .build(); + StatusBarNotification sbn = new StatusBarNotification( + "pkg", + "pkg", + 0, + "tag", + 0, + 0, + notification, + mContext.getUser(), + "", + 0); + + Bundle override = new Bundle(); + override.putInt(OVERRIDE_IMPORTANCE, IMPORTANCE_LOW); + mNotificationData.rankingOverrides.put(sbn.getKey(), override); + + NotificationEntry entry = new NotificationEntry(sbn); + entry.setRow(mRow); + + mNotificationData.add(entry); + + assertFalse(entry.isTopBucket()); + } + private void initStatusBarNotification(boolean allowDuringSetup) { Bundle bundle = new Bundle(); bundle.putBoolean(Notification.EXTRA_ALLOW_DURING_SETUP, allowDuringSetup);