NetworkNotificationManager: correctly handle existing notifications
This patch corrects a regression added by commitfd35080555that did not take into account the case of multiple notifications shown for a single network id. Given how network notifications are triggered, it can happen that NO_INTERNET and SIGN_IN notifications are both triggered for the same network when captive portal detection is slow. Contrary to the situation before commitfd35080555, a notification priority order is introduced so that SIGN_IN always overrides NO_INTERNET, and NO_INTERNET is ignored if SIGN_IN is already present. Bug: 63676954 Bug: 62503737 Test: runtest frameworks-net, added new unit tests Merged-In: Ib8658601e8d4dc6c41b335ab7dd8caa0cccd9531 Merged-In: I4432f66067ea1ab02e1d2dfe42530bcdafa52df6 Merged-In: I74631b0bfd14daf18a1641ed7f2ec323d636ebbf Merged-In: I73cc879e910d503946facdba498b300337f349fd Merged-In: Ieed9e3e7755e0c5f89dc41ef66f47d4dbf4c66a9 Merged-In: I0aa590170f1bd4c37175c7e35e54d52f1fb21347 (cherry picked from commit 5fcd050e0ecd5985cf184f55ea3df4434da8f824) Change-Id: I41675768ab59e9b23ca4275edf297b82595e5730
This commit is contained in:
@@ -142,6 +142,18 @@ public class NetworkNotificationManager {
|
||||
extraInfo = null;
|
||||
}
|
||||
|
||||
// Clear any previous notification with lower priority, otherwise return. http://b/63676954.
|
||||
// A new SIGN_IN notification with a new intent should override any existing one.
|
||||
final int previousEventId = mNotificationTypeMap.get(id);
|
||||
final NotificationType previousNotifyType = NotificationType.getFromId(previousEventId);
|
||||
if (priority(previousNotifyType) > priority(notifyType)) {
|
||||
Slog.d(TAG, String.format(
|
||||
"ignoring notification %s for network %s with existing notification %s",
|
||||
notifyType, id, previousNotifyType));
|
||||
return;
|
||||
}
|
||||
clearNotification(id);
|
||||
|
||||
if (DBG) {
|
||||
Slog.d(TAG, String.format(
|
||||
"showNotification tag=%s event=%s transport=%s extraInfo=%s highPrioriy=%s",
|
||||
@@ -274,4 +286,22 @@ public class NetworkNotificationManager {
|
||||
NotificationType t = NotificationType.getFromId(eventId);
|
||||
return (t != null) ? t.name() : "UNKNOWN";
|
||||
}
|
||||
|
||||
private static int priority(NotificationType t) {
|
||||
if (t == null) {
|
||||
return 0;
|
||||
}
|
||||
switch (t) {
|
||||
case SIGN_IN:
|
||||
return 4;
|
||||
case NO_INTERNET:
|
||||
return 3;
|
||||
case NETWORK_SWITCH:
|
||||
return 2;
|
||||
case LOST_INTERNET:
|
||||
return 1;
|
||||
default:
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,16 @@
|
||||
|
||||
package com.android.server.connectivity;
|
||||
|
||||
import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.*;
|
||||
import static org.mockito.Mockito.any;
|
||||
import static org.mockito.Mockito.anyInt;
|
||||
import static org.mockito.Mockito.eq;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.reset;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import android.app.Notification;
|
||||
import android.app.NotificationManager;
|
||||
import android.content.Context;
|
||||
@@ -37,15 +47,6 @@ import org.mockito.Mock;
|
||||
import org.mockito.Mockito;
|
||||
import org.mockito.MockitoAnnotations;
|
||||
|
||||
import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.*;
|
||||
import static org.mockito.Mockito.any;
|
||||
import static org.mockito.Mockito.anyInt;
|
||||
import static org.mockito.Mockito.eq;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
public class NetworkNotificationManagerTest extends TestCase {
|
||||
|
||||
static final NetworkCapabilities CELL_CAPABILITIES = new NetworkCapabilities();
|
||||
@@ -140,4 +141,47 @@ public class NetworkNotificationManagerTest extends TestCase {
|
||||
|
||||
verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any());
|
||||
}
|
||||
|
||||
@SmallTest
|
||||
public void testDuplicatedNotificationsNoInternetThenSignIn() {
|
||||
final int id = 101;
|
||||
final String tag = NetworkNotificationManager.tagFor(id);
|
||||
|
||||
// Show first NO_INTERNET
|
||||
mManager.showNotification(id, NO_INTERNET, mWifiNai, mCellNai, null, false);
|
||||
verify(mNotificationManager, times(1))
|
||||
.notifyAsUser(eq(tag), eq(NO_INTERNET.eventId), any(), any());
|
||||
|
||||
// Captive portal detection triggers SIGN_IN a bit later, clearing the previous NO_INTERNET
|
||||
mManager.showNotification(id, SIGN_IN, mWifiNai, mCellNai, null, false);
|
||||
verify(mNotificationManager, times(1))
|
||||
.cancelAsUser(eq(tag), eq(NO_INTERNET.eventId), any());
|
||||
verify(mNotificationManager, times(1))
|
||||
.notifyAsUser(eq(tag), eq(SIGN_IN.eventId), any(), any());
|
||||
|
||||
// Network disconnects
|
||||
mManager.clearNotification(id);
|
||||
verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(SIGN_IN.eventId), any());
|
||||
}
|
||||
|
||||
@SmallTest
|
||||
public void testDuplicatedNotificationsSignInThenNoInternet() {
|
||||
final int id = 101;
|
||||
final String tag = NetworkNotificationManager.tagFor(id);
|
||||
|
||||
// Show first SIGN_IN
|
||||
mManager.showNotification(id, SIGN_IN, mWifiNai, mCellNai, null, false);
|
||||
verify(mNotificationManager, times(1))
|
||||
.notifyAsUser(eq(tag), eq(SIGN_IN.eventId), any(), any());
|
||||
reset(mNotificationManager);
|
||||
|
||||
// NO_INTERNET arrives after, but is ignored.
|
||||
mManager.showNotification(id, NO_INTERNET, mWifiNai, mCellNai, null, false);
|
||||
verify(mNotificationManager, never()).cancelAsUser(any(), anyInt(), any());
|
||||
verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any());
|
||||
|
||||
// Network disconnects
|
||||
mManager.clearNotification(id);
|
||||
verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(SIGN_IN.eventId), any());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user