From 33834771a38c5e73c32941f3d20bb3025097e255 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 21 Feb 2020 09:28:00 +0000 Subject: [PATCH 1/3] Revert "Address comments on ag/10316753" This reverts commit 78b666729250286a1d61bfb9e8af2161c18c0a45. Reason for revert: Feature punted Change-Id: I4b17d1b186369f2ecfcecf4906cd2b99a66bc7a0 --- .../java/android/net/NetworkScoreTest.kt | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/tests/net/common/java/android/net/NetworkScoreTest.kt b/tests/net/common/java/android/net/NetworkScoreTest.kt index a7e33b3d63b61..3e10992c62d8e 100644 --- a/tests/net/common/java/android/net/NetworkScoreTest.kt +++ b/tests/net/common/java/android/net/NetworkScoreTest.kt @@ -16,6 +16,7 @@ package android.net +import android.net.NetworkScore.Metrics.BANDWIDTH_UNKNOWN import android.net.NetworkScore.POLICY_DEFAULT_SUBSCRIPTION import android.net.NetworkScore.POLICY_IGNORE_ON_WIFI import android.net.NetworkScore.RANGE_MEDIUM @@ -34,24 +35,20 @@ private const val TEST_SCORE = 80 @RunWith(AndroidJUnit4::class) @SmallTest class NetworkScoreTest { - private fun makeScoreBuilder() = NetworkScore.Builder() - .setLegacyScore(TEST_SCORE) - .addPolicy(POLICY_IGNORE_ON_WIFI) - .addPolicy(POLICY_DEFAULT_SUBSCRIPTION) - .setExiting(false) - .setEndToEndMetrics(NetworkScore.Metrics(145 /* latency */, - 2500 /* downlinkBandwidth */, 1430 /* uplinkBandwidth */)) - .setRange(RANGE_MEDIUM) - .setSignalStrength(400) - @Test fun testParcelNetworkScore() { val defaultCap = NetworkCapabilities() - val legacyBuilder = NetworkScore.Builder().setLegacyScore(TEST_SCORE) - assertEquals(TEST_SCORE, legacyBuilder.build().getLegacyScore()) - assertParcelSane(legacyBuilder.build(), 7) + val builder = NetworkScore.Builder().setLegacyScore(TEST_SCORE) + assertEquals(TEST_SCORE, builder.build().getLegacyScore()) + assertParcelSane(builder.build(), 7) - val builder = makeScoreBuilder() + builder.addPolicy(POLICY_IGNORE_ON_WIFI) + .addPolicy(POLICY_DEFAULT_SUBSCRIPTION) + .setLinkLayerMetrics(NetworkScore.Metrics(44 /* latency */, + 380 /* downlinkBandwidth */, BANDWIDTH_UNKNOWN /* uplinkBandwidth */)) + .setEndToEndMetrics(NetworkScore.Metrics(11 /* latency */, + BANDWIDTH_UNKNOWN /* downlinkBandwidth */, 100_000 /* uplinkBandwidth */)) + .setRange(NetworkScore.RANGE_MEDIUM) assertParcelSane(builder.build(), 7) builder.clearPolicy(POLICY_IGNORE_ON_WIFI) val ns = builder.build() @@ -82,7 +79,14 @@ class NetworkScoreTest { @Test fun testBuilderEquals() { - val ns = makeScoreBuilder().build() + val ns = NetworkScore.Builder() + .addPolicy(POLICY_IGNORE_ON_WIFI) + .addPolicy(POLICY_DEFAULT_SUBSCRIPTION) + .setExiting(true) + .setEndToEndMetrics(NetworkScore.Metrics(145, 2500, 1430)) + .setRange(RANGE_MEDIUM) + .setSignalStrength(400) + .build() assertEquals(ns, NetworkScore.Builder(ns).build()) } } From 9bcd42255b73842904ddb33fafb6fdca9d9101a8 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 20 Feb 2020 07:39:31 +0000 Subject: [PATCH 2/3] Revert "[NS D02] Mix in the ignore on wifi policy." This reverts commit 14af825dbed4bf2d4b3922686438b8236e0176cb. Reason for revert: The feature was punted out of R. Change-Id: Id9ecdac5292eeddf7c12f2330421248b0f8355a9 --- core/java/android/net/NetworkScore.java | 21 +++------- .../android/server/ConnectivityService.java | 41 +------------------ .../java/android/net/NetworkScoreTest.kt | 26 +++--------- 3 files changed, 13 insertions(+), 75 deletions(-) diff --git a/core/java/android/net/NetworkScore.java b/core/java/android/net/NetworkScore.java index d2e59eb89309b..ae17378cfc4cc 100644 --- a/core/java/android/net/NetworkScore.java +++ b/core/java/android/net/NetworkScore.java @@ -21,6 +21,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.SystemApi; import android.annotation.TestApi; +import android.os.Bundle; import android.os.Parcel; import android.os.Parcelable; @@ -86,7 +87,7 @@ public final class NetworkScore implements Parcelable { /** toString */ public String toString() { return "latency = " + latencyMs + " downlinkBandwidth = " + downlinkBandwidthKBps - + " uplinkBandwidth = " + uplinkBandwidthKBps; + + "uplinkBandwidth = " + uplinkBandwidthKBps; } @NonNull @@ -353,27 +354,17 @@ public final class NetworkScore implements Parcelable { private Metrics mLinkLayerMetrics = new Metrics(Metrics.LATENCY_UNKNOWN, Metrics.BANDWIDTH_UNKNOWN, Metrics.BANDWIDTH_UNKNOWN); @NonNull - private Metrics mEndToEndMetrics = new Metrics(Metrics.LATENCY_UNKNOWN, + private Metrics mEndToMetrics = new Metrics(Metrics.LATENCY_UNKNOWN, Metrics.BANDWIDTH_UNKNOWN, Metrics.BANDWIDTH_UNKNOWN); private int mSignalStrength = UNKNOWN_SIGNAL_STRENGTH; private int mRange = RANGE_UNKNOWN; private boolean mExiting = false; private int mLegacyScore = 0; + @NonNull private Bundle mExtensions = new Bundle(); /** Create a new builder. */ public Builder() { } - /** @hide */ - public Builder(@NonNull final NetworkScore source) { - mPolicy = source.mPolicy; - mLinkLayerMetrics = source.mLinkLayerMetrics; - mEndToEndMetrics = source.mEndToEndMetrics; - mSignalStrength = source.mSignalStrength; - mRange = source.mRange; - mExiting = source.mExiting; - mLegacyScore = source.mLegacyScore; - } - /** Add a policy flag. */ @NonNull public Builder addPolicy(@Policy final int policy) { mPolicy |= policy; @@ -394,7 +385,7 @@ public final class NetworkScore implements Parcelable { /** Set the end-to-end metrics. */ @NonNull public Builder setEndToEndMetrics(@NonNull final Metrics endToEndMetrics) { - mEndToEndMetrics = endToEndMetrics; + mEndToMetrics = endToEndMetrics; return this; } @@ -426,7 +417,7 @@ public final class NetworkScore implements Parcelable { /** Build the NetworkScore object represented by this builder. */ @NonNull public NetworkScore build() { - return new NetworkScore(mPolicy, mLinkLayerMetrics, mEndToEndMetrics, + return new NetworkScore(mPolicy, mLinkLayerMetrics, mEndToMetrics, mSignalStrength, mRange, mExiting, mLegacyScore); } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 7dab90d5cef07..60a30d3317762 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3817,9 +3817,8 @@ public class ConnectivityService extends IConnectivityManager.Stub return avoidBadWifi(); } + private void rematchForAvoidBadWifiUpdate() { - ensureRunningOnConnectivityServiceThread(); - mixInAllNetworkScores(); rematchAllNetworksAndRequests(); for (NetworkAgentInfo nai: mNetworkAgentInfos.values()) { if (nai.networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { @@ -7036,45 +7035,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - /** - * Re-mixin all network scores. - * This is called when some global setting like avoidBadWifi has changed. - * TODO : remove this when all usages have been removed. - */ - private void mixInAllNetworkScores() { - ensureRunningOnConnectivityServiceThread(); - for (final NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - nai.setNetworkScore(mixInNetworkScore(nai, nai.getNetworkScore())); - } - } - - /** - * Mix in the Connectivity-managed parts of the NetworkScore. - * @param nai The NAI this score applies to. - * @param sourceScore the score sent by the network agent, or the previous score of this NAI. - * @return A new score with the Connectivity-managed parts mixed in. - */ - @NonNull - private NetworkScore mixInNetworkScore(@NonNull final NetworkAgentInfo nai, - @NonNull final NetworkScore sourceScore) { - final NetworkScore.Builder score = new NetworkScore.Builder(sourceScore); - - // TODO : this should be done in Telephony. It should be handled per-network because - // it's a carrier-dependent config. - if (nai.networkCapabilities.hasTransport(TRANSPORT_CELLULAR)) { - if (mMultinetworkPolicyTracker.getAvoidBadWifi()) { - score.clearPolicy(NetworkScore.POLICY_IGNORE_ON_WIFI); - } else { - score.addPolicy(NetworkScore.POLICY_IGNORE_ON_WIFI); - } - } - - return score.build(); - } - private void updateNetworkScore(NetworkAgentInfo nai, NetworkScore ns) { if (VDBG || DDBG) log("updateNetworkScore for " + nai.toShortString() + " to " + ns); - nai.setNetworkScore(mixInNetworkScore(nai, ns)); + nai.setNetworkScore(ns); rematchAllNetworksAndRequests(); sendUpdatedScoreToFactories(nai); } diff --git a/tests/net/common/java/android/net/NetworkScoreTest.kt b/tests/net/common/java/android/net/NetworkScoreTest.kt index 3e10992c62d8e..a63d58d5a0f69 100644 --- a/tests/net/common/java/android/net/NetworkScoreTest.kt +++ b/tests/net/common/java/android/net/NetworkScoreTest.kt @@ -17,9 +17,6 @@ package android.net import android.net.NetworkScore.Metrics.BANDWIDTH_UNKNOWN -import android.net.NetworkScore.POLICY_DEFAULT_SUBSCRIPTION -import android.net.NetworkScore.POLICY_IGNORE_ON_WIFI -import android.net.NetworkScore.RANGE_MEDIUM import androidx.test.filters.SmallTest import androidx.test.runner.AndroidJUnit4 import com.android.testutils.assertParcelSane @@ -42,19 +39,19 @@ class NetworkScoreTest { assertEquals(TEST_SCORE, builder.build().getLegacyScore()) assertParcelSane(builder.build(), 7) - builder.addPolicy(POLICY_IGNORE_ON_WIFI) - .addPolicy(POLICY_DEFAULT_SUBSCRIPTION) + builder.addPolicy(NetworkScore.POLICY_IGNORE_ON_WIFI) + .addPolicy(NetworkScore.POLICY_DEFAULT_SUBSCRIPTION) .setLinkLayerMetrics(NetworkScore.Metrics(44 /* latency */, 380 /* downlinkBandwidth */, BANDWIDTH_UNKNOWN /* uplinkBandwidth */)) .setEndToEndMetrics(NetworkScore.Metrics(11 /* latency */, BANDWIDTH_UNKNOWN /* downlinkBandwidth */, 100_000 /* uplinkBandwidth */)) .setRange(NetworkScore.RANGE_MEDIUM) assertParcelSane(builder.build(), 7) - builder.clearPolicy(POLICY_IGNORE_ON_WIFI) + builder.clearPolicy(NetworkScore.POLICY_IGNORE_ON_WIFI) val ns = builder.build() assertParcelSane(ns, 7) - assertFalse(ns.hasPolicy(POLICY_IGNORE_ON_WIFI)) - assertTrue(ns.hasPolicy(POLICY_DEFAULT_SUBSCRIPTION)) + assertFalse(ns.hasPolicy(NetworkScore.POLICY_IGNORE_ON_WIFI)) + assertTrue(ns.hasPolicy(NetworkScore.POLICY_DEFAULT_SUBSCRIPTION)) val exitingNs = ns.withExiting(true) assertNotEquals(ns.isExiting, exitingNs.isExiting) @@ -76,17 +73,4 @@ class NetworkScoreTest { assertTrue(builder1.build().equals(builder2.build())) assertEquals(builder1.build().hashCode(), builder2.build().hashCode()) } - - @Test - fun testBuilderEquals() { - val ns = NetworkScore.Builder() - .addPolicy(POLICY_IGNORE_ON_WIFI) - .addPolicy(POLICY_DEFAULT_SUBSCRIPTION) - .setExiting(true) - .setEndToEndMetrics(NetworkScore.Metrics(145, 2500, 1430)) - .setRange(RANGE_MEDIUM) - .setSignalStrength(400) - .build() - assertEquals(ns, NetworkScore.Builder(ns).build()) - } } From 96726bcbb9cd58a3e3c214154c1f1e52f545006c Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 20 Feb 2020 08:41:38 +0000 Subject: [PATCH 3/3] Revert "[NS D01] Remove candidates that don't satisfy the request." This reverts commit 48c3f95878194461cd05f305d9cc9f052acadc1f. Reason for revert: The feature was punted out of R. Change-Id: Ia91b3b3c55f735dae64ffa3194614a6f2631a087 --- .../com/android/server/connectivity/NetworkRanker.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/NetworkRanker.java b/services/core/java/com/android/server/connectivity/NetworkRanker.java index 1ae7dc5c36767..d0aabf95d5720 100644 --- a/services/core/java/com/android/server/connectivity/NetworkRanker.java +++ b/services/core/java/com/android/server/connectivity/NetworkRanker.java @@ -20,7 +20,6 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.net.NetworkRequest; -import java.util.ArrayList; import java.util.Collection; /** @@ -32,15 +31,15 @@ public class NetworkRanker { /** * Find the best network satisfying this request among the list of passed networks. */ + // Almost equivalent to Collections.max(nais), but allows returning null if no network + // satisfies the request. @Nullable public NetworkAgentInfo getBestNetwork(@NonNull final NetworkRequest request, @NonNull final Collection nais) { - final ArrayList candidates = new ArrayList<>(nais); - candidates.removeIf(nai -> !nai.satisfies(request)); - NetworkAgentInfo bestNetwork = null; int bestScore = Integer.MIN_VALUE; - for (final NetworkAgentInfo nai : candidates) { + for (final NetworkAgentInfo nai : nais) { + if (!nai.satisfies(request)) continue; if (nai.getCurrentScore() > bestScore) { bestNetwork = nai; bestScore = nai.getCurrentScore();