Merge "Suppress group changes in group pruning"

This commit is contained in:
TreeHugger Robot
2020-09-11 16:52:06 +00:00
committed by Android (Google) Code Review
8 changed files with 353 additions and 28 deletions

View File

@@ -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;
}
}

View File

@@ -513,28 +513,66 @@ public class ShadeListBuilder implements Dumpable {
}
}
private void stabilizeGroupingNotifs(List<ListEntry> list) {
private void stabilizeGroupingNotifs(List<ListEntry> 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<NotificationEntry> 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<ListEntry> 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<ListEntry> 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?

View File

@@ -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)
}
}
}

View File

@@ -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;
}

View File

@@ -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'"
})
}

View File

@@ -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

View File

@@ -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<ListEntry> 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";

View File

@@ -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