Update the title & description of notification

Refer to the UX design doc to fix the title & description.

Bug: 150903640
Test: 1. Build pass
      2. atest NetworkStackTests
      3. Manual test to see if the title & description are shown
      as expected

Change-Id: I5ab49a3b5b220e144c69627eb9d02c6ddc438716
Merged-In: I354f9f19aa4d8057b49833107c8bd4d9c67d5525
(cherry picked from commit 07c905836d8a0d153ad3f02421b9776a0cee1108)

3 files changed, 32 insertions, 43 deletions

diff --git a/res/values/strings.xml b/res/values/strings.xml
index 58335b3..7b5b7e4 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -31,22 +31,15 @@
          information page, so that the user can access that page. [CHAR LIMIT=300] -->
     <string name="notification_channel_description_network_venue_info">Notifications shown to indicate the network has a venue information page</string>
 
-    <!-- Notifications are shown after the user logs in to a captive portal network, to indicate
-         that the network should now have internet connectivity. This is the title of the
-         notification, indicating that the device is connected to the network with the SSID given
-         as parameter. [CHAR LIMIT=50] -->
-    <string name="connected_to_ssid_param1">Connected to %1$s</string>
-
     <!-- A notification is shown after the user logs in to a captive portal network, to indicate
-         that the network should now have internet connectivity. This is the title of the
-         notification, indicating that the device is connected to the network without SSID
-         information. [CHAR LIMIT=50] -->
+         that the network should now have internet connectivity. This is the description of the
+         notification, indicating that the device is connected to the network. [CHAR LIMIT=50] -->
     <string name="connected">Connected</string>
 
     <!-- Notifications are shown when a user connects to a network that advertises a venue
-         information page, so that the user can access that page. This is the message of
+         information page, so that the user can access that page. This is the description of
          the notification. [CHAR LIMIT=50] -->
-    <string name="tap_for_info">Tap for venue information</string>
+    <string name="tap_for_info">Connected / Tap to view website</string>
 
     <!-- The label of application. It's also a sender name of notification. [CHAR LIMIT=50]-->
     <string name="application_label">Connectivity Manager</string>
diff --git a/src/com/android/networkstack/NetworkStackNotifier.java b/src/com/android/networkstack/NetworkStackNotifier.java
index d86e03b..1e97617 100644
--- a/src/com/android/networkstack/NetworkStackNotifier.java
+++ b/src/com/android/networkstack/NetworkStackNotifier.java
@@ -181,6 +181,9 @@ public class NetworkStackNotifier {
         final TrackedNetworkStatus networkStatus = mNetworkStatus.get(network);
         // The required network attributes callbacks were not fired yet for this network
         if (networkStatus == null) return;
+        // Don't show the notification when SSID is unknown to prevent sending something vague to
+        // the user.
+        final boolean hasSsid = !TextUtils.isEmpty(getSsid(networkStatus));
 
         final CaptivePortalDataShim capportData = getCaptivePortalData(networkStatus);
         final boolean showVenueInfo = capportData != null && capportData.getVenueInfoUrl() != null
@@ -191,9 +194,11 @@ public class NetworkStackNotifier {
                 && isVenueInfoNotificationEnabled()
                 // Most browsers do not yet support opening a page on a non-default network, so the
                 // venue info link should not be shown if the network is not the default one.
-                && network.equals(mDefaultNetwork);
+                && network.equals(mDefaultNetwork)
+                && hasSsid;
         final boolean showValidated =
-                networkStatus.mValidatedNotificationPending && networkStatus.isValidated();
+                networkStatus.mValidatedNotificationPending && networkStatus.isValidated()
+                && hasSsid;
         final String notificationTag = getNotificationTag(network);
 
         final Resources res = mContext.getResources();
@@ -216,7 +221,7 @@ public class NetworkStackNotifier {
             // channel even if the notification contains venue info: the "venue info" notification
             // then doubles as a "connected" notification.
             final String channel = showValidated ? CHANNEL_CONNECTED : CHANNEL_VENUE_INFO;
-            builder = getNotificationBuilder(channel, networkStatus, res)
+            builder = getNotificationBuilder(channel, networkStatus, res, getSsid(networkStatus))
                     .setContentText(res.getString(R.string.tap_for_info))
                     .setContentIntent(mDependencies.getActivityPendingIntent(
                             getContextAsUser(mContext, UserHandle.CURRENT),
@@ -226,8 +231,10 @@ public class NetworkStackNotifier {
         } else if (showValidated) {
             if (networkStatus.mShownNotification == NOTE_CONNECTED) return;
 
-            builder = getNotificationBuilder(CHANNEL_CONNECTED, networkStatus, res)
+            builder = getNotificationBuilder(CHANNEL_CONNECTED, networkStatus, res,
+                    getSsid(networkStatus))
                     .setTimeoutAfter(CONNECTED_NOTIFICATION_TIMEOUT_MS)
+                    .setContentText(res.getString(R.string.connected))
                     .setContentIntent(mDependencies.getActivityPendingIntent(
                             getContextAsUser(mContext, UserHandle.CURRENT),
                             new Intent(Settings.ACTION_WIFI_SETTINGS),
@@ -256,9 +263,10 @@ public class NetworkStackNotifier {
     }
 
     private Notification.Builder getNotificationBuilder(@NonNull String channelId,
-            @NonNull TrackedNetworkStatus networkStatus, @NonNull Resources res) {
+            @NonNull TrackedNetworkStatus networkStatus, @NonNull Resources res,
+            @NonNull String ssid) {
         return new Notification.Builder(mContext, channelId)
-                .setContentTitle(getConnectedNotificationTitle(res, networkStatus))
+                .setContentTitle(ssid)
                 .setSmallIcon(R.drawable.icon_wifi);
     }
 
@@ -279,16 +287,6 @@ public class NetworkStackNotifier {
         return mNotificationManager.getNotificationChannel(CHANNEL_VENUE_INFO) != null;
     }
 
-    private String getConnectedNotificationTitle(@NonNull Resources res,
-            @NonNull TrackedNetworkStatus status) {
-        final String ssid = getSsid(status);
-        if (TextUtils.isEmpty(ssid)) {
-            return res.getString(R.string.connected);
-        }
-
-        return res.getString(R.string.connected_to_ssid_param1, ssid);
-    }
-
     private static String getNotificationTag(@NonNull Network network) {
         return Long.toString(network.getNetworkHandle());
     }
diff --git a/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt b/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt
index 39d53af..9100c91 100644
--- a/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt
+++ b/tests/unit/src/com/android/networkstack/NetworkStackNotifierTest.kt
@@ -106,8 +106,8 @@ class NetworkStackNotifierTest {
             .addTransportType(TRANSPORT_WIFI)
             .addCapability(NET_CAPABILITY_VALIDATED)
 
-    private val TEST_CONNECTED_NO_SSID_TITLE = "Connected without SSID"
-    private val TEST_CONNECTED_SSID_TITLE = "Connected to TestSsid"
+    private val TEST_CONNECTED_DESCRIPTION = "Connected"
+    private val TEST_VENUE_DESCRIPTION = "Connected / Tap to view website"
 
     private val TEST_VENUE_INFO_URL = "https://testvenue.example.com/info"
     private val EMPTY_CAPPORT_LP = LinkProperties()
@@ -123,9 +123,8 @@ class NetworkStackNotifierTest {
         MockitoAnnotations.initMocks(this)
         mLooper = TestableLooper.get(this)
         doReturn(mResources).`when`(mContext).resources
-        doReturn(TEST_CONNECTED_NO_SSID_TITLE).`when`(mResources).getString(R.string.connected)
-        doReturn(TEST_CONNECTED_SSID_TITLE).`when`(mResources).getString(
-                R.string.connected_to_ssid_param1, TEST_SSID)
+        doReturn(TEST_CONNECTED_DESCRIPTION).`when`(mResources).getString(R.string.connected)
+        doReturn(TEST_VENUE_DESCRIPTION).`when`(mResources).getString(R.string.tap_for_info)
 
         // applicationInfo is used by Notification.Builder
         val realContext = InstrumentationRegistry.getInstrumentation().context
@@ -201,19 +200,15 @@ class NetworkStackNotifierTest {
         mNotifier.notifyCaptivePortalValidationPending(TEST_NETWORK)
         onCapabilitiesChanged(VALIDATED_CAPABILITIES)
         mLooper.processAllMessages()
-
-        verifyConnectedNotification()
-        verify(mResources).getString(R.string.connected)
-        verifyWifiSettingsIntent(mIntentCaptor.value)
-        verifyCanceledNotificationAfterNetworkLost()
+        // There is no notification when SSID is not set.
+        verify(mNm, never()).notify(any(), anyInt(), any())
     }
 
     @Test
     fun testConnectedNotification_WithSsid() {
         // NetworkCapabilities#getSSID is not available for API <= Q
         assumeTrue(NetworkInformationShimImpl.useApiAboveQ())
-        val capabilities = NetworkCapabilities(VALIDATED_CAPABILITIES)
-                .setSSID(TEST_SSID)
+        val capabilities = NetworkCapabilities(VALIDATED_CAPABILITIES).setSSID(TEST_SSID)
 
         onCapabilitiesChanged(EMPTY_CAPABILITIES)
         mNotifier.notifyCaptivePortalValidationPending(TEST_NETWORK)
@@ -221,7 +216,7 @@ class NetworkStackNotifierTest {
         mLooper.processAllMessages()
 
         verifyConnectedNotification()
-        verify(mResources).getString(R.string.connected_to_ssid_param1, TEST_SSID)
+        verify(mResources).getString(R.string.connected)
         verifyWifiSettingsIntent(mIntentCaptor.value)
         verifyCanceledNotificationAfterNetworkLost()
     }
@@ -233,7 +228,8 @@ class NetworkStackNotifierTest {
         mNotifier.notifyCaptivePortalValidationPending(TEST_NETWORK)
         onLinkPropertiesChanged(TEST_CAPPORT_LP)
         onDefaultNetworkAvailable(TEST_NETWORK)
-        onCapabilitiesChanged(VALIDATED_CAPABILITIES)
+        val capabilities = NetworkCapabilities(VALIDATED_CAPABILITIES).setSSID(TEST_SSID)
+        onCapabilitiesChanged(capabilities)
 
         mLooper.processAllMessages()
 
@@ -251,7 +247,8 @@ class NetworkStackNotifierTest {
         mNotifier.notifyCaptivePortalValidationPending(TEST_NETWORK)
         onLinkPropertiesChanged(TEST_CAPPORT_LP)
         onDefaultNetworkAvailable(TEST_NETWORK)
-        onCapabilitiesChanged(VALIDATED_CAPABILITIES)
+        val capabilities = NetworkCapabilities(VALIDATED_CAPABILITIES).setSSID(TEST_SSID)
+        onCapabilitiesChanged(capabilities)
         mLooper.processAllMessages()
 
         verifyConnectedNotification()
@@ -266,7 +263,8 @@ class NetworkStackNotifierTest {
         assumeTrue(NetworkInformationShimImpl.useApiAboveQ())
         onLinkPropertiesChanged(TEST_CAPPORT_LP)
         onDefaultNetworkAvailable(TEST_NETWORK)
-        onCapabilitiesChanged(VALIDATED_CAPABILITIES)
+        val capabilities = NetworkCapabilities(VALIDATED_CAPABILITIES).setSSID(TEST_SSID)
+        onCapabilitiesChanged(capabilities)
         mLooper.processAllMessages()
 
         verify(mNm).notify(eq(TEST_NETWORK_TAG), mNoteIdCaptor.capture(), mNoteCaptor.capture())