[NS A14] Move code notifying battery stats in its right place

This should be done once every time an interface comes online.
Doing this in updateLinkProperties guarantees this happens every
time a new interface comes online, but it doesn't do it more
often than needed.

Test: FrameworksNetTests NetworkStackTests
Change-Id: I0613c23f44192944266d76107308da8d1c541d1c

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

@@ -5651,7 +5651,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
         // are accurate.
         networkAgent.clatd.fixupLinkProperties(oldLp, newLp);
 
-        updateInterfaces(newLp, oldLp, netId, networkAgent.networkCapabilities);
+        updateInterfaces(newLp, oldLp, netId, networkAgent.networkCapabilities,
+                networkAgent.networkInfo.getType());
 
         // update filtering rules, need to happen after the interface update so netd knows about the
         // new interface (the interface name -> index map becomes initialized)
@@ -5730,21 +5731,26 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
     }
 
-    private void updateInterfaces(LinkProperties newLp, LinkProperties oldLp, int netId,
+    private void updateInterfaces(final @Nullable LinkProperties newLp,
-                                  NetworkCapabilities caps) {
+            final @Nullable LinkProperties oldLp, final int netId,
-        CompareResult<String> interfaceDiff = new CompareResult<>(
+            final @Nullable NetworkCapabilities caps, final int legacyType) {
+        final CompareResult<String> interfaceDiff = new CompareResult<>(
                 oldLp != null ? oldLp.getAllInterfaceNames() : null,
                 newLp != null ? newLp.getAllInterfaceNames() : null);
-        for (String iface : interfaceDiff.added) {
+        if (!interfaceDiff.added.isEmpty()) {
+            final IBatteryStats bs = mDeps.getBatteryStatsService();
+            for (final String iface : interfaceDiff.added) {
                 try {
                     if (DBG) log("Adding iface " + iface + " to network " + netId);
                     mNMS.addInterfaceToNetwork(iface, netId);
                     wakeupModifyInterface(iface, caps, true);
+                    bs.noteNetworkInterfaceType(iface, legacyType);
                 } catch (Exception e) {
                     loge("Exception adding interface: " + e);
                 }
             }
-        for (String iface : interfaceDiff.removed) {
+        }
+        for (final String iface : interfaceDiff.removed) {
             try {
                 if (DBG) log("Removing iface " + iface + " from network " + netId);
                 wakeupModifyInterface(iface, caps, false);
@@ -6503,24 +6509,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
             mLegacyTypeTracker.add(newNetwork.networkInfo.getType(), newNetwork);
             notifyLockdownVpn(newNetwork);
         }
-
-        if (reassignedRequests.containsValue(newNetwork) || newNetwork.isVPN()) {
-            // Notify battery stats service about this network, both the normal
-            // interface and any stacked links.
-            // TODO: Avoid redoing this; this must only be done once when a network comes online.
-            try {
-                final IBatteryStats bs = mDeps.getBatteryStatsService();
-                final int type = newNetwork.networkInfo.getType();
-
-                final String baseIface = newNetwork.linkProperties.getInterfaceName();
-                bs.noteNetworkInterfaceType(baseIface, type);
-                for (LinkProperties stacked : newNetwork.linkProperties.getStackedLinks()) {
-                    final String stackedIface = stacked.getInterfaceName();
-                    bs.noteNetworkInterfaceType(stackedIface, type);
-                }
-            } catch (RemoteException ignored) {
-            }
-        }
     }
 
     /**

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

@@ -5664,14 +5664,6 @@ public class ConnectivityServiceTest {
                 TYPE_WIFI);
         reset(mBatteryStatsService);
 
-        // TODO : In the current code, ConnectivityService only tells BatteryStatsService about
-        // the type of networks that satisfy a request. That is a bug in a sense, but it has no
-        // consequences because a network that never satisfies any request gets torn down right
-        // away. Because of this, in the context of this test, the cell network agent does not
-        // satisfy any request as long as WiFi is connected, so the test below would fail if
-        // the WiFi network agent is not disconnected first. When this bug is fixed, remove the
-        // WiFi disconnect for more precise testing.
-        mWiFiNetworkAgent.disconnect();
         mCellNetworkAgent.disconnect();
 
         cellLp.setInterfaceName("wifi0");
@@ -5722,13 +5714,12 @@ public class ConnectivityServiceTest {
         mCm.registerNetworkCallback(networkRequest, networkCallback);
 
         // Prepare ipv6 only link properties.
-        mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
-        final int cellNetId = mCellNetworkAgent.getNetwork().netId;
         final LinkProperties cellLp = new LinkProperties();
         cellLp.setInterfaceName(MOBILE_IFNAME);
         cellLp.addLinkAddress(myIpv6);
         cellLp.addRoute(new RouteInfo((IpPrefix) null, myIpv6.getAddress(), MOBILE_IFNAME));
         cellLp.addRoute(new RouteInfo(myIpv6, null, MOBILE_IFNAME));
+        mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp);
         reset(mNetworkManagementService);
         reset(mMockDnsResolver);
         reset(mMockNetd);
@@ -5737,8 +5728,8 @@ public class ConnectivityServiceTest {
                 .thenReturn(getClatInterfaceConfig(myIpv4));
 
         // Connect with ipv6 link properties. Expect prefix discovery to be started.
-        mCellNetworkAgent.sendLinkProperties(cellLp);
         mCellNetworkAgent.connect(true);
+        final int cellNetId = mCellNetworkAgent.getNetwork().netId;
         waitForIdle();
 
         verify(mMockNetd, times(1)).networkCreatePhysical(eq(cellNetId), anyInt());
@@ -5811,14 +5802,10 @@ public class ConnectivityServiceTest {
         assertEquals(1, resolvrParams.servers.length);
         assertTrue(ArrayUtils.contains(resolvrParams.servers, "8.8.8.8"));
 
-        // TODO : this should be invoked but in the current code there is no path to invoke
+        for (final LinkProperties stackedLp : stackedLpsAfterChange) {
-        // it. In practice, it will be invoked next time this network changes what requests it
+            verify(mBatteryStatsService).noteNetworkInterfaceType(stackedLp.getInterfaceName(),
-        // satisfies through rematchNetworkAndRequests, which may in fact be too late. This code
+                    TYPE_MOBILE);
-        // should be reinstated when the bug is fixed.
+        }
-//        for (final LinkProperties stackedLp : stackedLpsAfterChange) {
-//            verify(mBatteryStatsService).noteNetworkInterfaceType(stackedLp.getInterfaceName(),
-//                    TYPE_MOBILE);
-//        }
 
         // Add ipv4 address, expect that clatd and prefix discovery are stopped and stacked
         // linkproperties are cleaned up.