[NS13] Remove the last usage of the legacy int
Note that this requires removing part of a test, because
that part is testing that the a 50 bonus of the legacy
int is stronger than the validation penalty, which is
not a mechanic we want to have. When WiFi is unvalidated
and cell is unvalidated, cell should be kept in case it
validates, like is described in comments in
isNetworkPotentialSatisfier ; however this test is
checking that it *IS* reaped off if the wifi score is
strong enough. This should be incorrect, and should not
be tested, so this patch removes the check.
Test: ConnectivityServiceTest
Bug: 184834350
Merged-In: I3c2563d4ae4e3715d0c6270344ba8f7ef067872f
Merged-In: I8966abee59fea2d9f10f082aba87df6588b72762
Change-Id: I8966abee59fea2d9f10f082aba87df6588b72762
(cherry-picked from ag/14127306)
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 667e658..0166fc0 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -4038,17 +4038,16 @@
// multilayer requests, returning as soon as a NetworkAgentInfo satisfies a request
// is important so as to not evaluate lower priority requests further in
// nri.mRequests.
- final boolean isNetworkNeeded = candidate.isSatisfyingRequest(req.requestId)
- // Note that this catches two important cases:
- // 1. Unvalidated cellular will not be reaped when unvalidated WiFi
- // is currently satisfying the request. This is desirable when
- // cellular ends up validating but WiFi does not.
- // 2. Unvalidated WiFi will not be reaped when validated cellular
- // is currently satisfying the request. This is desirable when
- // WiFi ends up validating and out scoring cellular.
- || nri.getSatisfier().getCurrentScore()
- < candidate.getCurrentScoreAsValidated();
- return isNetworkNeeded;
+ final NetworkAgentInfo champion = req.equals(nri.getActiveRequest())
+ ? nri.getSatisfier() : null;
+ // Note that this catches two important cases:
+ // 1. Unvalidated cellular will not be reaped when unvalidated WiFi
+ // is currently satisfying the request. This is desirable when
+ // cellular ends up validating but WiFi does not.
+ // 2. Unvalidated WiFi will not be reaped when validated cellular
+ // is currently satisfying the request. This is desirable when
+ // WiFi ends up validating and out scoring cellular.
+ return mNetworkRanker.mightBeat(req, champion, candidate.getValidatedScoreable());
}
}
@@ -7887,7 +7886,7 @@
@NonNull final Collection<NetworkRequestInfo> networkRequests) {
final NetworkReassignment changes = new NetworkReassignment();
- // Gather the list of all relevant agents and sort them by score.
+ // Gather the list of all relevant agents.
final ArrayList<NetworkAgentInfo> nais = new ArrayList<>();
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
if (!nai.everConnected) {
diff --git a/service/src/com/android/server/connectivity/FullScore.java b/service/src/com/android/server/connectivity/FullScore.java
index 52da566..14cec09 100644
--- a/service/src/com/android/server/connectivity/FullScore.java
+++ b/service/src/com/android/server/connectivity/FullScore.java
@@ -259,6 +259,14 @@
}
/**
+ * Returns this score but validated.
+ */
+ public FullScore asValidated() {
+ return new FullScore(mLegacyInt, mPolicies | (1L << POLICY_IS_VALIDATED),
+ mKeepConnectedReason);
+ }
+
+ /**
* For backward compatibility, get the legacy int.
* This will be removed before S is published.
*/
diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
index 4d310cb..851096d 100644
--- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
@@ -938,6 +938,26 @@
}
/**
+ * Returns a Scoreable identical to this NAI, but validated.
+ *
+ * This is useful to probe what scoring would be if this network validated, to know
+ * whether to provisionally keep a network that may or may not validate.
+ *
+ * @return a Scoreable identical to this NAI, but validated.
+ */
+ public NetworkRanker.Scoreable getValidatedScoreable() {
+ return new NetworkRanker.Scoreable() {
+ @Override public FullScore getScore() {
+ return mScore.asValidated();
+ }
+
+ @Override public NetworkCapabilities getCapsNoCopy() {
+ return networkCapabilities;
+ }
+ };
+ }
+
+ /**
* Return a {@link NetworkStateSnapshot} for this network.
*/
@NonNull
diff --git a/service/src/com/android/server/connectivity/NetworkRanker.java b/service/src/com/android/server/connectivity/NetworkRanker.java
index 2b345e5..346af44 100644
--- a/service/src/com/android/server/connectivity/NetworkRanker.java
+++ b/service/src/com/android/server/connectivity/NetworkRanker.java
@@ -234,16 +234,17 @@
NetworkAgentInfo bestNetwork = null;
int bestScore = Integer.MIN_VALUE;
for (final NetworkAgentInfo nai : nais) {
- if (nai.getCurrentScore() > bestScore) {
+ final int naiScore = nai.getCurrentScore();
+ if (naiScore > bestScore) {
bestNetwork = nai;
- bestScore = nai.getCurrentScore();
+ bestScore = naiScore;
}
}
return bestNetwork;
}
/**
- * Returns whether an offer has a chance to beat a champion network for a request.
+ * Returns whether a {@link Scoreable} has a chance to beat a champion network for a request.
*
* Offers are sent by network providers when they think they might be able to make a network
* with the characteristics contained in the offer. If the offer has no chance to beat
@@ -257,15 +258,15 @@
*
* @param request The request to evaluate against.
* @param champion The currently best network for this request.
- * @param offer The offer.
+ * @param contestant The offer.
* @return Whether the offer stands a chance to beat the champion.
*/
public boolean mightBeat(@NonNull final NetworkRequest request,
@Nullable final NetworkAgentInfo champion,
- @NonNull final NetworkOffer offer) {
+ @NonNull final Scoreable contestant) {
// If this network can't even satisfy the request then it can't beat anything, not
// even an absence of network. It can't satisfy it anyway.
- if (!request.canBeSatisfiedBy(offer.caps)) return false;
+ if (!request.canBeSatisfiedBy(contestant.getCapsNoCopy())) return false;
// If there is no satisfying network, then this network can beat, because some network
// is always better than no network.
if (null == champion) return true;
@@ -274,25 +275,24 @@
// Otherwise rank them.
final ArrayList<Scoreable> candidates = new ArrayList<>();
candidates.add(champion);
- candidates.add(offer);
- return offer == getBestNetworkByPolicy(candidates, champion);
+ candidates.add(contestant);
+ return contestant == getBestNetworkByPolicy(candidates, champion);
} else {
- return mightBeatByLegacyInt(request, champion.getScore(), offer);
+ return mightBeatByLegacyInt(champion.getScore(), contestant);
}
}
/**
- * Returns whether an offer might beat a champion according to the legacy int.
+ * Returns whether a contestant might beat a champion according to the legacy int.
*/
- public boolean mightBeatByLegacyInt(@NonNull final NetworkRequest request,
- @Nullable final FullScore championScore,
- @NonNull final NetworkOffer offer) {
+ private boolean mightBeatByLegacyInt(@Nullable final FullScore championScore,
+ @NonNull final Scoreable contestant) {
final int offerIntScore;
- if (offer.caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
+ if (contestant.getCapsNoCopy().hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
// If the offer might have Internet access, then it might validate.
- offerIntScore = offer.score.getLegacyIntAsValidated();
+ offerIntScore = contestant.getScore().getLegacyIntAsValidated();
} else {
- offerIntScore = offer.score.getLegacyInt();
+ offerIntScore = contestant.getScore().getLegacyInt();
}
return championScore.getLegacyInt() < offerIntScore;
}
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 7ba415b..e275a5e 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -2683,25 +2683,6 @@
assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
- // Bring up wifi with a score of 70.
- // Cell is lingered because it would not satisfy any request, even if it validated.
- mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
- mWiFiNetworkAgent.adjustScore(50);
- mWiFiNetworkAgent.connect(false); // Score: 70
- callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
- callback.expectCallback(CallbackEntry.LOSING, mCellNetworkAgent);
- defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
- assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
- assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
-
- // Tear down wifi.
- mWiFiNetworkAgent.disconnect();
- callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
- defaultCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
- defaultCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent);
- assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
- assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork());
-
// Bring up wifi, then validate it. Previous versions would immediately tear down cell, but
// it's arguably correct to linger it, since it was the default network before it validated.
mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
@@ -3017,11 +2998,17 @@
callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
- // BUG: the network will no longer linger, even though it's validated and outscored.
- // TODO: fix this.
mEthernetNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_ETHERNET);
mEthernetNetworkAgent.connect(true);
+ // BUG: with the legacy int-based scoring code, the network will no longer linger, even
+ // though it's validated and outscored.
+ // The new policy-based scoring code fixes this.
+ // TODO: remove the line below and replace with the three commented lines when
+ // the policy-based scoring code is turned on.
callback.expectAvailableThenValidatedCallbacks(mEthernetNetworkAgent);
+ // callback.expectAvailableCallbacksUnvalidated(mEthernetNetworkAgent);
+ // callback.expectCallback(CallbackEntry.LOSING, mWiFiNetworkAgent);
+ // callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mEthernetNetworkAgent);
assertEquals(mEthernetNetworkAgent.getNetwork(), mCm.getActiveNetwork());
callback.assertNoCallback();