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 2baa0797b41f1..477db3fb2e3a4 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 @@ -641,28 +641,37 @@ public class ShadeListBuilder implements Dumpable { * 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(); + if (prevParent == null) { + // New entries are always allowed. + return false; + } final GroupEntry assignedParent = entry.getParent(); - if (prevParent != assignedParent - && !getStabilityManager().isGroupChangeAllowed(entry.getRepresentativeEntry())) { + if (prevParent == assignedParent) { + // Nothing to change. + return false; + } + if (prevParent != ROOT_ENTRY && prevParent.getParent() == null) { + // Previous parent was a group, which has been removed (hence, its parent is null). + // Always allow this group change, otherwise the child will remain attached to the + // removed group and be removed from the shade until visual stability ends. + return false; + } + // TODO: Rather than perform "half" of the move here and require the caller remove the child + // from the assignedParent, ideally we would have an atomic "move" operation. + if (!getStabilityManager().isGroupChangeAllowed(entry.getRepresentativeEntry())) { entry.getAttachState().getSuppressedChanges().setParent(assignedParent); entry.setParent(prevParent); if (prevParent == ROOT_ENTRY) { out.add(entry); - } else if (prevParent != null) { + } else { prevParent.addChild(entry); if (!mGroups.containsKey(prevParent.getKey())) { mGroups.put(prevParent.getKey(), prevParent); } } - return true; } - return false; } 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 f470715b92c4c..dd0c7589d7a1b 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 @@ -1172,6 +1172,29 @@ public class ShadeListBuilderTest extends SysuiTestCase { ).inOrder(); // Order is a bonus because this listener is before sort } + @Test + public void testStabilizeGroupsAlwaysAllowsGroupChangeFromDeletedGroupToRoot() { + // GIVEN a group w/ summary and two children + addGroupSummary(0, PACKAGE_1, GROUP_1); + addGroupChild(1, PACKAGE_1, GROUP_1); + addGroupChild(2, PACKAGE_1, GROUP_1); + dispatchBuild(); + + // GIVEN visual stability manager doesn't allow any group changes + mStabilityManager.setAllowGroupChanges(false); + + // WHEN we run the pipeline with the summary and one child removed + mEntrySet.remove(2); + mEntrySet.remove(0); + dispatchBuild(); + + // THEN all that remains is the one child at top-level, despite no group change allowed by + // visual stability manager. + verifyBuiltList( + notif(0) + ); + } + @Test public void testStabilizeGroupsDoesNotAllowGroupingExistingNotifications() { // GIVEN one group child without a summary yet