Use "don't actively prefer" timeout when avoiding bad wifi

Test: ConnectivityServiceTest
Change-Id: I6e99aff77c55805630d878e472f466bd31bba360

service/src/com/android/server/ConnectivityService.java

@@ -1519,6 +1519,23 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 throws SocketException, InterruptedIOException, ErrnoException {
             InetDiagMessage.destroyLiveTcpSocketsByOwnerUids(ownerUids);
         }
+
+        /**
+         * Schedule the evaluation timeout.
+         *
+         * When a network connects, it's "not evaluated" yet. Detection events cause the network
+         * to be "evaluated" (typically, validation or detection of a captive portal). If none
+         * of these events happen, this time will run out, after which the network is considered
+         * "evaluated" even if nothing happened to it. Notionally that means the system gave up
+         * on this network and considers it won't provide connectivity. In particular, that means
+         * it's when the system prefers it to cell if it's wifi and configuration says it should
+         * prefer bad wifi to cell.
+         */
+        public void scheduleEvaluationTimeout(@NonNull Handler handler,
+                @NonNull final Network network, final long delayMs) {
+            handler.sendMessageDelayed(
+                    handler.obtainMessage(EVENT_INITIAL_EVALUATION_TIMEOUT, network), delayMs);
+        }
     }
 
     public ConnectivityService(Context context) {
@@ -5273,8 +5290,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     /** Schedule evaluation timeout */
     @VisibleForTesting
     public void scheduleEvaluationTimeout(@NonNull final Network network, final long delayMs) {
-        mHandler.sendMessageDelayed(
+        mDeps.scheduleEvaluationTimeout(mHandler, network, delayMs);
-                mHandler.obtainMessage(EVENT_INITIAL_EVALUATION_TIMEOUT, network), delayMs);
     }
 
     @Override
@@ -9826,7 +9842,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 networkAgent.networkMonitor().notifyNetworkConnected(params.linkProperties,
                         params.networkCapabilities);
             }
-            final long delay = activelyPreferBadWifi()
+            final long delay = !avoidBadWifi() && activelyPreferBadWifi()
                     ? ACTIVELY_PREFER_BAD_WIFI_INITIAL_TIMEOUT_MS
                     : DONT_ACTIVELY_PREFER_BAD_WIFI_INITIAL_TIMEOUT_MS;
             scheduleEvaluationTimeout(networkAgent.network, delay);

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -2188,6 +2188,15 @@ public class ConnectivityServiceTest {
             // Call mocked destroyLiveTcpSocketsByOwnerUids so that test can verify this method call
             mDestroySocketsWrapper.destroyLiveTcpSocketsByOwnerUids(ownerUids);
         }
+
+        final ArrayTrackRecord<Long>.ReadHead mScheduledEvaluationTimeouts =
+                new ArrayTrackRecord<Long>().newReadHead();
+        @Override
+        public void scheduleEvaluationTimeout(@NonNull Handler handler,
+                @NonNull final Network network, final long delayMs) {
+            mScheduledEvaluationTimeouts.add(delayMs);
+            super.scheduleEvaluationTimeout(handler, network, delayMs);
+        }
     }
 
     private class AutomaticOnOffKeepaliveTrackerDependencies
@@ -6052,10 +6061,13 @@ public class ConnectivityServiceTest {
         wifiCallback.assertNoCallback();
     }
 
-    public void doTestPreferBadWifi(final boolean preferBadWifi) throws Exception {
+    public void doTestPreferBadWifi(final boolean avoidBadWifi,
+            final boolean preferBadWifi,
+            @NonNull Predicate<Long> checkUnvalidationTimeout) throws Exception {
         // Pretend we're on a carrier that restricts switching away from bad wifi, and
         // depending on the parameter one that may indeed prefer bad wifi.
-        doReturn(0).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi);
+        doReturn(avoidBadWifi ? 1 : 0).when(mResources)
+                .getInteger(R.integer.config_networkAvoidBadWifi);
         doReturn(preferBadWifi ? 1 : 0).when(mResources)
                 .getInteger(R.integer.config_activelyPreferBadWifi);
         mPolicyTracker.reevaluate();
@@ -6077,7 +6089,9 @@ public class ConnectivityServiceTest {
         mWiFiAgent.connect(false);
         wifiCallback.expectAvailableCallbacksUnvalidated(mWiFiAgent);
 
-        if (preferBadWifi) {
+        mDeps.mScheduledEvaluationTimeouts.poll(TIMEOUT_MS, t -> checkUnvalidationTimeout.test(t));
+
+        if (!avoidBadWifi && preferBadWifi) {
             expectUnvalidationCheckWillNotify(mWiFiAgent, NotificationType.LOST_INTERNET);
             mDefaultNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiAgent);
         } else {
@@ -6087,15 +6101,31 @@ public class ConnectivityServiceTest {
     }
 
     @Test
-    public void testPreferBadWifi_doNotPrefer() throws Exception {
+    public void testPreferBadWifi_doNotAvoid_doNotPrefer() throws Exception {
         // Starting with U this mode is no longer supported and can't actually be tested
         assumeFalse(SdkLevel.isAtLeastU());
-        doTestPreferBadWifi(false /* preferBadWifi */);
+        doTestPreferBadWifi(false /* avoidBadWifi */, false /* preferBadWifi */,
+                timeout -> timeout < 14_000);
     }
 
     @Test
-    public void testPreferBadWifi_doPrefer() throws Exception {
+    public void testPreferBadWifi_doNotAvoid_doPrefer() throws Exception {
-        doTestPreferBadWifi(true /* preferBadWifi */);
+        doTestPreferBadWifi(false /* avoidBadWifi */, true /* preferBadWifi */,
+                timeout -> timeout > 14_000);
+    }
+
+    @Test
+    public void testPreferBadWifi_doAvoid_doNotPrefer() throws Exception {
+        // If avoidBadWifi=true, then preferBadWifi should be irrelevant. Test anyway.
+        doTestPreferBadWifi(true /* avoidBadWifi */, false /* preferBadWifi */,
+                timeout -> timeout < 14_000);
+    }
+
+    @Test
+    public void testPreferBadWifi_doAvoid_doPrefer() throws Exception {
+        // If avoidBadWifi=true, then preferBadWifi should be irrelevant. Test anyway.
+        doTestPreferBadWifi(true /* avoidBadWifi */, true /* preferBadWifi */,
+                timeout -> timeout < 14_000);
     }
 
     @Test