Merge "[FUI19] Migrate ConnectivityService to use NetworkStatsManager"

services/core/java/com/android/server/ConnectivityService.java

@@ -70,6 +70,7 @@ import android.annotation.Nullable;
 import android.app.AppOpsManager;
 import android.app.BroadcastOptions;
 import android.app.PendingIntent;
+import android.app.usage.NetworkStatsManager;
 import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.ContentResolver;
@@ -95,7 +96,6 @@ import android.net.INetworkActivityListener;
 import android.net.INetworkMonitor;
 import android.net.INetworkMonitorCallbacks;
 import android.net.INetworkPolicyListener;
-import android.net.INetworkStatsService;
 import android.net.IOnSetOemNetworkPreferenceListener;
 import android.net.IQosCallback;
 import android.net.ISocketKeepaliveCallback;
@@ -331,7 +331,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     protected IDnsResolver mDnsResolver;
     @VisibleForTesting
     protected INetd mNetd;
-    private INetworkStatsService mStatsService;
+    private NetworkStatsManager mStatsManager;
     private NetworkPolicyManager mPolicyManager;
     private NetworkPolicyManagerInternal mPolicyManagerInternal;
     private final NetdCallback mNetdCallback;
@@ -1042,15 +1042,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
-    public ConnectivityService(Context context, INetworkStatsService statsService) {
+    public ConnectivityService(Context context) {
-        this(context, statsService, getDnsResolver(context), new IpConnectivityLog(),
+        this(context, getDnsResolver(context), new IpConnectivityLog(),
                 NetdService.getInstance(), new Dependencies());
     }
 
     @VisibleForTesting
-    protected ConnectivityService(Context context, INetworkStatsService statsService,
+    protected ConnectivityService(Context context, IDnsResolver dnsresolver,
-            IDnsResolver dnsresolver, IpConnectivityLog logger,
+            IpConnectivityLog logger, INetd netd, Dependencies deps) {
-            INetd netd, Dependencies deps) {
         if (DBG) log("ConnectivityService starting up");
 
         mDeps = Objects.requireNonNull(deps, "missing Dependencies");
@@ -1096,7 +1095,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         // TODO: Consider making the timer customizable.
         mNascentDelayMs = DEFAULT_NASCENT_DELAY_MS;
 
-        mStatsService = Objects.requireNonNull(statsService, "missing INetworkStatsService");
+        mStatsManager = mContext.getSystemService(NetworkStatsManager.class);
         mPolicyManager = mContext.getSystemService(NetworkPolicyManager.class);
         mPolicyManagerInternal = Objects.requireNonNull(
                 LocalServices.getService(NetworkPolicyManagerInternal.class),
@@ -7913,7 +7912,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
      *
      * Must be called on the handler thread.
      */
-    private Network[] getDefaultNetworks() {
+    @NonNull
+    private ArrayList<Network> getDefaultNetworks() {
         ensureRunningOnConnectivityServiceThread();
         final ArrayList<Network> defaultNetworks = new ArrayList<>();
         final Set<Integer> activeNetIds = new ArraySet<>();
@@ -7927,7 +7927,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 defaultNetworks.add(nai.network);
             }
         }
-        return defaultNetworks.toArray(new Network[0]);
+        return defaultNetworks;
     }
 
     /**
@@ -7952,8 +7952,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
                         state.legacyNetworkType);
                 snapshots.add(snapshot);
             }
-            mStatsService.forceUpdateIfaces(getDefaultNetworks(), snapshots.toArray(
+            mStatsManager.notifyNetworkStatus(getDefaultNetworks(),
-                    new NetworkStateSnapshot[0]), activeIface, underlyingNetworkInfos);
+                    snapshots, activeIface, Arrays.asList(underlyingNetworkInfos));
         } catch (Exception ignored) {
         }
     }

services/core/java/com/android/server/ConnectivityServiceInitializer.java

@@ -20,8 +20,6 @@ import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_HIGH;
 import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_NORMAL;
 
 import android.content.Context;
-import android.net.INetworkStatsService;
-import android.os.ServiceManager;
 import android.util.Log;
 
 /**
@@ -37,7 +35,7 @@ public final class ConnectivityServiceInitializer extends SystemService {
         // Load JNI libraries used by ConnectivityService and its dependencies
         System.loadLibrary("service-connectivity");
         // TODO: Define formal APIs to get the needed services.
-        mConnectivity = new ConnectivityService(context, getNetworkStatsService());
+        mConnectivity = new ConnectivityService(context);
     }
 
     @Override
@@ -46,9 +44,4 @@ public final class ConnectivityServiceInitializer extends SystemService {
         publishBinderService(Context.CONNECTIVITY_SERVICE, mConnectivity,
                 /* allowIsolated= */ false, DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL);
     }
-
-    private INetworkStatsService getNetworkStatsService() {
-        return INetworkStatsService.Stub.asInterface(
-                ServiceManager.getService(Context.NETWORK_STATS_SERVICE));
-    }
 }

tests/net/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt

@@ -16,6 +16,7 @@
 
 package com.android.server.net.integrationtests
 
+import android.app.usage.NetworkStatsManager
 import android.content.ComponentName
 import android.content.Context
 import android.content.Context.BIND_AUTO_CREATE
@@ -25,7 +26,6 @@ import android.content.ServiceConnection
 import android.net.ConnectivityManager
 import android.net.IDnsResolver
 import android.net.INetd
-import android.net.INetworkStatsService
 import android.net.LinkProperties
 import android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL
 import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET
@@ -37,7 +37,6 @@ import android.net.Uri
 import android.net.metrics.IpConnectivityLog
 import android.os.ConditionVariable
 import android.os.IBinder
-import android.os.INetworkManagementService
 import android.os.SystemConfigManager
 import android.os.UserHandle
 import android.testing.TestableContext
@@ -87,9 +86,7 @@ class ConnectivityServiceIntegrationTest {
     // lateinit used here for mocks as they need to be reinitialized between each test and the test
     // should crash if they are used before being initialized.
     @Mock
-    private lateinit var netManager: INetworkManagementService
+    private lateinit var statsManager: NetworkStatsManager
-    @Mock
-    private lateinit var statsService: INetworkStatsService
     @Mock
     private lateinit var log: IpConnectivityLog
     @Mock
@@ -172,12 +169,13 @@ class ConnectivityServiceIntegrationTest {
         service = TestConnectivityService(makeDependencies())
         cm = ConnectivityManager(context, service)
         context.addMockSystemService(Context.CONNECTIVITY_SERVICE, cm)
+        context.addMockSystemService(Context.NETWORK_STATS_SERVICE, statsManager)
 
         service.systemReadyInternal()
     }
 
     private inner class TestConnectivityService(deps: Dependencies) : ConnectivityService(
-            context, statsService, dnsResolver, log, netd, deps)
+            context, dnsResolver, log, netd, deps)
 
     private fun makeDependencies(): ConnectivityService.Dependencies {
         val deps = spy(ConnectivityService.Dependencies())

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -149,6 +149,7 @@ import android.app.AlarmManager;
 import android.app.AppOpsManager;
 import android.app.NotificationManager;
 import android.app.PendingIntent;
+import android.app.usage.NetworkStatsManager;
 import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.ContentProvider;
@@ -180,7 +181,6 @@ import android.net.INetd;
 import android.net.INetworkMonitor;
 import android.net.INetworkMonitorCallbacks;
 import android.net.INetworkPolicyListener;
-import android.net.INetworkStatsService;
 import android.net.IOnSetOemNetworkPreferenceListener;
 import android.net.IQosCallback;
 import android.net.InetAddresses;
@@ -203,7 +203,6 @@ import android.net.NetworkRequest;
 import android.net.NetworkSpecifier;
 import android.net.NetworkStack;
 import android.net.NetworkStackClient;
-import android.net.NetworkStateSnapshot;
 import android.net.NetworkTestResultParcelable;
 import android.net.OemNetworkPreferences;
 import android.net.ProxyInfo;
@@ -425,7 +424,7 @@ public class ConnectivityServiceTest {
 
     @Mock DeviceIdleInternal mDeviceIdleInternal;
     @Mock INetworkManagementService mNetworkManagementService;
-    @Mock INetworkStatsService mStatsService;
+    @Mock NetworkStatsManager mStatsManager;
     @Mock IBatteryStats mBatteryStatsService;
     @Mock IDnsResolver mMockDnsResolver;
     @Mock INetd mMockNetd;
@@ -541,6 +540,7 @@ public class ConnectivityServiceTest {
             if (Context.ETHERNET_SERVICE.equals(name)) return mEthernetManager;
             if (Context.NETWORK_POLICY_SERVICE.equals(name)) return mNetworkPolicyManager;
             if (Context.SYSTEM_CONFIG_SERVICE.equals(name)) return mSystemConfigManager;
+            if (Context.NETWORK_STATS_SERVICE.equals(name)) return mStatsManager;
             return super.getSystemService(name);
         }
 
@@ -1473,7 +1473,6 @@ public class ConnectivityServiceTest {
         mDeps = makeDependencies();
         returnRealCallingUid();
         mService = new ConnectivityService(mServiceContext,
-                mStatsService,
                 mMockDnsResolver,
                 mock(IpConnectivityLog.class),
                 mMockNetd,
@@ -5489,18 +5488,19 @@ public class ConnectivityServiceTest {
         assertEquals(expectedSet, actualSet);
     }
 
-    private void expectForceUpdateIfaces(Network[] networks, String defaultIface,
+    private void expectNetworkStatus(Network[] networks, String defaultIface,
             Integer vpnUid, String vpnIfname, String[] underlyingIfaces) throws Exception {
-        ArgumentCaptor<Network[]> networksCaptor = ArgumentCaptor.forClass(Network[].class);
+        ArgumentCaptor<List<Network>> networksCaptor = ArgumentCaptor.forClass(List.class);
-        ArgumentCaptor<UnderlyingNetworkInfo[]> vpnInfosCaptor = ArgumentCaptor.forClass(
+        ArgumentCaptor<List<UnderlyingNetworkInfo>> vpnInfosCaptor =
-                UnderlyingNetworkInfo[].class);
+                ArgumentCaptor.forClass(List.class);
 
-        verify(mStatsService, atLeastOnce()).forceUpdateIfaces(networksCaptor.capture(),
+        verify(mStatsManager, atLeastOnce()).notifyNetworkStatus(networksCaptor.capture(),
-                any(NetworkStateSnapshot[].class), eq(defaultIface), vpnInfosCaptor.capture());
+                any(List.class), eq(defaultIface), vpnInfosCaptor.capture());
 
-        assertSameElementsNoDuplicates(networksCaptor.getValue(), networks);
+        assertSameElementsNoDuplicates(networksCaptor.getValue().toArray(), networks);
 
-        UnderlyingNetworkInfo[] infos = vpnInfosCaptor.getValue();
+        UnderlyingNetworkInfo[] infos =
+                vpnInfosCaptor.getValue().toArray(new UnderlyingNetworkInfo[0]);
         if (vpnUid != null) {
             assertEquals("Should have exactly one VPN:", 1, infos.length);
             UnderlyingNetworkInfo info = infos[0];
@@ -5514,8 +5514,9 @@ public class ConnectivityServiceTest {
         }
     }
 
-    private void expectForceUpdateIfaces(Network[] networks, String defaultIface) throws Exception {
+    private void expectNetworkStatus(
-        expectForceUpdateIfaces(networks, defaultIface, null, null, new String[0]);
+            Network[] networks, String defaultIface) throws Exception {
+        expectNetworkStatus(networks, defaultIface, null, null, new String[0]);
     }
 
     @Test
@@ -5535,46 +5536,46 @@ public class ConnectivityServiceTest {
         mCellNetworkAgent.connect(false);
         mCellNetworkAgent.sendLinkProperties(cellLp);
         waitForIdle();
-        expectForceUpdateIfaces(onlyCell, MOBILE_IFNAME);
+        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Default network switch should update ifaces.
         mWiFiNetworkAgent.connect(false);
         mWiFiNetworkAgent.sendLinkProperties(wifiLp);
         waitForIdle();
         assertEquals(wifiLp, mService.getActiveLinkProperties());
-        expectForceUpdateIfaces(onlyWifi, WIFI_IFNAME);
+        expectNetworkStatus(onlyWifi, WIFI_IFNAME);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Disconnect should update ifaces.
         mWiFiNetworkAgent.disconnect();
         waitForIdle();
-        expectForceUpdateIfaces(onlyCell, MOBILE_IFNAME);
+        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Metered change should update ifaces
         mCellNetworkAgent.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED);
         waitForIdle();
-        expectForceUpdateIfaces(onlyCell, MOBILE_IFNAME);
+        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         mCellNetworkAgent.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED);
         waitForIdle();
-        expectForceUpdateIfaces(onlyCell, MOBILE_IFNAME);
+        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Temp metered change shouldn't update ifaces
         mCellNetworkAgent.addCapability(NetworkCapabilities.NET_CAPABILITY_TEMPORARILY_NOT_METERED);
         waitForIdle();
-        verify(mStatsService, never()).forceUpdateIfaces(eq(onlyCell), any(
+        verify(mStatsManager, never()).notifyNetworkStatus(eq(Arrays.asList(onlyCell)),
-                NetworkStateSnapshot[].class), eq(MOBILE_IFNAME), eq(new UnderlyingNetworkInfo[0]));
+                any(List.class), eq(MOBILE_IFNAME), any(List.class));
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Roaming change should update ifaces
         mCellNetworkAgent.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING);
         waitForIdle();
-        expectForceUpdateIfaces(onlyCell, MOBILE_IFNAME);
+        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Test VPNs.
         final LinkProperties lp = new LinkProperties();
@@ -5587,7 +5588,7 @@ public class ConnectivityServiceTest {
                 mCellNetworkAgent.getNetwork(), mMockVpn.getNetwork()};
 
         // A VPN with default (null) underlying networks sets the underlying network's interfaces...
-        expectForceUpdateIfaces(cellAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+        expectNetworkStatus(cellAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
                 new String[]{MOBILE_IFNAME});
 
         // ...and updates them as the default network switches.
@@ -5604,9 +5605,9 @@ public class ConnectivityServiceTest {
 
         waitForIdle();
         assertEquals(wifiLp, mService.getActiveLinkProperties());
-        expectForceUpdateIfaces(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
+        expectNetworkStatus(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
                 new String[]{WIFI_IFNAME});
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // A VPN that sets its underlying networks passes the underlying interfaces, and influences
         // the default interface sent to NetworkStatsService by virtue of applying to the system
@@ -5616,22 +5617,22 @@ public class ConnectivityServiceTest {
         // applies to the system server UID should not have any bearing on network stats.
         mMockVpn.setUnderlyingNetworks(onlyCell);
         waitForIdle();
-        expectForceUpdateIfaces(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
                 new String[]{MOBILE_IFNAME});
-        reset(mStatsService);
+        reset(mStatsManager);
 
         mMockVpn.setUnderlyingNetworks(cellAndWifi);
         waitForIdle();
-        expectForceUpdateIfaces(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
                 new String[]{MOBILE_IFNAME, WIFI_IFNAME});
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Null underlying networks are ignored.
         mMockVpn.setUnderlyingNetworks(cellNullAndWifi);
         waitForIdle();
-        expectForceUpdateIfaces(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
                 new String[]{MOBILE_IFNAME, WIFI_IFNAME});
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // If an underlying network disconnects, that interface should no longer be underlying.
         // This doesn't actually work because disconnectAndDestroyNetwork only notifies
@@ -5643,17 +5644,17 @@ public class ConnectivityServiceTest {
         mCellNetworkAgent.disconnect();
         waitForIdle();
         assertNull(mService.getLinkProperties(mCellNetworkAgent.getNetwork()));
-        expectForceUpdateIfaces(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
                 new String[]{MOBILE_IFNAME, WIFI_IFNAME});
 
         // Confirm that we never tell NetworkStatsService that cell is no longer the underlying
         // network for the VPN...
-        verify(mStatsService, never()).forceUpdateIfaces(any(Network[].class),
+        verify(mStatsManager, never()).notifyNetworkStatus(any(List.class),
-                any(NetworkStateSnapshot[].class), any() /* anyString() doesn't match null */,
+                any(List.class), any() /* anyString() doesn't match null */,
-                argThat(infos -> infos[0].underlyingIfaces.size() == 1
+                argThat(infos -> infos.get(0).underlyingIfaces.size() == 1
-                        && WIFI_IFNAME.equals(infos[0].underlyingIfaces.get(0))));
+                        && WIFI_IFNAME.equals(infos.get(0).underlyingIfaces.get(0))));
-        verifyNoMoreInteractions(mStatsService);
+        verifyNoMoreInteractions(mStatsManager);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // ... but if something else happens that causes notifyIfacesChangedForNetworkStats to be
         // called again, it does. For example, connect Ethernet, but with a low score, such that it
@@ -5662,13 +5663,13 @@ public class ConnectivityServiceTest {
         mEthernetNetworkAgent.adjustScore(-40);
         mEthernetNetworkAgent.connect(false);
         waitForIdle();
-        verify(mStatsService).forceUpdateIfaces(any(Network[].class),
+        verify(mStatsManager).notifyNetworkStatus(any(List.class),
-                any(NetworkStateSnapshot[].class), any() /* anyString() doesn't match null */,
+                any(List.class), any() /* anyString() doesn't match null */,
-                argThat(vpnInfos -> vpnInfos[0].underlyingIfaces.size() == 1
+                argThat(vpnInfos -> vpnInfos.get(0).underlyingIfaces.size() == 1
-                        && WIFI_IFNAME.equals(vpnInfos[0].underlyingIfaces.get(0))));
+                        && WIFI_IFNAME.equals(vpnInfos.get(0).underlyingIfaces.get(0))));
         mEthernetNetworkAgent.disconnect();
         waitForIdle();
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // When a VPN declares no underlying networks (i.e., no connectivity), getAllVpnInfo
         // does not return the VPN, so CS does not pass it to NetworkStatsService. This causes
@@ -5678,27 +5679,27 @@ public class ConnectivityServiceTest {
         // Also, for the same reason as above, the active interface passed in is null.
         mMockVpn.setUnderlyingNetworks(new Network[0]);
         waitForIdle();
-        expectForceUpdateIfaces(wifiAndVpn, null);
+        expectNetworkStatus(wifiAndVpn, null);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Specifying only a null underlying network is the same as no networks.
         mMockVpn.setUnderlyingNetworks(onlyNull);
         waitForIdle();
-        expectForceUpdateIfaces(wifiAndVpn, null);
+        expectNetworkStatus(wifiAndVpn, null);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Specifying networks that are all disconnected is the same as specifying no networks.
         mMockVpn.setUnderlyingNetworks(onlyCell);
         waitForIdle();
-        expectForceUpdateIfaces(wifiAndVpn, null);
+        expectNetworkStatus(wifiAndVpn, null);
-        reset(mStatsService);
+        reset(mStatsManager);
 
         // Passing in null again means follow the default network again.
         mMockVpn.setUnderlyingNetworks(null);
         waitForIdle();
-        expectForceUpdateIfaces(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
+        expectNetworkStatus(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
                 new String[]{WIFI_IFNAME});
-        reset(mStatsService);
+        reset(mStatsManager);
     }
 
     @Test