diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListEntry.java index 82c1f243dcdb6..fd0476b76a9ae 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListEntry.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListEntry.java @@ -110,4 +110,11 @@ public abstract class ListEntry { mPreviousAttachState.clone(mAttachState); mAttachState.reset(); } + + /** + * True if this entry was attached in the last pass, else false. + */ + public boolean wasAttachedInPreviousPass() { + return getPreviousAttachState().getParent() != null; + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java index df63fec62e4ee..a0ef1b674af39 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java @@ -513,28 +513,66 @@ public class ShadeListBuilder implements Dumpable { } } - private void stabilizeGroupingNotifs(List list) { + private void stabilizeGroupingNotifs(List topLevelList) { if (mNotifStabilityManager == null) { return; } - for (int i = 0; i < list.size(); i++) { - final ListEntry tle = list.get(i); - if (tle.getPreviousAttachState().getParent() == null) { - continue; // new entries are allowed - } - - final GroupEntry prevParent = tle.getPreviousAttachState().getParent(); - final GroupEntry assignedParent = tle.getParent(); - if (prevParent != assignedParent) { - if (!mNotifStabilityManager.isGroupChangeAllowed(tle.getRepresentativeEntry())) { - tle.getAttachState().getSuppressedChanges().setParent(assignedParent); - tle.setParent(prevParent); + for (int i = 0; i < topLevelList.size(); i++) { + final ListEntry tle = topLevelList.get(i); + if (tle instanceof GroupEntry) { + // maybe put children back into their old group (including moving back to top-level) + GroupEntry groupEntry = (GroupEntry) tle; + List children = groupEntry.getRawChildren(); + for (int j = 0; j < groupEntry.getChildren().size(); j++) { + if (maybeSuppressGroupChange(children.get(j), topLevelList)) { + // child was put back into its previous group, so we remove it from this + // group + children.remove(j); + j--; + } + } + } else { + // maybe put top-level-entries back into their previous groups + if (maybeSuppressGroupChange(tle.getRepresentativeEntry(), topLevelList)) { + // entry was put back into its previous group, so we remove it from the list of + // top-level-entries + topLevelList.remove(i); + i--; } } } } + /** + * Returns true if the group change was suppressed, else false + */ + private boolean maybeSuppressGroupChange(NotificationEntry entry, List out) { + if (!entry.wasAttachedInPreviousPass()) { + return false; // new entries are allowed + } + + final GroupEntry prevParent = entry.getPreviousAttachState().getParent(); + final GroupEntry assignedParent = entry.getParent(); + if (prevParent != assignedParent + && !mNotifStabilityManager.isGroupChangeAllowed(entry.getRepresentativeEntry())) { + entry.getAttachState().getSuppressedChanges().setParent(assignedParent); + entry.setParent(prevParent); + if (prevParent == ROOT_ENTRY) { + out.add(entry); + } else if (prevParent != null) { + prevParent.addChild(entry); + if (!mGroups.containsKey(prevParent.getKey())) { + mGroups.put(prevParent.getKey(), prevParent); + } + } + + return true; + } + + return false; + } + private void promoteNotifs(List list) { for (int i = 0; i < list.size(); i++) { final ListEntry tle = list.get(i); @@ -577,6 +615,17 @@ public class ShadeListBuilder implements Dumpable { } else if (group.getSummary() == null || children.size() < MIN_CHILDREN_FOR_GROUP) { + + if (group.getSummary() != null + && group.wasAttachedInPreviousPass() + && mNotifStabilityManager != null + && !mNotifStabilityManager.isGroupChangeAllowed(group.getSummary())) { + // if this group was previously attached and group changes aren't + // allowed, keep it around until group changes are allowed again + group.getAttachState().getSuppressedChanges().setWasPruneSuppressed(true); + continue; + } + // If the group doesn't provide a summary or is too small, ignore it and add // its children (if any) directly to top-level. @@ -718,6 +767,12 @@ public class ShadeListBuilder implements Dumpable { curr.getParent()); } + if (curr.getSuppressedChanges().getWasPruneSuppressed()) { + mLogger.logGroupPruningSuppressed( + mIterationCount, + curr.getParent()); + } + if (curr.getExcludingFilter() != prev.getExcludingFilter()) { mLogger.logFilterChanged( mIterationCount, @@ -867,9 +922,9 @@ public class ShadeListBuilder implements Dumpable { NotifSection finalSection = newSection; - // are we changing sections of this entry? + // have we seen this entry before and are we changing its section? if (mNotifStabilityManager != null - && prevAttachState.getParent() != null + && entry.wasAttachedInPreviousPass() && newSection != prevAttachState.getSection()) { // are section changes allowed? diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/SuppressedAttachState.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/SuppressedAttachState.kt index 3eb2e610f329c..584563b6c3885 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/SuppressedAttachState.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/SuppressedAttachState.kt @@ -24,28 +24,37 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.NotifS */ data class SuppressedAttachState private constructor( /** - * Null if not attached to the current shade list. If top-level, then the shade list root. If - * part of a group, then that group's GroupEntry. + * The suppressed section assignment for this ListEntry. + * Null if no section change was suppressed. + */ + var section: NotifSection?, + + /** + * The suppressed parent assignment for this ListEntry. + * - Null if no parent change was suppressed. + * - Root if suppressing group change to top-level + * - GroupEntry if suppressing group change to a different group */ var parent: GroupEntry?, /** - * The assigned section for this ListEntry. If the child of the group, this will be the - * parent's section. Null if not attached to the list. + * Whether the ListEntry would have been pruned had its group change not been suppressed. */ - var section: NotifSection? + var wasPruneSuppressed: Boolean ) { /** Copies the state of another instance. */ fun clone(other: SuppressedAttachState) { parent = other.parent section = other.section + wasPruneSuppressed = other.wasPruneSuppressed } /** Resets back to a "clean" state (the same as created by the factory method) */ fun reset() { parent = null section = null + wasPruneSuppressed = false } companion object { @@ -53,7 +62,8 @@ data class SuppressedAttachState private constructor( fun create(): SuppressedAttachState { return SuppressedAttachState( null, - null) + null, + false) } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java index 08030f8201a24..5d6c0437ce9b5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java @@ -110,7 +110,7 @@ public class VisualStabilityCoordinator implements Coordinator { public boolean isGroupChangeAllowed(NotificationEntry entry) { final boolean isGroupChangeAllowedForEntry = mReorderingAllowed || mHeadsUpManager.isAlerting(entry.getKey()); - mIsSuppressingGroupChange |= isGroupChangeAllowedForEntry; + mIsSuppressingGroupChange |= !isGroupChangeAllowedForEntry; return isGroupChangeAllowedForEntry; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt index 9ee7db738c206..5a35127397b45 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt @@ -174,8 +174,19 @@ class ShadeListBuilderLogger @Inject constructor( str1 = suppressedParent?.key str2 = keepingParent?.key }, { - "(Build $long1) Change of parent to '$str1' suppressed; " + - "keeping parent '$str2'" + "(Build $long1) Change of parent to '$str1' suppressed; keeping parent '$str2'" + }) + } + + fun logGroupPruningSuppressed( + buildId: Int, + keepingParent: GroupEntry? + ) { + buffer.log(TAG, INFO, { + int1 = buildId + str1 = keepingParent?.key + }, { + "(Build $long1) Group pruning suppressed; keeping parent '$str1'" }) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java index c1f468a3072f6..e784ec62c8e6e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/GroupMembershipManagerImpl.java @@ -56,7 +56,7 @@ public class GroupMembershipManagerImpl implements GroupMembershipManager { return false; } - return entry.getParent().getChildren().size() == 1; + return !isGroupSummary(entry) && entry.getParent().getChildren().size() == 1; } @Nullable diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java index 8acb705c744d5..2ce22a6f71c96 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java @@ -58,6 +58,7 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter; 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.listbuilder.pluggable.NotifStabilityManager; import com.android.systemui.statusbar.notification.collection.notifcollection.CollectionReadyForBuildListener; import com.android.systemui.util.time.FakeSystemClock; @@ -963,6 +964,198 @@ public class ShadeListBuilderTest extends SysuiTestCase { ); } + @Test + public void testStabilizeGroupsDoesNotAllowGrouping() { + // GIVEN one group child without a summary yet + addGroupChild(0, PACKAGE_1, GROUP_1); + + dispatchBuild(); + + // GIVEN visual stability manager doesn't allow any group changes + mListBuilder.setNotifStabilityManager( + new TestableStabilityManager().setAllowGroupChanges(false)); + + // WHEN we run the pipeline with the addition of a group summary & child + addGroupSummary(1, PACKAGE_1, GROUP_1); + addGroupChild(2, PACKAGE_1, GROUP_1); + + dispatchBuild(); + + // THEN all notifications are top-level and the summary doesn't show yet + // because group changes aren't allowed by the stability manager + verifyBuiltList( + notif(0), + notif(2) + ); + } + + @Test + public void testStabilizeGroupsAllowsGroupingAllNewNotifications() { + // GIVEN visual stability manager doesn't allow any group changes + mListBuilder.setNotifStabilityManager( + new TestableStabilityManager().setAllowGroupChanges(false)); + + // WHEN we run the pipeline with all new notification groups + addGroupChild(0, PACKAGE_1, GROUP_1); + addGroupSummary(1, PACKAGE_1, GROUP_1); + addGroupChild(2, PACKAGE_1, GROUP_1); + addGroupSummary(3, PACKAGE_2, GROUP_2); + addGroupChild(4, PACKAGE_2, GROUP_2); + addGroupChild(5, PACKAGE_2, GROUP_2); + + dispatchBuild(); + + // THEN all notifications are grouped since they're all new + verifyBuiltList( + group( + summary(1), + child(0), + child(2) + ), + group( + summary(3), + child(4), + child(5) + ) + ); + } + + + @Test + public void testStabilizeGroupsAllowsGroupingOnlyNewNotifications() { + // GIVEN one group child without a summary yet + addGroupChild(0, PACKAGE_1, GROUP_1); + + dispatchBuild(); + + // GIVEN visual stability manager doesn't allow any group changes + mListBuilder.setNotifStabilityManager( + new TestableStabilityManager().setAllowGroupChanges(false)); + + // WHEN we run the pipeline with the addition of a group summary & child + addGroupSummary(1, PACKAGE_1, GROUP_1); + addGroupChild(2, PACKAGE_1, GROUP_1); + addGroupSummary(3, PACKAGE_2, GROUP_2); + addGroupChild(4, PACKAGE_2, GROUP_2); + addGroupChild(5, PACKAGE_2, GROUP_2); + + dispatchBuild(); + + // THEN all notifications are top-level and the summary doesn't show yet + // because group changes aren't allowed by the stability manager + verifyBuiltList( + notif(0), + group( + summary(3), + child(4), + child(5) + ), + notif(2) + ); + } + + @Test + public void testStabilizeGroupsHidesGroupSummary() { + // GIVEN one group child with a summary + addGroupChild(0, PACKAGE_1, GROUP_1); + addGroupSummary(1, PACKAGE_1, GROUP_1); + + dispatchBuild(); // group summary is hidden because it needs at least 2 children to group + + // GIVEN visual stability manager doesn't allow any group changes + mListBuilder.setNotifStabilityManager( + new TestableStabilityManager().setAllowGroupChanges(false)); + + // WHEN we run the pipeline with the addition of a child + addGroupChild(2, PACKAGE_1, GROUP_1); + + dispatchBuild(); + + // THEN the children notifications are top-level and the summary still doesn't show yet + // because group changes aren't allowed by the stability manager + verifyBuiltList( + notif(0), + notif(2) + ); + } + + @Test + public void testStabilizeGroupsDelayedSummaryRendersAllNotifsTopLevel() { + // GIVEN group children posted without a summary + addGroupChild(0, PACKAGE_1, GROUP_1); + addGroupChild(1, PACKAGE_1, GROUP_1); + addGroupChild(2, PACKAGE_1, GROUP_1); + addGroupChild(3, PACKAGE_1, GROUP_1); + + dispatchBuild(); + + // GIVEN visual stability manager doesn't allow any group changes + final TestableStabilityManager stabilityManager = + new TestableStabilityManager().setAllowGroupChanges(false); + mListBuilder.setNotifStabilityManager(stabilityManager); + + // WHEN the delayed summary is posted + addGroupSummary(4, PACKAGE_1, GROUP_1); + + dispatchBuild(); + + // THEN all entries are top-level since group changes aren't allowed + verifyBuiltList( + notif(0), + notif(1), + notif(2), + notif(3), + notif(4) + ); + + // WHEN visual stability manager allows group changes again + stabilityManager.setAllowGroupChanges(true); + stabilityManager.invalidateList(); + + // THEN entries are grouped + verifyBuiltList( + group( + summary(4), + child(0), + child(1), + child(2), + child(3) + ) + ); + } + + @Test + public void testStabilizeSectionDisallowsNewSection() { + // GIVEN one non-default sections + final NotifSectioner originalSectioner = new PackageSectioner(PACKAGE_1); + mListBuilder.setSectioners(List.of(originalSectioner)); + + // GIVEN notifications that's sectioned by sectioner1 + addNotif(0, PACKAGE_1); + dispatchBuild(); + assertEquals(originalSectioner, mEntrySet.get(0).getSection().getSectioner()); + + // WHEN section changes aren't allowed + final TestableStabilityManager stabilityManager = + new TestableStabilityManager().setAllowSectionChanges(false); + mListBuilder.setNotifStabilityManager(stabilityManager); + + // WHEN we try to change the section + final NotifSectioner newSectioner = new PackageSectioner(PACKAGE_1); + mListBuilder.setSectioners(List.of(newSectioner, originalSectioner)); + dispatchBuild(); + + // THEN the section remains the same since section changes aren't allowed + assertEquals(originalSectioner, mEntrySet.get(0).getSection().getSectioner()); + + // WHEN section changes are allowed again + stabilityManager.setAllowSectionChanges(true); + stabilityManager.invalidateList(); + + // THEN the section updates + assertEquals(newSectioner, mEntrySet.get(0).getSection().getSectioner()); + } + @Test public void testDispatchListOnBeforeSort() { // GIVEN a registered OnBeforeSortListener @@ -999,8 +1192,8 @@ public class ShadeListBuilderTest extends SysuiTestCase { @Test public void testDispatchListOnBeforeRender() { // GIVEN a registered OnBeforeRenderList - RecordingOnBeforeRenderistener listener = - new RecordingOnBeforeRenderistener(); + RecordingOnBeforeRenderListener listener = + new RecordingOnBeforeRenderListener(); mListBuilder.addOnBeforeRenderListListener(listener); // GIVEN some new notifs out of order @@ -1450,7 +1643,7 @@ public class ShadeListBuilderTest extends SysuiTestCase { } } - private static class RecordingOnBeforeRenderistener + private static class RecordingOnBeforeRenderListener implements OnBeforeRenderListListener { List mEntriesReceived; @@ -1460,6 +1653,39 @@ public class ShadeListBuilderTest extends SysuiTestCase { } } + private static class TestableStabilityManager extends NotifStabilityManager { + boolean mAllowGroupChanges = true; + boolean mAllowSectionChanges = true; + + TestableStabilityManager() { + super("Test"); + } + + TestableStabilityManager setAllowGroupChanges(boolean allowGroupChanges) { + mAllowGroupChanges = allowGroupChanges; + return this; + } + + TestableStabilityManager setAllowSectionChanges(boolean allowSectionChanges) { + mAllowSectionChanges = allowSectionChanges; + return this; + } + + @Override + public void onBeginRun() { + } + + @Override + public boolean isGroupChangeAllowed(NotificationEntry entry) { + return mAllowGroupChanges; + } + + @Override + public boolean isSectionChangeAllowed(NotificationEntry entry) { + return mAllowSectionChanges; + } + } + private static final String PACKAGE_1 = "com.test1"; private static final String PACKAGE_2 = "com.test2"; private static final String PACKAGE_3 = "org.test3"; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java index 605b4d18d2f1f..4edca7dd43d94 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java @@ -295,6 +295,22 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { verifyInvalidateCalled(false); } + @Test + public void testNotSuppressingGroupChangesAnymore_invalidationCalled() { + // GIVEN visual stability is being maintained b/c panel is expanded + setPulsing(false); + setScreenOn(true); + setPanelExpanded(true); + + assertFalse(mNotifStabilityManager.isGroupChangeAllowed(mEntry)); + + // WHEN the panel isn't expanded anymore + setPanelExpanded(false); + + // invalidate is called because we were previously suppressing a group change + verifyInvalidateCalled(true); + } + @Test public void testHeadsUp_allowedToChangeGroupAndSection() { // GIVEN group + section changes disallowed