DO NOT MERGE: Don't let NotificationEntryManager keep around old RankingMaps
When a notification becomes lifetime-extended, NotificationEntryManager was holding onto the RankingMap that was passed at the time of removal of _that_ notification, and using it again in the NotificationSafeToRemoveCallback. The problem here is that when onSafeToRemove gets called, it was passing that same stale ranking map to removeNotification, which caused any notification that arrived in the intervening time to get improperly ranked. This fixes an issue where any notification that arrives while another is lifetime-extended can get the wrong ranking applied to it, causing trouble later in time such as mis-ranking and mis-sorting until the next update from system server. Bug: 146046016 Bug: 119041698 Test: atest SystemUITests Test: manual - Post a FGS notification and immediately cancel, then post a regular notification and wait for the FGS notification to dismiss. Note that the regular notification keeps showing in the status bar. Change-Id: I3df1279f13c424fcedd878bae2095fadc75d61b4
This commit is contained in:
@@ -85,7 +85,6 @@ public class NotificationEntryManager implements
|
|||||||
private NotificationRowBinder mNotificationRowBinder;
|
private NotificationRowBinder mNotificationRowBinder;
|
||||||
|
|
||||||
private NotificationPresenter mPresenter;
|
private NotificationPresenter mPresenter;
|
||||||
private NotificationListenerService.RankingMap mLatestRankingMap;
|
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
protected NotificationData mNotificationData;
|
protected NotificationData mNotificationData;
|
||||||
|
|
||||||
@@ -163,8 +162,7 @@ public class NotificationEntryManager implements
|
|||||||
/** Adds a {@link NotificationLifetimeExtender}. */
|
/** Adds a {@link NotificationLifetimeExtender}. */
|
||||||
public void addNotificationLifetimeExtender(NotificationLifetimeExtender extender) {
|
public void addNotificationLifetimeExtender(NotificationLifetimeExtender extender) {
|
||||||
mNotificationLifetimeExtenders.add(extender);
|
mNotificationLifetimeExtenders.add(extender);
|
||||||
extender.setCallback(key -> removeNotification(key, mLatestRankingMap,
|
extender.setCallback(key -> removeNotification(key, null, UNDEFINED_DISMISS_REASON));
|
||||||
UNDEFINED_DISMISS_REASON));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public NotificationData getNotificationData() {
|
public NotificationData getNotificationData() {
|
||||||
@@ -302,7 +300,6 @@ public class NotificationEntryManager implements
|
|||||||
if (!forceRemove && !entryDismissed) {
|
if (!forceRemove && !entryDismissed) {
|
||||||
for (NotificationLifetimeExtender extender : mNotificationLifetimeExtenders) {
|
for (NotificationLifetimeExtender extender : mNotificationLifetimeExtenders) {
|
||||||
if (extender.shouldExtendLifetime(entry)) {
|
if (extender.shouldExtendLifetime(entry)) {
|
||||||
mLatestRankingMap = ranking;
|
|
||||||
extendLifetime(entry, extender);
|
extendLifetime(entry, extender);
|
||||||
lifetimeExtended = true;
|
lifetimeExtended = true;
|
||||||
break;
|
break;
|
||||||
|
|||||||
@@ -203,6 +203,9 @@ public class NotificationData {
|
|||||||
removed = mEntries.remove(key);
|
removed = mEntries.remove(key);
|
||||||
}
|
}
|
||||||
if (removed == null) return null;
|
if (removed == null) return null;
|
||||||
|
// NEM may pass us a null ranking map if removing a lifetime-extended notification,
|
||||||
|
// so use the most recent ranking
|
||||||
|
if (ranking == null) ranking = mRankingMap;
|
||||||
mGroupManager.onEntryRemoved(removed);
|
mGroupManager.onEntryRemoved(removed);
|
||||||
updateRankingAndSort(ranking);
|
updateRankingAndSort(ranking);
|
||||||
return removed;
|
return removed;
|
||||||
|
|||||||
Reference in New Issue
Block a user