Allow additional reason/surface combinations for cancel

Bubbles log REASON_GROUP_SUMMARY_CANCELED/DISMISSAL_BUBBLE, which wasn't
an allowed combination.

To make this more robust, only look at surface values for REASON_CANCEL,
which is the only reason that needs them. Also, use wtf instead of
exceptions so we can track issues without crashing.

Test: atest NotificationRecordLogger
Fixes: 288516227
Change-Id: I054969dc82da5277002049d063abdb044dcc811e
(cherry picked from commit 80ce161ff1)
Merged-In: I054969dc82da5277002049d063abdb044dcc811e

services/core/java/com/android/server/notification/NotificationRecordLogger.java

@@ -30,6 +30,7 @@ import android.app.Person;
 import android.os.Bundle;
 import android.service.notification.NotificationListenerService;
 import android.service.notification.NotificationStats;
+import android.util.Log;
 
 import com.android.internal.logging.InstanceId;
 import com.android.internal.logging.UiEvent;
@@ -45,6 +46,8 @@ import java.util.Objects;
  */
 interface NotificationRecordLogger {
 
+    static final String TAG = "NotificationRecordLogger";
+
     // The high-level interface used by clients.
 
     /**
@@ -225,33 +228,13 @@ interface NotificationRecordLogger {
                 @NotificationStats.DismissalSurface int surface) {
             // Shouldn't be possible to get a non-dismissed notification here.
             if (surface == NotificationStats.DISMISSAL_NOT_DISMISSED) {
-                if (NotificationManagerService.DBG) {
+                Log.wtf(TAG, "Unexpected surface: " + surface + " with reason " + reason);
-                    throw new IllegalArgumentException("Unexpected surface " + surface);
-                }
-                return INVALID;
-            }
-            // Most cancel reasons do not have a meaningful surface. Reason codes map directly
-            // to NotificationCancelledEvent codes.
-            if (surface == NotificationStats.DISMISSAL_OTHER) {
-                if ((REASON_CLICK <= reason) && (reason <= REASON_CLEAR_DATA)) {
-                    return NotificationCancelledEvent.values()[reason];
-                }
-                if (reason == REASON_ASSISTANT_CANCEL) {
-                    return NotificationCancelledEvent.NOTIFICATION_CANCEL_ASSISTANT;
-                }
-                if (NotificationManagerService.DBG) {
-                    throw new IllegalArgumentException("Unexpected cancel reason " + reason);
-                }
                 return INVALID;
             }
+
             // User cancels have a meaningful surface, which we differentiate by. See b/149038335
             // for caveats.
-            if (reason != REASON_CANCEL) {
+            if (reason == REASON_CANCEL) {
-                if (NotificationManagerService.DBG) {
-                    throw new IllegalArgumentException("Unexpected cancel with surface " + reason);
-                }
-                return INVALID;
-            }
                 switch (surface) {
                     case NotificationStats.DISMISSAL_PEEK:
                         return NOTIFICATION_CANCEL_USER_PEEK;
@@ -263,11 +246,20 @@ interface NotificationRecordLogger {
                         return NOTIFICATION_CANCEL_USER_BUBBLE;
                     case NotificationStats.DISMISSAL_LOCKSCREEN:
                         return NOTIFICATION_CANCEL_USER_LOCKSCREEN;
+                    case NotificationStats.DISMISSAL_OTHER:
+                        return NOTIFICATION_CANCEL_USER_OTHER;
                     default:
-                    if (NotificationManagerService.DBG) {
+                        Log.wtf(TAG, "Unexpected surface: " + surface + " with reason " + reason);
-                        throw new IllegalArgumentException("Unexpected surface for user-dismiss "
+                        return INVALID;
-                                + reason);
                 }
+            } else {
+                if ((REASON_CLICK <= reason) && (reason <= REASON_CLEAR_DATA)) {
+                    return NotificationCancelledEvent.values()[reason];
+                }
+                if (reason == REASON_ASSISTANT_CANCEL) {
+                    return NotificationCancelledEvent.NOTIFICATION_CANCEL_ASSISTANT;
+                }
+                Log.wtf(TAG, "Unexpected reason: " + reason + " with surface " + surface);
                 return INVALID;
             }
         }

services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordLoggerTest.java

@@ -19,6 +19,15 @@ package com.android.server.notification;
 import static android.app.Notification.FLAG_FOREGROUND_SERVICE;
 import static android.app.NotificationManager.IMPORTANCE_DEFAULT;
 
+import static android.service.notification.NotificationListenerService.REASON_CANCEL;
+import static android.service.notification.NotificationListenerService.REASON_GROUP_SUMMARY_CANCELED;
+import static android.service.notification.NotificationStats.DISMISSAL_BUBBLE;
+import static android.service.notification.NotificationStats.DISMISSAL_OTHER;
+
+import static com.android.server.notification.NotificationRecordLogger.NotificationCancelledEvent.NOTIFICATION_CANCEL_CLICK;
+import static com.android.server.notification.NotificationRecordLogger.NotificationCancelledEvent.NOTIFICATION_CANCEL_GROUP_SUMMARY_CANCELED;
+import static com.android.server.notification.NotificationRecordLogger.NotificationCancelledEvent.NOTIFICATION_CANCEL_USER_OTHER;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
@@ -155,4 +164,18 @@ public class NotificationRecordLoggerTest extends UiServiceTestCase {
         // Then: should return false
         assertFalse(NotificationRecordLogger.isNonDismissible(p.r));
     }
+
+    @Test
+    public void testBubbleGroupSummaryDismissal() {
+        assertEquals(NOTIFICATION_CANCEL_GROUP_SUMMARY_CANCELED,
+                NotificationRecordLogger.NotificationCancelledEvent.fromCancelReason(
+                REASON_GROUP_SUMMARY_CANCELED, DISMISSAL_BUBBLE));
+    }
+
+    @Test
+    public void testOtherNotificationCancel() {
+        assertEquals(NOTIFICATION_CANCEL_USER_OTHER,
+                NotificationRecordLogger.NotificationCancelledEvent.fromCancelReason(
+                        REASON_CANCEL, DISMISSAL_OTHER));
+    }
 }