From 1cbef0b29d6a71ed9733054cd9058ea6036e8cf0 Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Mon, 3 Feb 2020 14:23:52 -0500 Subject: Don't crash if we're given an incomplete ranking Right now there's a race condition in NotificationListener that means that we can (briefly) be exposed to rankings that don't contain all of the notifications that we know about. The order is as follows: (system server) queue onListenerConnected queue onNotificationPosted queue onNotificationPosted (sysui offthread) see onListenerConnected read list of current notifs (2 total) post to main.onListenerConnected see onNotificationPosted, post to main.onNotifPosted see onNotificationPosted, post to main.onNotifPosted (sysui main thread) see main.onListenerConnected, add notifs #1 and #2 see main.onNotificationPosted, attempt to re-add notif#1 (counts as update, fine), attempt to update ranking (ERROR, missing ranking for notif#2, crash) Filed b/148791039 to track the overall race condition. Short-term fix is to just disable this check and be sad. Fixes: 148291993 Test: atest Change-Id: Id03dccfed3f2c061b7603b7e9e3b5210aeaaf962 --- .../notification/collection/NotifCollection.java | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java index cb8cef8fdd72..64d460bfb8fe 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotifCollection.java @@ -308,17 +308,23 @@ public class NotifCollection implements Dumpable { private void applyRanking(@NonNull RankingMap rankingMap) { for (NotificationEntry entry : mNotificationSet.values()) { if (!isLifetimeExtended(entry)) { - Ranking ranking = requireRanking(rankingMap, entry.getKey()); - entry.setRanking(ranking); - - // TODO: (b/145659174) update the sbn's overrideGroupKey in - // NotificationEntry.setRanking instead of here once we fully migrate to the - // NewNotifPipeline - if (mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { - final String newOverrideGroupKey = ranking.getOverrideGroupKey(); - if (!Objects.equals(entry.getSbn().getOverrideGroupKey(), - newOverrideGroupKey)) { - entry.getSbn().setOverrideGroupKey(newOverrideGroupKey); + + // TODO: (b/148791039) We should crash if we are ever handed a ranking with + // incomplete entries. Right now, there's a race condition in NotificationListener + // that means this might occur when SystemUI is starting up. + Ranking ranking = new Ranking(); + if (rankingMap.getRanking(entry.getKey(), ranking)) { + entry.setRanking(ranking); + + // TODO: (b/145659174) update the sbn's overrideGroupKey in + // NotificationEntry.setRanking instead of here once we fully migrate to the + // NewNotifPipeline + if (mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { + final String newOverrideGroupKey = ranking.getOverrideGroupKey(); + if (!Objects.equals(entry.getSbn().getOverrideGroupKey(), + newOverrideGroupKey)) { + entry.getSbn().setOverrideGroupKey(newOverrideGroupKey); + } } } } -- cgit v1.2.3-59-g8ed1b