[WifiCandidates] Remove the nominator score

The nominators are no longer generating scores, so do not bother to
store them.

Relax the candidate replacement rules so that new candidate can replace
one with a numerically lower (or equal) nominatorId. This is to allow
a candidate for the current connection to be added as a default, with
the possibility of replacing it using via the nominators.

Bug: 147751334
Test: atest WifiCandidatesTest WifiNetworkSelectorTest
Change-Id: I9d231ecccc6821e8446b4047fdd7509691859ecd

5 files changed, 52 insertions, 77 deletions

diff --git a/service/java/com/android/server/wifi/WifiCandidates.java b/service/java/com/android/server/wifi/WifiCandidates.java
index 604086024..255c80fe0 100644
--- a/service/java/com/android/server/wifi/WifiCandidates.java
+++ b/service/java/com/android/server/wifi/WifiCandidates.java
@@ -87,13 +87,7 @@ public class WifiCandidates {
          */
         @WifiNetworkSelector.NetworkNominator.NominatorId
         int getNominatorId();
-        /**
-         * Gets the score that was provided by the nominator.
-         *
-         * Not all nominators provide a useful score. Scores from different nominators
-         * are not directly comparable.
-         */
-        int getNominatorScore();
+
         /**
          * Returns true if the candidate is in the same network as the
          * current connection.
@@ -136,7 +130,6 @@ public class WifiCandidates {
         public final WifiConfiguration config;
         // First nominator to nominate this config
         public final @WifiNetworkSelector.NetworkNominator.NominatorId int nominatorId;
-        public final int nominatorScore;        // Score provided by first nominator
         public final double lastSelectionWeight; // Value between 0 and 1
 
         private final int mScanRssi;
@@ -151,7 +144,6 @@ public class WifiCandidates {
                 ScanDetail scanDetail,
                 WifiConfiguration config,
                 @WifiNetworkSelector.NetworkNominator.NominatorId int nominatorId,
-                int nominatorScore,
                 WifiScoreCard.PerBssid perBssid,
                 double lastSelectionWeight,
                 boolean isCurrentNetwork,
@@ -161,7 +153,6 @@ public class WifiCandidates {
             this.key = key;
             this.config = config;
             this.nominatorId = nominatorId;
-            this.nominatorScore = nominatorScore;
             this.mScanRssi = scanDetail.getScanResult().level;
             this.mFrequency = scanDetail.getScanResult().frequency;
             this.mPerBssid = perBssid;
@@ -214,11 +205,6 @@ public class WifiCandidates {
         }
 
         @Override
-        public int getNominatorScore() {
-            return nominatorScore;
-        }
-
-        @Override
         public double getLastSelectionWeight() {
             return lastSelectionWeight;
         }
@@ -363,15 +349,14 @@ public class WifiCandidates {
     /**
      * Adds a new candidate
      *
-     * @returns true if added or replaced, false otherwise
+     * @return true if added or replaced, false otherwise
      */
     public boolean add(ScanDetail scanDetail,
-                    WifiConfiguration config,
-                    @WifiNetworkSelector.NetworkNominator.NominatorId int nominatorId,
-                    int nominatorScore,
-                    double lastSelectionWeightBetweenZeroAndOne,
-                    boolean isMetered,
-                    int predictedThroughputMbps) {
+            WifiConfiguration config,
+            @WifiNetworkSelector.NetworkNominator.NominatorId int nominatorId,
+            double lastSelectionWeightBetweenZeroAndOne,
+            boolean isMetered,
+            int predictedThroughputMbps) {
         if (!validConfigAndScanDetail(config, scanDetail)) return false;
         ScanResult scanResult = scanDetail.getScanResult();
         MacAddress bssid = MacAddress.fromString(scanResult.BSSID);
@@ -379,9 +364,7 @@ public class WifiCandidates {
         CandidateImpl old = mCandidates.get(key);
         if (old != null) {
             // check if we want to replace this old candidate
-            if (nominatorId < old.nominatorId) return failure();
             if (nominatorId > old.nominatorId) return false;
-            if (nominatorScore <= old.nominatorScore) return false;
             remove(old);
         }
         WifiScoreCard.PerBssid perBssid = mWifiScoreCard.lookupBssid(
@@ -391,7 +374,7 @@ public class WifiCandidates {
                 WifiScoreCardProto.SecurityType.forNumber(key.matchInfo.networkType));
         perBssid.setNetworkConfigId(config.networkId);
         CandidateImpl candidate = new CandidateImpl(key,
-                scanDetail, config, nominatorId, nominatorScore, perBssid,
+                scanDetail, config, nominatorId, perBssid,
                 Math.min(Math.max(lastSelectionWeightBetweenZeroAndOne, 0.0), 1.0),
                 config.networkId == mCurrentNetworkId,
                 bssid.equals(mCurrentBssid),
@@ -433,7 +416,7 @@ public class WifiCandidates {
 
     /**
      * Removes a candidate
-     * @returns true if the candidate was successfully removed
+     * @return true if the candidate was successfully removed
      */
     public boolean remove(Candidate candidate) {
         if (!(candidate instanceof CandidateImpl)) return failure();
@@ -466,7 +449,7 @@ public class WifiCandidates {
     /**
      * Make a choice from among the candidates, using the provided scorer.
      *
-     * @returns the chosen scored candidate, or ScoredCandidate.NONE.
+     * @return the chosen scored candidate, or ScoredCandidate.NONE.
      */
     public @NonNull ScoredCandidate choose(@NonNull CandidateScorer candidateScorer) {
         Preconditions.checkNotNull(candidateScorer);
@@ -511,7 +494,7 @@ public class WifiCandidates {
      * This captures a stack trace, so don't bother to construct a string message, just
      * supply any culprits (convertible to strings) that might aid diagnosis.
      *
-     * @returns false
+     * @return false
      * @throws RuntimeException (if in picky mode)
      */
     private boolean failure(Object... culprits) {
diff --git a/service/java/com/android/server/wifi/WifiNetworkSelector.java b/service/java/com/android/server/wifi/WifiNetworkSelector.java
index 90a5e88c5..5d43b5536 100644
--- a/service/java/com/android/server/wifi/WifiNetworkSelector.java
+++ b/service/java/com/android/server/wifi/WifiNetworkSelector.java
@@ -714,7 +714,6 @@ public class WifiNetworkSelector {
                         if (config != null) {
                             boolean added = wifiCandidates.add(scanDetail, config,
                                     registeredNominator.getId(),
-                                    0,
                                     (config.networkId == lastUserSelectedNetworkId)
                                             ? lastSelectionWeight : 0.0,
                                     WifiConfiguration.isMetered(config, wifiInfo),
diff --git a/tests/wifitests/src/com/android/server/wifi/CandidateScorerTest.java b/tests/wifitests/src/com/android/server/wifi/CandidateScorerTest.java
index 611f29264..0512307c8 100644
--- a/tests/wifitests/src/com/android/server/wifi/CandidateScorerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/CandidateScorerTest.java
@@ -106,9 +106,9 @@ public class CandidateScorerTest extends WifiBaseTest {
     @Before
     public void setUp() throws Exception {
         mScoringParams.update("");
-        mCandidate1 = new ConcreteCandidate().setNominatorId(0).setNominatorScore(66)
+        mCandidate1 = new ConcreteCandidate().setNominatorId(0)
                 .setScanRssi(-50).setFrequency(5180);
-        mCandidate2 = new ConcreteCandidate().setNominatorId(0).setNominatorScore(99)
+        mCandidate2 = new ConcreteCandidate().setNominatorId(0)
                 .setScanRssi(-50).setFrequency(5180);
     }
 
diff --git a/tests/wifitests/src/com/android/server/wifi/ConcreteCandidate.java b/tests/wifitests/src/com/android/server/wifi/ConcreteCandidate.java
index 632319c45..cf75ad56c 100644
--- a/tests/wifitests/src/com/android/server/wifi/ConcreteCandidate.java
+++ b/tests/wifitests/src/com/android/server/wifi/ConcreteCandidate.java
@@ -33,7 +33,6 @@ public final class ConcreteCandidate implements WifiCandidates.Candidate {
     private boolean mIsTrusted;
     private boolean mIsMetered;
     private int mNominatorId = -1;
-    private int mNominatorScore = Integer.MIN_VALUE;
     private double mLastSelectionWeight;
     private int mScanRssi = -127;
     private int mFrequency = -1;
@@ -56,7 +55,6 @@ public final class ConcreteCandidate implements WifiCandidates.Candidate {
         mIsTrusted = candidate.isTrusted();
         mIsMetered = candidate.isMetered();
         mNominatorId = candidate.getNominatorId();
-        mNominatorScore = candidate.getNominatorScore();
         mLastSelectionWeight = candidate.getLastSelectionWeight();
         mScanRssi = candidate.getScanRssi();
         mFrequency = candidate.getFrequency();
@@ -149,16 +147,6 @@ public final class ConcreteCandidate implements WifiCandidates.Candidate {
         return mNominatorId;
     }
 
-    public ConcreteCandidate setNominatorScore(int nominatorScore) {
-        mNominatorScore = nominatorScore;
-        return this;
-    }
-
-    @Override
-    public int getNominatorScore() {
-        return mNominatorScore;
-    }
-
     public ConcreteCandidate setLastSelectionWeight(double lastSelectionWeight) {
         mLastSelectionWeight = lastSelectionWeight;
         return this;
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiCandidatesTest.java b/tests/wifitests/src/com/android/server/wifi/WifiCandidatesTest.java
index 2a725774c..3a305cf44 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiCandidatesTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiCandidatesTest.java
@@ -90,10 +90,10 @@ public class WifiCandidatesTest extends WifiBaseTest {
      */
     @Test
     public void testDontDieFromNulls() throws Exception {
-        mWifiCandidates.add(null, mConfig1, 1, 42, 0.0, false, 100);
-        mWifiCandidates.add(mScanDetail1, null, 2, 16, 0.0, false, 100);
+        mWifiCandidates.add(null, mConfig1, 1, 0.0, false, 100);
+        mWifiCandidates.add(mScanDetail1, null, 2, 0.0, false, 100);
         doReturn(null).when(mScanDetail2).getScanResult();
-        mWifiCandidates.add(mScanDetail2, mConfig2, 3, 314, 1.0, true, 100);
+        mWifiCandidates.add(mScanDetail2, mConfig2, 3, 1.0, true, 100);
         assertFalse(mWifiCandidates.remove(null));
 
         assertEquals(0, mWifiCandidates.size());
@@ -104,7 +104,7 @@ public class WifiCandidatesTest extends WifiBaseTest {
      */
     @Test
     public void testAddJustOne() throws Exception {
-        assertTrue(mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, false, 100));
+        assertTrue(mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100));
 
         assertEquals(1, mWifiCandidates.size());
         assertEquals(0, mWifiCandidates.getFaultCount());
@@ -119,7 +119,7 @@ public class WifiCandidatesTest extends WifiBaseTest {
     public void testQuotingBotch() throws Exception {
         // Unfortunately ScanResult.SSID is not quoted; make sure we catch that
         mScanResult1.SSID = mConfig1.SSID;
-        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, true, 100);
+        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, true, 100);
 
         // Should not have added this one
         assertEquals(0, mWifiCandidates.size());
@@ -178,7 +178,7 @@ public class WifiCandidatesTest extends WifiBaseTest {
         assertTrue(mWifiCandidates == mWifiCandidates.setPicky(true));
         try {
             mScanResult1.SSID = mConfig1.SSID; // As in testQuotingBotch()
-            mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, false, 100);
+            mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100);
             fail("Exception not raised in picky mode");
         } catch (IllegalArgumentException e) {
             assertEquals(1, mWifiCandidates.getFaultCount());
@@ -192,25 +192,32 @@ public class WifiCandidatesTest extends WifiBaseTest {
     @Test
     public void testNoOverwriteCases() throws Exception {
         // Setup is to add the first candidate
-        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, false, 100);
+        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100);
         assertEquals(1, mWifiCandidates.size());
 
-        // Same nominator
-        // , same score. Should not add.
-        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, false, 100));
-        assertEquals(0, mWifiCandidates.getFaultCount()); // But not considered a fault
-        // Same nominator, lower score. Should not add.
-        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 2, 13, 0.0, false, 100));
-        assertEquals(0, mWifiCandidates.getFaultCount()); // Also not a fault
-        // Later nominator. Should not add (regardless of score).
-        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 5, 13, 0.0, false, 100));
-        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 5, 15, 0.0, false, 100));
+        // Later nominator. Should not add.
+        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 5, 0.0, false, 100));
+        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 5, 0.0, false, 100));
         assertEquals(0, mWifiCandidates.getFaultCount()); // Still no faults
-        // Nominator out of order. Should not add (regardless of score).
-        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 1, 12, 0.0, false, 100));
-        assertNotNull(mWifiCandidates.getLastFault()); // This one is considered a caller error
-        assertFalse(mWifiCandidates.add(mScanDetail1, mConfig1, 1, 15, 0.0, false, 100));
-        assertEquals(2, mWifiCandidates.getFaultCount());
+        // After all that, only one candidate should be there.
+        assertEquals(1, mWifiCandidates.size());
+    }
+
+    /**
+     * Try cases where we do overwrite existing candidates
+     */
+    @Test
+    public void testOverwriteCases() throws Exception {
+        // Setup is to add the first candidate
+        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100);
+        assertEquals(1, mWifiCandidates.size());
+
+        // Same nominator, should replace.
+        assertTrue(mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100));
+        assertEquals(0, mWifiCandidates.getFaultCount()); // No fault
+        // Nominator out of order. Should replace.
+        assertTrue(mWifiCandidates.add(mScanDetail1, mConfig1, 1, 0.0, false, 100));
+        assertEquals(0, mWifiCandidates.getFaultCount());  // But not considered a fault
         // After all that, only one candidate should be there.
         assertEquals(1, mWifiCandidates.size());
     }
@@ -222,12 +229,12 @@ public class WifiCandidatesTest extends WifiBaseTest {
     public void testBssidValidation() throws Exception {
         // Null BSSID.
         mScanResult1.BSSID = null;
-        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, false, 100);
+        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100);
         assertTrue("Expecting NPE, got " + mWifiCandidates.getLastFault(),
                 mWifiCandidates.getLastFault() instanceof NullPointerException);
         // Malformed BSSID
         mScanResult1.BSSID = "NotaBssid!";
-        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, false, 100);
+        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100);
         assertTrue("Expecting IAE, got " + mWifiCandidates.getLastFault(),
                 mWifiCandidates.getLastFault() instanceof IllegalArgumentException);
         assertEquals(0, mWifiCandidates.size());
@@ -244,8 +251,8 @@ public class WifiCandidatesTest extends WifiBaseTest {
         mScanResult2.SSID = mScanResult1.SSID;
         mScanResult2.BSSID = mScanResult1.BSSID.replace('1', '2');
         // Add both
-        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 14, 0.0, false, 100);
-        mWifiCandidates.add(mScanDetail2, mConfig2, 2, 14, 0.0, false, 100);
+        mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 100);
+        mWifiCandidates.add(mScanDetail2, mConfig2, 2, 0.0, false, 100);
         // We expect them both to be there
         assertEquals(2, mWifiCandidates.size());
         // But just one group
@@ -276,18 +283,16 @@ public class WifiCandidatesTest extends WifiBaseTest {
         // And the scan result
         mScanResult2.SSID = mScanResult1.SSID;
         mScanResult2.BSSID = mScanResult1.BSSID;
-        // Try adding them both, the higher-scoring one second
-        assertTrue(mWifiCandidates.add(mScanDetail2, mConfig2, 2, 14, 0.0, false, 100));
-        assertTrue(mWifiCandidates.add(mScanDetail1, mConfig1, 2, 15, 0.0, false, 90));
+        // Try adding them both, in a known order
+        assertTrue(mWifiCandidates.add(mScanDetail2, mConfig2, 2, 0.0, false, 100));
+        assertTrue(mWifiCandidates.add(mScanDetail1, mConfig1, 2, 0.0, false, 90));
         // Only one should survive
         assertEquals(1, mWifiCandidates.size());
-        // And no faults
         assertEquals(0, mWifiCandidates.getFaultCount());
-        // Make sure we kept the one with a higher nominator score
+        // Make sure we kept the second one
         WifiCandidates.Candidate c;
         c = mWifiCandidates.getGroupedCandidates().iterator().next().iterator().next();
         assertEquals(90, c.getPredictedThroughputMbps());
-        assertEquals(15, c.getNominatorScore());
     }
 
     /**
@@ -300,8 +305,8 @@ public class WifiCandidatesTest extends WifiBaseTest {
         WifiConfiguration config1 = WifiConfigurationTestUtil.createPasspointNetwork();
         mScanResult2.BSSID = mScanResult1.BSSID.replace('1', '2');
         // Add candidates with different scanDetail for same passpoint WifiConfig.
-        assertTrue(mWifiCandidates.add(mScanDetail1, config1, 2, 0, 0.0, false, 100));
-        assertTrue(mWifiCandidates.add(mScanDetail2, config1, 2, 0, 0.0, false, 100));
+        assertTrue(mWifiCandidates.add(mScanDetail1, config1, 2, 0.0, false, 100));
+        assertTrue(mWifiCandidates.add(mScanDetail2, config1, 2, 0.0, false, 100));
         // Both should survive and no faults.
         assertEquals(2, mWifiCandidates.size());
         assertEquals(0, mWifiCandidates.getFaultCount());