From 29cb06a3e0ac0af782346958604237b21c3ee449 Mon Sep 17 00:00:00 2001
From: Chalard Jean <jchalard@google.com>
Date: Mon, 5 Apr 2021 17:35:32 +0900
Subject: [PATCH] [NS12] Address comments on NS09

Test: ConnectivityServiceTest
Fixes: 184507247
Merged-In: I3c2563d4ae4e3715d0c6270344ba8f7ef067872f
Merged-In: Ic4500dc85507fa68538c9ec179ec6eb4f44c5022
Change-Id: Ic4500dc85507fa68538c9ec179ec6eb4f44c5022
  (cherry-picked from ag/14091168)
---
 .../server/connectivity/NetworkAgentInfo.java |  2 +-
 .../server/connectivity/NetworkOffer.java     |  2 +-
 .../server/connectivity/NetworkRanker.java    | 19 ++++++++++---------
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
index 6142d70931..4d310cbc06 100644
--- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
@@ -894,7 +894,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo>, NetworkRa
     // Caller must not mutate. This method is called frequently and making a defensive copy
     // would be too expensive. This is used by NetworkRanker.Scoreable, so it can be compared
     // against other scoreables.
-    @Override public NetworkCapabilities getCaps() {
+    @Override public NetworkCapabilities getCapsNoCopy() {
         return networkCapabilities;
     }
 
diff --git a/service/src/com/android/server/connectivity/NetworkOffer.java b/service/src/com/android/server/connectivity/NetworkOffer.java
index 5336593b40..2bad596a59 100644
--- a/service/src/com/android/server/connectivity/NetworkOffer.java
+++ b/service/src/com/android/server/connectivity/NetworkOffer.java
@@ -76,7 +76,7 @@ public class NetworkOffer implements NetworkRanker.Scoreable {
     /**
      * Get the capabilities filter of this offer
      */
-    @Override @NonNull public NetworkCapabilities getCaps() {
+    @Override @NonNull public NetworkCapabilities getCapsNoCopy() {
         return caps;
     }
 
diff --git a/service/src/com/android/server/connectivity/NetworkRanker.java b/service/src/com/android/server/connectivity/NetworkRanker.java
index 66d6ccb499..c123ea75c6 100644
--- a/service/src/com/android/server/connectivity/NetworkRanker.java
+++ b/service/src/com/android/server/connectivity/NetworkRanker.java
@@ -59,7 +59,7 @@ public class NetworkRanker {
         /** Get score of this scoreable */
         FullScore getScore();
         /** Get capabilities of this scoreable */
-        NetworkCapabilities getCaps();
+        NetworkCapabilities getCapsNoCopy();
     }
 
     private static final boolean USE_POLICY_RANKING = false;
@@ -159,11 +159,12 @@ public class NetworkRanker {
         if (accepted.size() == 1) return accepted.get(0);
         if (accepted.size() > 0 && rejected.size() > 0) candidates = new ArrayList<>(accepted);
 
-        // Yield to bad wifi policy : if any wifi has ever been validated, keep only networks
-        // that don't yield to such a wifi network.
+        // Yield to bad wifi policy : if any wifi has ever been validated (even if it's now
+        // unvalidated), and unless it's been explicitly avoided when bad in UI, then keep only
+        // networks that don't yield to such a wifi network.
         final boolean anyWiFiEverValidated = CollectionUtils.any(candidates,
                 nai -> nai.getScore().hasPolicy(POLICY_EVER_VALIDATED_NOT_AVOIDED_WHEN_BAD)
-                        && nai.getCaps().hasTransport(TRANSPORT_WIFI));
+                        && nai.getCapsNoCopy().hasTransport(TRANSPORT_WIFI));
         if (anyWiFiEverValidated) {
             partitionInto(candidates, nai -> !nai.getScore().hasPolicy(POLICY_YIELD_TO_BAD_WIFI),
                     accepted, rejected);
@@ -207,18 +208,18 @@ public class NetworkRanker {
         for (final Scoreable defaultSubNai : accepted) {
             // Remove all networks without the DEFAULT_SUBSCRIPTION policy and the same transports
             // as a network that has it.
-            final int[] transports = defaultSubNai.getCaps().getTransportTypes();
+            final int[] transports = defaultSubNai.getCapsNoCopy().getTransportTypes();
             candidates.removeIf(nai -> !nai.getScore().hasPolicy(POLICY_TRANSPORT_PRIMARY)
-                    && Arrays.equals(transports, nai.getCaps().getTransportTypes()));
+                    && Arrays.equals(transports, nai.getCapsNoCopy().getTransportTypes()));
         }
         if (1 == candidates.size()) return candidates.get(0);
-        // It's guaranteed candidates.size() > 0 because there is at least one with DEFAULT_SUB
-        // policy and only those without it were removed.
+        // It's guaranteed candidates.size() > 0 because there is at least one with the
+        // TRANSPORT_PRIMARY policy and only those without it were removed.
 
         // If some of the networks have a better transport than others, keep only the ones with
         // the best transports.
         for (final int transport : PREFERRED_TRANSPORTS_ORDER) {
-            partitionInto(candidates, nai -> nai.getCaps().hasTransport(transport),
+            partitionInto(candidates, nai -> nai.getCapsNoCopy().hasTransport(transport),
                     accepted, rejected);
             if (accepted.size() == 1) return accepted.get(0);
             if (accepted.size() > 0 && rejected.size() > 0) {