New Pipeline: Separate sectioning and sorting steps.

* Fixes a bug where the last NotifSectioner with entries would not have onEntriesUpdated called.
* Changes the contract of onEntriesUpdated so that its called for empty sections.
* Update RankingCoordinator - remove resetClearAllFlags() now that onEntriesUpdated() is called when the section is empty.

Bug: 205348428
Test: atest ShadeListBuilderTest
Change-Id: Idafbc6643aed11f4ee9db23c59a6d5d5da45b09f
This commit is contained in:
Jeff DeCew
2021-11-05 20:08:56 -04:00
parent f918416763
commit b9df9d9653
4 changed files with 108 additions and 73 deletions

View File

@@ -28,6 +28,8 @@ import static com.android.systemui.statusbar.notification.collection.listbuilder
import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_SORTING;
import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_TRANSFORMING;
import static java.util.Objects.requireNonNull;
import android.annotation.MainThread;
import android.annotation.Nullable;
import android.os.Trace;
@@ -372,12 +374,14 @@ public class ShadeListBuilder implements Dumpable {
applyNewNotifList();
pruneIncompleteGroups(mNotifList);
// Step 6: Sort
// Assign each top-level entry a section, then sort the list by section and then within
// section by our list of custom comparators
// Step 6: Section & Sort
// Assign each top-level entry a section, and copy to all of its children
dispatchOnBeforeSort(mReadOnlyNotifList);
mPipelineState.incrementTo(STATE_SORTING);
sortListAndNotifySections();
assignSections();
notifySectionEntriesUpdated();
// Sort the list by section and then within section by our list of custom comparators
sortListAndGroups();
// Step 7: Lock in our group structure and log anything that's changed since the last run
mPipelineState.incrementTo(STATE_FINALIZING);
@@ -408,18 +412,15 @@ public class ShadeListBuilder implements Dumpable {
private void notifySectionEntriesUpdated() {
Trace.beginSection("ShadeListBuilder.notifySectionEntriesUpdated");
NotifSection currentSection = null;
mTempSectionMembers.clear();
for (int i = 0; i < mNotifList.size(); i++) {
ListEntry currentEntry = mNotifList.get(i);
if (currentSection != currentEntry.getSection()) {
if (currentSection != null) {
currentSection.getSectioner().onEntriesUpdated(mTempSectionMembers);
mTempSectionMembers.clear();
for (NotifSection section : mNotifSections) {
for (ListEntry entry : mNotifList) {
if (section == entry.getSection()) {
mTempSectionMembers.add(entry);
}
currentSection = currentEntry.getSection();
}
mTempSectionMembers.add(currentEntry);
section.getSectioner().onEntriesUpdated(mTempSectionMembers);
mTempSectionMembers.clear();
}
Trace.endSection();
}
@@ -765,9 +766,9 @@ public class ShadeListBuilder implements Dumpable {
}
}
private void sortListAndNotifySections() {
Trace.beginSection("ShadeListBuilder.sortListAndNotifySections");
// Assign sections to top-level elements and sort their children
private void assignSections() {
Trace.beginSection("ShadeListBuilder.assignSections");
// Assign sections to top-level elements and their children
for (ListEntry entry : mNotifList) {
NotifSection section = applySections(entry);
if (entry instanceof GroupEntry) {
@@ -775,27 +776,35 @@ public class ShadeListBuilder implements Dumpable {
for (NotificationEntry child : parent.getChildren()) {
setEntrySection(child, section);
}
}
}
Trace.endSection();
}
private void sortListAndGroups() {
Trace.beginSection("ShadeListBuilder.sortListAndGroups");
// Assign sections to top-level elements and sort their children
for (ListEntry entry : mNotifList) {
if (entry instanceof GroupEntry) {
GroupEntry parent = (GroupEntry) entry;
parent.sortChildren(mGroupChildrenComparator);
}
}
mNotifList.sort(mTopLevelComparator);
assignIndexes(mNotifList);
notifySectionEntriesUpdated();
Trace.endSection();
}
/**
* Assign the index of each notification relative to the total order
* @param notifList
*/
private void assignIndexes(List<ListEntry> notifList) {
private static void assignIndexes(List<ListEntry> notifList) {
if (notifList.size() == 0) return;
NotifSection currentSection = notifList.get(0).getSection();
NotifSection currentSection = requireNonNull(notifList.get(0).getSection());
int sectionMemberIndex = 0;
for (int i = 0; i < notifList.size(); i++) {
ListEntry entry = notifList.get(i);
NotifSection section = entry.getSection();
NotifSection section = requireNonNull(entry.getSection());
if (section.getIndex() != currentSection.getIndex()) {
sectionMemberIndex = 0;
currentSection = section;

View File

@@ -19,7 +19,6 @@ package com.android.systemui.statusbar.notification.collection.coordinator;
import android.annotation.NonNull;
import android.annotation.Nullable;
import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
@@ -76,7 +75,6 @@ public class RankingCoordinator implements Coordinator {
pipeline.addPreGroupFilter(mSuspendedFilter);
pipeline.addPreGroupFilter(mDndVisualEffectsFilter);
pipeline.addOnBeforeSortListener(entries -> resetClearAllFlags());
}
public NotifSectioner getAlertingSectioner() {
@@ -126,6 +124,7 @@ public class RankingCoordinator implements Coordinator {
@Nullable
@Override
public void onEntriesUpdated(@NonNull List<ListEntry> entries) {
mHasSilentEntries = false;
for (int i = 0; i < entries.size(); i++) {
if (entries.get(i).getRepresentativeEntry().getSbn().isClearable()) {
mHasSilentEntries = true;
@@ -154,6 +153,7 @@ public class RankingCoordinator implements Coordinator {
@Nullable
@Override
public void onEntriesUpdated(@NonNull List<ListEntry> entries) {
mHasMinimizedEntries = false;
for (int i = 0; i < entries.size(); i++) {
if (entries.get(i).getRepresentativeEntry().getSbn().isClearable()) {
mHasMinimizedEntries = true;
@@ -189,12 +189,6 @@ public class RankingCoordinator implements Coordinator {
}
};
@VisibleForTesting
protected void resetClearAllFlags() {
mHasSilentEntries = false;
mHasMinimizedEntries = false;
}
private final StatusBarStateController.StateListener mStatusBarStateCallback =
new StatusBarStateController.StateListener() {
@Override

View File

@@ -18,6 +18,8 @@ package com.android.systemui.statusbar.notification.collection;
import static com.android.systemui.statusbar.notification.collection.ListDumper.dumpTree;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -33,7 +35,6 @@ import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static java.util.Arrays.asList;
@@ -44,7 +45,6 @@ import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;
import android.util.ArrayMap;
import androidx.annotation.Nullable;
import androidx.test.filters.SmallTest;
import com.android.systemui.SysuiTestCase;
@@ -82,7 +82,6 @@ import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
@SmallTest
@@ -618,26 +617,53 @@ public class ShadeListBuilderTest extends SysuiTestCase {
@Test
public void testNotifSectionsChildrenUpdated() {
AtomicBoolean validChildren = new AtomicBoolean(false);
ArrayList<ListEntry> pkg1Entries = new ArrayList<>();
ArrayList<ListEntry> pkg2Entries = new ArrayList<>();
ArrayList<ListEntry> pkg3Entries = new ArrayList<>();
final NotifSectioner pkg1Sectioner = spy(new PackageSectioner(PACKAGE_1) {
@Nullable
@Override
public void onEntriesUpdated(List<ListEntry> entries) {
super.onEntriesUpdated(entries);
validChildren.set(entries.size() == 2);
pkg1Entries.addAll(entries);
}
});
mListBuilder.setSectioners(asList(pkg1Sectioner));
final NotifSectioner pkg2Sectioner = spy(new PackageSectioner(PACKAGE_2) {
@Override
public void onEntriesUpdated(List<ListEntry> entries) {
super.onEntriesUpdated(entries);
pkg2Entries.addAll(entries);
}
});
final NotifSectioner pkg3Sectioner = spy(new PackageSectioner(PACKAGE_3) {
@Override
public void onEntriesUpdated(List<ListEntry> entries) {
super.onEntriesUpdated(entries);
pkg3Entries.addAll(entries);
}
});
mListBuilder.setSectioners(asList(pkg1Sectioner, pkg2Sectioner, pkg3Sectioner));
addNotif(0, PACKAGE_4);
addNotif(0, PACKAGE_1);
addNotif(1, PACKAGE_1);
addNotif(2, PACKAGE_1);
addNotif(2, PACKAGE_3);
addNotif(3, PACKAGE_3);
addNotif(4, PACKAGE_3);
dispatchBuild();
verify(pkg1Sectioner, times(1)).onEntriesUpdated(any());
assertTrue(validChildren.get());
verify(pkg1Sectioner).onEntriesUpdated(any());
verify(pkg2Sectioner).onEntriesUpdated(any());
verify(pkg3Sectioner).onEntriesUpdated(any());
assertThat(pkg1Entries).containsExactly(
mEntrySet.get(0),
mEntrySet.get(1)
).inOrder();
assertThat(pkg2Entries).isEmpty();
assertThat(pkg3Entries).containsExactly(
mEntrySet.get(2),
mEntrySet.get(3),
mEntrySet.get(4)
).inOrder();
}
@Test

View File

@@ -19,6 +19,7 @@ package com.android.systemui.statusbar.notification.collection.coordinator;
import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_AMBIENT;
import static android.app.NotificationManager.Policy.SUPPRESSED_EFFECT_NOTIFICATION_LIST;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
@@ -28,8 +29,7 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.app.Notification;
import android.service.notification.NotificationListenerService;
import android.service.notification.StatusBarNotification;
import android.app.NotificationManager;
import android.testing.AndroidTestingRunner;
import androidx.annotation.Nullable;
@@ -38,6 +38,7 @@ import androidx.test.filters.SmallTest;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.statusbar.RankingBuilder;
import com.android.systemui.statusbar.SbnBuilder;
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.statusbar.notification.collection.NotifPipeline;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -54,7 +55,6 @@ import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import java.util.ArrayList;
@@ -70,8 +70,6 @@ public class RankingCoordinatorTest extends SysuiTestCase {
@Mock private NodeController mAlertingHeaderController;
@Mock private NodeController mSilentNodeController;
@Mock private SectionHeaderController mSilentHeaderController;
@Mock private NotificationListenerService.Ranking mRanking;
@Mock private StatusBarNotification mSbn;
@Captor private ArgumentCaptor<NotifFilter> mNotifFilterCaptor;
@@ -92,9 +90,7 @@ public class RankingCoordinatorTest extends SysuiTestCase {
mStatusBarStateController, mHighPriorityProvider, mAlertingHeaderController,
mSilentHeaderController, mSilentNodeController);
mEntry = spy(new NotificationEntryBuilder().build());
mRanking = spy(getRankingForUnfilteredNotif().build());
mEntry.setRanking(mRanking);
when(mEntry.getSbn()).thenReturn(mSbn);
mEntry.setRanking(getRankingForUnfilteredNotif().build());
mRankingCoordinator.attach(mNotifPipeline);
verify(mNotifPipeline, times(2)).addPreGroupFilter(mNotifFilterCaptor.capture());
@@ -109,23 +105,19 @@ public class RankingCoordinatorTest extends SysuiTestCase {
@Test
public void testSilentHeaderClearableChildrenUpdate() {
StatusBarNotification sbn = Mockito.mock(StatusBarNotification.class);
Mockito.doReturn("key").when(sbn).getKey();
Mockito.doReturn(Mockito.mock(Notification.class)).when(sbn).getNotification();
NotificationEntry entry = new NotificationEntryBuilder().setSbn(sbn).build();
ListEntry listEntry = new ListEntry("key", 0L) {
ListEntry listEntry = new ListEntry(mEntry.getKey(), 0L) {
@Nullable
@Override
public NotificationEntry getRepresentativeEntry() {
return entry;
return mEntry;
}
};
Mockito.doReturn(true).when(sbn).isClearable();
setRankingAmbient(false);
setSbnClearable(true);
mSilentSectioner.onEntriesUpdated(Arrays.asList(listEntry));
when(mRanking.isAmbient()).thenReturn(false);
verify(mSilentHeaderController).setClearSectionEnabled(eq(true));
mRankingCoordinator.resetClearAllFlags();
Mockito.doReturn(false).when(sbn).isClearable();
setSbnClearable(false);
mSilentSectioner.onEntriesUpdated(Arrays.asList(listEntry));
verify(mSilentHeaderController).setClearSectionEnabled(eq(false));
}
@@ -204,7 +196,7 @@ public class RankingCoordinatorTest extends SysuiTestCase {
public void testIncludeInSectionSilent() {
// GIVEN the entry isn't high priority
when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false);
when(mRanking.isAmbient()).thenReturn(false);
setRankingAmbient(false);
// THEN entry is in the silent section
assertFalse(mAlertingSectioner.isInSection(mEntry));
@@ -213,24 +205,23 @@ public class RankingCoordinatorTest extends SysuiTestCase {
@Test
public void testMinSection() {
when(mEntry.getRanking()).thenReturn(mRanking);
when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false);
when(mRanking.isAmbient()).thenReturn(true);
setRankingAmbient(true);
assertInSection(mEntry, mMinimizedSectioner);
}
@Test
public void testSilentSection() {
when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false);
when(mRanking.isAmbient()).thenReturn(false);
setRankingAmbient(false);
assertInSection(mEntry, mSilentSectioner);
}
@Test
public void testClearableSilentSection() {
when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false);
when(mSbn.isClearable()).thenReturn(true);
when(mRanking.isAmbient()).thenReturn(false);
setSbnClearable(true);
setRankingAmbient(false);
mSilentSectioner.onEntriesUpdated(Arrays.asList(mEntry));
verify(mSilentHeaderController).setClearSectionEnabled(eq(true));
}
@@ -238,17 +229,17 @@ public class RankingCoordinatorTest extends SysuiTestCase {
@Test
public void testClearableMinimizedSection() {
when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false);
when(mSbn.isClearable()).thenReturn(true);
when(mRanking.isAmbient()).thenReturn(true);
setSbnClearable(true);
setRankingAmbient(true);
mMinimizedSectioner.onEntriesUpdated(Arrays.asList(mEntry));
verify(mSilentHeaderController).setClearSectionEnabled(eq(true));
}
@Test
public void testNotClearableSilentSection() {
when(mSbn.isClearable()).thenReturn(false);
setSbnClearable(false);
when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false);
when(mRanking.isAmbient()).thenReturn(false);
setRankingAmbient(false);
mSilentSectioner.onEntriesUpdated(Arrays.asList(mEntry));
mMinimizedSectioner.onEntriesUpdated(Arrays.asList(mEntry));
mAlertingSectioner.onEntriesUpdated(Arrays.asList(mEntry));
@@ -257,9 +248,9 @@ public class RankingCoordinatorTest extends SysuiTestCase {
@Test
public void testNotClearableMinimizedSection() {
when(mSbn.isClearable()).thenReturn(false);
setSbnClearable(false);
when(mHighPriorityProvider.isHighPriority(mEntry)).thenReturn(false);
when(mRanking.isAmbient()).thenReturn(true);
setRankingAmbient(true);
mSilentSectioner.onEntriesUpdated(Arrays.asList(mEntry));
mMinimizedSectioner.onEntriesUpdated(Arrays.asList(mEntry));
mAlertingSectioner.onEntriesUpdated(Arrays.asList(mEntry));
@@ -277,9 +268,24 @@ public class RankingCoordinatorTest extends SysuiTestCase {
}
private RankingBuilder getRankingForUnfilteredNotif() {
return new RankingBuilder()
.setKey(mEntry.getKey())
return new RankingBuilder(mEntry.getRanking())
.setSuppressedVisualEffects(0)
.setSuspended(false);
}
private void setSbnClearable(boolean clearable) {
mEntry.setSbn(new SbnBuilder(mEntry.getSbn())
.setFlag(mContext, Notification.FLAG_NO_CLEAR, !clearable)
.build());
assertEquals(clearable, mEntry.getSbn().isClearable());
}
private void setRankingAmbient(boolean ambient) {
mEntry.setRanking(new RankingBuilder(mEntry.getRanking())
.setImportance(ambient
? NotificationManager.IMPORTANCE_MIN
: NotificationManager.IMPORTANCE_DEFAULT)
.build());
assertEquals(ambient, mEntry.getRanking().isAmbient());
}
}