Allow additional reason/surface combinations for cancel am: 4c39ad7c

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/24147764



Change-Id: I2c343cf6796c239a577daf9497a983804b4ca92d
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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,51 +228,40 @@ 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) {
-                    throw new IllegalArgumentException("Unexpected surface " + surface);
-                }
+                Log.wtf(TAG, "Unexpected surface: " + surface + " with reason " + reason);
                 return INVALID;
             }
-            // Most cancel reasons do not have a meaningful surface. Reason codes map directly
-            // to NotificationCancelledEvent codes.
-            if (surface == NotificationStats.DISMISSAL_OTHER) {
+
+            // User cancels have a meaningful surface, which we differentiate by. See b/149038335
+            // for caveats.
+            if (reason == REASON_CANCEL) {
+                switch (surface) {
+                    case NotificationStats.DISMISSAL_PEEK:
+                        return NOTIFICATION_CANCEL_USER_PEEK;
+                    case NotificationStats.DISMISSAL_AOD:
+                        return NOTIFICATION_CANCEL_USER_AOD;
+                    case NotificationStats.DISMISSAL_SHADE:
+                        return NOTIFICATION_CANCEL_USER_SHADE;
+                    case NotificationStats.DISMISSAL_BUBBLE:
+                        return NOTIFICATION_CANCEL_USER_BUBBLE;
+                    case NotificationStats.DISMISSAL_LOCKSCREEN:
+                        return NOTIFICATION_CANCEL_USER_LOCKSCREEN;
+                    case NotificationStats.DISMISSAL_OTHER:
+                        return NOTIFICATION_CANCEL_USER_OTHER;
+                    default:
+                        Log.wtf(TAG, "Unexpected surface: " + surface + " with reason " + reason);
+                        return INVALID;
+                }
+            } else {
                 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);
-                }
+                Log.wtf(TAG, "Unexpected reason: " + reason + " with surface " + surface);
                 return INVALID;
             }
-            // User cancels have a meaningful surface, which we differentiate by. See b/149038335
-            // for caveats.
-            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;
-                case NotificationStats.DISMISSAL_AOD:
-                    return NOTIFICATION_CANCEL_USER_AOD;
-                case NotificationStats.DISMISSAL_SHADE:
-                    return NOTIFICATION_CANCEL_USER_SHADE;
-                case NotificationStats.DISMISSAL_BUBBLE:
-                    return NOTIFICATION_CANCEL_USER_BUBBLE;
-                case NotificationStats.DISMISSAL_LOCKSCREEN:
-                    return NOTIFICATION_CANCEL_USER_LOCKSCREEN;
-                default:
-                    if (NotificationManagerService.DBG) {
-                        throw new IllegalArgumentException("Unexpected surface for user-dismiss "
-                                + reason);
-                    }
-                    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));
+    }
 }