DO NOT MERGE Network notifications: revamp keying scheme

am: 84daaea8fd

Change-Id: Ibad7d0210b2c69826462b781f43a82d65849ebfb

2 files changed, 70 insertions, 25 deletions

diff --git a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java
index 9ffa40b7522f..c051642a2151 100644
--- a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java
+++ b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java
@@ -19,7 +19,6 @@ package com.android.server.connectivity;
 import android.app.Notification;
 import android.app.NotificationManager;
 import android.app.PendingIntent;
-import android.widget.Toast;
 import android.content.Context;
 import android.content.Intent;
 import android.content.res.Resources;
@@ -27,18 +26,40 @@ import android.net.NetworkCapabilities;
 import android.os.UserHandle;
 import android.telephony.TelephonyManager;
 import android.util.Slog;
-import com.android.internal.annotations.VisibleForTesting;
+import android.util.SparseArray;
+import android.util.SparseIntArray;
+import android.widget.Toast;
 import com.android.internal.R;
+import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.logging.MetricsProto.MetricsEvent;
 
-import static android.net.NetworkCapabilities.*;
-
+import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
+import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
+import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
 
 public class NetworkNotificationManager {
 
-    public static enum NotificationType { SIGN_IN, NO_INTERNET, LOST_INTERNET, NETWORK_SWITCH };
+    public static enum NotificationType {
+        LOST_INTERNET(MetricsEvent.NOTIFICATION_NETWORK_LOST_INTERNET),
+        NETWORK_SWITCH(MetricsEvent.NOTIFICATION_NETWORK_SWITCH),
+        NO_INTERNET(MetricsEvent.NOTIFICATION_NETWORK_NO_INTERNET),
+        SIGN_IN(MetricsEvent.NOTIFICATION_NETWORK_SIGN_IN);
 
-    @VisibleForTesting
-    static final String NOTIFICATION_ID = "Connectivity.Notification";
+        public final int eventId;
+
+        NotificationType(int eventId) {
+            this.eventId = eventId;
+            Holder.sIdToTypeMap.put(eventId, this);
+        }
+
+        private static class Holder {
+            private static SparseArray<NotificationType> sIdToTypeMap = new SparseArray<>();
+        }
+
+        public static NotificationType getFromId(int id) {
+            return Holder.sIdToTypeMap.get(id);
+        }
+    };
 
     private static final String TAG = NetworkNotificationManager.class.getSimpleName();
     private static final boolean DBG = true;
@@ -47,11 +68,14 @@ public class NetworkNotificationManager {
     private final Context mContext;
     private final TelephonyManager mTelephonyManager;
     private final NotificationManager mNotificationManager;
+    // Tracks the types of notifications managed by this instance, from creation to cancellation.
+    private final SparseIntArray mNotificationTypeMap;
 
     public NetworkNotificationManager(Context c, TelephonyManager t, NotificationManager n) {
         mContext = c;
         mTelephonyManager = t;
         mNotificationManager = n;
+        mNotificationTypeMap = new SparseIntArray();
     }
 
     // TODO: deal more gracefully with multi-transport networks.
@@ -101,8 +125,10 @@ public class NetworkNotificationManager {
      */
     public void showNotification(int id, NotificationType notifyType, NetworkAgentInfo nai,
             NetworkAgentInfo switchToNai, PendingIntent intent, boolean highPriority) {
-        int transportType;
-        String extraInfo;
+        final String tag = tagFor(id);
+        final int eventId = notifyType.eventId;
+        final int transportType;
+        final String extraInfo;
         if (nai != null) {
             transportType = getFirstTransportType(nai);
             extraInfo = nai.networkInfo.getExtraInfo();
@@ -115,9 +141,9 @@ public class NetworkNotificationManager {
         }
 
         if (DBG) {
-            Slog.d(TAG, "showNotification id=" + id + " " + notifyType
-                    + " transportType=" + getTransportName(transportType)
-                    + " extraInfo=" + extraInfo + " highPriority=" + highPriority);
+            Slog.d(TAG, String.format(
+                    "showNotification tag=%s event=%s transport=%s extraInfo=%d highPrioriy=%s",
+                    tag, nameOf(eventId), getTransportName(transportType), extraInfo, highPriority));
         }
 
         Resources r = Resources.getSystem();
@@ -185,22 +211,31 @@ public class NetworkNotificationManager {
 
         Notification notification = builder.build();
 
+        mNotificationTypeMap.put(id, eventId);
         try {
-            mNotificationManager.notifyAsUser(NOTIFICATION_ID, id, notification, UserHandle.ALL);
+            mNotificationManager.notifyAsUser(tag, eventId, notification, UserHandle.ALL);
         } catch (NullPointerException npe) {
             Slog.d(TAG, "setNotificationVisible: visible notificationManager error", npe);
         }
     }
 
     public void clearNotification(int id) {
+        final String tag = tagFor(id);
+        if (mNotificationTypeMap.indexOfKey(id) < 0) {
+            Slog.e(TAG, "cannot clear unknown notification with tag=" + tag);
+            return;
+        }
+        final int eventId = mNotificationTypeMap.get(id);
         if (DBG) {
-            Slog.d(TAG, "clearNotification id=" + id);
+            Slog.d(TAG, String.format("clearing notification tag=%s event=", tag, nameOf(eventId)));
         }
         try {
-            mNotificationManager.cancelAsUser(NOTIFICATION_ID, id, UserHandle.ALL);
+            mNotificationManager.cancelAsUser(tag, eventId, UserHandle.ALL);
         } catch (NullPointerException npe) {
-            Slog.d(TAG, "setNotificationVisible: cancel notificationManager error", npe);
+            Slog.d(TAG, String.format(
+                    "failed to clear notification tag=%s event=", tag, nameOf(eventId)), npe);
         }
+        mNotificationTypeMap.delete(id);
     }
 
     /**
@@ -223,4 +258,15 @@ public class NetworkNotificationManager {
                 R.string.network_switch_metered_toast, fromTransport, toTransport);
         Toast.makeText(mContext, text, Toast.LENGTH_LONG).show();
     }
+
+    @VisibleForTesting
+    static String tagFor(int id) {
+        return String.format("ConnectivityNotification:%d", id);
+    }
+
+    @VisibleForTesting
+    static String nameOf(int eventId) {
+        NotificationType t = NotificationType.getFromId(eventId);
+        return (t != null) ? t.name() : "UNKNOWN";
+    }
 }
diff --git a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java
index 813e92811881..98073ce1e59b 100644
--- a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java
+++ b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java
@@ -48,8 +48,6 @@ import static org.mockito.Mockito.when;
 
 public class NetworkNotificationManagerTest extends TestCase {
 
-    static final String NOTIFICATION_ID = NetworkNotificationManager.NOTIFICATION_ID;
-
     static final NetworkCapabilities CELL_CAPABILITIES = new NetworkCapabilities();
     static final NetworkCapabilities WIFI_CAPABILITIES = new NetworkCapabilities();
     static {
@@ -108,11 +106,11 @@ public class NetworkNotificationManagerTest extends TestCase {
         }
 
         for (int i = 0; i < ids.size(); i++) {
-            final int expectedId = NETWORK_ID_BASE + i;
-            verify(mNotificationManager, times(1))
-                    .notifyAsUser(eq(NOTIFICATION_ID), eq(expectedId), any(), any());
-            verify(mNotificationManager, times(1))
-                    .cancelAsUser(eq(NOTIFICATION_ID), eq(expectedId), any());
+            final int id = ids.get(i);
+            final int eventId = types.get(i).eventId;
+            final String tag = NetworkNotificationManager.tagFor(id);
+            verify(mNotificationManager, times(1)).notifyAsUser(eq(tag), eq(eventId), any(), any());
+            verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(eventId), any());
         }
     }
 
@@ -125,8 +123,9 @@ public class NetworkNotificationManagerTest extends TestCase {
 
         mManager.showNotification(102, NO_INTERNET, mWifiNai, mCellNai, null, false);
 
-        verify(mNotificationManager, times(1))
-                .notifyAsUser(eq(NOTIFICATION_ID), eq(102), any(), any());
+        final int eventId = NO_INTERNET.eventId;
+        final String tag = NetworkNotificationManager.tagFor(102);
+        verify(mNotificationManager, times(1)).notifyAsUser(eq(tag), eq(eventId), any(), any());
     }
 
     @SmallTest