DO NOT MERGE Network notifications: revamp keying scheme
This patch changes the (tag: String, id: Int) keying scheme for network
notifications so that TRON notification counters can count network
related notifications unambiguously.
TRON instruments all notifications shown for package "android" as well
as user interactions with these Notifications. These counters are
grouped by id. However the network notifications ("no internet" dialog,
"captive portal sign in" dialog, ...) use a static tag and a dynamic id
for keying notifications, preventing the counters to correctly
aggregate. In addition there is also the risk of collision with other
SystemUi notification ids not managed by NetworkNotificationManager.
In order to make the TRON counters useful for network notifications,
the id is now encoding the network notification type in a stable way
while the tag is used to uniquely identify network notifications.
Test: change covered by previously added new unit tests.
Bug: 32198726
Bug: 33030620
(cherry picked from commit 51727428fcfa9e09bfd70e710ceef02325994942)
Change-Id: Iadf7f15da38de28587090ed0395f15c24d4ad442
diff --git a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java
index 9ffa40b..c051642 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 @@
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.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 @@
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 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 @@
}
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 @@
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 @@
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 813e928..98073ce 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 @@
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 @@
}
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 @@
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