diff options
| author | 2021-04-20 20:27:51 -0400 | |
|---|---|---|
| committer | 2021-05-11 15:45:55 -0400 | |
| commit | c8242de17af167efa716c01dbbb127f931811e76 (patch) | |
| tree | 281949902e9aa93adb93eaa460b8f8de47523527 | |
| parent | ec6c6835ca1bfa24c53e13da58483764b14e0c4d (diff) | |
Fix VIP conversations alerting incorrectly.
Transfer alerts (HUN/pulse) from the group to priority conversations which are isolated from that group.
Note: In cases where the alert is transferred from a group with 2+ children to the priority, there can be very small flickering because the group is inflated synchronously, so it starts displaying before the message is received to transfer the alert to the priority conversation, which then needs to be inflated asynchronously.
Fixes: 185680162
Test: atest NotificationGroupAlertTransferHelperTest NotificationGroupManagerLegacyTest
Test: Using WA, receive messages from 2+ people, where some but not all of those conversations have been set to Priority. Test different numbers of people in each category (single vs. multile) and ensure that whenever an alert displays it shows the message that was just received.
Cherry-picks commit 64c85ac6f5f20d031a7e192366b8335fab0253c2
This re-lands a change that was reverted due to the following, however no causation could be found, and subsequent tests have not reproduced the bug.
Bug: 187009701
Change-Id: I4b0f9249572011d783204d973fa11c7bf6a9ddda
5 files changed, 946 insertions, 132 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java index 7f31fddbfb6c..5437ce63475e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListener.java @@ -18,7 +18,6 @@ package com.android.systemui.statusbar; import static com.android.systemui.statusbar.RemoteInputController.processForRemoteInput; import static com.android.systemui.statusbar.notification.NotificationEntryManager.UNDEFINED_DISMISS_REASON; -import static com.android.systemui.statusbar.phone.StatusBar.DEBUG; import android.annotation.NonNull; import android.annotation.SuppressLint; @@ -35,6 +34,7 @@ import android.util.Log; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.statusbar.dagger.StatusBarModule; import com.android.systemui.statusbar.phone.NotificationListenerWithPlugins; +import com.android.systemui.statusbar.phone.StatusBar; import java.util.ArrayList; import java.util.List; @@ -46,6 +46,7 @@ import java.util.List; @SuppressLint("OverrideAbstract") public class NotificationListener extends NotificationListenerWithPlugins { private static final String TAG = "NotificationListener"; + private static final boolean DEBUG = StatusBar.DEBUG; private final Context mContext; private final NotificationManager mNotificationManager; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/NotificationGroupManagerLegacy.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/NotificationGroupManagerLegacy.java index d6356de5ea51..f40f24a935c4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/NotificationGroupManagerLegacy.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/legacy/NotificationGroupManagerLegacy.java @@ -16,7 +16,9 @@ package com.android.systemui.statusbar.notification.collection.legacy; +import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.Notification; import android.service.notification.StatusBarNotification; import android.util.ArraySet; import android.util.Log; @@ -31,6 +33,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManager; import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager; import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier; +import com.android.systemui.statusbar.phone.StatusBar; import com.android.systemui.statusbar.policy.HeadsUpManager; import com.android.systemui.statusbar.policy.OnHeadsUpChangedListener; import com.android.wm.shell.bubbles.Bubbles; @@ -39,10 +42,12 @@ import java.io.FileDescriptor; import java.io.PrintWriter; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.TreeSet; import javax.inject.Inject; @@ -58,13 +63,21 @@ import dagger.Lazy; public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, StateListener, GroupMembershipManager, GroupExpansionManager, Dumpable { - private static final String TAG = "NotificationGroupManager"; + private static final String TAG = "NotifGroupManager"; + private static final boolean DEBUG = StatusBar.DEBUG; + private static final boolean SPEW = StatusBar.SPEW; + /** + * The maximum amount of time (in ms) between the posting of notifications that can be + * considered part of the same update batch. + */ + private static final long POST_BATCH_MAX_AGE = 5000; private final HashMap<String, NotificationGroup> mGroupMap = new HashMap<>(); private final ArraySet<OnGroupExpansionChangeListener> mExpansionChangeListeners = new ArraySet<>(); private final ArraySet<OnGroupChangeListener> mGroupChangeListeners = new ArraySet<>(); private final Lazy<PeopleNotificationIdentifier> mPeopleNotificationIdentifier; private final Optional<Bubbles> mBubblesOptional; + private final EventBuffer mEventBuffer = new EventBuffer(); private int mBarState = -1; private HashMap<String, StatusBarNotification> mIsolatedEntries = new HashMap<>(); private HeadsUpManager mHeadsUpManager; @@ -134,8 +147,14 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, * When we want to remove an entry from being tracked for grouping */ public void onEntryRemoved(NotificationEntry removed) { + if (SPEW) { + Log.d(TAG, "onEntryRemoved: entry=" + removed); + } onEntryRemovedInternal(removed, removed.getSbn()); - mIsolatedEntries.remove(removed.getKey()); + StatusBarNotification oldSbn = mIsolatedEntries.remove(removed.getKey()); + if (oldSbn != null) { + updateSuppression(mGroupMap.get(oldSbn.getGroupKey())); + } } /** @@ -162,6 +181,9 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, // the close future. See b/23676310 for reference. return; } + if (SPEW) { + Log.d(TAG, "onEntryRemovedInternal: entry=" + removed + " group=" + group.groupKey); + } if (isGroupChild(removed.getKey(), isGroup, isGroupSummary)) { group.children.remove(removed.getKey()); } else { @@ -182,6 +204,9 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, * Notify the group manager that a new entry was added */ public void onEntryAdded(final NotificationEntry added) { + if (SPEW) { + Log.d(TAG, "onEntryAdded: entry=" + added); + } updateIsolation(added); onEntryAddedInternal(added); } @@ -195,13 +220,16 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, String groupKey = getGroupKey(sbn); NotificationGroup group = mGroupMap.get(groupKey); if (group == null) { - group = new NotificationGroup(); + group = new NotificationGroup(groupKey); mGroupMap.put(groupKey, group); for (OnGroupChangeListener listener : mGroupChangeListeners) { listener.onGroupCreated(group, groupKey); } } + if (SPEW) { + Log.d(TAG, "onEntryAddedInternal: entry=" + added + " group=" + group.groupKey); + } if (isGroupChild) { NotificationEntry existing = group.children.get(added.getKey()); if (existing != null && existing != added) { @@ -213,9 +241,11 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, + " added removed" + added.isRowRemoved(), new Throwable()); } group.children.put(added.getKey(), added); + addToPostBatchHistory(group, added); updateSuppression(group); } else { group.summary = added; + addToPostBatchHistory(group, added); group.expanded = added.areChildrenExpanded(); updateSuppression(group); if (!group.children.isEmpty()) { @@ -231,6 +261,27 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, } } + private void addToPostBatchHistory(NotificationGroup group, @Nullable NotificationEntry entry) { + if (entry == null) { + return; + } + boolean didAdd = group.postBatchHistory.add(new PostRecord(entry)); + if (didAdd) { + trimPostBatchHistory(group.postBatchHistory); + } + } + + /** remove all history that's too old to be in the batch. */ + private void trimPostBatchHistory(@NonNull TreeSet<PostRecord> postBatchHistory) { + if (postBatchHistory.size() <= 1) { + return; + } + long batchStartTime = postBatchHistory.last().postTime - POST_BATCH_MAX_AGE; + while (!postBatchHistory.isEmpty() && postBatchHistory.first().postTime < batchStartTime) { + postBatchHistory.pollFirst(); + } + } + private void onEntryBecomingChild(NotificationEntry entry) { updateIsolation(entry); } @@ -239,6 +290,9 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, if (group == null) { return; } + NotificationEntry prevAlertOverride = group.alertOverride; + group.alertOverride = getPriorityConversationAlertOverride(group); + int childCount = 0; boolean hasBubbles = false; for (NotificationEntry entry : group.children.values()) { @@ -255,18 +309,150 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, group.suppressed = group.summary != null && !group.expanded && (childCount == 1 || (childCount == 0 - && group.summary.getSbn().getNotification().isGroupSummary() - && (hasIsolatedChildren(group) || hasBubbles))); - if (prevSuppressed != group.suppressed) { - for (OnGroupChangeListener listener : mGroupChangeListeners) { - if (!mIsUpdatingUnchangedGroup) { - listener.onGroupSuppressionChanged(group, group.suppressed); - listener.onGroupsChanged(); + && group.summary.getSbn().getNotification().isGroupSummary() + && (hasIsolatedChildren(group) || hasBubbles))); + + boolean alertOverrideChanged = prevAlertOverride != group.alertOverride; + boolean suppressionChanged = prevSuppressed != group.suppressed; + if (alertOverrideChanged || suppressionChanged) { + if (DEBUG && alertOverrideChanged) { + Log.d(TAG, "updateSuppression: alertOverride was=" + prevAlertOverride + + " now=" + group.alertOverride + " group:\n" + group); + } + if (DEBUG && suppressionChanged) { + Log.d(TAG, + "updateSuppression: suppressed changed to " + group.suppressed + + " group:\n" + group); + } + if (!mIsUpdatingUnchangedGroup) { + if (alertOverrideChanged) { + mEventBuffer.notifyAlertOverrideChanged(group, prevAlertOverride); + } + if (suppressionChanged) { + for (OnGroupChangeListener listener : mGroupChangeListeners) { + listener.onGroupSuppressionChanged(group, group.suppressed); + } + } + mEventBuffer.notifyGroupsChanged(); + } else { + if (DEBUG) { + Log.d(TAG, group + " did not notify listeners of above change(s)"); } } } } + /** + * Finds the isolated logical child of this group which is should be alerted instead. + * + * Notifications from priority conversations are isolated from their groups to make them more + * prominent, however apps may post these with a GroupAlertBehavior that has the group receiving + * the alert. This would lead to the group alerting even though the conversation that was + * updated was not actually a part of that group. This method finds the best priority + * conversation in this situation, if there is one, so they can be set as the alertOverride of + * the group. + * + * @param group the group to check + * @return the entry which should receive the alert instead of the group, if any. + */ + @Nullable + private NotificationEntry getPriorityConversationAlertOverride(NotificationGroup group) { + // GOAL: if there is a priority child which wouldn't alert based on its groupAlertBehavior, + // but which should be alerting (because priority conversations are isolated), find it. + if (group == null || group.summary == null) { + if (SPEW) { + Log.d(TAG, "getPriorityConversationAlertOverride: null group or summary"); + } + return null; + } + if (isIsolated(group.summary.getKey())) { + if (SPEW) { + Log.d(TAG, "getPriorityConversationAlertOverride: isolated group"); + } + return null; + } + + // Precondiions: + // * Only necessary when all notifications in the group use GROUP_ALERT_SUMMARY + // * Only necessary when at least one notification in the group is on a priority channel + if (group.summary.getSbn().getNotification().getGroupAlertBehavior() + != Notification.GROUP_ALERT_SUMMARY) { + if (SPEW) { + Log.d(TAG, "getPriorityConversationAlertOverride: summary != GROUP_ALERT_SUMMARY"); + } + return null; + } + + // Get the important children first, copy the keys for the final importance check, + // then add the non-isolated children to the map for unified lookup. + HashMap<String, NotificationEntry> children = getImportantConversations(group); + if (children == null || children.isEmpty()) { + if (SPEW) { + Log.d(TAG, "getPriorityConversationAlertOverride: no important conversations"); + } + return null; + } + HashSet<String> importantChildKeys = new HashSet<>(children.keySet()); + children.putAll(group.children); + + // Ensure all children have GROUP_ALERT_SUMMARY + for (NotificationEntry child : children.values()) { + if (child.getSbn().getNotification().getGroupAlertBehavior() + != Notification.GROUP_ALERT_SUMMARY) { + if (SPEW) { + Log.d(TAG, "getPriorityConversationAlertOverride: " + + "child != GROUP_ALERT_SUMMARY"); + } + return null; + } + } + + // Create a merged post history from all the children + TreeSet<PostRecord> combinedHistory = new TreeSet<>(group.postBatchHistory); + for (String importantChildKey : importantChildKeys) { + NotificationGroup importantChildGroup = mGroupMap.get(importantChildKey); + combinedHistory.addAll(importantChildGroup.postBatchHistory); + } + trimPostBatchHistory(combinedHistory); + + // This is a streamlined implementation of the following idea: + // * From the subset of notifications in the latest 'batch' of updates. A batch is: + // * Notifs posted less than POST_BATCH_MAX_AGE before the most recently posted. + // * Only including notifs newer than the second-to-last post of any notification. + // * Find the newest child in the batch -- the with the largest 'when' value. + // * If the newest child is a priority conversation, set that as the override. + HashSet<String> batchKeys = new HashSet<>(); + long newestChildWhen = -1; + NotificationEntry newestChild = null; + // Iterate backwards through the post history, tracking the child with the smallest sort key + for (PostRecord record : combinedHistory.descendingSet()) { + if (batchKeys.contains(record.key)) { + // Once you see a notification again, the batch has ended + break; + } + batchKeys.add(record.key); + NotificationEntry child = children.get(record.key); + if (child != null) { + long childWhen = child.getSbn().getNotification().when; + if (newestChild == null || childWhen > newestChildWhen) { + newestChildWhen = childWhen; + newestChild = child; + } + } + } + if (newestChild != null && importantChildKeys.contains(newestChild.getKey())) { + if (SPEW) { + Log.d(TAG, "getPriorityConversationAlertOverride: result=" + newestChild); + } + return newestChild; + } + if (SPEW) { + Log.d(TAG, "getPriorityConversationAlertOverride: result=null, newestChild=" + + newestChild); + } + return null; + } + private boolean hasIsolatedChildren(NotificationGroup group) { return getNumberOfIsolatedChildren(group.summary.getSbn().getGroupKey()) != 0; } @@ -281,12 +467,33 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, return count; } + @Nullable + private HashMap<String, NotificationEntry> getImportantConversations(NotificationGroup group) { + String groupKey = group.summary.getSbn().getGroupKey(); + HashMap<String, NotificationEntry> result = null; + for (StatusBarNotification sbn : mIsolatedEntries.values()) { + if (sbn.getGroupKey().equals(groupKey)) { + NotificationEntry entry = mGroupMap.get(sbn.getKey()).summary; + if (isImportantConversation(entry)) { + if (result == null) { + result = new HashMap<>(); + } + result.put(sbn.getKey(), entry); + } + } + } + return result; + } + /** * Update an entry's group information * @param entry notification entry to update * @param oldNotification previous notification info before this update */ public void onEntryUpdated(NotificationEntry entry, StatusBarNotification oldNotification) { + if (SPEW) { + Log.d(TAG, "onEntryUpdated: entry=" + entry); + } onEntryUpdated(entry, oldNotification.getGroupKey(), oldNotification.isGroup(), oldNotification.getNotification().isGroupSummary()); } @@ -325,7 +532,17 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, * Whether the given notification is the summary of a group that is being suppressed */ public boolean isSummaryOfSuppressedGroup(StatusBarNotification sbn) { - return isGroupSuppressed(getGroupKey(sbn)) && sbn.getNotification().isGroupSummary(); + return sbn.getNotification().isGroupSummary() && isGroupSuppressed(getGroupKey(sbn)); + } + + /** + * If the given notification is a summary, get the group for it. + */ + public NotificationGroup getGroupForSummary(StatusBarNotification sbn) { + if (sbn.getNotification().isGroupSummary()) { + return mGroupMap.get(getGroupKey(sbn)); + } + return null; } private boolean isOnlyChild(StatusBarNotification sbn) { @@ -545,9 +762,7 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, if (!sbn.isGroup() || sbn.getNotification().isGroupSummary()) { return false; } - int peopleNotificationType = - mPeopleNotificationIdentifier.get().getPeopleNotificationType(entry); - if (peopleNotificationType == PeopleNotificationIdentifier.TYPE_IMPORTANT_PERSON) { + if (isImportantConversation(entry)) { return true; } if (mHeadsUpManager != null && !mHeadsUpManager.isAlerting(entry.getKey())) { @@ -560,18 +775,25 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, || isGroupNotFullyVisible(notificationGroup)); } + private boolean isImportantConversation(NotificationEntry entry) { + int peopleNotificationType = + mPeopleNotificationIdentifier.get().getPeopleNotificationType(entry); + return peopleNotificationType == PeopleNotificationIdentifier.TYPE_IMPORTANT_PERSON; + } + /** * Isolate a notification from its group so that it visually shows as its own group. * * @param entry the notification to isolate */ private void isolateNotification(NotificationEntry entry) { - StatusBarNotification sbn = entry.getSbn(); - + if (SPEW) { + Log.d(TAG, "isolateNotification: entry=" + entry); + } // We will be isolated now, so lets update the groups onEntryRemovedInternal(entry, entry.getSbn()); - mIsolatedEntries.put(sbn.getKey(), sbn); + mIsolatedEntries.put(entry.getKey(), entry.getSbn()); onEntryAddedInternal(entry); // We also need to update the suppression of the old group, because this call comes @@ -588,6 +810,14 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, * Update the isolation of an entry, splitting it from the group. */ public void updateIsolation(NotificationEntry entry) { + // We need to buffer a few events because we do isolation changes in 3 steps: + // removeInternal, update mIsolatedEntries, addInternal. This means that often the + // alertOverride will update on the removal, however processing the event in that case can + // cause problems because the mIsolatedEntries map is not in its final state, so the event + // listener may be unable to correctly determine the true state of the group. By delaying + // the alertOverride change until after the add phase, we can ensure that listeners only + // have to handle a consistent state. + mEventBuffer.startBuffering(); boolean isIsolated = isIsolated(entry.getSbn().getKey()); if (shouldIsolate(entry)) { if (!isIsolated) { @@ -596,6 +826,7 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, } else if (isIsolated) { stopIsolatingNotification(entry); } + mEventBuffer.flushAndStopBuffering(); } /** @@ -604,15 +835,15 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, * @param entry the notification to un-isolate */ private void stopIsolatingNotification(NotificationEntry entry) { - StatusBarNotification sbn = entry.getSbn(); - if (isIsolated(sbn.getKey())) { - // not isolated anymore, we need to update the groups - onEntryRemovedInternal(entry, entry.getSbn()); - mIsolatedEntries.remove(sbn.getKey()); - onEntryAddedInternal(entry); - for (OnGroupChangeListener listener : mGroupChangeListeners) { - listener.onGroupsChanged(); - } + if (SPEW) { + Log.d(TAG, "stopIsolatingNotification: entry=" + entry); + } + // not isolated anymore, we need to update the groups + onEntryRemovedInternal(entry, entry.getSbn()); + mIsolatedEntries.remove(entry.getKey()); + onEntryAddedInternal(entry); + for (OnGroupChangeListener listener : mGroupChangeListeners) { + listener.onGroupsChanged(); } } @@ -648,33 +879,154 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, } /** + * A record of a notification being posted, containing the time of the post and the key of the + * notification entry. These are stored in a TreeSet by the NotificationGroup and used to + * calculate a batch of notifications. + */ + public static class PostRecord implements Comparable<PostRecord> { + public final long postTime; + public final String key; + + /** constructs a record containing the post time and key from the notification entry */ + public PostRecord(@NonNull NotificationEntry entry) { + this.postTime = entry.getSbn().getPostTime(); + this.key = entry.getKey(); + } + + @Override + public int compareTo(PostRecord o) { + int postTimeComparison = Long.compare(this.postTime, o.postTime); + return postTimeComparison == 0 + ? String.CASE_INSENSITIVE_ORDER.compare(this.key, o.key) + : postTimeComparison; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PostRecord that = (PostRecord) o; + return postTime == that.postTime && key.equals(that.key); + } + + @Override + public int hashCode() { + return Objects.hash(postTime, key); + } + } + + /** * Represents a notification group in the notification shade. */ public static class NotificationGroup { + public final String groupKey; public final HashMap<String, NotificationEntry> children = new HashMap<>(); + public final TreeSet<PostRecord> postBatchHistory = new TreeSet<>(); public NotificationEntry summary; public boolean expanded; /** * Is this notification group suppressed, i.e its summary is hidden */ public boolean suppressed; + /** + * The child (which is isolated from this group) to which the alert should be transferred, + * due to priority conversations. + */ + public NotificationEntry alertOverride; + + NotificationGroup(String groupKey) { + this.groupKey = groupKey; + } @Override public String toString() { - String result = " summary:\n " - + (summary != null ? summary.getSbn() : "null") - + (summary != null && summary.getDebugThrowable() != null - ? Log.getStackTraceString(summary.getDebugThrowable()) - : ""); - result += "\n children size: " + children.size(); + StringBuilder sb = new StringBuilder(); + sb.append(" groupKey: ").append(groupKey); + sb.append("\n summary:"); + appendEntry(sb, summary); + sb.append("\n children size: ").append(children.size()); for (NotificationEntry child : children.values()) { - result += "\n " + child.getSbn() - + (child.getDebugThrowable() != null - ? Log.getStackTraceString(child.getDebugThrowable()) - : ""); + appendEntry(sb, child); + } + sb.append("\n alertOverride:"); + appendEntry(sb, alertOverride); + sb.append("\n summary suppressed: ").append(suppressed); + return sb.toString(); + } + + private void appendEntry(StringBuilder sb, NotificationEntry entry) { + sb.append("\n ").append(entry != null ? entry.getSbn() : "null"); + if (entry != null && entry.getDebugThrowable() != null) { + sb.append(Log.getStackTraceString(entry.getDebugThrowable())); + } + } + } + + /** + * This class is a toggleable buffer for a subset of events of {@link OnGroupChangeListener}. + * When buffering, instead of notifying the listeners it will set internal state that will allow + * it to notify listeners of those events later + */ + private class EventBuffer { + private final HashMap<String, NotificationEntry> mOldAlertOverrideByGroup = new HashMap<>(); + private boolean mIsBuffering = false; + private boolean mDidGroupsChange = false; + + void notifyAlertOverrideChanged(NotificationGroup group, + NotificationEntry oldAlertOverride) { + if (mIsBuffering) { + // The value in this map is the override before the event. If there is an entry + // already in the map, then we are effectively coalescing two events, which means + // we need to preserve the original initial value. + mOldAlertOverrideByGroup.putIfAbsent(group.groupKey, oldAlertOverride); + } else { + for (OnGroupChangeListener listener : mGroupChangeListeners) { + listener.onGroupAlertOverrideChanged(group, oldAlertOverride, + group.alertOverride); + } + } + } + + void notifyGroupsChanged() { + if (mIsBuffering) { + mDidGroupsChange = true; + } else { + for (OnGroupChangeListener listener : mGroupChangeListeners) { + listener.onGroupsChanged(); + } + } + } + + void startBuffering() { + mIsBuffering = true; + } + + void flushAndStopBuffering() { + // stop buffering so that we can call our own helpers + mIsBuffering = false; + // alert all group alert override changes for groups that were not removed + for (Map.Entry<String, NotificationEntry> entry : mOldAlertOverrideByGroup.entrySet()) { + NotificationGroup group = mGroupMap.get(entry.getKey()); + if (group == null) { + // The group can be null if this alertOverride changed before the group was + // permanently removed, meaning that there's no guarantee that listeners will + // that field clear. + continue; + } + NotificationEntry oldAlertOverride = entry.getValue(); + if (group.alertOverride == oldAlertOverride) { + // If the final alertOverride equals the initial, it means we coalesced two + // events which undid the change, so we can drop it entirely. + continue; + } + notifyAlertOverrideChanged(group, oldAlertOverride); + } + mOldAlertOverrideByGroup.clear(); + // alert that groups changed + if (mDidGroupsChange) { + notifyGroupsChanged(); + mDidGroupsChange = false; } - result += "\n summary suppressed: " + suppressed; - return result; } } @@ -714,6 +1066,18 @@ public class NotificationGroupManagerLegacy implements OnHeadsUpChangedListener, boolean suppressed) {} /** + * The alert override of a group has changed. + * + * @param group the group that has changed + * @param oldAlertOverride the previous notification to which the group's alerts were sent + * @param newAlertOverride the notification to which the group's alerts should now be sent + */ + default void onGroupAlertOverrideChanged( + NotificationGroup group, + @Nullable NotificationEntry oldAlertOverride, + @Nullable NotificationEntry newAlertOverride) {} + + /** * A group of children just received a summary notification and should therefore become * children of it. * diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java index 3181f520dca2..9787a9446019 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelper.java @@ -22,12 +22,12 @@ import android.app.Notification; import android.os.SystemClock; import android.service.notification.StatusBarNotification; import android.util.ArrayMap; +import android.util.Log; import com.android.internal.statusbar.NotificationVisibility; import com.android.systemui.Dependency; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.plugins.statusbar.StatusBarStateController.StateListener; -import com.android.systemui.statusbar.AlertingNotificationManager; import com.android.systemui.statusbar.notification.NotificationEntryListener; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; @@ -41,17 +41,21 @@ import com.android.systemui.statusbar.policy.HeadsUpManager; import com.android.systemui.statusbar.policy.OnHeadsUpChangedListener; import java.util.ArrayList; +import java.util.List; import java.util.Objects; /** * A helper class dealing with the alert interactions between {@link NotificationGroupManagerLegacy} * and {@link HeadsUpManager}. In particular, this class deals with keeping - * the correct notification in a group alerting based off the group suppression. + * the correct notification in a group alerting based off the group suppression and alertOverride. */ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedListener, StateListener { private static final long ALERT_TRANSFER_TIMEOUT = 300; + private static final String TAG = "NotifGroupAlertTransfer"; + private static final boolean DEBUG = StatusBar.DEBUG; + private static final boolean SPEW = StatusBar.SPEW; /** * The list of entries containing group alert metadata for each group. Keyed by group key. @@ -142,41 +146,98 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis @Override public void onGroupSuppressionChanged(NotificationGroup group, boolean suppressed) { - if (suppressed) { - if (mHeadsUpManager.isAlerting(group.summary.getKey())) { - handleSuppressedSummaryAlerted(group.summary, mHeadsUpManager); + if (DEBUG) { + Log.d(TAG, "!! onGroupSuppressionChanged: group.summary=" + group.summary + + " suppressed=" + suppressed); + } + NotificationEntry oldAlertOverride = group.alertOverride; + onGroupChanged(group, oldAlertOverride); + } + + @Override + public void onGroupAlertOverrideChanged(NotificationGroup group, + @Nullable NotificationEntry oldAlertOverride, + @Nullable NotificationEntry newAlertOverride) { + if (DEBUG) { + Log.d(TAG, "!! onGroupAlertOverrideChanged: group.summary=" + group.summary + + " oldAlertOverride=" + oldAlertOverride + + " newAlertOverride=" + newAlertOverride); + } + onGroupChanged(group, oldAlertOverride); + } + }; + + /** + * Called when either the suppressed or alertOverride fields of the group changed + * + * @param group the group which changed + * @param oldAlertOverride the previous value of group.alertOverride + */ + private void onGroupChanged(NotificationGroup group, + NotificationEntry oldAlertOverride) { + // Group summary can be null if we are no longer suppressed because the summary was + // removed. In that case, we don't need to alert the summary. + if (group.summary == null) { + if (DEBUG) { + Log.d(TAG, "onGroupChanged: summary is null"); + } + return; + } + if (group.suppressed || group.alertOverride != null) { + checkForForwardAlertTransfer(group.summary, oldAlertOverride); + } else { + if (DEBUG) { + Log.d(TAG, "onGroupChanged: maybe transfer back"); + } + GroupAlertEntry groupAlertEntry = mGroupAlertEntries.get(mGroupManager.getGroupKey( + group.summary.getSbn())); + // Group is no longer suppressed or overridden. + // We should check if we need to transfer the alert back to the summary. + if (groupAlertEntry.mAlertSummaryOnNextAddition) { + if (!mHeadsUpManager.isAlerting(group.summary.getKey())) { + alertNotificationWhenPossible(group.summary); } + groupAlertEntry.mAlertSummaryOnNextAddition = false; } else { - // Group summary can be null if we are no longer suppressed because the summary was - // removed. In that case, we don't need to alert the summary. - if (group.summary == null) { - return; - } - GroupAlertEntry groupAlertEntry = mGroupAlertEntries.get(mGroupManager.getGroupKey( - group.summary.getSbn())); - // Group is no longer suppressed. We should check if we need to transfer the alert - // back to the summary now that it's no longer suppressed. - if (groupAlertEntry.mAlertSummaryOnNextAddition) { - if (!mHeadsUpManager.isAlerting(group.summary.getKey())) { - alertNotificationWhenPossible(group.summary, mHeadsUpManager); - } - groupAlertEntry.mAlertSummaryOnNextAddition = false; - } else { - checkShouldTransferBack(groupAlertEntry); - } + checkShouldTransferBack(groupAlertEntry); } } - }; + } @Override public void onHeadsUpStateChanged(NotificationEntry entry, boolean isHeadsUp) { - onAlertStateChanged(entry, isHeadsUp, mHeadsUpManager); + if (DEBUG) { + Log.d(TAG, "!! onHeadsUpStateChanged: entry=" + entry + " isHeadsUp=" + isHeadsUp); + } + if (isHeadsUp && entry.getSbn().getNotification().isGroupSummary()) { + // a group summary is alerting; trigger the forward transfer checks + checkForForwardAlertTransfer(entry, /* oldAlertOverride */ null); + } } - private void onAlertStateChanged(NotificationEntry entry, boolean isAlerting, - AlertingNotificationManager alertManager) { - if (isAlerting && mGroupManager.isSummaryOfSuppressedGroup(entry.getSbn())) { - handleSuppressedSummaryAlerted(entry, alertManager); + /** + * Handles changes in a group's suppression or alertOverride, but where at least one of those + * conditions is still true (either the group is suppressed, the group has an alertOverride, + * or both). The method determined which kind of child needs to receive the alert, finds the + * entry currently alerting, and makes the transfer. + * + * Internally, this is handled with two main cases: the override needs the alert, or there is + * no override but the summary is suppressed (so an isolated child needs the alert). + * + * @param summary the notification entry of the summary of the logical group. + * @param oldAlertOverride the former value of group.alertOverride, before whatever event + * required us to check for for a transfer condition. + */ + private void checkForForwardAlertTransfer(NotificationEntry summary, + NotificationEntry oldAlertOverride) { + if (DEBUG) { + Log.d(TAG, "checkForForwardAlertTransfer: enter"); + } + NotificationGroup group = mGroupManager.getGroupForSummary(summary.getSbn()); + if (group != null && group.alertOverride != null) { + handleOverriddenSummaryAlerted(summary); + } else if (mGroupManager.isSummaryOfSuppressedGroup(summary.getSbn())) { + handleSuppressedSummaryAlerted(summary, oldAlertOverride); } } @@ -186,9 +247,16 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis // see as early as we can if we need to abort a transfer. @Override public void onPendingEntryAdded(NotificationEntry entry) { + if (DEBUG) { + Log.d(TAG, "!! onPendingEntryAdded: entry=" + entry); + } String groupKey = mGroupManager.getGroupKey(entry.getSbn()); GroupAlertEntry groupAlertEntry = mGroupAlertEntries.get(groupKey); - if (groupAlertEntry != null) { + if (groupAlertEntry != null && groupAlertEntry.mGroup.alertOverride == null) { + // new pending group entries require us to transfer back from the child to the + // group, but alertOverrides are only present in very limited circumstances, so + // while it's possible the group should ALSO alert, the previous detection which set + // this alertOverride won't be invalidated by this notification added to this group. checkShouldTransferBack(groupAlertEntry); } } @@ -262,43 +330,128 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis } /** - * Handles the scenario where a summary that has been suppressed is alerted. A suppressed + * Handles the scenario where a summary that has been suppressed is itself, or has a former + * alertOverride (in the form of an isolated logical child) which was alerted. A suppressed * summary should for all intents and purposes be invisible to the user and as a result should * not alert. When this is the case, it is our responsibility to pass the alert to the * appropriate child which will be the representative notification alerting for the group. * - * @param summary the summary that is suppressed and alerting - * @param alertManager the alert manager that manages the alerting summary + * @param summary the summary that is suppressed and (potentially) alerting + * @param oldAlertOverride the alertOverride before whatever event triggered this method. If + * the alert override was removed, this will be the entry that should + * be transferred back from. */ private void handleSuppressedSummaryAlerted(@NonNull NotificationEntry summary, - @NonNull AlertingNotificationManager alertManager) { - StatusBarNotification sbn = summary.getSbn(); + NotificationEntry oldAlertOverride) { + if (DEBUG) { + Log.d(TAG, "handleSuppressedSummaryAlerted: summary=" + summary); + } GroupAlertEntry groupAlertEntry = - mGroupAlertEntries.get(mGroupManager.getGroupKey(sbn)); + mGroupAlertEntries.get(mGroupManager.getGroupKey(summary.getSbn())); + if (!mGroupManager.isSummaryOfSuppressedGroup(summary.getSbn()) - || !alertManager.isAlerting(sbn.getKey()) || groupAlertEntry == null) { + if (DEBUG) { + Log.d(TAG, "handleSuppressedSummaryAlerted: invalid state"); + } + return; + } + boolean summaryIsAlerting = mHeadsUpManager.isAlerting(summary.getKey()); + boolean priorityIsAlerting = oldAlertOverride != null + && mHeadsUpManager.isAlerting(oldAlertOverride.getKey()); + if (!summaryIsAlerting && !priorityIsAlerting) { + if (DEBUG) { + Log.d(TAG, "handleSuppressedSummaryAlerted: no summary or override alerting"); + } return; } if (pendingInflationsWillAddChildren(groupAlertEntry.mGroup)) { // New children will actually be added to this group, let's not transfer the alert. + if (DEBUG) { + Log.d(TAG, "handleSuppressedSummaryAlerted: pending inflations"); + } return; } NotificationEntry child = mGroupManager.getLogicalChildren(summary.getSbn()).iterator().next(); - if (child != null) { - if (child.getRow().keepInParent() - || child.isRowRemoved() - || child.isRowDismissed()) { - // The notification is actually already removed. No need to alert it. - return; + if (summaryIsAlerting) { + if (DEBUG) { + Log.d(TAG, "handleSuppressedSummaryAlerted: transfer summary -> child"); } - if (!alertManager.isAlerting(child.getKey()) && onlySummaryAlerts(summary)) { - groupAlertEntry.mLastAlertTransferTime = SystemClock.elapsedRealtime(); + tryTransferAlertState(summary, /*from*/ summary, /*to*/ child, groupAlertEntry); + return; + } + // Summary didn't have the alert, so we're in "transfer back" territory. First, make sure + // it's not too late to transfer back, then transfer the alert from the oldAlertOverride to + // the isolated child which should receive the alert. + if (!canStillTransferBack(groupAlertEntry)) { + if (DEBUG) { + Log.d(TAG, "handleSuppressedSummaryAlerted: transfer from override: too late"); + } + return; + } + + if (DEBUG) { + Log.d(TAG, "handleSuppressedSummaryAlerted: transfer override -> child"); + } + tryTransferAlertState(summary, /*from*/ oldAlertOverride, /*to*/ child, groupAlertEntry); + } + + /** + * Checks for and handles the scenario where the given entry is the summary of a group which + * has an alertOverride, and either the summary itself or one of its logical isolated children + * is currently alerting (which happens if the summary is suppressed). + */ + private void handleOverriddenSummaryAlerted(NotificationEntry summary) { + if (DEBUG) { + Log.d(TAG, "handleOverriddenSummaryAlerted: summary=" + summary); + } + GroupAlertEntry groupAlertEntry = + mGroupAlertEntries.get(mGroupManager.getGroupKey(summary.getSbn())); + NotificationGroup group = mGroupManager.getGroupForSummary(summary.getSbn()); + if (group == null || group.alertOverride == null || groupAlertEntry == null) { + if (DEBUG) { + Log.d(TAG, "handleOverriddenSummaryAlerted: invalid state"); + } + return; + } + boolean summaryIsAlerting = mHeadsUpManager.isAlerting(summary.getKey()); + if (summaryIsAlerting) { + if (DEBUG) { + Log.d(TAG, "handleOverriddenSummaryAlerted: transfer summary -> override"); + } + tryTransferAlertState(summary, /*from*/ summary, group.alertOverride, groupAlertEntry); + return; + } + // Summary didn't have the alert, so we're in "transfer back" territory. First, make sure + // it's not too late to transfer back, then remove the alert from any of the logical + // children, and if one of them was alerting, we can alert the override. + if (!canStillTransferBack(groupAlertEntry)) { + if (DEBUG) { + Log.d(TAG, "handleOverriddenSummaryAlerted: transfer from child: too late"); + } + return; + } + List<NotificationEntry> children = mGroupManager.getLogicalChildren(summary.getSbn()); + if (children == null) { + if (DEBUG) { + Log.d(TAG, "handleOverriddenSummaryAlerted: no children"); + } + return; + } + children.remove(group.alertOverride); // do not release the alert on our desired destination + boolean releasedChild = releaseChildAlerts(children); + if (releasedChild) { + if (DEBUG) { + Log.d(TAG, "handleOverriddenSummaryAlerted: transfer child -> override"); + } + tryTransferAlertState(summary, /*from*/ null, group.alertOverride, groupAlertEntry); + } else { + if (DEBUG) { + Log.d(TAG, "handleOverriddenSummaryAlerted: no child alert released"); } - transferAlertState(summary, child, alertManager); } } @@ -307,14 +460,37 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis * immediately to have the incorrect one up as short as possible. The second should alert * when possible. * + * @param summary entry of the summary * @param fromEntry entry to transfer alert from * @param toEntry entry to transfer to - * @param alertManager alert manager for the alert type */ - private void transferAlertState(@NonNull NotificationEntry fromEntry, @NonNull NotificationEntry toEntry, - @NonNull AlertingNotificationManager alertManager) { - alertManager.removeNotification(fromEntry.getKey(), true /* releaseImmediately */); - alertNotificationWhenPossible(toEntry, alertManager); + private void tryTransferAlertState( + NotificationEntry summary, + NotificationEntry fromEntry, + NotificationEntry toEntry, + GroupAlertEntry groupAlertEntry) { + if (toEntry != null) { + if (toEntry.getRow().keepInParent() + || toEntry.isRowRemoved() + || toEntry.isRowDismissed()) { + // The notification is actually already removed. No need to alert it. + return; + } + if (!mHeadsUpManager.isAlerting(toEntry.getKey()) && onlySummaryAlerts(summary)) { + groupAlertEntry.mLastAlertTransferTime = SystemClock.elapsedRealtime(); + } + if (DEBUG) { + Log.d(TAG, "transferAlertState: fromEntry=" + fromEntry + " toEntry=" + toEntry); + } + transferAlertState(fromEntry, toEntry); + } + } + private void transferAlertState(@Nullable NotificationEntry fromEntry, + @NonNull NotificationEntry toEntry) { + if (fromEntry != null) { + mHeadsUpManager.removeNotification(fromEntry.getKey(), true /* releaseImmediately */); + } + alertNotificationWhenPossible(toEntry); } /** @@ -326,11 +502,13 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis * more children are coming. Thus, if a child is added within a certain timeframe after we * transfer, we back out and alert the summary again. * + * An alert can only transfer back within a small window of time after a transfer away from the + * summary to a child happened. + * * @param groupAlertEntry group alert entry to check */ private void checkShouldTransferBack(@NonNull GroupAlertEntry groupAlertEntry) { - if (SystemClock.elapsedRealtime() - groupAlertEntry.mLastAlertTransferTime - < ALERT_TRANSFER_TIMEOUT) { + if (canStillTransferBack(groupAlertEntry)) { NotificationEntry summary = groupAlertEntry.mGroup.summary; if (!onlySummaryAlerts(summary)) { @@ -338,30 +516,17 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis } ArrayList<NotificationEntry> children = mGroupManager.getLogicalChildren( summary.getSbn()); - int numChildren = children.size(); + int numActiveChildren = children.size(); int numPendingChildren = getPendingChildrenNotAlerting(groupAlertEntry.mGroup); - numChildren += numPendingChildren; + int numChildren = numActiveChildren + numPendingChildren; if (numChildren <= 1) { return; } - boolean releasedChild = false; - for (int i = 0; i < children.size(); i++) { - NotificationEntry entry = children.get(i); - if (onlySummaryAlerts(entry) && mHeadsUpManager.isAlerting(entry.getKey())) { - releasedChild = true; - mHeadsUpManager.removeNotification( - entry.getKey(), true /* releaseImmediately */); - } - if (mPendingAlerts.containsKey(entry.getKey())) { - // This is the child that would've been removed if it was inflated. - releasedChild = true; - mPendingAlerts.get(entry.getKey()).mAbortOnInflation = true; - } - } + boolean releasedChild = releaseChildAlerts(children); if (releasedChild && !mHeadsUpManager.isAlerting(summary.getKey())) { - boolean notifyImmediately = (numChildren - numPendingChildren) > 1; + boolean notifyImmediately = numActiveChildren > 1; if (notifyImmediately) { - alertNotificationWhenPossible(summary, mHeadsUpManager); + alertNotificationWhenPossible(summary); } else { // Should wait until the pending child inflates before alerting. groupAlertEntry.mAlertSummaryOnNextAddition = true; @@ -371,25 +536,61 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis } } + private boolean canStillTransferBack(@NonNull GroupAlertEntry groupAlertEntry) { + return SystemClock.elapsedRealtime() - groupAlertEntry.mLastAlertTransferTime + < ALERT_TRANSFER_TIMEOUT; + } + + private boolean releaseChildAlerts(List<NotificationEntry> children) { + boolean releasedChild = false; + if (SPEW) { + Log.d(TAG, "releaseChildAlerts: numChildren=" + children.size()); + } + for (int i = 0; i < children.size(); i++) { + NotificationEntry entry = children.get(i); + if (SPEW) { + Log.d(TAG, "releaseChildAlerts: checking i=" + i + " entry=" + entry + + " onlySummaryAlerts=" + onlySummaryAlerts(entry) + + " isAlerting=" + mHeadsUpManager.isAlerting(entry.getKey()) + + " isPendingAlert=" + mPendingAlerts.containsKey(entry.getKey())); + } + if (onlySummaryAlerts(entry) && mHeadsUpManager.isAlerting(entry.getKey())) { + releasedChild = true; + mHeadsUpManager.removeNotification( + entry.getKey(), true /* releaseImmediately */); + } + if (mPendingAlerts.containsKey(entry.getKey())) { + // This is the child that would've been removed if it was inflated. + releasedChild = true; + mPendingAlerts.get(entry.getKey()).mAbortOnInflation = true; + } + } + if (SPEW) { + Log.d(TAG, "releaseChildAlerts: didRelease=" + releasedChild); + } + return releasedChild; + } + /** * Tries to alert the notification. If its content view is not inflated, we inflate and continue * when the entry finishes inflating the view. * * @param entry entry to show - * @param alertManager alert manager for the alert type */ - private void alertNotificationWhenPossible(@NonNull NotificationEntry entry, - @NonNull AlertingNotificationManager alertManager) { - @InflationFlag int contentFlag = alertManager.getContentFlag(); + private void alertNotificationWhenPossible(@NonNull NotificationEntry entry) { + @InflationFlag int contentFlag = mHeadsUpManager.getContentFlag(); final RowContentBindParams params = mRowContentBindStage.getStageParams(entry); if ((params.getContentViews() & contentFlag) == 0) { + if (DEBUG) { + Log.d(TAG, "alertNotificationWhenPossible: async requestRebind entry=" + entry); + } mPendingAlerts.put(entry.getKey(), new PendingAlertInfo(entry)); params.requireContentViews(contentFlag); mRowContentBindStage.requestRebind(entry, en -> { PendingAlertInfo alertInfo = mPendingAlerts.remove(entry.getKey()); if (alertInfo != null) { if (alertInfo.isStillValid()) { - alertNotificationWhenPossible(entry, mHeadsUpManager); + alertNotificationWhenPossible(entry); } else { // The transfer is no longer valid. Free the content. mRowContentBindStage.getStageParams(entry).markContentViewsFreeable( @@ -400,10 +601,16 @@ public class NotificationGroupAlertTransferHelper implements OnHeadsUpChangedLis }); return; } - if (alertManager.isAlerting(entry.getKey())) { - alertManager.updateNotification(entry.getKey(), true /* alert */); + if (mHeadsUpManager.isAlerting(entry.getKey())) { + if (DEBUG) { + Log.d(TAG, "alertNotificationWhenPossible: continue alerting entry=" + entry); + } + mHeadsUpManager.updateNotification(entry.getKey(), true /* alert */); } else { - alertManager.showNotification(entry); + if (DEBUG) { + Log.d(TAG, "alertNotificationWhenPossible: start alerting entry=" + entry); + } + mHeadsUpManager.showNotification(entry); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java index 7c8b413833eb..88852f111c4b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupAlertTransferHelperTest.java @@ -74,6 +74,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { private HeadsUpManager mHeadsUpManager; @Mock private NotificationEntryManager mNotificationEntryManager; @Mock private RowContentBindStage mBindStage; + @Mock PeopleNotificationIdentifier mPeopleNotificationIdentifier; @Captor private ArgumentCaptor<NotificationEntryListener> mListenerCaptor; private NotificationEntryListener mNotificationEntryListener; private final HashMap<String, NotificationEntry> mPendingEntries = new HashMap<>(); @@ -91,7 +92,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { mGroupManager = new NotificationGroupManagerLegacy( mock(StatusBarStateController.class), - () -> mock(PeopleNotificationIdentifier.class), + () -> mPeopleNotificationIdentifier, Optional.of(mock(Bubbles.class))); mDependency.injectTestDependency(NotificationGroupManagerLegacy.class, mGroupManager); mGroupManager.setHeadsUpManager(mHeadsUpManager); @@ -107,15 +108,31 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { mHeadsUpManager.addListener(mGroupAlertTransferHelper); } + private void mockHasHeadsUpContentView(NotificationEntry entry, + boolean hasHeadsUpContentView) { + RowContentBindParams params = new RowContentBindParams(); + if (hasHeadsUpContentView) { + params.requireContentViews(FLAG_CONTENT_VIEW_HEADS_UP); + } + when(mBindStage.getStageParams(eq(entry))).thenReturn(params); + } + + private void mockHasHeadsUpContentView(NotificationEntry entry) { + mockHasHeadsUpContentView(entry, true); + } + + private void mockIsPriority(NotificationEntry priorityEntry) { + when(mPeopleNotificationIdentifier.getPeopleNotificationType(eq(priorityEntry))) + .thenReturn(PeopleNotificationIdentifier.TYPE_IMPORTANT_PERSON); + } + @Test public void testSuppressedSummaryHeadsUpTransfersToChild() { NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(); mHeadsUpManager.showNotification(summaryEntry); NotificationEntry childEntry = mGroupTestHelper.createChildNotification(); - RowContentBindParams params = new RowContentBindParams(); - params.requireContentViews(FLAG_CONTENT_VIEW_HEADS_UP); - when(mBindStage.getStageParams(eq(childEntry))).thenReturn(params); + mockHasHeadsUpContentView(childEntry); // Summary will be suppressed because there is only one child. mGroupManager.onEntryAdded(summaryEntry); @@ -180,8 +197,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(); mHeadsUpManager.showNotification(summaryEntry); NotificationEntry childEntry = mGroupTestHelper.createChildNotification(); - RowContentBindParams params = new RowContentBindParams(); - when(mBindStage.getStageParams(eq(childEntry))).thenReturn(params); + mockHasHeadsUpContentView(childEntry, false); mGroupManager.onEntryAdded(summaryEntry); mGroupManager.onEntryAdded(childEntry); @@ -198,8 +214,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(); mHeadsUpManager.showNotification(summaryEntry); NotificationEntry childEntry = mGroupTestHelper.createChildNotification(); - RowContentBindParams params = new RowContentBindParams(); - when(mBindStage.getStageParams(eq(childEntry))).thenReturn(params); + mockHasHeadsUpContentView(childEntry, false); mGroupManager.onEntryAdded(summaryEntry); mGroupManager.onEntryAdded(childEntry); @@ -250,8 +265,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { mGroupTestHelper.createSummaryNotification(Notification.GROUP_ALERT_SUMMARY); NotificationEntry childEntry = mGroupTestHelper.createChildNotification(Notification.GROUP_ALERT_SUMMARY); - RowContentBindParams params = new RowContentBindParams(); - when(mBindStage.getStageParams(eq(childEntry))).thenReturn(params); + mockHasHeadsUpContentView(childEntry, false); mHeadsUpManager.showNotification(summaryEntry); // Trigger a transfer of alert state from summary to child. @@ -270,8 +284,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { mGroupTestHelper.createSummaryNotification(Notification.GROUP_ALERT_SUMMARY); NotificationEntry childEntry = mGroupTestHelper.createChildNotification(Notification.GROUP_ALERT_SUMMARY); - RowContentBindParams params = new RowContentBindParams(); - when(mBindStage.getStageParams(eq(childEntry))).thenReturn(params); + mockHasHeadsUpContentView(childEntry, false); mHeadsUpManager.showNotification(summaryEntry); // Trigger a transfer of alert state from summary to child. @@ -294,8 +307,7 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { mGroupTestHelper.createSummaryNotification(Notification.GROUP_ALERT_SUMMARY); NotificationEntry childEntry = mGroupTestHelper.createChildNotification(Notification.GROUP_ALERT_SUMMARY, 47); - RowContentBindParams params = new RowContentBindParams(); - when(mBindStage.getStageParams(eq(childEntry))).thenReturn(params); + mockHasHeadsUpContentView(childEntry, false); mHeadsUpManager.showNotification(summaryEntry); // Trigger a transfer of alert state from summary to child. @@ -311,4 +323,160 @@ public class NotificationGroupAlertTransferHelperTest extends SysuiTestCase { assertFalse(mGroupAlertTransferHelper.isAlertTransferPending(childEntry)); } + + @Test + public void testOverriddenSummaryHeadsUpTransfersToPriority() { + // Creation order is oldest to newest, meaning the priority will be deemed newest + int groupAlert = Notification.GROUP_ALERT_SUMMARY; + NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(groupAlert); + NotificationEntry childEntry = mGroupTestHelper.createChildNotification(groupAlert); + NotificationEntry priorityEntry = mGroupTestHelper.createChildNotification(groupAlert); + mockIsPriority(priorityEntry); + + // summary gets heads up + mHeadsUpManager.showNotification(summaryEntry); + + mockHasHeadsUpContentView(summaryEntry); + mockHasHeadsUpContentView(priorityEntry); + mockHasHeadsUpContentView(childEntry); + + // Summary will have an alertOverride. + mGroupManager.onEntryAdded(summaryEntry); + mGroupManager.onEntryAdded(priorityEntry); + mGroupManager.onEntryAdded(childEntry); + + // An overridden summary should transfer its alert state to the priority. + assertFalse(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(childEntry.getKey())); + assertTrue(mHeadsUpManager.isAlerting(priorityEntry.getKey())); + } + + @Test + public void testOverriddenSummaryHeadsUpTransferDoesNotAlertPriorityIfUninflated() { + // Creation order is oldest to newest, meaning the priority will be deemed newest + int groupAlert = Notification.GROUP_ALERT_SUMMARY; + NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(groupAlert); + NotificationEntry childEntry = mGroupTestHelper.createChildNotification(groupAlert); + NotificationEntry priorityEntry = mGroupTestHelper.createChildNotification(groupAlert); + mockIsPriority(priorityEntry); + + // summary gets heads up + mHeadsUpManager.showNotification(summaryEntry); + + mockHasHeadsUpContentView(summaryEntry); + mockHasHeadsUpContentView(priorityEntry, false); + mockHasHeadsUpContentView(childEntry); + + // Summary will have an alertOverride. + mGroupManager.onEntryAdded(summaryEntry); + mGroupManager.onEntryAdded(priorityEntry); + mGroupManager.onEntryAdded(childEntry); + + // Alert is immediately removed from summary, but we do not show priority yet either as its + // content is not inflated. + assertFalse(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(childEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(priorityEntry.getKey())); + assertTrue(mGroupAlertTransferHelper.isAlertTransferPending(priorityEntry)); + } + + @Test + public void testOverriddenSummaryHeadsUpTransfersToPriorityButBackAgain() { + // Creation order is oldest to newest, meaning the child2 will ultimately be deemed newest + int groupAlert = Notification.GROUP_ALERT_SUMMARY; + NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(groupAlert); + NotificationEntry childEntry = mGroupTestHelper.createChildNotification(groupAlert); + NotificationEntry priorityEntry = mGroupTestHelper.createChildNotification(groupAlert); + NotificationEntry childEntry2 = mGroupTestHelper.createChildNotification(groupAlert); + mockIsPriority(priorityEntry); + + // summary gets heads up + mHeadsUpManager.showNotification(summaryEntry); + + mockHasHeadsUpContentView(summaryEntry); + mockHasHeadsUpContentView(priorityEntry); + mockHasHeadsUpContentView(childEntry); + mockHasHeadsUpContentView(childEntry2); + + // Summary will have an alertOverride. + mGroupManager.onEntryAdded(summaryEntry); + mGroupManager.onEntryAdded(priorityEntry); + mGroupManager.onEntryAdded(childEntry); + + // An overridden summary should transfer its alert state to the priority. + assertFalse(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(childEntry.getKey())); + assertTrue(mHeadsUpManager.isAlerting(priorityEntry.getKey())); + + mGroupManager.onEntryAdded(childEntry2); + + // An overridden summary should transfer its alert state to the priority. + assertTrue(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(childEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(childEntry2.getKey())); + assertFalse(mHeadsUpManager.isAlerting(priorityEntry.getKey())); + } + + @Test + public void testOverriddenSuppressedSummaryHeadsUpTransfersToChildThenToPriority() { + // Creation order is oldest to newest, meaning the priority will ultimately be deemed newest + int groupAlert = Notification.GROUP_ALERT_SUMMARY; + NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(groupAlert); + NotificationEntry childEntry = mGroupTestHelper.createChildNotification(groupAlert); + NotificationEntry priorityEntry = mGroupTestHelper.createChildNotification(groupAlert); + mockIsPriority(priorityEntry); + + // summary gets heads up + mHeadsUpManager.showNotification(summaryEntry); + + mockHasHeadsUpContentView(summaryEntry); + mockHasHeadsUpContentView(priorityEntry); + mockHasHeadsUpContentView(childEntry); + + // Summary will be suppressed, and the child will receive the alert + mGroupManager.onEntryAdded(summaryEntry); + mGroupManager.onEntryAdded(childEntry); + + assertFalse(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertTrue(mHeadsUpManager.isAlerting(childEntry.getKey())); + + // Alert should be transferred "back" from the child to the priority + mGroupManager.onEntryAdded(priorityEntry); + + assertFalse(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(childEntry.getKey())); + assertTrue(mHeadsUpManager.isAlerting(priorityEntry.getKey())); + } + + @Test + public void testOverriddenSuppressedSummaryHeadsUpTransfersToPriorityThenToChild() { + // Creation order is oldest to newest, meaning the child will ultimately be deemed newest + int groupAlert = Notification.GROUP_ALERT_SUMMARY; + NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(groupAlert); + NotificationEntry priorityEntry = mGroupTestHelper.createChildNotification(groupAlert); + NotificationEntry childEntry = mGroupTestHelper.createChildNotification(groupAlert); + mockIsPriority(priorityEntry); + + // summary gets heads up + mHeadsUpManager.showNotification(summaryEntry); + + mockHasHeadsUpContentView(summaryEntry); + mockHasHeadsUpContentView(priorityEntry); + mockHasHeadsUpContentView(childEntry); + + // Summary will have alert override of the priority + mGroupManager.onEntryAdded(summaryEntry); + mGroupManager.onEntryAdded(priorityEntry); + + assertFalse(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertTrue(mHeadsUpManager.isAlerting(priorityEntry.getKey())); + + // Alert should be transferred "back" from the priority to the child (which is newer) + mGroupManager.onEntryAdded(childEntry); + + assertFalse(mHeadsUpManager.isAlerting(summaryEntry.getKey())); + assertTrue(mHeadsUpManager.isAlerting(childEntry.getKey())); + assertFalse(mHeadsUpManager.isAlerting(priorityEntry.getKey())); + } + } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupManagerLegacyTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupManagerLegacyTest.java index 3e9fd5189129..0110d7b58cef 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupManagerLegacyTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationGroupManagerLegacyTest.java @@ -21,9 +21,12 @@ import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.app.Notification; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; @@ -33,6 +36,8 @@ import com.android.systemui.SysuiTestCase; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy; +import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy.NotificationGroup; +import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy.OnGroupChangeListener; import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier; import com.android.systemui.statusbar.policy.HeadsUpManager; import com.android.wm.shell.bubbles.Bubbles; @@ -58,6 +63,7 @@ public class NotificationGroupManagerLegacyTest extends SysuiTestCase { private final NotificationGroupTestHelper mGroupTestHelper = new NotificationGroupTestHelper(mContext); + @Mock PeopleNotificationIdentifier mPeopleNotificationIdentifier; @Mock HeadsUpManager mHeadsUpManager; @Before @@ -69,7 +75,7 @@ public class NotificationGroupManagerLegacyTest extends SysuiTestCase { private void initializeGroupManager() { mGroupManager = new NotificationGroupManagerLegacy( mock(StatusBarStateController.class), - () -> mock(PeopleNotificationIdentifier.class), + () -> mPeopleNotificationIdentifier, Optional.of(mock(Bubbles.class))); mGroupManager.setHeadsUpManager(mHeadsUpManager); } @@ -153,4 +159,72 @@ public class NotificationGroupManagerLegacyTest extends SysuiTestCase { assertEquals(childEntry, mGroupManager.getGroupSummary(childEntry)); assertEquals(summaryEntry, mGroupManager.getLogicalGroupSummary(childEntry)); } + + @Test + public void testAlertOverrideWithSiblings_0() { + helpTestAlertOverrideWithSiblings(0); + } + + @Test + public void testAlertOverrideWithSiblings_1() { + helpTestAlertOverrideWithSiblings(1); + } + + @Test + public void testAlertOverrideWithSiblings_2() { + helpTestAlertOverrideWithSiblings(2); + } + + /** + * This tests, for a group with a priority entry and the given number of siblings, that: + * 1) the priority entry is identified as the alertOverride for the group + * 2) the onAlertOverrideChanged method is called at that time + * 3) when the priority entry is removed, these are reversed + */ + private void helpTestAlertOverrideWithSiblings(int numSiblings) { + int groupAlert = Notification.GROUP_ALERT_SUMMARY; + // Create entries in an order so that the priority entry can be deemed the newest child. + NotificationEntry[] siblings = new NotificationEntry[numSiblings]; + for (int i = 0; i < numSiblings; i++) { + siblings[i] = mGroupTestHelper.createChildNotification(groupAlert); + } + NotificationEntry priorityEntry = mGroupTestHelper.createChildNotification(groupAlert); + NotificationEntry summaryEntry = mGroupTestHelper.createSummaryNotification(groupAlert); + + // The priority entry is an important conversation. + when(mPeopleNotificationIdentifier.getPeopleNotificationType(eq(priorityEntry))) + .thenReturn(PeopleNotificationIdentifier.TYPE_IMPORTANT_PERSON); + + // Register a listener so we can verify that the event is sent. + OnGroupChangeListener groupChangeListener = mock(OnGroupChangeListener.class); + mGroupManager.registerGroupChangeListener(groupChangeListener); + + // Add all the entries. The order here shouldn't matter. + mGroupManager.onEntryAdded(summaryEntry); + for (int i = 0; i < numSiblings; i++) { + mGroupManager.onEntryAdded(siblings[i]); + } + mGroupManager.onEntryAdded(priorityEntry); + + // Verify that the summary group has the priority child as its alertOverride + NotificationGroup summaryGroup = mGroupManager.getGroupForSummary(summaryEntry.getSbn()); + assertEquals(priorityEntry, summaryGroup.alertOverride); + verify(groupChangeListener).onGroupAlertOverrideChanged(summaryGroup, null, priorityEntry); + + // Verify that only the priority notification is isolated from the group + assertEquals(priorityEntry, mGroupManager.getGroupSummary(priorityEntry)); + assertEquals(summaryEntry, mGroupManager.getLogicalGroupSummary(priorityEntry)); + // Verify that the siblings are NOT isolated from the group + for (int i = 0; i < numSiblings; i++) { + assertEquals(summaryEntry, mGroupManager.getGroupSummary(siblings[i])); + assertEquals(summaryEntry, mGroupManager.getLogicalGroupSummary(siblings[i])); + } + + // Remove the priority notification to validate that it is removed as the alertOverride + mGroupManager.onEntryRemoved(priorityEntry); + + // verify that the alertOverride is removed when the priority notification is + assertNull(summaryGroup.alertOverride); + verify(groupChangeListener).onGroupAlertOverrideChanged(summaryGroup, null, priorityEntry); + } } |