Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

New Pipeline: Fix groups not grouping when shade is open.

Browse Source 
* Add logging when the reason an entry's group is pruned changes.
* Remove the incorrectly implemented ListEntry.hasBeenAttachedBefore(), and replace with ListEntry.wasAttachedInPreviousPass() which is sufficient.
* Fix ShadeListBuilder "change" logs containing many rows that didn't really change just because stableIndex changes frequently.
* Fix ShadeListBuilderLogger errors with the buildId not appearing.

Bug: 205099899
Test: atest PreparationCoordinatorTest ShadeListBuilderTest
Change-Id: Icf6bbcdce557ec6bb5f18da734caca89649f177a
This commit is contained in:
Jeff DeCew
2021-11-24 21:38:54 +00:00
parent 99e1b4485b
commit 3850e10f2f
10 changed files with 475 additions and 175 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListAttachState.kt
View File

@@ -30,11 +30,6 @@ data class ListAttachState private constructor(
*/
var parent: GroupEntry?,
/**
* Identifies the notification order in the entire notification list
*/
var stableIndex: Int = -1,
/**
* The section that this ListEntry was sorted into. If the child of the group, this will be the
* parent's section. Null if not attached to the list.
@@ -52,6 +47,11 @@ data class ListAttachState private constructor(
*/
var promoter: NotifPromoter?,
/**
* If an entry's group was pruned from the list by NotifPipeline logic, the reason is here.
*/
var groupPruneReason: String?,
/**
* If the [VisualStabilityManager] is suppressing group or section changes for this entry,
* suppressedChanges will contain the new parent or section that we would have assigned to
@@ -60,14 +60,23 @@ data class ListAttachState private constructor(
var suppressedChanges: SuppressedAttachState
) {
/**
* Identifies the notification order in the entire notification list.
* NOTE: this property is intentionally excluded from equals calculation (by not making it a
* constructor arg) because its value changes based on the presence of other members in the
* list, rather than anything having to do with this entry's attachment.
*/
var stableIndex: Int = -1
/** Copies the state of another instance. */
fun clone(other: ListAttachState) {
parent = other.parent
stableIndex = other.stableIndex
section = other.section
excludingFilter = other.excludingFilter
promoter = other.promoter
groupPruneReason = other.groupPruneReason
suppressedChanges.clone(other.suppressedChanges)
stableIndex = other.stableIndex
}
/** Resets back to a "clean" state (the same as created by the factory method) */
@@ -76,20 +85,22 @@ data class ListAttachState private constructor(
section = null
excludingFilter = null
promoter = null
stableIndex = -1
groupPruneReason = null
suppressedChanges.reset()
stableIndex = -1
}
companion object {
@JvmStatic
fun create(): ListAttachState {
return ListAttachState(
null,
-1,
null,
null,
null,
SuppressedAttachState.create())
null,
null,
null,
null,
null,
SuppressedAttachState.create()
)
}
}
}

10
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ListEntry.java
View File

@@ -31,8 +31,6 @@ public abstract class ListEntry {
private final String mKey;
private final long mCreationTime;
int mFirstAddedIteration = -1;
private final ListAttachState mPreviousAttachState = ListAttachState.create();
private final ListAttachState mAttachState = ListAttachState.create();
@@ -94,14 +92,6 @@ public abstract class ListEntry {
return mPreviousAttachState;
}
/**
* True if this entry has been attached to the shade at least once in its lifetime (it may not
* currently be attached).
*/
public boolean hasBeenAttachedBefore() {
return mFirstAddedIteration != -1;
}
/**
* Stores the current attach state into {@link #getPreviousAttachState()}} and then starts a
* fresh attach state (all entries will be null/default-initialized).

242
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilder.java
View File

@@ -16,6 +16,8 @@
package com.android.systemui.statusbar.notification.collection;
import static com.android.internal.util.Preconditions.checkArgument;
import static com.android.internal.util.Preconditions.checkState;
import static com.android.systemui.statusbar.notification.collection.GroupEntry.ROOT_ENTRY;
import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_BUILD_STARTED;
import static com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState.STATE_FINALIZE_FILTERING;
@@ -34,6 +36,7 @@ import android.annotation.MainThread;
import android.annotation.Nullable;
import android.os.Trace;
import android.util.ArrayMap;
import android.util.ArraySet;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
@@ -50,6 +53,7 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.OnBefo
import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeTransformGroupsListener;
import com.android.systemui.statusbar.notification.collection.listbuilder.PipelineState;
import com.android.systemui.statusbar.notification.collection.listbuilder.ShadeListBuilderLogger;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.DefaultNotifStabilityManager;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Invalidator;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifComparator;
import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.NotifFilter;
@@ -72,6 +76,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import javax.inject.Inject;
@@ -103,7 +108,7 @@ public class ShadeListBuilder implements Dumpable {
private final List<NotifFilter> mNotifFinalizeFilters = new ArrayList<>();
private final List<NotifComparator> mNotifComparators = new ArrayList<>();
private final List<NotifSection> mNotifSections = new ArrayList<>();
@Nullable private NotifStabilityManager mNotifStabilityManager;
private NotifStabilityManager mNotifStabilityManager;
private final List<OnBeforeTransformGroupsListener> mOnBeforeTransformGroupsListeners =
new ArrayList<>();
@@ -228,7 +233,7 @@ public class ShadeListBuilder implements Dumpable {
mNotifSections.add(new NotifSection(DEFAULT_SECTIONER, mNotifSections.size()));
}
void setNotifStabilityManager(NotifStabilityManager notifStabilityManager) {
void setNotifStabilityManager(@NonNull NotifStabilityManager notifStabilityManager) {
Assert.isMainThread();
mPipelineState.requireState(STATE_IDLE);
@@ -244,6 +249,14 @@ public class ShadeListBuilder implements Dumpable {
mNotifStabilityManager.setInvalidationListener(this::onReorderingAllowedInvalidated);
}
@NonNull
private NotifStabilityManager getStabilityManager() {
if (mNotifStabilityManager == null) {
return DefaultNotifStabilityManager.INSTANCE;
}
return mNotifStabilityManager;
}
void setComparators(List<NotifComparator> comparators) {
Assert.isMainThread();
mPipelineState.requireState(STATE_IDLE);
@@ -460,10 +473,6 @@ public class ShadeListBuilder implements Dumpable {
for (NotificationEntry entry : mAllEntries) {
entry.beginNewAttachState();
if (entry.mFirstAddedIteration == -1) {
entry.mFirstAddedIteration = mIterationCount;
}
}
mNotifList.clear();
@@ -519,7 +528,6 @@ public class ShadeListBuilder implements Dumpable {
GroupEntry group = mGroups.get(topLevelKey);
if (group == null) {
group = new GroupEntry(topLevelKey, mSystemClock.uptimeMillis());
group.mFirstAddedIteration = mIterationCount;
mGroups.put(topLevelKey, group);
}
if (group.getParent() == null) {
@@ -569,7 +577,7 @@ public class ShadeListBuilder implements Dumpable {
}
private void stabilizeGroupingNotifs(List<ListEntry> topLevelList) {
if (mNotifStabilityManager == null) {
if (getStabilityManager().isEveryChangeAllowed()) {
return;
}
Trace.beginSection("ShadeListBuilder.stabilizeGroupingNotifs");
@@ -612,7 +620,7 @@ public class ShadeListBuilder implements Dumpable {
final GroupEntry prevParent = entry.getPreviousAttachState().getParent();
final GroupEntry assignedParent = entry.getParent();
if (prevParent != assignedParent
&& !mNotifStabilityManager.isGroupChangeAllowed(entry.getRepresentativeEntry())) {
&& !getStabilityManager().isGroupChangeAllowed(entry.getRepresentativeEntry())) {
entry.getAttachState().getSuppressedChanges().setParent(assignedParent);
entry.setParent(prevParent);
if (prevParent == ROOT_ENTRY) {
@@ -655,56 +663,60 @@ public class ShadeListBuilder implements Dumpable {
private void pruneIncompleteGroups(List<ListEntry> shadeList) {
Trace.beginSection("ShadeListBuilder.pruneIncompleteGroups");
// Any group which lost a child on this run to stability is exempt from being pruned or
// having its summary promoted, regardless of how many children it has
Set<String> groupsWithChildrenLostToStability =
getGroupsWithChildrenLostToStability(shadeList);
for (int i = 0; i < shadeList.size(); i++) {
final ListEntry tle = shadeList.get(i);
if (tle instanceof GroupEntry) {
final GroupEntry group = (GroupEntry) tle;
final List<NotificationEntry> children = group.getRawChildren();
final boolean hasSummary = group.getSummary() != null;
if (group.getSummary() != null && children.size() == 0) {
NotificationEntry summary = group.getSummary();
summary.setParent(ROOT_ENTRY);
// The list may be sorted; replace the group with the summary, in its place
shadeList.set(i, summary);
group.setSummary(null);
annulAddition(group, shadeList);
if (hasSummary && children.size() == 0) {
if (groupsWithChildrenLostToStability.contains(group.getKey())) {
// This group lost a child on this run to stability, so it is exempt from
// having its summary promoted to the top level, so prune it.
// It has no children, so it will just vanish.
pruneGroupAtIndexAndPromoteAnyChildren(shadeList, group, i);
} else {
// For any other summary with no children, promote the summary.
pruneGroupAtIndexAndPromoteSummary(shadeList, group, i);
}
i--; // The node we visited is gone, so be sure to visit this index again.
} else if (group.getSummary() == null
|| children.size() < MIN_CHILDREN_FOR_GROUP) {
} else if (!hasSummary) {
// If the group doesn't provide a summary, ignore it and add
// any children it may have directly to top-level.
pruneGroupAtIndexAndPromoteAnyChildren(shadeList, group, i);
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
i--; // The node we visited is gone, so be sure to visit this index again.
} else if (children.size() < MIN_CHILDREN_FOR_GROUP) {
// This group has a summary and insufficient, but nonzero children.
checkState(hasSummary, "group must have summary at this point");
checkState(!children.isEmpty(), "empty group should have been promoted");
if (groupsWithChildrenLostToStability.contains(group.getKey())) {
// This group lost a child on this run to stability, so it is exempt from
// the "min children" requirement; keep it around in case more children are
// added before changes are allowed again.
group.getAttachState().getSuppressedChanges().setWasPruneSuppressed(true);
continue;
}
if (group.wasAttachedInPreviousPass()
&& !getStabilityManager().isGroupChangeAllowed(group.getSummary())) {
checkState(!children.isEmpty(), "empty group should have been pruned");
// 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
// The group is too small, ignore it and add
// its children (if any) directly to top-level.
shadeList.remove(i);
if (group.getSummary() != null) {
final NotificationEntry summary = group.getSummary();
group.setSummary(null);
annulAddition(summary, shadeList);
}
for (int j = 0; j < children.size(); j++) {
final NotificationEntry child = children.get(j);
child.setParent(ROOT_ENTRY);
// The list may be sorted, so add the children in order where the group was.
shadeList.add(i + j, child);
}
children.clear();
annulAddition(group, shadeList);
pruneGroupAtIndexAndPromoteAnyChildren(shadeList, group, i);
i--; // The node we visited is gone, so be sure to visit this index again.
}
@@ -713,6 +725,99 @@ public class ShadeListBuilder implements Dumpable {
Trace.endSection();
}
private void pruneGroupAtIndexAndPromoteSummary(List<ListEntry> shadeList,
GroupEntry group, int index) {
// Validate that the group has no children
checkArgument(group.getChildren().isEmpty(), "group should have no children");
NotificationEntry summary = group.getSummary();
summary.setParent(ROOT_ENTRY);
// The list may be sorted; replace the group with the summary, in its place
ListEntry oldEntry = shadeList.set(index, summary);
// Validate that the replaced entry was the group entry
checkState(oldEntry == group);
group.setSummary(null);
annulAddition(group, shadeList);
summary.getAttachState().setGroupPruneReason(
"SUMMARY with no children @ " + mPipelineState.getStateName());
}
private void pruneGroupAtIndexAndPromoteAnyChildren(List<ListEntry> shadeList,
GroupEntry group, int index) {
// REMOVE the GroupEntry at this index
ListEntry oldEntry = shadeList.remove(index);
// Validate that the replaced entry was the group entry
checkState(oldEntry == group);
List<NotificationEntry> children = group.getRawChildren();
boolean hasSummary = group.getSummary() != null;
// Remove the group summary, if present, and leave detached.
if (hasSummary) {
final NotificationEntry summary = group.getSummary();
group.setSummary(null);
annulAddition(summary, shadeList);
summary.getAttachState().setGroupPruneReason(
"SUMMARY with too few children @ " + mPipelineState.getStateName());
}
// Promote any children
if (!children.isEmpty()) {
// create the reason we will report on the child for why its group was pruned.
String childReason = hasSummary
? ("CHILD with " + (children.size() - 1) + " siblings @ "
+ mPipelineState.getStateName())
: ("CHILD with no summary @ " + mPipelineState.getStateName());
// Remove children from the group and add them to the shadeList.
for (int j = 0; j < children.size(); j++) {
final NotificationEntry child = children.get(j);
child.setParent(ROOT_ENTRY);
child.getAttachState().setGroupPruneReason(requireNonNull(childReason));
}
// The list may be sorted, so add the children in order where the group was.
shadeList.addAll(index, children);
children.clear();
}
annulAddition(group, shadeList);
}
/**
* Collect the keys of any groups which have already lost a child to stability this run.
*
* If stability is being enforced, then {@link #stabilizeGroupingNotifs(List)} might have
* detached some children from their groups and left them at the top level because the child was
* previously attached at the top level. Doing so would set the
* {@link SuppressedAttachState#getParent() suppressed parent} for the current attach state.
*
* If we've already removed a child from this group, we don't want to remove any more children
* from the group (even if that would leave only a single notification in the group) because
* that could cascade over multiple runs and allow a large group of notifications all show up as
* top level (ungrouped) notifications.
*/
@NonNull
private Set<String> getGroupsWithChildrenLostToStability(List<ListEntry> shadeList) {
if (getStabilityManager().isEveryChangeAllowed()) {
return Collections.emptySet();
}
ArraySet<String> groupsWithChildrenLostToStability = new ArraySet<>();
for (int i = 0; i < shadeList.size(); i++) {
final ListEntry tle = shadeList.get(i);
final GroupEntry suppressedParent =
tle.getAttachState().getSuppressedChanges().getParent();
if (suppressedParent != null) {
// This top-level-entry was supposed to be attached to this group,
// so mark the group as having lost a child to stability.
groupsWithChildrenLostToStability.add(suppressedParent.getKey());
}
}
return groupsWithChildrenLostToStability;
}
/**
* If a ListEntry was added to the shade list and then later removed (e.g. because it was a
* group that was broken up), this method will erase any bookkeeping traces of that addition
@@ -727,10 +832,9 @@ public class ShadeListBuilder implements Dumpable {
// lot of them), it will put the system into an inconsistent state. So we check all of them
// here.
if (entry.getParent() == null || entry.mFirstAddedIteration == -1) {
if (entry.getParent() == null) {
throw new IllegalStateException(
"Cannot nullify addition of " + entry.getKey() + ": no such addition. ("
+ entry.getParent() + " " + entry.mFirstAddedIteration + ")");
"Cannot nullify addition of " + entry.getKey() + ": no parent.");
}
if (entry.getParent() == ROOT_ENTRY) {
@@ -771,9 +875,6 @@ public class ShadeListBuilder implements Dumpable {
entry.setParent(null);
entry.getAttachState().setSection(null);
entry.getAttachState().setPromoter(null);
if (entry.mFirstAddedIteration == mIterationCount) {
entry.mFirstAddedIteration = -1;
}
}
private void assignSections() {
@@ -804,12 +905,12 @@ public class ShadeListBuilder implements Dumpable {
assignIndexes(mNotifList);
// Check for suppressed order changes
if (!mNotifStabilityManager.isEveryChangeAllowed()) {
if (!getStabilityManager().isEveryChangeAllowed()) {
mForceReorderable = true;
boolean isSorted = isSorted(mNotifList, mTopLevelComparator);
mForceReorderable = false;
if (!isSorted) {
mNotifStabilityManager.onEntryReorderSuppressed();
getStabilityManager().onEntryReorderSuppressed();
}
}
Trace.endSection();
@@ -897,12 +998,26 @@ public class ShadeListBuilder implements Dumpable {
curr.getParent());
}
if (curr.getSuppressedChanges().getSection() != null) {
mLogger.logSectionChangeSuppressed(
mIterationCount,
curr.getSuppressedChanges().getSection(),
curr.getSection());
}
if (curr.getSuppressedChanges().getWasPruneSuppressed()) {
mLogger.logGroupPruningSuppressed(
mIterationCount,
curr.getParent());
}
if (!Objects.equals(curr.getGroupPruneReason(), prev.getGroupPruneReason())) {
mLogger.logPrunedReasonChanged(
mIterationCount,
prev.getGroupPruneReason(),
curr.getGroupPruneReason());
}
if (curr.getExcludingFilter() != prev.getExcludingFilter()) {
mLogger.logFilterChanged(
mIterationCount,
@@ -927,20 +1042,11 @@ public class ShadeListBuilder implements Dumpable {
prev.getSection(),
curr.getSection());
}
if (curr.getSuppressedChanges().getSection() != null) {
mLogger.logSectionChangeSuppressed(
mIterationCount,
curr.getSuppressedChanges().getSection(),
curr.getSection());
}
}
}
private void onBeginRun() {
if (mNotifStabilityManager != null) {
mNotifStabilityManager.onBeginRun();
}
getStabilityManager().onBeginRun();
}
private void cleanupPluggables() {
@@ -953,9 +1059,7 @@ public class ShadeListBuilder implements Dumpable {
mNotifSections.get(i).getSectioner().onCleanup();
}
if (mNotifStabilityManager != null) {
callOnCleanup(List.of(mNotifStabilityManager));
}
callOnCleanup(List.of(getStabilityManager()));
}
private void callOnCleanup(List<? extends Pluggable<?>> pluggables) {
@@ -1015,7 +1119,7 @@ public class ShadeListBuilder implements Dumpable {
private boolean mForceReorderable = false;
private boolean canReorder(ListEntry entry) {
return mForceReorderable || mNotifStabilityManager.isEntryReorderingAllowed(entry);
return mForceReorderable || getStabilityManager().isEntryReorderingAllowed(entry);
}
private boolean applyFilters(NotificationEntry entry, long now, List<NotifFilter> filters) {
@@ -1064,12 +1168,10 @@ public class ShadeListBuilder implements Dumpable {
NotifSection finalSection = newSection;
// have we seen this entry before and are we changing its section?
if (mNotifStabilityManager != null
&& entry.wasAttachedInPreviousPass()
&& newSection != prevAttachState.getSection()) {
if (entry.wasAttachedInPreviousPass() && newSection != prevAttachState.getSection()) {
// are section changes allowed?
if (!mNotifStabilityManager.isSectionChangeAllowed(entry.getRepresentativeEntry())) {
if (!getStabilityManager().isSectionChangeAllowed(entry.getRepresentativeEntry())) {
// record the section that we wanted to change to
entry.getAttachState().getSuppressedChanges().setSection(newSection);

5
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinator.java
View File

@@ -385,7 +385,7 @@ public class PreparationCoordinator implements Coordinator {
}
private boolean shouldWaitForGroupToInflate(GroupEntry group, long now) {
if (group == GroupEntry.ROOT_ENTRY || group.hasBeenAttachedBefore()) {
if (group == GroupEntry.ROOT_ENTRY || group.wasAttachedInPreviousPass()) {
return false;
}
if (isBeyondGroupInitializationWindow(group, now)) {
@@ -397,11 +397,12 @@ public class PreparationCoordinator implements Coordinator {
return true;
}
for (NotificationEntry child : group.getChildren()) {
if (mInflatingNotifs.contains(child) && !child.hasBeenAttachedBefore()) {
if (mInflatingNotifs.contains(child) && !child.wasAttachedInPreviousPass()) {
mLogger.logDelayingGroupRelease(group.getKey(), child.getKey());
return true;
}
}
mLogger.logDoneWaitingForGroupInflation(group.getKey());
return false;
}

10
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/PreparationCoordinatorLogger.kt
View File

@@ -41,11 +41,19 @@ class PreparationCoordinatorLogger @Inject constructor(
})
}
fun logDoneWaitingForGroupInflation(groupKey: String) {
buffer.log(TAG, LogLevel.DEBUG, {
str1 = groupKey
}, {
"Finished inflating all members of group $str1, releasing group"
})
}
fun logGroupInflationTookTooLong(groupKey: String) {
buffer.log(TAG, LogLevel.WARNING, {
str1 = groupKey
}, {
"Group inflation took too long far $str1, releasing children early"
"Group inflation took too long for $str1, releasing children early"
})
}

6
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java
View File

@@ -118,7 +118,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
}
@Override
public boolean isGroupChangeAllowed(NotificationEntry entry) {
public boolean isGroupChangeAllowed(@NonNull NotificationEntry entry) {
final boolean isGroupChangeAllowedForEntry =
mReorderingAllowed || mHeadsUpManager.isAlerting(entry.getKey());
mIsSuppressingGroupChange |= !isGroupChangeAllowedForEntry;
@@ -126,7 +126,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
}
@Override
public boolean isSectionChangeAllowed(NotificationEntry entry) {
public boolean isSectionChangeAllowed(@NonNull NotificationEntry entry) {
final boolean isSectionChangeAllowedForEntry =
mReorderingAllowed
|| mHeadsUpManager.isAlerting(entry.getKey())
@@ -138,7 +138,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable {
}
@Override
public boolean isEntryReorderingAllowed(ListEntry section) {
public boolean isEntryReorderingAllowed(@NonNull ListEntry section) {
return mReorderingAllowed;
}

38
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/PipelineState.java
View File

@@ -18,6 +18,8 @@ package com.android.systemui.statusbar.notification.collection.listbuilder;
import android.annotation.IntDef;
import androidx.annotation.NonNull;
import com.android.systemui.statusbar.notification.collection.ShadeListBuilder;
import java.lang.annotation.Retention;
@@ -35,6 +37,7 @@ public class PipelineState {
return state == mState;
}
/** Get the current state's integer representation */
public @StateName int getState() {
return mState;
}
@@ -75,6 +78,41 @@ public class PipelineState {
}
}
/** Get the current state's string representation */
@NonNull
public String getStateName() {
return getStateName(mState);
}
/** Get the given state's string representation */
@NonNull
public static String getStateName(@StateName int state) {
switch (state) {
case STATE_IDLE:
return "STATE_IDLE";
case STATE_BUILD_STARTED:
return "STATE_BUILD_STARTED";
case STATE_RESETTING:
return "STATE_RESETTING";
case STATE_PRE_GROUP_FILTERING:
return "STATE_PRE_GROUP_FILTERING";
case STATE_GROUPING:
return "STATE_GROUPING";
case STATE_TRANSFORMING:
return "STATE_TRANSFORMING";
case STATE_GROUP_STABILIZING:
return "STATE_GROUP_STABILIZING";
case STATE_SORTING:
return "STATE_SORTING";
case STATE_FINALIZE_FILTERING:
return "STATE_FINALIZE_FILTERING";
case STATE_FINALIZING:
return "STATE_FINALIZING";
default:
return "STATE:" + state;
}
}
public static final int STATE_IDLE = 0;
public static final int STATE_BUILD_STARTED = 1;
public static final int STATE_RESETTING = 2;

50
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/ShadeListBuilderLogger.kt
View File

@@ -37,9 +37,9 @@ class ShadeListBuilderLogger @Inject constructor(
})
}
fun logEndBuildList(iterationCount: Int, topLevelEntries: Int, numChildren: Int) {
fun logEndBuildList(buildId: Int, topLevelEntries: Int, numChildren: Int) {
buffer.log(TAG, INFO, {
long1 = iterationCount.toLong()
long1 = buildId.toLong()
int1 = topLevelEntries
int2 = numChildren
}, {
@@ -112,21 +112,21 @@ class ShadeListBuilderLogger @Inject constructor(
fun logDuplicateSummary(buildId: Int, groupKey: String, existingKey: String, newKey: String) {
buffer.log(TAG, WARNING, {
int1 = buildId
long1 = buildId.toLong()
str1 = groupKey
str2 = existingKey
str3 = newKey
}, {
"""(Build $int1) Duplicate summary for group "$str1": "$str2" vs. "$str3""""
"""(Build $long1) Duplicate summary for group "$str1": "$str2" vs. "$str3""""
})
}
fun logDuplicateTopLevelKey(buildId: Int, topLevelKey: String) {
buffer.log(TAG, WARNING, {
int1 = buildId
long1 = buildId.toLong()
str1 = topLevelKey
}, {
"(Build $int1) Duplicate top-level key: $str1"
"(Build $long1) Duplicate top-level key: $str1"
})
}
@@ -137,7 +137,7 @@ class ShadeListBuilderLogger @Inject constructor(
newParent: GroupEntry?
) {
buffer.log(TAG, INFO, {
int1 = buildId
long1 = buildId.toLong()
str1 = key
str2 = prevParent?.key
str3 = newParent?.key
@@ -153,22 +153,22 @@ class ShadeListBuilderLogger @Inject constructor(
"MODIFIED (ATTACHED)"
}
"(Build $int1) $action {$str1}"
"(Build $long1) $action {$str1}"
})
}
fun logParentChanged(buildId: Int, prevParent: GroupEntry?, newParent: GroupEntry?) {
buffer.log(TAG, INFO, {
int1 = buildId
long1 = buildId.toLong()
str1 = prevParent?.key
str2 = newParent?.key
}, {
if (str1 == null && str2 != null) {
"(Build $int1) Parent is {$str2}"
"(Build $long1) Parent is {$str2}"
} else if (str1 != null && str2 == null) {
"(Build $int1) Parent was {$str1}"
"(Build $long1) Parent was {$str1}"
} else {
"(Build $int1) Reparent: {$str1} -> {$str2}"
"(Build $long1) Reparent: {$str1} -> {$str2}"
}
})
}
@@ -179,7 +179,7 @@ class ShadeListBuilderLogger @Inject constructor(
keepingParent: GroupEntry?
) {
buffer.log(TAG, INFO, {
int1 = buildId
long1 = buildId.toLong()
str1 = suppressedParent?.key
str2 = keepingParent?.key
}, {
@@ -192,24 +192,38 @@ class ShadeListBuilderLogger @Inject constructor(
keepingParent: GroupEntry?
) {
buffer.log(TAG, INFO, {
int1 = buildId
long1 = buildId.toLong()
str1 = keepingParent?.key
}, {
"(Build $long1) Group pruning suppressed; keeping parent '$str1'"
})
}
fun logPrunedReasonChanged(
buildId: Int,
prevReason: String?,
newReason: String?
) {
buffer.log(TAG, INFO, {
long1 = buildId.toLong()
str1 = prevReason
str2 = newReason
}, {
"(Build $long1) Pruned reason changed: $str1 -> $str2"
})
}
fun logFilterChanged(
buildId: Int,
prevFilter: NotifFilter?,
newFilter: NotifFilter?
) {
buffer.log(TAG, INFO, {
int1 = buildId
long1 = buildId.toLong()
str1 = prevFilter?.name
str2 = newFilter?.name
}, {
"(Build $int1) Filter changed: $str1 -> $str2"
"(Build $long1) Filter changed: $str1 -> $str2"
})
}
@@ -219,11 +233,11 @@ class ShadeListBuilderLogger @Inject constructor(
newPromoter: NotifPromoter?
) {
buffer.log(TAG, INFO, {
int1 = buildId
long1 = buildId.toLong()
str1 = prevPromoter?.name
str2 = newPromoter?.name
}, {
"(Build $int1) Promoter changed: $str1 -> $str2"
"(Build $long1) Promoter changed: $str1 -> $str2"
})
}

51
packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifStabilityManager.java → packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/listbuilder/pluggable/NotifStabilityManager.kt
View File

@@ -13,11 +13,10 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.systemui.statusbar.notification.collection.listbuilder.pluggable
package com.android.systemui.statusbar.notification.collection.listbuilder.pluggable;
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.ListEntry
import com.android.systemui.statusbar.notification.collection.NotificationEntry
/**
* Pluggable for participating in notif stabilization. In particular, suppressing group and
@@ -26,40 +25,38 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry;
* The stability manager should be invalidated when previously suppressed a group or
* section change is now allowed.
*/
public abstract class NotifStabilityManager extends Pluggable<NotifStabilityManager> {
protected NotifStabilityManager(String name) {
super(name);
}
abstract class NotifStabilityManager protected constructor(name: String) :
Pluggable<NotifStabilityManager>(name) {
/**
* Called at the beginning of every pipeline run to perform any necessary cleanup from the
* previous run.
*/
public abstract void onBeginRun();
abstract fun onBeginRun()
/**
* Returns whether this notification can currently change groups/parents.
* Per iteration of the notification pipeline, locally stores this information until the next
* run of the pipeline. When this method returns true, it's expected that a group change for
* run of the pipeline. When this method returns false, it's expected that a group change for
* this entry is being suppressed.
*/
public abstract boolean isGroupChangeAllowed(NotificationEntry entry);
abstract fun isGroupChangeAllowed(entry: NotificationEntry): Boolean
/**
* Returns whether this notification entry can currently change sections.
* Per iteration of the notification pipeline, locally stores this information until the next
* run of the pipeline. When this method returns true, it's expected that a section change is
* run of the pipeline. When this method returns false, it's expected that a section change is
* being suppressed.
*/
public abstract boolean isSectionChangeAllowed(NotificationEntry entry);
abstract fun isSectionChangeAllowed(entry: NotificationEntry): Boolean
/**
* Is a notification entry is allowed be reordered
* @param entry
* @return if can re-order
* Returns whether this list entry is allowed to be reordered within its section.
* Unlike [isGroupChangeAllowed] or [isSectionChangeAllowed], this method is called on every
* entry, so an implementation may not assume that returning false means an order change is
* being suppressed. However, if an order change is suppressed, that will be reported to ths
* implementation by calling [onEntryReorderSuppressed] after ordering is complete.
*/
public abstract boolean isEntryReorderingAllowed(ListEntry entry);
abstract fun isEntryReorderingAllowed(entry: ListEntry): Boolean
/**
* Called by the pipeline to determine if every call to the other stability methods would
@@ -68,11 +65,21 @@ public abstract class NotifStabilityManager extends Pluggable<NotifStabilityMana
*
* @return true if all other methods will return true for any parameters.
*/
public abstract boolean isEveryChangeAllowed();
abstract fun isEveryChangeAllowed(): Boolean
/**
* Called by the pipeline to inform the stability manager that an entry reordering was indeed
* suppressed as the result of a previous call to {@link #isEntryReorderingAllowed(ListEntry)}.
* suppressed as the result of a previous call to [.isEntryReorderingAllowed].
*/
public abstract void onEntryReorderSuppressed();
abstract fun onEntryReorderSuppressed()
}
/** The default, no-op instance of the stability manager which always allows all changes */
object DefaultNotifStabilityManager : NotifStabilityManager("DefaultNotifStabilityManager") {
override fun onBeginRun() {}
override fun isGroupChangeAllowed(entry: NotificationEntry): Boolean = true
override fun isSectionChangeAllowed(entry: NotificationEntry): Boolean = true
override fun isEntryReorderingAllowed(entry: ListEntry): Boolean = true
override fun isEveryChangeAllowed(): Boolean = true
override fun onEntryReorderSuppressed() {}
}

201
packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/ShadeListBuilderTest.java
View File

@@ -114,6 +114,7 @@ public class ShadeListBuilderTest extends SysuiTestCase {
private List<NotificationEntry> mEntrySet = new ArrayList<>();
private List<ListEntry> mBuiltList;
private TestableStabilityManager mStabilityManager;
private TestableNotifFilter mFinalizeFilter;
private Map<String, Integer> mNextIdMap = new ArrayMap<>();
private int mNextRank = 0;
@@ -136,6 +137,8 @@ public class ShadeListBuilderTest extends SysuiTestCase {
mStabilityManager = spy(new TestableStabilityManager());
mListBuilder.setNotifStabilityManager(mStabilityManager);
mFinalizeFilter = spy(new TestableNotifFilter());
mListBuilder.addFinalizeFilter(mFinalizeFilter);
Mockito.verify(mNotifCollection).setBuildListener(mBuildListenerCaptor.capture());
mReadyForBuildListener = Objects.requireNonNull(mBuildListenerCaptor.getValue());
@@ -408,7 +411,6 @@ public class ShadeListBuilderTest extends SysuiTestCase {
// THEN the summary has a null parent and an unset firstAddedIteration
assertNull(mEntrySet.get(1).getParent());
assertEquals(-1, mEntrySet.get(1).mFirstAddedIteration);
}
@Test
@@ -1037,7 +1039,7 @@ public class ShadeListBuilderTest extends SysuiTestCase {
}
@Test
public void testStabilizeGroupsDoesNotAllowGrouping() {
public void testStabilizeGroupsDoesNotAllowGroupingExistingNotifications() {
// GIVEN one group child without a summary yet
addGroupChild(0, PACKAGE_1, GROUP_1);
@@ -1056,7 +1058,10 @@ public class ShadeListBuilderTest extends SysuiTestCase {
// because group changes aren't allowed by the stability manager
verifyBuiltList(
notif(0),
notif(2)
group(
summary(1),
child(2)
)
);
}
@@ -1110,11 +1115,13 @@ public class ShadeListBuilderTest extends SysuiTestCase {
dispatchBuild();
// THEN all notifications are top-level and the summary doesn't show yet
// because group changes aren't allowed by the stability manager
// THEN first notification stays top-level but the other notifications are grouped.
verifyBuiltList(
notif(0),
notif(2),
group(
summary(1),
child(2)
),
group(
summary(3),
child(4),
@@ -1208,6 +1215,140 @@ public class ShadeListBuilderTest extends SysuiTestCase {
);
}
@Test
public void testStabilityIsolationAllowsGroupToHaveSingleChild() {
// GIVEN a group with only one child was already drawn
addGroupSummary(0, PACKAGE_1, GROUP_1);
addGroupChild(1, PACKAGE_1, GROUP_1);
dispatchBuild();
// NOTICE that the group is pruned and the child is moved to the top level
verifyBuiltList(
notif(1) // group with only one child is promoted
);
// WHEN another child is added while group changes are disabled.
mStabilityManager.setAllowGroupChanges(false);
addGroupChild(2, PACKAGE_1, GROUP_1);
dispatchBuild();
// THEN the new child should be added to the group
verifyBuiltList(
group(
summary(0),
child(2)
),
notif(1)
);
}
@Test
public void testStabilityIsolationExemptsGroupWithFinalizeFilteredChildFromShowingSummary() {
// GIVEN a group with only one child was already drawn
addGroupSummary(0, PACKAGE_1, GROUP_1);
addGroupChild(1, PACKAGE_1, GROUP_1);
dispatchBuild();
// NOTICE that the group is pruned and the child is moved to the top level
verifyBuiltList(
notif(1) // group with only one child is promoted
);
// WHEN another child is added but still filtered while group changes are disabled.
mStabilityManager.setAllowGroupChanges(false);
mFinalizeFilter.mIndicesToFilter.add(2);
addGroupChild(2, PACKAGE_1, GROUP_1);
dispatchBuild();
// THEN the new child should be shown without the summary
verifyBuiltList(
notif(1) // previously promoted child
);
}
@Test
public void testStabilityIsolationOfRemovedChildDoesNotExemptGroupFromPrune() {
// GIVEN a group with only one child was already drawn
addGroupSummary(0, PACKAGE_1, GROUP_1);
addGroupChild(1, PACKAGE_1, GROUP_1);
dispatchBuild();
// NOTICE that the group is pruned and the child is moved to the top level
verifyBuiltList(
notif(1) // group with only one child is promoted
);
// WHEN a new child is added and the old one gets filtered while group changes are disabled.
mStabilityManager.setAllowGroupChanges(false);
mFinalizeFilter.mIndicesToFilter.add(1);
addGroupChild(2, PACKAGE_1, GROUP_1);
dispatchBuild();
// THEN the new child should be shown without a group
verifyBuiltList(
notif(2) // previously promoted child
);
}
@Test
public void testFinalizeFilteredSummaryPromotesChildren() {
// GIVEN a group with only one child was already drawn
addGroupSummary(0, PACKAGE_1, GROUP_1);
addGroupChild(1, PACKAGE_1, GROUP_1);
addGroupChild(2, PACKAGE_1, GROUP_1);
// WHEN the parent is filtered out at the finalize step
mFinalizeFilter.mIndicesToFilter.add(0);
dispatchBuild();
// THEN the children should be promoted to the top level
verifyBuiltList(
notif(1),
notif(2)
);
}
@Test
public void testFinalizeFilteredChildrenPromotesSummary() {
// GIVEN a group with only one child was already drawn
addGroupSummary(0, PACKAGE_1, GROUP_1);
addGroupChild(1, PACKAGE_1, GROUP_1);
addGroupChild(2, PACKAGE_1, GROUP_1);
// WHEN the parent is filtered out at the finalize step
mFinalizeFilter.mIndicesToFilter.add(1);
mFinalizeFilter.mIndicesToFilter.add(2);
dispatchBuild();
// THEN the children should be promoted to the top level
verifyBuiltList(
notif(0)
);
}
@Test
public void testFinalizeFilteredChildPromotesSibling() {
// GIVEN a group with only one child was already drawn
addGroupSummary(0, PACKAGE_1, GROUP_1);
addGroupChild(1, PACKAGE_1, GROUP_1);
addGroupChild(2, PACKAGE_1, GROUP_1);
// WHEN the parent is filtered out at the finalize step
mFinalizeFilter.mIndicesToFilter.add(1);
dispatchBuild();
// THEN the children should be promoted to the top level
verifyBuiltList(
notif(2)
);
}
@Test
public void testBrokenGroupNotificationOrdering() {
// GIVEN two group children with different sections & without a summary yet
@@ -1227,30 +1368,6 @@ public class ShadeListBuilderTest extends SysuiTestCase {
);
}
@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
mStabilityManager.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
@@ -1269,13 +1386,12 @@ public class ShadeListBuilderTest extends SysuiTestCase {
dispatchBuild();
// THEN all entries are top-level since group changes aren't allowed
// THEN all entries are top-level, but summary is suppressed
verifyBuiltList(
notif(0),
notif(1),
notif(2),
notif(3),
notif(4)
notif(3)
);
// WHEN visual stability manager allows group changes again
@@ -1907,6 +2023,19 @@ public class ShadeListBuilderTest extends SysuiTestCase {
}
}
private class TestableNotifFilter extends NotifFilter {
ArrayList<Integer> mIndicesToFilter = new ArrayList<>();
protected TestableNotifFilter() {
super("TestFilter");
}
@Override
public boolean shouldFilterOut(@NonNull NotificationEntry entry, long now) {
return mIndicesToFilter.stream().anyMatch(i -> notif(i).entry == entry);
}
}
private static class TestableStabilityManager extends NotifStabilityManager {
boolean mAllowGroupChanges = true;
boolean mAllowSectionChanges = true;
@@ -1937,17 +2066,17 @@ public class ShadeListBuilderTest extends SysuiTestCase {
}
@Override
public boolean isGroupChangeAllowed(NotificationEntry entry) {
public boolean isGroupChangeAllowed(@NonNull NotificationEntry entry) {
return mAllowGroupChanges;
}
@Override
public boolean isSectionChangeAllowed(NotificationEntry entry) {
public boolean isSectionChangeAllowed(@NonNull NotificationEntry entry) {
return mAllowSectionChanges;
}
@Override
public boolean isEntryReorderingAllowed(ListEntry entry) {
public boolean isEntryReorderingAllowed(@NonNull ListEntry entry) {
return mAllowEntryReodering;
}