From efa1a74bb8ae7d7dabc728cb3dec9c5946221866 Mon Sep 17 00:00:00 2001 From: Kensuke Matsui Date: Thu, 3 Aug 2017 12:12:36 +0900 Subject: [PATCH] Fix NPE which could occur when showing notification guts SystemUI keeps shown heads-up notification for minimum display time even if the notification is canceled right after it's posted. This is intentional behavior but causes inconsistency between mEntry and mRankingMap in NotificationData. That inconsistency could cause NPE when showing notification guts. To avoid this, update an Entry only when the corresponding Ranking is available. Fixes: 65567562 Test: manual - long press a missed call notification immediately after receiving an incoming call Test: runtest -x packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java Change-Id: I6dc70d8e57efa7af0f23862a00d0e18cf39dfebb --- .../systemui/statusbar/NotificationData.java | 38 +++++++++++++------ .../statusbar/NotificationDataTest.java | 6 +++ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java index ddc7dd063d4ec..d0417b59448da 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationData.java @@ -292,8 +292,8 @@ public class NotificationData { if (mRankingMap != null) { // RankingMap as received from NoMan - mRankingMap.getRanking(a.key, mRankingA); - mRankingMap.getRanking(b.key, mRankingB); + getRanking(a.key, mRankingA); + getRanking(b.key, mRankingB); aImportance = mRankingA.getImportance(); bImportance = mRankingB.getImportance(); aRank = mRankingA.getRank(); @@ -381,7 +381,7 @@ public class NotificationData { public boolean isAmbient(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return mTmpRanking.isAmbient(); } return false; @@ -389,7 +389,7 @@ public class NotificationData { public int getVisibilityOverride(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return mTmpRanking.getVisibilityOverride(); } return Ranking.VISIBILITY_NO_OVERRIDE; @@ -397,7 +397,7 @@ public class NotificationData { public boolean shouldSuppressScreenOff(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return (mTmpRanking.getSuppressedVisualEffects() & NotificationListenerService.SUPPRESSED_EFFECT_SCREEN_OFF) != 0; } @@ -406,7 +406,7 @@ public class NotificationData { public boolean shouldSuppressScreenOn(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return (mTmpRanking.getSuppressedVisualEffects() & NotificationListenerService.SUPPRESSED_EFFECT_SCREEN_ON) != 0; } @@ -415,7 +415,7 @@ public class NotificationData { public int getImportance(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return mTmpRanking.getImportance(); } return NotificationManager.IMPORTANCE_UNSPECIFIED; @@ -423,7 +423,7 @@ public class NotificationData { public String getOverrideGroupKey(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return mTmpRanking.getOverrideGroupKey(); } return null; @@ -431,7 +431,7 @@ public class NotificationData { public List getSnoozeCriteria(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return mTmpRanking.getSnoozeCriteria(); } return null; @@ -439,7 +439,7 @@ public class NotificationData { public NotificationChannel getChannel(String key) { if (mRankingMap != null) { - mRankingMap.getRanking(key, mTmpRanking); + getRanking(key, mTmpRanking); return mTmpRanking.getChannel(); } return null; @@ -452,6 +452,9 @@ public class NotificationData { final int N = mEntries.size(); for (int i = 0; i < N; i++) { Entry entry = mEntries.valueAt(i); + if (!getRanking(entry.key, mTmpRanking)) { + continue; + } final StatusBarNotification oldSbn = entry.notification.cloneLight(); final String overrideGroupKey = getOverrideGroupKey(entry.key); if (!Objects.equals(oldSbn.getOverrideGroupKey(), overrideGroupKey)) { @@ -466,6 +469,19 @@ public class NotificationData { filterAndSort(); } + /** + * Get the ranking from the current ranking map. + * + * @param key the key to look up + * @param outRanking the ranking to populate + * + * @return {@code true} if the ranking was properly obtained. + */ + @VisibleForTesting + protected boolean getRanking(String key, Ranking outRanking) { + return mRankingMap.getRanking(key, outRanking); + } + // TODO: This should not be public. Instead the Environment should notify this class when // anything changed, and this class should call back the UI so it updates itself. public void filterAndSort() { @@ -573,7 +589,7 @@ public class NotificationData { } private void dumpEntry(PrintWriter pw, String indent, int i, Entry e) { - mRankingMap.getRanking(e.key, mTmpRanking); + getRanking(e.key, mTmpRanking); pw.print(indent); pw.println(" [" + i + "] key=" + e.key + " icon=" + e.icon); StatusBarNotification n = e.notification; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java index 18c5756b3fb68..972eddb469015 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationDataTest.java @@ -135,5 +135,11 @@ public class NotificationDataTest extends SysuiTestCase { public NotificationChannel getChannel(String key) { return new NotificationChannel(null, null, 0); } + + @Override + protected boolean getRanking(String key, NotificationListenerService.Ranking outRanking) { + super.getRanking(key, outRanking); + return true; + } } }