Merge "Fix ranking of people notifications and peoplehub"

2019-12-05 18:16:07 +00:00
parent e880831db9 960f4cede3
commit 4aafce53ee
5 changed files with 61 additions and 101 deletions
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManager.kt
@@ -16,36 +16,28 @@

 package com.android.systemui.statusbar.notification.collection

-import android.app.Notification
 import android.app.NotificationManager.IMPORTANCE_DEFAULT
 import android.app.NotificationManager.IMPORTANCE_HIGH
 import android.app.NotificationManager.IMPORTANCE_LOW
 import android.app.NotificationManager.IMPORTANCE_MIN
-import android.app.Person
 import android.service.notification.NotificationListenerService.Ranking
 import android.service.notification.NotificationListenerService.RankingMap
 import android.service.notification.StatusBarNotification
 import com.android.internal.annotations.VisibleForTesting
-
 import com.android.systemui.statusbar.NotificationMediaManager
 import com.android.systemui.statusbar.notification.NotificationFilter
 import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager
 import com.android.systemui.statusbar.notification.logging.NotifEvent
 import com.android.systemui.statusbar.notification.logging.NotifLog
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_ALERTING
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_PEOPLE
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_SILENT
 import com.android.systemui.statusbar.phone.NotificationGroupManager
 import com.android.systemui.statusbar.policy.HeadsUpManager
-
-import java.util.Objects
-import java.util.ArrayList
-
-import javax.inject.Inject
-
-import kotlin.Comparator
-
 import dagger.Lazy
+import java.util.Objects
+import javax.inject.Inject

 private const val TAG = "NotifRankingManager"

@@ -64,7 +56,8 @@ open class NotificationRankingManager @Inject constructor(
    private val headsUpManager: HeadsUpManager,
    private val notifFilter: NotificationFilter,
    private val notifLog: NotifLog,
-    sectionsFeatureManager: NotificationSectionsFeatureManager
+    sectionsFeatureManager: NotificationSectionsFeatureManager,
+    private val peopleNotificationIdentifier: PeopleNotificationIdentifier
 ) {

    var rankingMap: RankingMap? = null
@@ -79,6 +72,9 @@ open class NotificationRankingManager @Inject constructor(
        val aRank = a.ranking.rank
        val bRank = b.ranking.rank

+        val aIsPeople = a.isPeopleNotification()
+        val bIsPeople = b.isPeopleNotification()
+
        val aMedia = isImportantMedia(a)
        val bMedia = isImportantMedia(b)

@@ -88,25 +84,19 @@ open class NotificationRankingManager @Inject constructor(
        val aHeadsUp = a.isRowHeadsUp
        val bHeadsUp = b.isRowHeadsUp

-        if (usePeopleFiltering && a.isPeopleNotification() != b.isPeopleNotification()) {
-            if (a.isPeopleNotification()) -1 else 1
-        } else if (aHeadsUp != bHeadsUp) {
-            if (aHeadsUp) -1 else 1
-        } else if (aHeadsUp) {
+        when {
+            usePeopleFiltering && aIsPeople != bIsPeople -> if (aIsPeople) -1 else 1
+            aHeadsUp != bHeadsUp -> if (aHeadsUp) -1 else 1
            // Provide consistent ranking with headsUpManager
-            headsUpManager.compare(a, b)
-        } else if (aMedia != bMedia) {
+            aHeadsUp -> headsUpManager.compare(a, b)
            // Upsort current media notification.
-            if (aMedia) -1 else 1
-        } else if (aSystemMax != bSystemMax) {
+            aMedia != bMedia -> if (aMedia) -1 else 1
            // Upsort PRIORITY_MAX system notifications
-            if (aSystemMax) -1 else 1
-        } else if (a.isHighPriority != b.isHighPriority) {
-            -1 * java.lang.Boolean.compare(a.isHighPriority, b.isHighPriority)
-        } else if (aRank != bRank) {
-            aRank - bRank
-        } else {
-            nb.notification.`when`.compareTo(na.notification.`when`)
+            aSystemMax != bSystemMax -> if (aSystemMax) -1 else 1
+            a.isHighPriority != b.isHighPriority ->
+                -1 * a.isHighPriority.compareTo(b.isHighPriority)
+            aRank != bRank -> aRank - bRank
+            else -> nb.notification.`when`.compareTo(na.notification.`when`)
        }
    }

@@ -138,10 +128,9 @@ open class NotificationRankingManager @Inject constructor(
        val c = entry.channel
        val n = entry.sbn.notification

-        if (((n.isForegroundService && entry.ranking.importance >= IMPORTANCE_LOW) ||
-            n.hasMediaSession() ||
-            n.hasPerson() ||
-            n.hasStyle(Notification.MessagingStyle::class.java))) {
+        if ((n.isForegroundService && entry.ranking.importance >= IMPORTANCE_LOW) ||
+                n.hasMediaSession() ||
+                entry.isPeopleNotification()) {
            // Users who have long pressed and demoted to silent should not see the notification
            // in the top section
            if (c != null && c.hasUserSetImportance()) {
@@ -204,7 +193,7 @@ open class NotificationRankingManager @Inject constructor(
        isMedia: Boolean,
        isSystemMax: Boolean
    ) {
-        if (usePeopleFiltering && entry.hasAssociatedPeople()) {
+        if (usePeopleFiltering && entry.isPeopleNotification()) {
            entry.bucket = BUCKET_PEOPLE
        } else if (isHeadsUp || isMedia || isSystemMax || entry.isHighPriority) {
            entry.bucket = BUCKET_ALERTING
@@ -235,6 +224,11 @@ open class NotificationRankingManager @Inject constructor(
            }
        }
    }
+
+    private fun NotificationEntry.isPeopleNotification() =
+            sbn.isPeopleNotification()
+    private fun StatusBarNotification.isPeopleNotification() =
+            peopleNotificationIdentifier.isPeopleNotification(this)
 }

 // Convenience functions
@@ -245,16 +239,3 @@ private fun NotificationEntry.isSystemMax(): Boolean {
 private fun StatusBarNotification.isSystemNotification(): Boolean {
    return "android" == packageName || "com.android.systemui" == packageName
 }
-
-private fun Notification.hasPerson(): Boolean {
-    val people: ArrayList<Person> =
-            (extras?.getParcelableArrayList(Notification.EXTRA_PEOPLE_LIST)) ?: ArrayList()
-    return people.isNotEmpty()
-}
-
-private fun Notification.hasStyle(targetStyleClass: Class<*>): Boolean {
-    return targetStyleClass == notificationStyle
-}
-
-private fun NotificationEntry.isPeopleNotification(): Boolean =
-        sbn.notification.hasStyle(Notification.MessagingStyle::class.java)
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java
@@ -318,7 +318,7 @@ public class NotificationSectionsManager implements StackScrollAlgorithm.Section
                }
                mParent.addView(mPeopleHubView, targetIndex);
                return true;
-            } else if (currentHubIndex != targetIndex - 1) {
+            } else if (currentHubIndex != targetIndex) {
                if (currentHubIndex < targetIndex) {
                    targetIndex--;
                }
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/NotificationEntryManagerTest.java
@@ -85,6 +85,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationRowBin
 import com.android.systemui.statusbar.notification.collection.NotificationRowBinderImpl;
 import com.android.systemui.statusbar.notification.logging.NotifLog;
 import com.android.systemui.statusbar.notification.logging.NotificationLogger;
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
 import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
 import com.android.systemui.statusbar.notification.row.NotificationGutsManager;
 import com.android.systemui.statusbar.notification.row.RowInflaterTask;
@@ -228,7 +229,8 @@ public class NotificationEntryManagerTest extends SysuiTestCase {
                        mHeadsUpManager,
                        mock(NotificationFilter.class),
                        mNotifLog,
-                        mock(NotificationSectionsFeatureManager.class)),
+                        mock(NotificationSectionsFeatureManager.class),
+                        mock(PeopleNotificationIdentifier.class)),
                mEnvironment
        );
        Dependency.get(InitController.class).executePostInitTasks();
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/NotificationRankingManagerTest.kt
@@ -20,7 +20,6 @@ import android.app.Notification
 import android.app.NotificationChannel
 import android.app.NotificationManager.IMPORTANCE_DEFAULT
 import android.app.NotificationManager.IMPORTANCE_LOW
-import android.app.Person
 import android.service.notification.NotificationListenerService.RankingMap
 import android.service.notification.StatusBarNotification
 import android.testing.AndroidTestingRunner
@@ -36,6 +35,7 @@ import com.android.systemui.statusbar.NotificationMediaManager
 import com.android.systemui.statusbar.notification.NotificationFilter
 import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager
 import com.android.systemui.statusbar.notification.logging.NotifLog
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_ALERTING
 import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_SILENT
 import com.android.systemui.statusbar.phone.NotificationGroupManager
@@ -52,42 +52,39 @@ import org.mockito.Mockito.mock

@SmallTest
@RunWith(AndroidTestingRunner::class)
-class NotificationRankingManagerTest
-    : SysuiTestCase() {
+class NotificationRankingManagerTest : SysuiTestCase() {

-    private var lazyMedia: Lazy<NotificationMediaManager> = Lazy {
-        mock<NotificationMediaManager>(NotificationMediaManager::class.java)
+    private val lazyMedia: Lazy<NotificationMediaManager> = Lazy {
+        mock(NotificationMediaManager::class.java)
    }
-
-    private val rankingManager = TestableNotificationRankingManager(
-            lazyMedia,
-            mock<NotificationGroupManager>(NotificationGroupManager::class.java),
-            mock<HeadsUpManager>(HeadsUpManager::class.java),
-            mock<NotificationFilter>(NotificationFilter::class.java),
-            mock<NotifLog>(NotifLog::class.java),
-            mock<NotificationSectionsFeatureManager>(NotificationSectionsFeatureManager::class.java)
-    )
+    private lateinit var personNotificationIdentifier: PeopleNotificationIdentifier
+    private lateinit var rankingManager: TestableNotificationRankingManager

    @Before
    fun setup() {
+        personNotificationIdentifier =
+                mock(PeopleNotificationIdentifier::class.java)
+        rankingManager = TestableNotificationRankingManager(
+                lazyMedia,
+                mock(NotificationGroupManager::class.java),
+                mock(HeadsUpManager::class.java),
+                mock(NotificationFilter::class.java),
+                mock(NotifLog::class.java),
+                mock(NotificationSectionsFeatureManager::class.java),
+                personNotificationIdentifier
+        )
    }

    @Test
    fun testPeopleNotification_isHighPriority() {
-        val person = Person.Builder()
-                .setName("name")
-                .setKey("abc")
-                .setUri("uri")
-                .setBot(true)
-                .build()
-
        val notification = Notification.Builder(mContext, "test")
-                .addPerson(person)
                .build()

        val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
                notification, mContext.user, "", 0)

+        `when`(personNotificationIdentifier.isPeopleNotification(sbn)).thenReturn(true)
+
        val e = NotificationEntryBuilder()
                .setNotification(notification)
                .setSbn(sbn)
@@ -96,28 +93,10 @@ class NotificationRankingManagerTest
        assertTrue(rankingManager.isHighPriority2(e))
    }

-    @Test
-    fun messagingStyleHighPriority() {
-
-        val notif = Notification.Builder(mContext, "test")
-                .setStyle(Notification.MessagingStyle(""))
-                .build()
-
-        val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
-                notif, mContext.getUser(), "", 0)
-
-        val e = NotificationEntryBuilder()
-                .setNotification(notif)
-                .setSbn(sbn)
-                .build()
-
-        assertTrue(rankingManager.isHighPriority2(e))
-    }
-
    @Test
    fun lowForegroundHighPriority() {
        val notification = mock(Notification::class.java)
-        `when`<Boolean>(notification.isForegroundService).thenReturn(true)
+        `when`(notification.isForegroundService).thenReturn(true)

        val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
                notification, mContext.user, "", 0)
@@ -136,15 +115,7 @@ class NotificationRankingManagerTest

    @Test
    fun userChangeTrumpsHighPriorityCharacteristics() {
-        val person = Person.Builder()
-                .setName("name")
-                .setKey("abc")
-                .setUri("uri")
-                .setBot(true)
-                .build()
-
        val notification = Notification.Builder(mContext, "test")
-                .addPerson(person)
                .setStyle(Notification.MessagingStyle(""))
                .setFlag(Notification.FLAG_FOREGROUND_SERVICE, true)
                .build()
@@ -152,6 +123,8 @@ class NotificationRankingManagerTest
        val sbn = StatusBarNotification("pkg", "pkg", 0, "tag", 0, 0,
                notification, mContext.user, "", 0)

+        `when`(personNotificationIdentifier.isPeopleNotification(sbn)).thenReturn(true)
+
        val channel = NotificationChannel("a", "a", IMPORTANCE_LOW)
        channel.lockFields(NotificationChannel.USER_LOCKED_IMPORTANCE)

@@ -297,14 +270,16 @@ class NotificationRankingManagerTest
        headsUpManager: HeadsUpManager,
        filter: NotificationFilter,
        notifLog: NotifLog,
-        sectionsFeatureManager: NotificationSectionsFeatureManager
+        sectionsFeatureManager: NotificationSectionsFeatureManager,
+        peopleNotificationIdentifier: PeopleNotificationIdentifier
    ) : NotificationRankingManager(
        mediaManager,
        groupManager,
        headsUpManager,
        filter,
        notifLog,
-        sectionsFeatureManager
+        sectionsFeatureManager,
+        peopleNotificationIdentifier
    ) {

        fun isHighPriority2(e: NotificationEntry): Boolean {
@@ -315,4 +290,4 @@ class NotificationRankingManagerTest
            rankingMap = r
        }
    }
-}
+}
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java
@@ -74,6 +74,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry;
 import com.android.systemui.statusbar.notification.collection.NotificationRankingManager;
 import com.android.systemui.statusbar.notification.logging.NotifLog;
 import com.android.systemui.statusbar.notification.people.PeopleHubSectionFooterViewAdapter;
+import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier;
 import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
 import com.android.systemui.statusbar.notification.row.FooterView;
 import com.android.systemui.statusbar.notification.row.NotificationBlockingHelperManager;
@@ -164,7 +165,8 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase {
                        mHeadsUpManager,
                        mock(NotificationFilter.class),
                        mock(NotifLog.class),
-                        mock(NotificationSectionsFeatureManager.class)
+                        mock(NotificationSectionsFeatureManager.class),
+                        mock(PeopleNotificationIdentifier.class)
                ),
                mock(NotificationEntryManager.KeyguardEnvironment.class));
        mDependency.injectTestDependency(NotificationEntryManager.class, mEntryManager);