Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit bbaa33e1 authored by Chalard Jean's avatar Chalard Jean
Browse files

[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
parent 24b7fe01

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

+18 −30
Original line number Diff line number Diff line
@@ -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,

                                  NetworkCapabilities caps) {

        CompareResult<String> interfaceDiff = new CompareResult<>(

    private void updateInterfaces(final @Nullable LinkProperties newLp,

            final @Nullable LinkProperties oldLp, final int netId,

            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

+6 −19
Original line number Diff line number Diff line
@@ -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

        // it. In practice, it will be invoked next time this network changes what requests it

        // satisfies through rematchNetworkAndRequests, which may in fact be too late. This code

        // should be reinstated when the bug is fixed.

//        for (final LinkProperties stackedLp : stackedLpsAfterChange) {

//            verify(mBatteryStatsService).noteNetworkInterfaceType(stackedLp.getInterfaceName(),

//                    TYPE_MOBILE);

//        }

        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.