From 57c0380135b928377113b1fe12d41f6da370a56f Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 31 Mar 2021 17:07:45 +0900 Subject: [PATCH] Add option to make sign-in notification ongoing Add an overlay boolean that allows setting the SIGN_IN notification as an ongoing notification. This can be useful to make sure users can always easily find the notification to sign in to a captive portal, as studies have found that some users have a tendency to dismiss notifications before reading them. At the same time the notification shade is generally too crowded, which is what causes such behaviors in the first place, so this option is not enabled by default and should generally not be enabled without proper user studies or metrics. Bug: 173171709 Test: atest NetworkNotificationManagerTest Merged-In: I3c2563d4ae4e3715d0c6270344ba8f7ef067872f Change-Id: Ic187d2a2b7e49ad152ea2aa35bb784864b97473c --- .../res/values/config.xml | 4 ++ .../res/values/overlayable.xml | 1 + .../NetworkNotificationManager.java | 6 ++- .../NetworkNotificationManagerTest.java | 46 +++++++++++++++++-- 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/service/ServiceConnectivityResources/res/values/config.xml b/service/ServiceConnectivityResources/res/values/config.xml index 078a9eb582..70ddb9a7e9 100644 --- a/service/ServiceConnectivityResources/res/values/config.xml +++ b/service/ServiceConnectivityResources/res/values/config.xml @@ -107,4 +107,8 @@ + + false + diff --git a/service/ServiceConnectivityResources/res/values/overlayable.xml b/service/ServiceConnectivityResources/res/values/overlayable.xml index f0f4ae8022..fd235661dc 100644 --- a/service/ServiceConnectivityResources/res/values/overlayable.xml +++ b/service/ServiceConnectivityResources/res/values/overlayable.xml @@ -30,6 +30,7 @@ + diff --git a/service/src/com/android/server/connectivity/NetworkNotificationManager.java b/service/src/com/android/server/connectivity/NetworkNotificationManager.java index b57ad5d84e..3dc79c51d8 100644 --- a/service/src/com/android/server/connectivity/NetworkNotificationManager.java +++ b/service/src/com/android/server/connectivity/NetworkNotificationManager.java @@ -280,7 +280,11 @@ public class NetworkNotificationManager { .setContentTitle(title) .setContentIntent(intent) .setLocalOnly(true) - .setOnlyAlertOnce(true); + .setOnlyAlertOnce(true) + // TODO: consider having action buttons to disconnect on the sign-in notification + // especially if it is ongoing + .setOngoing(notifyType == NotificationType.SIGN_IN + && r.getBoolean(R.bool.config_ongoingSignInNotification)); if (notifyType == NotificationType.NETWORK_SWITCH) { builder.setStyle(new Notification.BigTextStyle().bigText(details)); diff --git a/tests/unit/java/com/android/server/connectivity/NetworkNotificationManagerTest.java b/tests/unit/java/com/android/server/connectivity/NetworkNotificationManagerTest.java index 3adf08c199..1e8d8087b2 100644 --- a/tests/unit/java/com/android/server/connectivity/NetworkNotificationManagerTest.java +++ b/tests/unit/java/com/android/server/connectivity/NetworkNotificationManagerTest.java @@ -16,8 +16,16 @@ package com.android.server.connectivity; -import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.*; +import static android.app.Notification.FLAG_ONGOING_EVENT; +import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.LOST_INTERNET; +import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.NETWORK_SWITCH; +import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.NO_INTERNET; +import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.PARTIAL_CONNECTIVITY; +import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.PRIVATE_DNS_BROKEN; +import static com.android.server.connectivity.NetworkNotificationManager.NotificationType.SIGN_IN; + +import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.clearInvocations; @@ -228,19 +236,47 @@ public class NetworkNotificationManagerTest { verify(mNotificationManager, never()).notify(any(), anyInt(), any()); } + private void assertNotification(NotificationType type, boolean ongoing) { + final int id = 101; + final String tag = NetworkNotificationManager.tagFor(id); + final ArgumentCaptor noteCaptor = ArgumentCaptor.forClass(Notification.class); + mManager.showNotification(id, type, mWifiNai, mCellNai, null, false); + verify(mNotificationManager, times(1)).notify(eq(tag), eq(type.eventId), + noteCaptor.capture()); + + assertEquals("Notification ongoing flag should be " + (ongoing ? "set" : "unset"), + ongoing, (noteCaptor.getValue().flags & FLAG_ONGOING_EVENT) != 0); + } + @Test 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)).notify(eq(tag), eq(NO_INTERNET.eventId), any()); + assertNotification(NO_INTERNET, false /* ongoing */); // Captive portal detection triggers SIGN_IN a bit later, clearing the previous NO_INTERNET - mManager.showNotification(id, SIGN_IN, mWifiNai, mCellNai, null, false); + assertNotification(SIGN_IN, false /* ongoing */); + verify(mNotificationManager, times(1)).cancel(eq(tag), eq(NO_INTERNET.eventId)); + + // Network disconnects + mManager.clearNotification(id); + verify(mNotificationManager, times(1)).cancel(eq(tag), eq(SIGN_IN.eventId)); + } + + @Test + public void testOngoingSignInNotification() { + doReturn(true).when(mResources).getBoolean(R.bool.config_ongoingSignInNotification); + final int id = 101; + final String tag = NetworkNotificationManager.tagFor(id); + + // Show first NO_INTERNET + assertNotification(NO_INTERNET, false /* ongoing */); + + // Captive portal detection triggers SIGN_IN a bit later, clearing the previous NO_INTERNET + assertNotification(SIGN_IN, true /* ongoing */); verify(mNotificationManager, times(1)).cancel(eq(tag), eq(NO_INTERNET.eventId)); - verify(mNotificationManager, times(1)).notify(eq(tag), eq(SIGN_IN.eventId), any()); // Network disconnects mManager.clearNotification(id);