Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

New Pipeline: Implement per-section comparators; sort HUNs

Browse Source 
Fixes: 217188002
Test: atest HeadsUpManagerTest ShadeListBuilderTest
Change-Id: I39d4e4c27e5c65d175726fa6c623982b9c38c9c1
This commit is contained in:
Jeff DeCew
2022-01-31 17:10:22 +00:00
parent a9f0101e33
commit 92c826152d
10 changed files with 186 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java
View File

@@ -226,8 +226,13 @@ public class ShadeListBuilder implements Dumpable {
mNotifSections.clear();
for (NotifSectioner sectioner : sectioners) {
mNotifSections.add(new NotifSection(sectioner, mNotifSections.size()));
final NotifSection section = new NotifSection(sectioner, mNotifSections.size());
final NotifComparator sectionComparator = section.getComparator();
mNotifSections.add(section);
sectioner.setInvalidationListener(this::onNotifSectionInvalidated);
if (sectionComparator != null) {
sectionComparator.setInvalidationListener(this::onNotifComparatorInvalidated);
}
}
mNotifSections.add(new NotifSection(DEFAULT_SECTIONER, mNotifSections.size()));
@@ -1089,7 +1094,12 @@ public class ShadeListBuilder implements Dumpable {
callOnCleanup(mNotifComparators);
for (int i = 0; i < mNotifSections.size(); i++) {
mNotifSections.get(i).getSectioner().onCleanup();
final NotifSection notifSection = mNotifSections.get(i);
notifSection.getSectioner().onCleanup();
final NotifComparator comparator = notifSection.getComparator();
if (comparator != null) {
comparator.onCleanup();
}
}
callOnCleanup(List.of(getStabilityManager()));
@@ -1101,6 +1111,19 @@ public class ShadeListBuilder implements Dumpable {
}
}
@Nullable
private NotifComparator getSectionComparator(
@NonNull ListEntry o1, @NonNull ListEntry o2) {
final NotifSection section = o1.getSection();
if (section != o2.getSection()) {
throw new RuntimeException("Entry ordering should only be done within sections");
}
if (section != null) {
return section.getComparator();
}
return null;
}
private final Comparator<ListEntry> mTopLevelComparator = (o1, o2) -> {
int cmp = Integer.compare(
o1.getSectionIndex(),
@@ -1112,6 +1135,12 @@ public class ShadeListBuilder implements Dumpable {
cmp = Integer.compare(index1, index2);
if (cmp != 0) return cmp;
NotifComparator sectionComparator = getSectionComparator(o1, o2);
if (sectionComparator != null) {
cmp = sectionComparator.compare(o1, o2);
if (cmp != 0) return cmp;
}
for (int i = 0; i < mNotifComparators.size(); i++) {
cmp = mNotifComparators.get(i).compare(o1, o2);
if (cmp != 0) return cmp;

21
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinator.kt
View File

@@ -51,23 +51,20 @@ class ConversationCoordinator @Inject constructor(
val sectioner = object : NotifSectioner("People", BUCKET_PEOPLE) {
override fun isInSection(entry: ListEntry): Boolean =
isConversation(entry)
override fun getComparator() = object : NotifComparator("People") {
override fun compare(entry1: ListEntry, entry2: ListEntry): Int {
val type1 = getPeopleType(entry1)
val type2 = getPeopleType(entry2)
return type2.compareTo(type1)
}
}
override fun getHeaderNodeController() =
// TODO: remove SHOW_ALL_SECTIONS, this redundant method, and peopleHeaderController
if (RankingCoordinator.SHOW_ALL_SECTIONS) peopleHeaderController else null
}
val comparator = object : NotifComparator("People") {
override fun compare(entry1: ListEntry, entry2: ListEntry): Int {
assert(entry1.section === entry2.section)
if (entry1.section?.sectioner !== sectioner) {
return 0
}
val type1 = getPeopleType(entry1)
val type2 = getPeopleType(entry2)
return type2.compareTo(type1)
}
}
override fun attach(pipeline: NotifPipeline) {
pipeline.addPromoter(notificationPromoter)
}

8
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/HeadsUpCoordinator.kt
View File

@@ -26,6 +26,7 @@ import com.android.systemui.statusbar.notification.collection.ListEntry
import com.android.systemui.statusbar.notification.collection.NotifPipeline
import com.android.systemui.statusbar.notification.collection.NotificationEntry
import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifPromoter
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner
import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener
@@ -456,6 +457,13 @@ class HeadsUpCoordinator @Inject constructor(
// TODO: This check won't notice if a child of the group is going to HUN...
isGoingToShowHunNoRetract(entry)
override fun getComparator(): NotifComparator {
return object : NotifComparator("HeadsUp") {
override fun compare(o1: ListEntry, o2: ListEntry): Int =
mHeadsUpManager.compare(o1.representativeEntry, o2.representativeEntry)
}
}
override fun getHeaderNodeController(): NodeController? =
// TODO: remove SHOW_ALL_SECTIONS, this redundant method, and mIncomingHeaderController
if (RankingCoordinator.SHOW_ALL_SECTIONS) mIncomingHeaderController else null

6
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/NotifCoordinators.kt
View File

@@ -20,7 +20,6 @@ import com.android.systemui.dump.DumpManager
import com.android.systemui.statusbar.notification.NotifPipelineFlags
import com.android.systemui.statusbar.notification.collection.NotifPipeline
import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner
import java.io.FileDescriptor
import java.io.PrintWriter
@@ -64,7 +63,6 @@ class NotifCoordinatorsImpl @Inject constructor(
private val mCoordinators: MutableList<Coordinator> = ArrayList()
private val mOrderedSections: MutableList<NotifSectioner> = ArrayList()
private val mOrderedComparators: MutableList<NotifComparator> = ArrayList()
/**
* Creates all the coordinators.
@@ -119,9 +117,6 @@ class NotifCoordinatorsImpl @Inject constructor(
mOrderedSections.add(rankingCoordinator.alertingSectioner) // Alerting
mOrderedSections.add(rankingCoordinator.silentSectioner) // Silent
mOrderedSections.add(rankingCoordinator.minimizedSectioner) // Minimized
// Manually add ordered comparators
mOrderedComparators.add(conversationCoordinator.comparator)
}
/**
@@ -133,7 +128,6 @@ class NotifCoordinatorsImpl @Inject constructor(
c.attach(pipeline)
}
pipeline.setSections(mOrderedSections)
pipeline.setComparators(mOrderedComparators)
}
override fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array<String>) {

3
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/NotifSection.kt
View File

@@ -16,6 +16,7 @@
package com.android.systemui.statusbar.notification.collection.listbuilder
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifSectioner
import com.android.systemui.statusbar.notification.collection.render.NodeController
import com.android.systemui.statusbar.notification.stack.PriorityBucket
@@ -29,5 +30,7 @@ data class NotifSection(
val headerController: NodeController? = sectioner.headerNodeController
val comparator: NotifComparator? = sectioner.comparator
@PriorityBucket val bucket: Int = sectioner.bucket
}

14
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifSectioner.java
View File

@@ -54,9 +54,23 @@ public abstract class NotifSectioner extends Pluggable<NotifSectioner> {
*/
public abstract boolean isInSection(ListEntry entry);
/**
* Returns an optional {@link NotifComparator} to sort entries only in this section.
* {@link ListEntry} instances passed to this comparator are guaranteed to have this section,
* and this ordering will take precedence over any global comparators supplied to {@link
* com.android.systemui.statusbar.notification.collection.NotifPipeline#setComparators(List)}.
*
* NOTE: this method is only called once when the Sectioner is attached.
*/
public @Nullable NotifComparator getComparator() {
return null;
}
/**
* Returns an optional {@link NodeSpec} for the section header. If {@code null}, no header will
* be used for the section.
*
* NOTE: this method is only called once when the Sectioner is attached.
*/
public @Nullable NodeController getHeaderNodeController() {
return null;

7
packages/SystemUI/src/com/android/systemui/statusbar/policy/HeadsUpManager.java
View File

@@ -350,11 +350,14 @@ public abstract class HeadsUpManager extends AlertingNotificationManager {
* @return -1 if the first argument should be ranked higher than the second, 1 if the second
* one should be ranked higher and 0 if they are equal.
*/
public int compare(@NonNull NotificationEntry a, @NonNull NotificationEntry b) {
public int compare(@Nullable NotificationEntry a, @Nullable NotificationEntry b) {
if (a == null || b == null) {
return Boolean.compare(a == null, b == null);
}
AlertEntry aEntry = getHeadsUpEntry(a.getKey());
AlertEntry bEntry = getHeadsUpEntry(b.getKey());
if (aEntry == null || bEntry == null) {
return aEntry == null ? 1 : -1;
return Boolean.compare(aEntry == null, bEntry == null);
}
return aEntry.compareTo(bEntry);
}

97
packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java
View File

@@ -29,6 +29,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.inOrder;
@@ -46,6 +47,7 @@ import android.testing.TestableLooper;
import android.util.ArrayMap;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.test.filters.SmallTest;
import com.android.systemui.SysuiTestCase;
@@ -871,6 +873,73 @@ public class ShadeListBuilderTest extends SysuiTestCase {
);
}
@Test
public void testThatSectionComparatorsAreCalled() {
// GIVEN a section with a comparator that elevates some packages over others
NotifComparator comparator = spy(new HypeComparator(PACKAGE_2, PACKAGE_4));
NotifSectioner sectioner = new PackageSectioner(
List.of(PACKAGE_1, PACKAGE_2, PACKAGE_4, PACKAGE_5), comparator);
mListBuilder.setSectioners(List.of(sectioner));
// WHEN the pipeline is kicked off on a bunch of notifications
addNotif(0, PACKAGE_0);
addNotif(1, PACKAGE_1);
addNotif(2, PACKAGE_2);
addNotif(3, PACKAGE_3);
addNotif(4, PACKAGE_4);
addNotif(5, PACKAGE_5);
dispatchBuild();
// THEN the notifs are sorted according to both sectioning and the section's comparator
verifyBuiltList(
notif(2),
notif(4),
notif(1),
notif(5),
notif(0),
notif(3)
);
// VERIFY that the comparator is invoked at least 3 times
verify(comparator, atLeast(3)).compare(any(), any());
// VERIFY that the comparator is never invoked with the entry from package 0 or 3.
final NotificationEntry package0Entry = mEntrySet.get(0);
verify(comparator, never()).compare(eq(package0Entry), any());
verify(comparator, never()).compare(any(), eq(package0Entry));
final NotificationEntry package3Entry = mEntrySet.get(3);
verify(comparator, never()).compare(eq(package3Entry), any());
verify(comparator, never()).compare(any(), eq(package3Entry));
}
@Test
public void testThatSectionComparatorsAreNotCalledForSectionWithSingleEntry() {
// GIVEN a section with a comparator that will have only 1 element
NotifComparator comparator = spy(new HypeComparator(PACKAGE_3));
NotifSectioner sectioner = new PackageSectioner(List.of(PACKAGE_3), comparator);
mListBuilder.setSectioners(List.of(sectioner));
// WHEN the pipeline is kicked off on a bunch of notifications
addNotif(0, PACKAGE_1);
addNotif(1, PACKAGE_2);
addNotif(2, PACKAGE_3);
addNotif(3, PACKAGE_4);
addNotif(4, PACKAGE_5);
dispatchBuild();
// THEN the notifs are sorted according to the sectioning
verifyBuiltList(
notif(2),
notif(0),
notif(1),
notif(3),
notif(4)
);
// VERIFY that the comparator is never invoked
verify(comparator, never()).compare(any(), any());
}
@Test
public void testListenersAndPluggablesAreFiredInOrder() {
// GIVEN a bunch of registered listeners and pluggables
@@ -934,7 +1003,8 @@ public class ShadeListBuilderTest extends SysuiTestCase {
// GIVEN a variety of pluggables
NotifFilter packageFilter = new PackageFilter(PACKAGE_1);
NotifPromoter idPromoter = new IdPromoter(4);
NotifSectioner section = new PackageSectioner(PACKAGE_1);
NotifComparator sectionComparator = new HypeComparator(PACKAGE_1);
NotifSectioner section = new PackageSectioner(List.of(PACKAGE_1), sectionComparator);
NotifComparator hypeComparator = new HypeComparator(PACKAGE_2);
Invalidator preRenderInvalidator = new Invalidator("PreRenderInvalidator") {};
@@ -968,6 +1038,10 @@ public class ShadeListBuilderTest extends SysuiTestCase {
hypeComparator.invalidateList();
verify(mOnRenderListListener).onRenderList(anyList());
clearInvocations(mOnRenderListListener);
sectionComparator.invalidateList();
verify(mOnRenderListListener).onRenderList(anyList());
clearInvocations(mOnRenderListListener);
preRenderInvalidator.invalidateList();
verify(mOnRenderListListener).onRenderList(anyList());
@@ -2037,16 +2111,30 @@ public class ShadeListBuilderTest extends SysuiTestCase {
/** Represents a section for the passed pkg */
private static class PackageSectioner extends NotifSectioner {
private final String mPackage;
private final List<String> mPackages;
private final NotifComparator mComparator;
PackageSectioner(List<String> pkgs, NotifComparator comparator) {
super("PackageSection_" + pkgs, 0);
mPackages = pkgs;
mComparator = comparator;
}
PackageSectioner(String pkg) {
super("PackageSection_" + pkg, 0);
mPackage = pkg;
mPackages = List.of(pkg);
mComparator = null;
}
@Nullable
@Override
public NotifComparator getComparator() {
return mComparator;
}
@Override
public boolean isInSection(ListEntry entry) {
return entry.getRepresentativeEntry().getSbn().getPackageName().equals(mPackage);
return mPackages.contains(entry.getRepresentativeEntry().getSbn().getPackageName());
}
}
@@ -2157,6 +2245,7 @@ public class ShadeListBuilderTest extends SysuiTestCase {
}
}
private static final String PACKAGE_0 = "com.test0";
private static final String PACKAGE_1 = "com.test1";
private static final String PACKAGE_2 = "com.test2";
private static final String PACKAGE_3 = "org.test3";

14
packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ConversationCoordinatorTest.kt
View File

@@ -32,7 +32,6 @@ import com.android.systemui.statusbar.notification.collection.render.NodeControl
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_IMPORTANT_PERSON
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier.Companion.TYPE_PERSON
import com.android.systemui.util.mockito.any
import com.android.systemui.util.mockito.withArgCaptor
import com.google.common.truth.Truth.assertThat
import org.junit.Assert.assertFalse
@@ -41,7 +40,6 @@ import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
import org.mockito.Mockito.`when` as whenever
@@ -79,7 +77,7 @@ class ConversationCoordinatorTest : SysuiTestCase() {
}
peopleSectioner = coordinator.sectioner
peopleComparator = coordinator.comparator
peopleComparator = peopleSectioner.comparator!!
entry = NotificationEntryBuilder().setChannel(channel).build()
@@ -107,16 +105,6 @@ class ConversationCoordinatorTest : SysuiTestCase() {
assertFalse(peopleSectioner.isInSection(NotificationEntryBuilder().build()))
}
@Test
fun testComparatorIgnoresFromOtherSection() {
val e1 = NotificationEntryBuilder().setId(1).setChannel(channel).build()
val e2 = NotificationEntryBuilder().setId(2).setChannel(channel).build()
// wrong section -- never classify
assertThat(peopleComparator.compare(e1, e2)).isEqualTo(0)
verify(peopleNotificationIdentifier, never()).getPeopleNotificationType(any())
}
@Test
fun testComparatorPutsImportantPeopleFirst() {
whenever(peopleNotificationIdentifier.getPeopleNotificationType(entryA))

22
packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/HeadsUpManagerTest.java
View File

@@ -127,6 +127,28 @@ public class HeadsUpManagerTest extends AlertingNotificationManagerTest {
assertFalse(mHeadsUpManager.isAlerting(mEntry.getKey()));
}
@Test
public void testCompareTo_withNullEntries() {
NotificationEntry alertEntry = new NotificationEntryBuilder().setTag("alert").build();
mHeadsUpManager.showNotification(alertEntry);
assertThat(mHeadsUpManager.compare(alertEntry, null)).isLessThan(0);
assertThat(mHeadsUpManager.compare(null, alertEntry)).isGreaterThan(0);
assertThat(mHeadsUpManager.compare(null, null)).isEqualTo(0);
}
@Test
public void testCompareTo_withNonAlertEntries() {
NotificationEntry nonAlertEntry1 = new NotificationEntryBuilder().setTag("nae1").build();
NotificationEntry nonAlertEntry2 = new NotificationEntryBuilder().setTag("nae2").build();
NotificationEntry alertEntry = new NotificationEntryBuilder().setTag("alert").build();
mHeadsUpManager.showNotification(alertEntry);
assertThat(mHeadsUpManager.compare(alertEntry, nonAlertEntry1)).isLessThan(0);
assertThat(mHeadsUpManager.compare(nonAlertEntry1, alertEntry)).isGreaterThan(0);
assertThat(mHeadsUpManager.compare(nonAlertEntry1, nonAlertEntry2)).isEqualTo(0);
}
@Test
public void testAlertEntryCompareTo_ongoingCallLessThanActiveRemoteInput() {
HeadsUpManager.HeadsUpEntry ongoingCall = mHeadsUpManager.new HeadsUpEntry();