Merge "Revert "Revert "Don't filter out groups if summary is re-inflating""" into tm-qpr-dev

This commit is contained in:
Steve Elliott
2022-10-21 18:40:59 +00:00
committed by Android (Google) Code Review
3 changed files with 88 additions and 15 deletions

View File

@@ -407,7 +407,10 @@ public class PreparationCoordinator implements Coordinator {
mLogger.logGroupInflationTookTooLong(group); mLogger.logGroupInflationTookTooLong(group);
return false; return false;
} }
if (mInflatingNotifs.contains(group.getSummary())) { // Only delay release if the summary is not inflated.
// TODO(253454977): Once we ensure that all other pipeline filtering and pruning has been
// done by this point, we can revert back to checking for mInflatingNotifs.contains(...)
if (group.getSummary() != null && !isInflated(group.getSummary())) {
mLogger.logDelayingGroupRelease(group, group.getSummary()); mLogger.logDelayingGroupRelease(group, group.getSummary());
return true; return true;
} }

View File

@@ -31,8 +31,8 @@ public class GroupEntryBuilder {
private long mCreationTime = 0; private long mCreationTime = 0;
@Nullable private GroupEntry mParent = GroupEntry.ROOT_ENTRY; @Nullable private GroupEntry mParent = GroupEntry.ROOT_ENTRY;
private NotifSection mNotifSection; private NotifSection mNotifSection;
private NotificationEntry mSummary = null; @Nullable private NotificationEntry mSummary = null;
private List<NotificationEntry> mChildren = new ArrayList<>(); private final List<NotificationEntry> mChildren = new ArrayList<>();
/** Builds a new instance of GroupEntry */ /** Builds a new instance of GroupEntry */
public GroupEntry build() { public GroupEntry build() {
@@ -41,7 +41,9 @@ public class GroupEntryBuilder {
ge.getAttachState().setSection(mNotifSection); ge.getAttachState().setSection(mNotifSection);
ge.setSummary(mSummary); ge.setSummary(mSummary);
mSummary.setParent(ge); if (mSummary != null) {
mSummary.setParent(ge);
}
for (NotificationEntry child : mChildren) { for (NotificationEntry child : mChildren) {
ge.addChild(child); ge.addChild(child);

View File

@@ -181,7 +181,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test @Test
public void testInflatesNewNotification() { public void testInflatesNewNotification() {
// WHEN there is a new notification // WHEN there is a new notification
mCollectionListener.onEntryAdded(mEntry); mCollectionListener.onEntryInit(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
// THEN we inflate it // THEN we inflate it
@@ -194,7 +194,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test @Test
public void testRebindsInflatedNotificationsOnUpdate() { public void testRebindsInflatedNotificationsOnUpdate() {
// GIVEN an inflated notification // GIVEN an inflated notification
mCollectionListener.onEntryAdded(mEntry); mCollectionListener.onEntryInit(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), any(), any()); verify(mNotifInflater).inflateViews(eq(mEntry), any(), any());
mNotifInflater.invokeInflateCallbackForEntry(mEntry); mNotifInflater.invokeInflateCallbackForEntry(mEntry);
@@ -213,7 +213,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test @Test
public void testEntrySmartReplyAdditionWillRebindViews() { public void testEntrySmartReplyAdditionWillRebindViews() {
// GIVEN an inflated notification // GIVEN an inflated notification
mCollectionListener.onEntryAdded(mEntry); mCollectionListener.onEntryInit(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), any(), any()); verify(mNotifInflater).inflateViews(eq(mEntry), any(), any());
mNotifInflater.invokeInflateCallbackForEntry(mEntry); mNotifInflater.invokeInflateCallbackForEntry(mEntry);
@@ -232,7 +232,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test @Test
public void testEntryChangedToMinimizedSectionWillRebindViews() { public void testEntryChangedToMinimizedSectionWillRebindViews() {
// GIVEN an inflated notification // GIVEN an inflated notification
mCollectionListener.onEntryAdded(mEntry); mCollectionListener.onEntryInit(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), mParamsCaptor.capture(), any()); verify(mNotifInflater).inflateViews(eq(mEntry), mParamsCaptor.capture(), any());
assertFalse(mParamsCaptor.getValue().isLowPriority()); assertFalse(mParamsCaptor.getValue().isLowPriority());
@@ -254,7 +254,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
public void testMinimizedEntryMovedIntoGroupWillRebindViews() { public void testMinimizedEntryMovedIntoGroupWillRebindViews() {
// GIVEN an inflated, minimized notification // GIVEN an inflated, minimized notification
setSectionIsLowPriority(true); setSectionIsLowPriority(true);
mCollectionListener.onEntryAdded(mEntry); mCollectionListener.onEntryInit(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), mParamsCaptor.capture(), any()); verify(mNotifInflater).inflateViews(eq(mEntry), mParamsCaptor.capture(), any());
assertTrue(mParamsCaptor.getValue().isLowPriority()); assertTrue(mParamsCaptor.getValue().isLowPriority());
@@ -275,7 +275,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test @Test
public void testEntryRankChangeWillNotRebindViews() { public void testEntryRankChangeWillNotRebindViews() {
// GIVEN an inflated notification // GIVEN an inflated notification
mCollectionListener.onEntryAdded(mEntry); mCollectionListener.onEntryInit(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), any(), any()); verify(mNotifInflater).inflateViews(eq(mEntry), any(), any());
mNotifInflater.invokeInflateCallbackForEntry(mEntry); mNotifInflater.invokeInflateCallbackForEntry(mEntry);
@@ -294,7 +294,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
@Test @Test
public void testDoesntFilterInflatedNotifs() { public void testDoesntFilterInflatedNotifs() {
// GIVEN an inflated notification // GIVEN an inflated notification
mCollectionListener.onEntryAdded(mEntry); mCollectionListener.onEntryInit(mEntry);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(mEntry));
verify(mNotifInflater).inflateViews(eq(mEntry), any(), any()); verify(mNotifInflater).inflateViews(eq(mEntry), any(), any());
mNotifInflater.invokeInflateCallbackForEntry(mEntry); mNotifInflater.invokeInflateCallbackForEntry(mEntry);
@@ -330,9 +330,9 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
mCollectionListener.onEntryInit(entry); mCollectionListener.onEntryInit(entry);
} }
mCollectionListener.onEntryAdded(summary); mCollectionListener.onEntryInit(summary);
for (NotificationEntry entry : children) { for (NotificationEntry entry : children) {
mCollectionListener.onEntryAdded(entry); mCollectionListener.onEntryInit(entry);
} }
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(groupEntry)); mBeforeFilterListener.onBeforeFinalizeFilter(List.of(groupEntry));
@@ -392,6 +392,70 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
assertTrue(mUninflatedFilter.shouldFilterOut(child1, 401)); assertTrue(mUninflatedFilter.shouldFilterOut(child1, 401));
} }
@Test
public void testNullGroupSummary() {
// GIVEN a newly-posted group with a summary and two children
final GroupEntry group = new GroupEntryBuilder()
.setCreationTime(400)
.setSummary(getNotificationEntryBuilder().setId(1).build())
.addChild(getNotificationEntryBuilder().setId(2).build())
.addChild(getNotificationEntryBuilder().setId(3).build())
.build();
fireAddEvents(List.of(group));
final NotificationEntry child0 = group.getChildren().get(0);
final NotificationEntry child1 = group.getChildren().get(1);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(group));
// WHEN the summary is pruned
new GroupEntryBuilder()
.setCreationTime(400)
.addChild(child0)
.addChild(child1)
.build();
// WHEN all of the children (but not the summary) finish inflating
mNotifInflater.invokeInflateCallbackForEntry(child0);
mNotifInflater.invokeInflateCallbackForEntry(child1);
// THEN the entire group is not filtered out
assertFalse(mUninflatedFilter.shouldFilterOut(child0, 401));
assertFalse(mUninflatedFilter.shouldFilterOut(child1, 401));
}
@Test
public void testPartiallyInflatedGroupsAreNotFilteredOutIfSummaryReinflate() {
// GIVEN a newly-posted group with a summary and two children
final String groupKey = "test_reinflate_group";
final int summaryId = 1;
final GroupEntry group = new GroupEntryBuilder()
.setKey(groupKey)
.setCreationTime(400)
.setSummary(getNotificationEntryBuilder().setId(summaryId).setImportance(1).build())
.addChild(getNotificationEntryBuilder().setId(2).build())
.addChild(getNotificationEntryBuilder().setId(3).build())
.build();
fireAddEvents(List.of(group));
final NotificationEntry summary = group.getSummary();
final NotificationEntry child0 = group.getChildren().get(0);
final NotificationEntry child1 = group.getChildren().get(1);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(group));
// WHEN all of the children (but not the summary) finish inflating
mNotifInflater.invokeInflateCallbackForEntry(child0);
mNotifInflater.invokeInflateCallbackForEntry(child1);
mNotifInflater.invokeInflateCallbackForEntry(summary);
// WHEN the summary is updated and starts re-inflating
summary.setRanking(new RankingBuilder(summary.getRanking()).setImportance(4).build());
fireUpdateEvents(summary);
mBeforeFilterListener.onBeforeFinalizeFilter(List.of(group));
// THEN the entire group is still not filtered out
assertFalse(mUninflatedFilter.shouldFilterOut(summary, 401));
assertFalse(mUninflatedFilter.shouldFilterOut(child0, 401));
assertFalse(mUninflatedFilter.shouldFilterOut(child1, 401));
}
@Test @Test
public void testCompletedInflatedGroupsAreReleased() { public void testCompletedInflatedGroupsAreReleased() {
// GIVEN a newly-posted group with a summary and two children // GIVEN a newly-posted group with a summary and two children
@@ -412,7 +476,7 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
mNotifInflater.invokeInflateCallbackForEntry(child1); mNotifInflater.invokeInflateCallbackForEntry(child1);
mNotifInflater.invokeInflateCallbackForEntry(summary); mNotifInflater.invokeInflateCallbackForEntry(summary);
// THEN the entire group is still filtered out // THEN the entire group is no longer filtered out
assertFalse(mUninflatedFilter.shouldFilterOut(summary, 401)); assertFalse(mUninflatedFilter.shouldFilterOut(summary, 401));
assertFalse(mUninflatedFilter.shouldFilterOut(child0, 401)); assertFalse(mUninflatedFilter.shouldFilterOut(child0, 401));
assertFalse(mUninflatedFilter.shouldFilterOut(child1, 401)); assertFalse(mUninflatedFilter.shouldFilterOut(child1, 401));
@@ -494,7 +558,11 @@ public class PreparationCoordinatorTest extends SysuiTestCase {
private void fireAddEvents(NotificationEntry entry) { private void fireAddEvents(NotificationEntry entry) {
mCollectionListener.onEntryInit(entry); mCollectionListener.onEntryInit(entry);
mCollectionListener.onEntryAdded(entry); mCollectionListener.onEntryInit(entry);
}
private void fireUpdateEvents(NotificationEntry entry) {
mCollectionListener.onEntryUpdated(entry);
} }
private static final String TEST_MESSAGE = "TEST_MESSAGE"; private static final String TEST_MESSAGE = "TEST_MESSAGE";