Fixed a bug where autobundling could lead to bugs

Notifications could appear twice if a notification with an app
group became bundled and one could not be swiped away anymore.
This is because we were unconditionally copying over the override
key even if the notification just became a group.

This also fixes a nullpointer that could happen with a racecondition
when a notification was cancelled and it tried to be used for autobundling.

This also fixes an issue that the autobundling wasn't working correctly
when a notification didn't have a group set but was set to be a
group summary.

Change-Id: Icd971f16ae0804ce162fd3171fcdd99bd04f3885
Fixes: 28706404
Fixes: 28628237

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 41f62e0..fe6ecbd 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -22,7 +22,6 @@ import static android.service.notification.NotificationRankerService.REASON_DELE
 import static android.service.notification.NotificationRankerService.REASON_DELEGATE_CANCEL_ALL;
 import static android.service.notification.NotificationRankerService.REASON_DELEGATE_CLICK;
 import static android.service.notification.NotificationRankerService.REASON_DELEGATE_ERROR;
-import static android.service.notification.NotificationRankerService.REASON_GROUP_OPTIMIZATION;
 import static android.service.notification.NotificationRankerService.REASON_GROUP_SUMMARY_CANCELED;
 import static android.service.notification.NotificationRankerService.REASON_LISTENER_CANCEL;
 import static android.service.notification.NotificationRankerService.REASON_LISTENER_CANCEL_ALL;
@@ -164,7 +163,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
@@ -2221,8 +2219,14 @@ public class NotificationManagerService extends SystemService {
                 int userId = -1;
                 NotificationRecord summaryRecord = null;
                 synchronized (mNotificationList) {
-                    final StatusBarNotification adjustedSbn
-                            = mNotificationsByKey.get(adjustment.getKey()).sbn;
+                    NotificationRecord notificationRecord =
+                            mNotificationsByKey.get(adjustment.getKey());
+                    if (notificationRecord == null) {
+                        // The notification could have been cancelled again already. A successive
+                        // adjustment will post a summary if needed.
+                        return;
+                    }
+                    final StatusBarNotification adjustedSbn = notificationRecord.sbn;
                     userId = adjustedSbn.getUser().getIdentifier();
                     ArrayMap<String, String> summaries = mAutobundledSummaries.get(userId);
                     if (summaries == null) {
@@ -2666,6 +2670,12 @@ public class NotificationManagerService extends SystemService {
             int callingUid, int callingPid) {
         StatusBarNotification sbn = r.sbn;
         Notification n = sbn.getNotification();
+        if (n.isGroupSummary() && !sbn.isAppGroup())  {
+            // notifications without a group shouldn't be a summary, otherwise autobundling can
+            // lead to bugs
+            n.flags &= ~Notification.FLAG_GROUP_SUMMARY;
+        }
+
         String group = sbn.getGroupKey();
         boolean isSummary = n.isGroupSummary();
 

diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java
index 1315bcb..7c89e9f 100644 (file)
--- a/services/core/java/com/android/server/notification/NotificationRecord.java
+++ b/services/core/java/com/android/server/notification/NotificationRecord.java
@@ -176,7 +176,7 @@ public final class NotificationRecord {
         mRankingTimeMs = calculateRankingTimeMs(previous.getRankingTimeMs());
         mCreationTimeMs = previous.mCreationTimeMs;
         mVisibleSinceMs = previous.mVisibleSinceMs;
-        if(previous.sbn.getOverrideGroupKey() != null) {
+        if (previous.sbn.getOverrideGroupKey() != null && !sbn.isAppGroup()) {
             sbn.setOverrideGroupKey(previous.sbn.getOverrideGroupKey());
         }
         // Don't copy importance information or mGlobalSortKey, recompute them.