Fix issue with media notifs being misbucketed

Previous code assumed that "isHighPriority" == "is in top section",
which is not always true. Media notifs and some other notifs can appear
in the top section even if they're not high priority. Because we detect
section boundaries by iterating through the list until we find the first
notif where isHighPriority == false, we were sometimes drawing the
section boundary way too high. This change creates a new propery,
isInTopSection() that accurately tracks this state.

Setting this value in the proper location would require some seriously
destabilizing refactors, so instead we set it in the list comparator,
which is awful but here are.

Test: manual
Bug: 138320173
Change-Id: I19223720bac534ab92219a2962169097819e8efb
Merged-In: I19223720bac534ab92219a2962169097819e8efb
(cherry picked from commit 8c1b763dcf5df6e7fc7177dad5e7390adb229fa4)
(cherry picked from commit 91e425d793495ab4ea496ecdb80a6e7d33e28a8e)

10 files changed, 44 insertions, 25 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java
index 0fe5f8a6af5a..4cc5b2144adc 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManager.java
@@ -15,7 +15,6 @@
 package com.android.systemui.statusbar;
 
 import android.content.pm.UserInfo;
-import android.service.notification.StatusBarNotification;
 import android.util.SparseArray;
 
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -58,7 +57,7 @@ public interface NotificationLockscreenUserManager {
 
     boolean shouldHideNotifications(int userId);
     boolean shouldHideNotifications(String key);
-    boolean shouldShowOnKeyguard(StatusBarNotification sbn);
+    boolean shouldShowOnKeyguard(NotificationEntry entry);
 
     boolean isAnyProfilePublicMode();
 
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java
index 4ea1ed5b9451..e08a5ae07bd8 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerImpl.java
@@ -33,7 +33,6 @@ import android.os.ServiceManager;
 import android.os.UserHandle;
 import android.os.UserManager;
 import android.provider.Settings;
-import android.service.notification.StatusBarNotification;
 import android.util.Log;
 import android.util.SparseArray;
 import android.util.SparseBooleanArray;
@@ -302,7 +301,7 @@ public class NotificationLockscreenUserManagerImpl implements
                         Notification.VISIBILITY_SECRET;
     }
 
-    public boolean shouldShowOnKeyguard(StatusBarNotification sbn) {
+    public boolean shouldShowOnKeyguard(NotificationEntry entry) {
         if (getEntryManager() == null) {
             Log.wtf(TAG, "mEntryManager was null!", new Throwable());
             return false;
@@ -310,10 +309,10 @@ public class NotificationLockscreenUserManagerImpl implements
         boolean exceedsPriorityThreshold;
         if (NotificationUtils.useNewInterruptionModel(mContext)
                 && hideSilentNotificationsOnLockscreen()) {
-            exceedsPriorityThreshold = getEntryManager().getNotificationData().isHighPriority(sbn);
+            exceedsPriorityThreshold = entry.isTopBucket();
         } else {
             exceedsPriorityThreshold =
-                    !getEntryManager().getNotificationData().isAmbient(sbn.getKey());
+                    !getEntryManager().getNotificationData().isAmbient(entry.key);
         }
         return mShowLockscreenNotifications && exceedsPriorityThreshold;
     }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java
index 404920187351..9c3ee96fe25b 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java
@@ -394,15 +394,13 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
             int userId = entry.notification.getUserId();
             boolean suppressedSummary = mGroupManager.isSummaryOfSuppressedGroup(
                     entry.notification) && !entry.isRowRemoved();
-            boolean showOnKeyguard = mLockscreenUserManager.shouldShowOnKeyguard(entry
-                    .notification);
+            boolean showOnKeyguard = mLockscreenUserManager.shouldShowOnKeyguard(entry);
             if (!showOnKeyguard) {
                 // min priority notifications should show if their summary is showing
                 if (mGroupManager.isChildInGroupWithSummary(entry.notification)) {
                     NotificationEntry summary = mGroupManager.getLogicalGroupSummary(
                             entry.notification);
-                    if (summary != null && mLockscreenUserManager.shouldShowOnKeyguard(
-                            summary.notification))         {
+                    if (summary != null && mLockscreenUserManager.shouldShowOnKeyguard(summary)) {
                         showOnKeyguard = true;
                     }
                 }
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 64b2f048ce2e..13f8f1a6a548 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
@@ -25,7 +25,6 @@ import android.service.notification.NotificationListenerService.RankingMap;
 import android.service.notification.SnoozeCriterion;
 import android.service.notification.StatusBarNotification;
 import android.util.ArrayMap;
-import android.util.Slog;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.systemui.Dependency;
@@ -108,10 +107,19 @@ public class NotificationData {
             boolean bSystemMax = bImportance >= NotificationManager.IMPORTANCE_HIGH
                     && isSystemNotification(nb);
 
-            boolean isHeadsUp = a.getRow().isHeadsUp();
-            if (isHeadsUp != b.getRow().isHeadsUp()) {
-                return isHeadsUp ? -1 : 1;
-            } else if (isHeadsUp) {
+
+            boolean aHeadsUp = a.getRow().isHeadsUp();
+            boolean bHeadsUp = b.getRow().isHeadsUp();
+
+            // HACK: This should really go elsewhere, but it's currently not straightforward to
+            // extract the comparison code and we're guaranteed to touch every element, so this is
+            // the best place to set the buckets for the moment.
+            a.setIsTopBucket(aHeadsUp || aMedia || aSystemMax || a.isHighPriority());
+            b.setIsTopBucket(bHeadsUp || bMedia || bSystemMax || b.isHighPriority());
+
+            if (aHeadsUp != bHeadsUp) {
+                return aHeadsUp ? -1 : 1;
+            } else if (aHeadsUp) {
                 // Provide consistent ranking with headsUpManager
                 return mHeadsUpManager.compare(a, b);
             } else if (a.getRow().showingAmbientPulsing() != b.getRow().showingAmbientPulsing()) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
index 92c261c4cad7..d157f06c03e9 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java
@@ -173,6 +173,8 @@ public final class NotificationEntry {
      */
     private boolean mHighPriority;
 
+    private boolean mIsTopBucket;
+
     public NotificationEntry(StatusBarNotification n) {
         this(n, null);
     }
@@ -220,6 +222,18 @@ public final class NotificationEntry {
         this.mHighPriority = highPriority;
     }
 
+    /**
+     * @return True if the notif should appear in the "top" or "important" section of notifications
+     * (as opposed to the "bottom" or "silent" section). This is usually the same as
+     * {@link #isHighPriority()}, but there are certain exceptions, such as media notifs.
+     */
+    public boolean isTopBucket() {
+        return mIsTopBucket;
+    }
+    public void setIsTopBucket(boolean isTopBucket) {
+        mIsTopBucket = isTopBucket;
+    }
+
     public boolean isBubble() {
         return (notification.getNotification().flags & FLAG_BUBBLE) != 0;
     }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
index 170a4d570688..d119fb79e4c6 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
@@ -133,7 +133,7 @@ class NotificationSectionsManager implements StackScrollAlgorithm.SectionProvide
             if (child instanceof ExpandableNotificationRow
                     && child.getVisibility() != View.GONE) {
                 ExpandableNotificationRow row = (ExpandableNotificationRow) child;
-                if (!row.getEntry().isHighPriority()) {
+                if (!row.getEntry().isTopBucket()) {
                     firstGentleNotifIndex = i;
                     mFirstGentleNotif = row;
                     break;
@@ -248,7 +248,7 @@ class NotificationSectionsManager implements StackScrollAlgorithm.SectionProvide
             View child = mParent.getChildAt(i);
             if (child.getVisibility() != View.GONE && child instanceof ExpandableNotificationRow) {
                 ExpandableNotificationRow row = (ExpandableNotificationRow) child;
-                if (!row.getEntry().isHighPriority()) {
+                if (!row.getEntry().isTopBucket()) {
                     break;
                 } else {
                     lastChildBeforeGap = row;
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
index 80e1ac10e319..09352cfd17a3 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java
@@ -5776,7 +5776,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd
             currentIndex++;
             boolean beforeSpeedBump;
             if (mHighPriorityBeforeSpeedBump) {
-                beforeSpeedBump = row.getEntry().isHighPriority();
+                beforeSpeedBump = row.getEntry().isTopBucket();
             } else {
                 beforeSpeedBump = !row.getEntry().ambient;
             }
@@ -5834,9 +5834,9 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd
             case ROWS_ALL:
                 return true;
             case ROWS_HIGH_PRIORITY:
-                return row.getEntry().isHighPriority();
+                return row.getEntry().isTopBucket();
             case ROWS_GENTLE:
-                return !row.getEntry().isHighPriority();
+                return !row.getEntry().isTopBucket();
             default:
                 throw new IllegalArgumentException("Unknown selection: " + selection);
         }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelView.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelView.java
index a423adfd34b7..bc205d676914 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelView.java
@@ -734,8 +734,7 @@ public class NotificationPanelView extends PanelView implements
             if (suppressedSummary) {
                 continue;
             }
-            if (!mLockscreenUserManager.shouldShowOnKeyguard(
-                    row.getStatusBarNotification())) {
+            if (!mLockscreenUserManager.shouldShowOnKeyguard(row.getEntry())) {
                 continue;
             }
             if (row.isRemoved()) {
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java
index 49a263a8d781..57dd8c94c790 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationLockscreenUserManagerTest.java
@@ -38,7 +38,6 @@ import android.os.Handler;
 import android.os.Looper;
 import android.os.UserManager;
 import android.provider.Settings;
-import android.service.notification.StatusBarNotification;
 import android.testing.AndroidTestingRunner;
 import android.testing.TestableLooper;
 
@@ -48,6 +47,7 @@ import com.android.systemui.Dependency;
 import com.android.systemui.SysuiTestCase;
 import com.android.systemui.statusbar.notification.NotificationEntryManager;
 import com.android.systemui.statusbar.notification.collection.NotificationData;
+import com.android.systemui.statusbar.notification.collection.NotificationEntry;
 import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager;
 import com.android.systemui.statusbar.policy.DeviceProvisionedController;
 
@@ -166,7 +166,7 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase {
                 Settings.Secure.LOCK_SCREEN_SHOW_SILENT_NOTIFICATIONS, 1);
         when(mNotificationData.isHighPriority(any())).thenReturn(false);
 
-        assertTrue(mLockscreenUserManager.shouldShowOnKeyguard(mock(StatusBarNotification.class)));
+        assertTrue(mLockscreenUserManager.shouldShowOnKeyguard(mock(NotificationEntry.class)));
     }
 
     @Test
@@ -179,7 +179,7 @@ public class NotificationLockscreenUserManagerTest extends SysuiTestCase {
                 Settings.Secure.LOCK_SCREEN_SHOW_SILENT_NOTIFICATIONS, 0);
         when(mNotificationData.isHighPriority(any())).thenReturn(false);
 
-        assertFalse(mLockscreenUserManager.shouldShowOnKeyguard(mock(StatusBarNotification.class)));
+        assertFalse(mLockscreenUserManager.shouldShowOnKeyguard(mock(NotificationEntry.class)));
     }
 
     private class TestNotificationLockscreenUserManager
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java
index 73abda9a5da7..59d0f912d38e 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java
@@ -263,6 +263,8 @@ public class NotificationSectionsManagerTest extends SysuiTestCase {
                     when(notifRow.getVisibility()).thenReturn(View.VISIBLE);
                     when(notifRow.getEntry().isHighPriority())
                             .thenReturn(children[i] == ChildType.HIPRI);
+                    when(notifRow.getEntry().isTopBucket())
+                            .thenReturn(children[i] == ChildType.HIPRI);
                     when(notifRow.getParent()).thenReturn(mNssl);
                     child = notifRow;
                     break;