From d49aab92c9506464e35edce3f1356f856be9ec19 Mon Sep 17 00:00:00 2001 From: junyulai Date: Tue, 29 Dec 2020 19:17:01 +0800 Subject: [PATCH 1/2] [FUI06] Stop using NetworkInfo in NetworkState from external callers This is achieved by: 1. Use legacy network type inside NetworkState to replace the needs of referencing NetworkInfo.getType(). 2. Let getAllNetworkState only return networks with isConnected() equals true. This allows callers such as NPMS or NSS does not have to reference to NetworkInfo.isConnected(). Test: atest FrameworksNetTests NetworkPolicyManagerServiceTest Bug: 174123988 Change-Id: I1c4eb08d18ca973eb8f41d06258872eabc0006b8 --- core/java/android/net/NetworkState.java | 6 ++++++ .../core/java/com/android/server/ConnectivityService.java | 3 ++- tests/net/java/android/net/NetworkTemplateTest.kt | 1 - 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/NetworkState.java b/core/java/android/net/NetworkState.java index e1ef8b5ea5..2d6ddcbb5d 100644 --- a/core/java/android/net/NetworkState.java +++ b/core/java/android/net/NetworkState.java @@ -41,6 +41,7 @@ public class NetworkState implements Parcelable { public final Network network; public final String subscriberId; public final String networkId; + public final int legacyNetworkType; private NetworkState() { networkInfo = null; @@ -49,6 +50,7 @@ public class NetworkState implements Parcelable { network = null; subscriberId = null; networkId = null; + legacyNetworkType = 0; } public NetworkState(@NonNull NetworkInfo networkInfo, @NonNull LinkProperties linkProperties, @@ -60,6 +62,8 @@ public class NetworkState implements Parcelable { this.network = network; this.subscriberId = subscriberId; this.networkId = networkId; + // TODO: Pass legacyNetworkType directly from parameters and remove NetworkInfo. + this.legacyNetworkType = this.networkInfo.getType(); // This object is an atomic view of a network, so the various components // should always agree on roaming state. @@ -80,6 +84,7 @@ public class NetworkState implements Parcelable { network = in.readParcelable(null); subscriberId = in.readString(); networkId = in.readString(); + legacyNetworkType = in.readInt(); } @Override @@ -95,6 +100,7 @@ public class NetworkState implements Parcelable { out.writeParcelable(network, flags); out.writeString(subscriberId); out.writeString(networkId); + out.writeInt(legacyNetworkType); } @UnsupportedAppUsage diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 96c3e573a8..8ad51f24ff 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1854,7 +1854,8 @@ public class ConnectivityService extends IConnectivityManager.Stub final ArrayList result = new ArrayList<>(); for (Network network : getAllNetworks()) { final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network); - if (nai != null) { + // TODO: Consider include SUSPENDED networks. + if (nai != null && nai.networkInfo.isConnected()) { // TODO (b/73321673) : NetworkState contains a copy of the // NetworkCapabilities, which may contain UIDs of apps to which the // network applies. Should the UIDs be cleared so as not to leak or diff --git a/tests/net/java/android/net/NetworkTemplateTest.kt b/tests/net/java/android/net/NetworkTemplateTest.kt index 91fcbc0fd5..6f1bcc3beb 100644 --- a/tests/net/java/android/net/NetworkTemplateTest.kt +++ b/tests/net/java/android/net/NetworkTemplateTest.kt @@ -62,7 +62,6 @@ class NetworkTemplateTest { ): NetworkState { val info = mock(NetworkInfo::class.java) doReturn(type).`when`(info).type - doReturn(NetworkInfo.State.CONNECTED).`when`(info).state val lp = LinkProperties() val caps = NetworkCapabilities().apply { setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED, false) From 23e22618cf086ee665ee0fa288c9deeb972d2084 Mon Sep 17 00:00:00 2001 From: junyulai Date: Wed, 30 Dec 2020 19:33:45 +0800 Subject: [PATCH 2/2] [FUI07] Stop making NetworkState with NetworkInfo from external callers Follow-up from previous patch. This change stop accessing the constructor that needs NetworkInfo to create NetworkState, which is no longer accessible after ConnectivityService become mainline module. Instead, pass a legacy network type which is dedicated for the need of the type. Test: atest FrameworksNetTests NetworkPolicyManagerServiceTest Bug: 174123988 Change-Id: I24157bc33e5a5819eccd6a3111d2049f531c1d43 --- core/java/android/net/NetworkState.java | 20 +++++++++++++++++-- .../java/android/net/NetworkTemplateTest.kt | 5 +---- .../server/net/NetworkStatsServiceTest.java | 17 ++++------------ 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/core/java/android/net/NetworkState.java b/core/java/android/net/NetworkState.java index 2d6ddcbb5d..e466d2e626 100644 --- a/core/java/android/net/NetworkState.java +++ b/core/java/android/net/NetworkState.java @@ -17,6 +17,7 @@ package android.net; import android.annotation.NonNull; +import android.annotation.Nullable; import android.compat.annotation.UnsupportedAppUsage; import android.os.Build; import android.os.Parcel; @@ -53,17 +54,32 @@ public class NetworkState implements Parcelable { legacyNetworkType = 0; } + public NetworkState(int legacyNetworkType, @NonNull LinkProperties linkProperties, + @NonNull NetworkCapabilities networkCapabilities, @NonNull Network network, + @Nullable String subscriberId, @Nullable String networkId) { + this(legacyNetworkType, new NetworkInfo(legacyNetworkType, 0, null, null), linkProperties, + networkCapabilities, network, subscriberId, networkId); + } + + // Constructor that used internally in ConnectivityService mainline module. public NetworkState(@NonNull NetworkInfo networkInfo, @NonNull LinkProperties linkProperties, @NonNull NetworkCapabilities networkCapabilities, @NonNull Network network, String subscriberId, String networkId) { + this(networkInfo.getType(), networkInfo, linkProperties, + networkCapabilities, network, subscriberId, networkId); + } + + public NetworkState(int legacyNetworkType, @NonNull NetworkInfo networkInfo, + @NonNull LinkProperties linkProperties, + @NonNull NetworkCapabilities networkCapabilities, @NonNull Network network, + String subscriberId, String networkId) { this.networkInfo = networkInfo; this.linkProperties = linkProperties; this.networkCapabilities = networkCapabilities; this.network = network; this.subscriberId = subscriberId; this.networkId = networkId; - // TODO: Pass legacyNetworkType directly from parameters and remove NetworkInfo. - this.legacyNetworkType = this.networkInfo.getType(); + this.legacyNetworkType = legacyNetworkType; // This object is an atomic view of a network, so the various components // should always agree on roaming state. diff --git a/tests/net/java/android/net/NetworkTemplateTest.kt b/tests/net/java/android/net/NetworkTemplateTest.kt index 6f1bcc3beb..1f8f6f3110 100644 --- a/tests/net/java/android/net/NetworkTemplateTest.kt +++ b/tests/net/java/android/net/NetworkTemplateTest.kt @@ -35,7 +35,6 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.JUnit4 -import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock import org.mockito.MockitoAnnotations import kotlin.test.assertFalse @@ -60,15 +59,13 @@ class NetworkTemplateTest { subscriberId: String? = null, ssid: String? = null ): NetworkState { - val info = mock(NetworkInfo::class.java) - doReturn(type).`when`(info).type val lp = LinkProperties() val caps = NetworkCapabilities().apply { setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED, false) setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING, true) setSSID(ssid) } - return NetworkState(info, lp, caps, mock(Network::class.java), subscriberId, ssid) + return NetworkState(type, lp, caps, mock(Network::class.java), subscriberId, ssid) } private fun NetworkTemplate.assertMatches(ident: NetworkIdentity) = diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index dde78aa541..214c82da17 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -80,8 +80,6 @@ import android.net.INetworkStatsSession; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; -import android.net.NetworkInfo; -import android.net.NetworkInfo.DetailedState; import android.net.NetworkState; import android.net.NetworkStats; import android.net.NetworkStatsHistory; @@ -1456,8 +1454,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } private static NetworkState buildWifiState(boolean isMetered, @NonNull String iface) { - final NetworkInfo info = new NetworkInfo(TYPE_WIFI, 0, null, null); - info.setDetailedState(DetailedState.CONNECTED, null, null); final LinkProperties prop = new LinkProperties(); prop.setInterfaceName(iface); final NetworkCapabilities capabilities = new NetworkCapabilities(); @@ -1465,7 +1461,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { capabilities.setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING, true); capabilities.addTransportType(NetworkCapabilities.TRANSPORT_WIFI); capabilities.setSSID(TEST_SSID); - return new NetworkState(info, prop, capabilities, WIFI_NETWORK, null, TEST_SSID); + return new NetworkState(TYPE_WIFI, prop, capabilities, WIFI_NETWORK, null, TEST_SSID); } private static NetworkState buildMobile3gState(String subscriberId) { @@ -1473,17 +1469,14 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } private static NetworkState buildMobile3gState(String subscriberId, boolean isRoaming) { - final NetworkInfo info = new NetworkInfo( - TYPE_MOBILE, TelephonyManager.NETWORK_TYPE_UMTS, null, null); - info.setDetailedState(DetailedState.CONNECTED, null, null); - info.setRoaming(isRoaming); final LinkProperties prop = new LinkProperties(); prop.setInterfaceName(TEST_IFACE); final NetworkCapabilities capabilities = new NetworkCapabilities(); capabilities.setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED, false); capabilities.setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING, !isRoaming); capabilities.addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR); - return new NetworkState(info, prop, capabilities, MOBILE_NETWORK, subscriberId, null); + return new NetworkState( + TYPE_MOBILE, prop, capabilities, MOBILE_NETWORK, subscriberId, null); } private NetworkStats buildEmptyStats() { @@ -1491,11 +1484,9 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } private static NetworkState buildVpnState() { - final NetworkInfo info = new NetworkInfo(TYPE_VPN, 0, null, null); - info.setDetailedState(DetailedState.CONNECTED, null, null); final LinkProperties prop = new LinkProperties(); prop.setInterfaceName(TUN_IFACE); - return new NetworkState(info, prop, new NetworkCapabilities(), VPN_NETWORK, null, null); + return new NetworkState(TYPE_VPN, prop, new NetworkCapabilities(), VPN_NETWORK, null, null); } private long getElapsedRealtime() {