From 117bde44fa934c0186889374cefdf346e1a3f1c0 Mon Sep 17 00:00:00 2001 From: paulhu Date: Wed, 7 Oct 2020 19:03:42 +0800 Subject: [PATCH] Replace NotificationManager @hide APIs Connectivity service module is using some NotificationManager @hide APIs but they are not able to call after CS become a mainline module. Thus, replace them with similar System APIs. Bug: 170593746 Test: atest FrameworksNetTests Change-Id: I2644867cfc01d8d651c7029134294a9d44fdb471 --- .../android/server/ConnectivityService.java | 6 ++- .../NetworkNotificationManager.java | 8 ++-- .../server/ConnectivityServiceTest.java | 20 ++++++--- .../NetworkNotificationManagerTest.java | 44 ++++++++----------- .../android/server/connectivity/VpnTest.java | 21 +++++---- 5 files changed, 55 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 85d77f246b..ce3522a51a 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1147,7 +1147,11 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker = new KeepaliveTracker(mContext, mHandler); mNotifier = new NetworkNotificationManager(mContext, mTelephonyManager, - mContext.getSystemService(NotificationManager.class)); + // Pass a NotificationManager obtained from a context with UserHandle.ALL, then + // NetworkNotificationManager can put up a notification to all users. + // TODO: Create NotificationManager in NetworkNotificationManager directly. + (NotificationManager) mContext.createContextAsUser(UserHandle.ALL, 0 /* flags */) + .getSystemService(Context.NOTIFICATION_SERVICE)); final int dailyLimit = Settings.Global.getInt(mContext.getContentResolver(), Settings.Global.NETWORK_SWITCH_NOTIFICATION_DAILY_LIMIT, diff --git a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java index 26356b440d..3d22d6d37c 100644 --- a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java +++ b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java @@ -30,7 +30,6 @@ import android.content.res.Resources; import android.net.NetworkSpecifier; import android.net.TelephonyNetworkSpecifier; import android.net.wifi.WifiInfo; -import android.os.UserHandle; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; import android.text.TextUtils; @@ -75,8 +74,11 @@ public class NetworkNotificationManager { private static final boolean DBG = true; private static final boolean VDBG = false; + // The context is for the current user (system server) private final Context mContext; private final TelephonyManager mTelephonyManager; + // The notification manager is created from a context for User.ALL, so notifications + // will be sent to all users. private final NotificationManager mNotificationManager; // Tracks the types of notifications managed by this instance, from creation to cancellation. private final SparseIntArray mNotificationTypeMap; @@ -282,7 +284,7 @@ public class NetworkNotificationManager { mNotificationTypeMap.put(id, eventId); try { - mNotificationManager.notifyAsUser(tag, eventId, notification, UserHandle.ALL); + mNotificationManager.notify(tag, eventId, notification); } catch (NullPointerException npe) { Slog.d(TAG, "setNotificationVisible: visible notificationManager error", npe); } @@ -311,7 +313,7 @@ public class NetworkNotificationManager { nameOf(eventId))); } try { - mNotificationManager.cancelAsUser(tag, eventId, UserHandle.ALL); + mNotificationManager.cancel(tag, eventId); } catch (NullPointerException npe) { Slog.d(TAG, String.format( "failed to clear notification tag=%s event=%s", tag, nameOf(eventId)), npe); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 9e770c5bbf..9e7d4ad360 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -251,6 +251,7 @@ import org.junit.Before; import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.AdditionalAnswers; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mock; @@ -451,6 +452,13 @@ public class ConnectivityServiceTest { return super.getSystemService(name); } + @Override + public Context createContextAsUser(UserHandle user, int flags) { + final Context asUser = mock(Context.class, AdditionalAnswers.delegatesTo(this)); + doReturn(user).when(asUser).getUser(); + return asUser; + } + @Override public ContentResolver getContentResolver() { return mContentResolver; @@ -4992,22 +5000,22 @@ public class ConnectivityServiceTest { // simulate that situation and check if ConnectivityService could filter that case. mWiFiNetworkAgent.mNetworkMonitor.forceReevaluation(Process.myUid()); waitForIdle(); - verify(mNotificationManager, timeout(TIMEOUT_MS).times(1)).notifyAsUser(anyString(), - eq(NotificationType.PRIVATE_DNS_BROKEN.eventId), any(), eq(UserHandle.ALL)); + verify(mNotificationManager, timeout(TIMEOUT_MS).times(1)).notify(anyString(), + eq(NotificationType.PRIVATE_DNS_BROKEN.eventId), any()); // If private DNS resolution successful, the PRIVATE_DNS_BROKEN notification shouldn't be // shown. mWiFiNetworkAgent.setNetworkValid(true /* isStrictMode */); mWiFiNetworkAgent.mNetworkMonitor.forceReevaluation(Process.myUid()); waitForIdle(); - verify(mNotificationManager, timeout(TIMEOUT_MS).times(1)).cancelAsUser(anyString(), - eq(NotificationType.PRIVATE_DNS_BROKEN.eventId), eq(UserHandle.ALL)); + verify(mNotificationManager, timeout(TIMEOUT_MS).times(1)).cancel(anyString(), + eq(NotificationType.PRIVATE_DNS_BROKEN.eventId)); // If private DNS resolution failed again, the PRIVATE_DNS_BROKEN notification should be // shown again. mWiFiNetworkAgent.setNetworkInvalid(true /* isStrictMode */); mWiFiNetworkAgent.mNetworkMonitor.forceReevaluation(Process.myUid()); waitForIdle(); - verify(mNotificationManager, timeout(TIMEOUT_MS).times(2)).notifyAsUser(anyString(), - eq(NotificationType.PRIVATE_DNS_BROKEN.eventId), any(), eq(UserHandle.ALL)); + verify(mNotificationManager, timeout(TIMEOUT_MS).times(2)).notify(anyString(), + eq(NotificationType.PRIVATE_DNS_BROKEN.eventId), any()); } @Test diff --git a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java index 47db5d4316..fa4df4e8b1 100644 --- a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java +++ b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java @@ -114,7 +114,7 @@ public class NetworkNotificationManagerTest { final String tag = NetworkNotificationManager.tagFor(id); mManager.showNotification(id, PRIVATE_DNS_BROKEN, nai, null, null, true); verify(mNotificationManager, times(1)) - .notifyAsUser(eq(tag), eq(PRIVATE_DNS_BROKEN.eventId), any(), any()); + .notify(eq(tag), eq(PRIVATE_DNS_BROKEN.eventId), any()); final int transportType = NetworkNotificationManager.approximateTransportType(nai); if (transportType == NetworkCapabilities.TRANSPORT_WIFI) { verify(mResources, times(1)).getString(title, eq(any())); @@ -164,8 +164,8 @@ public class NetworkNotificationManagerTest { 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()); + verify(mNotificationManager, times(1)).notify(eq(tag), eq(eventId), any()); + verify(mNotificationManager, times(1)).cancel(eq(tag), eq(eventId)); } } @@ -174,13 +174,13 @@ public class NetworkNotificationManagerTest { mManager.showNotification(100, NO_INTERNET, mCellNai, mWifiNai, null, false); mManager.showNotification(101, LOST_INTERNET, mCellNai, mWifiNai, null, false); - verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any()); + verify(mNotificationManager, never()).notify(any(), anyInt(), any()); mManager.showNotification(102, NO_INTERNET, mWifiNai, mCellNai, null, false); final int eventId = NO_INTERNET.eventId; final String tag = NetworkNotificationManager.tagFor(102); - verify(mNotificationManager, times(1)).notifyAsUser(eq(tag), eq(eventId), any(), any()); + verify(mNotificationManager, times(1)).notify(eq(tag), eq(eventId), any()); } @Test @@ -191,7 +191,7 @@ public class NetworkNotificationManagerTest { mManager.showNotification(103, LOST_INTERNET, mWifiNai, mCellNai, null, false); mManager.showNotification(104, NETWORK_SWITCH, mWifiNai, mCellNai, null, false); - verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any()); + verify(mNotificationManager, never()).notify(any(), anyInt(), any()); } @Test @@ -201,19 +201,16 @@ public class NetworkNotificationManagerTest { // 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()); + verify(mNotificationManager, times(1)).notify(eq(tag), eq(NO_INTERNET.eventId), 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()); + 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); - verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(SIGN_IN.eventId), any()); + verify(mNotificationManager, times(1)).cancel(eq(tag), eq(SIGN_IN.eventId)); } @Test @@ -223,18 +220,17 @@ public class NetworkNotificationManagerTest { // 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()); + verify(mNotificationManager, times(1)).notify(eq(tag), eq(SIGN_IN.eventId), 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()); + verify(mNotificationManager, never()).cancel(any(), anyInt()); + verify(mNotificationManager, never()).notify(any(), anyInt(), any()); // Network disconnects mManager.clearNotification(id); - verify(mNotificationManager, times(1)).cancelAsUser(eq(tag), eq(SIGN_IN.eventId), any()); + verify(mNotificationManager, times(1)).cancel(eq(tag), eq(SIGN_IN.eventId)); } @Test @@ -246,24 +242,20 @@ public class NetworkNotificationManagerTest { // to previous type or not. If they are equal then clear the notification; if they are not // equal then return. mManager.showNotification(id, NO_INTERNET, mWifiNai, mCellNai, null, false); - verify(mNotificationManager, times(1)) - .notifyAsUser(eq(tag), eq(NO_INTERNET.eventId), any(), any()); + verify(mNotificationManager, times(1)).notify(eq(tag), eq(NO_INTERNET.eventId), any()); // Previous notification is NO_INTERNET and given type is NO_INTERNET too. The notification // should be cleared. mManager.clearNotification(id, NO_INTERNET); - verify(mNotificationManager, times(1)) - .cancelAsUser(eq(tag), eq(NO_INTERNET.eventId), any()); + verify(mNotificationManager, times(1)).cancel(eq(tag), eq(NO_INTERNET.eventId)); // SIGN_IN is popped-up. mManager.showNotification(id, SIGN_IN, mWifiNai, mCellNai, null, false); - verify(mNotificationManager, times(1)) - .notifyAsUser(eq(tag), eq(SIGN_IN.eventId), any(), any()); + verify(mNotificationManager, times(1)).notify(eq(tag), eq(SIGN_IN.eventId), any()); // The notification type is not matching previous one, PARTIAL_CONNECTIVITY won't be // cleared. mManager.clearNotification(id, PARTIAL_CONNECTIVITY); - verify(mNotificationManager, never()) - .cancelAsUser(eq(tag), eq(PARTIAL_CONNECTIVITY.eventId), any()); + verify(mNotificationManager, never()).cancel(eq(tag), eq(PARTIAL_CONNECTIVITY.eventId)); } } diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index daa2627d64..2fa0914dab 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -45,6 +45,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -106,6 +107,7 @@ import com.android.server.IpSecService; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.AdditionalAnswers; import org.mockito.Answers; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; @@ -215,6 +217,8 @@ public class VpnTest { when(mContext.getOpPackageName()).thenReturn(TEST_VPN_PKG); when(mContext.getSystemService(eq(Context.USER_SERVICE))).thenReturn(mUserManager); when(mContext.getSystemService(eq(Context.APP_OPS_SERVICE))).thenReturn(mAppOps); + when(mContext.getSystemServiceName(NotificationManager.class)) + .thenReturn(Context.NOTIFICATION_SERVICE); when(mContext.getSystemService(eq(Context.NOTIFICATION_SERVICE))) .thenReturn(mNotificationManager); when(mContext.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) @@ -594,26 +598,23 @@ public class VpnTest { // Don't show a notification for regular disconnected states. vpn.updateState(DetailedState.DISCONNECTED, TAG); - order.verify(mNotificationManager, atLeastOnce()) - .cancelAsUser(anyString(), anyInt(), eq(userHandle)); + order.verify(mNotificationManager, atLeastOnce()).cancel(anyString(), anyInt()); // Start showing a notification for disconnected once always-on. vpn.setAlwaysOnPackage(PKGS[0], false, null, mKeyStore); - order.verify(mNotificationManager) - .notifyAsUser(anyString(), anyInt(), any(), eq(userHandle)); + order.verify(mNotificationManager).notify(anyString(), anyInt(), any()); // Stop showing the notification once connected. vpn.updateState(DetailedState.CONNECTED, TAG); - order.verify(mNotificationManager).cancelAsUser(anyString(), anyInt(), eq(userHandle)); + order.verify(mNotificationManager).cancel(anyString(), anyInt()); // Show the notification if we disconnect again. vpn.updateState(DetailedState.DISCONNECTED, TAG); - order.verify(mNotificationManager) - .notifyAsUser(anyString(), anyInt(), any(), eq(userHandle)); + order.verify(mNotificationManager).notify(anyString(), anyInt(), any()); // Notification should be cleared after unsetting always-on package. vpn.setAlwaysOnPackage(null, false, null, mKeyStore); - order.verify(mNotificationManager).cancelAsUser(anyString(), anyInt(), eq(userHandle)); + order.verify(mNotificationManager).cancel(anyString(), anyInt()); } @Test @@ -1272,6 +1273,10 @@ public class VpnTest { * Mock some methods of vpn object. */ private Vpn createVpn(@UserIdInt int userId) { + final Context asUserContext = mock(Context.class, AdditionalAnswers.delegatesTo(mContext)); + doReturn(UserHandle.of(userId)).when(asUserContext).getUser(); + when(mContext.createContextAsUser(eq(UserHandle.of(userId)), anyInt())) + .thenReturn(asUserContext); return new Vpn(Looper.myLooper(), mContext, new TestDeps(), mNetService, userId, mKeyStore, mSystemServices, mIkev2SessionCreator); }