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

Merge "New Pipeline: Implement per-section comparators; sort HUNs"

Browse Source
This commit is contained in:
TreeHugger Robot
2022-02-03 10:18:42 +00:00
committed by Android (Google) Code Review
parent 17dd24445b 92c826152d
commit 1a6f762f29
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()));
@@ -1098,7 +1103,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()));
@@ -1111,6 +1121,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(),
@@ -1122,6 +1145,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();