Merge "Cleanup test (fix comment + make a helper easier to use)"
diff --git a/framework/src/android/net/util/MultinetworkPolicyTracker.java b/framework/src/android/net/util/MultinetworkPolicyTracker.java
index 4217921..c50df4a 100644
--- a/framework/src/android/net/util/MultinetworkPolicyTracker.java
+++ b/framework/src/android/net/util/MultinetworkPolicyTracker.java
@@ -100,7 +100,6 @@
      * doesn't provide internet access, unless there is a captive portal on that wifi.
      * This is the behavior in U and above.
      */
-    // TODO : implement the behavior.
     private boolean mActivelyPreferBadWifi;
 
     // Mainline module can't use internal HandlerExecutor, so add an identical executor here.
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 3c9886c..eb2674e 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -3748,17 +3748,6 @@
                 case EVENT_PROVISIONING_NOTIFICATION: {
                     final boolean visible = toBool(msg.arg1);
                     // If captive portal status has changed, update capabilities or disconnect.
-                    if (nai != null && (visible != nai.captivePortalDetected())) {
-                        nai.setCaptivePortalDetected(visible);
-                        if (visible && ConnectivitySettingsManager.CAPTIVE_PORTAL_MODE_AVOID
-                                == getCaptivePortalMode()) {
-                            if (DBG) log("Avoiding captive portal network: " + nai.toShortString());
-                            nai.onPreventAutomaticReconnect();
-                            teardownUnneededNetwork(nai);
-                            break;
-                        }
-                        updateCapabilitiesForNetwork(nai);
-                    }
                     if (!visible) {
                         // Only clear SIGN_IN and NETWORK_SWITCH notifications here, or else other
                         // notifications belong to the same network may be cleared unexpectedly.
@@ -3796,12 +3785,12 @@
                 @NonNull NetworkAgentInfo nai, int testResult, @NonNull String redirectUrl) {
             final boolean valid = (testResult & NETWORK_VALIDATION_RESULT_VALID) != 0;
             final boolean partial = (testResult & NETWORK_VALIDATION_RESULT_PARTIAL) != 0;
-            final boolean captive = !TextUtils.isEmpty(redirectUrl);
+            final boolean portal = !TextUtils.isEmpty(redirectUrl);
 
             // If there is any kind of working networking, then the NAI has been evaluated
             // once. {@see NetworkAgentInfo#setEvaluated}, which returns whether this is
             // the first time this ever happened.
-            final boolean someConnectivity = (valid || partial || captive);
+            final boolean someConnectivity = (valid || partial || portal);
             final boolean becameEvaluated = someConnectivity && nai.setEvaluated();
             // Because of b/245893397, if the score is updated when updateCapabilities is called,
             // any callback that receives onAvailable for that rematch receives an extra caps
@@ -3820,8 +3809,12 @@
 
             final boolean wasValidated = nai.isValidated();
             final boolean wasPartial = nai.partialConnectivity();
-            nai.setPartialConnectivity((testResult & NETWORK_VALIDATION_RESULT_PARTIAL) != 0);
-            final boolean partialConnectivityChanged = (wasPartial != nai.partialConnectivity());
+            final boolean wasPortal = nai.captivePortalDetected();
+            nai.setPartialConnectivity(partial);
+            nai.setCaptivePortalDetected(portal);
+            nai.updateScoreForNetworkAgentUpdate();
+            final boolean partialConnectivityChanged = (wasPartial != partial);
+            final boolean portalChanged = (wasPortal != portal);
 
             if (DBG) {
                 final String logMsg = !TextUtils.isEmpty(redirectUrl)
@@ -3829,7 +3822,7 @@
                         : "";
                 log(nai.toShortString() + " validation " + (valid ? "passed" : "failed") + logMsg);
             }
-            if (valid != nai.isValidated()) {
+            if (valid != wasValidated) {
                 final FullScore oldScore = nai.getScore();
                 nai.setValidated(valid);
                 updateCapabilities(oldScore, nai, nai.networkCapabilities);
@@ -3852,6 +3845,16 @@
                 }
             } else if (partialConnectivityChanged) {
                 updateCapabilitiesForNetwork(nai);
+            } else if (portalChanged) {
+                if (portal && ConnectivitySettingsManager.CAPTIVE_PORTAL_MODE_AVOID
+                        == getCaptivePortalMode()) {
+                    if (DBG) log("Avoiding captive portal network: " + nai.toShortString());
+                    nai.onPreventAutomaticReconnect();
+                    teardownUnneededNetwork(nai);
+                    return;
+                } else {
+                    updateCapabilitiesForNetwork(nai);
+                }
             } else if (becameEvaluated) {
                 // If valid or partial connectivity changed, updateCapabilities* has
                 // done the rematch.
@@ -5080,8 +5083,14 @@
     private void updateAvoidBadWifi() {
         ensureRunningOnConnectivityServiceThread();
         // Agent info scores and offer scores depend on whether cells yields to bad wifi.
+        final boolean avoidBadWifi = avoidBadWifi();
         for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
             nai.updateScoreForNetworkAgentUpdate();
+            if (avoidBadWifi) {
+                // If the device is now avoiding bad wifi, remove notifications that might have
+                // been put up when the device didn't.
+                mNotifier.clearNotification(nai.network.getNetId(), NotificationType.LOST_INTERNET);
+            }
         }
         // UpdateOfferScore will update mNetworkOffers inline, so make a copy first.
         final ArrayList<NetworkOfferInfo> offersToUpdate = new ArrayList<>(mNetworkOffers);
@@ -5229,6 +5238,16 @@
             // network was found not to have Internet access.
             nai.updateScoreForNetworkAgentUpdate();
             rematchAllNetworksAndRequests();
+
+            // Also, if this is WiFi and it should be preferred actively, now is the time to
+            // prompt the user that they walked past and connected to a bad WiFi.
+            if (nai.networkCapabilities.hasTransport(TRANSPORT_WIFI)
+                    && !avoidBadWifi()
+                    && activelyPreferBadWifi()) {
+                // The notification will be removed if the network validates or disconnects.
+                showNetworkNotification(nai, NotificationType.LOST_INTERNET);
+                return;
+            }
         }
 
         if (!shouldPromptUnvalidated(nai)) return;
diff --git a/service/src/com/android/server/connectivity/NetworkRanker.java b/service/src/com/android/server/connectivity/NetworkRanker.java
index d56e171..735505a 100644
--- a/service/src/com/android/server/connectivity/NetworkRanker.java
+++ b/service/src/com/android/server/connectivity/NetworkRanker.java
@@ -16,6 +16,7 @@
 
 package com.android.server.connectivity;
 
+import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL;
 import static android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET;
@@ -27,6 +28,7 @@
 import static com.android.net.module.util.CollectionUtils.filter;
 import static com.android.server.connectivity.FullScore.POLICY_ACCEPT_UNVALIDATED;
 import static com.android.server.connectivity.FullScore.POLICY_AVOIDED_WHEN_UNVALIDATED;
+import static com.android.server.connectivity.FullScore.POLICY_EVER_EVALUATED;
 import static com.android.server.connectivity.FullScore.POLICY_EVER_USER_SELECTED;
 import static com.android.server.connectivity.FullScore.POLICY_EVER_VALIDATED;
 import static com.android.server.connectivity.FullScore.POLICY_IS_DESTROYED;
@@ -67,7 +69,6 @@
         /**
          * @see MultinetworkPolicyTracker#getActivelyPreferBadWifi()
          */
-        // TODO : implement the behavior.
         public boolean activelyPreferBadWifi() {
             return mActivelyPreferBadWifi;
         }
@@ -142,11 +143,42 @@
         }
     }
 
-    private <T extends Scoreable> boolean isBadWiFi(@NonNull final T candidate) {
+    /**
+     * Returns whether the wifi passed as an argument is a preferred network to yielding cell.
+     *
+     * When comparing bad wifi to cell with POLICY_YIELD_TO_BAD_WIFI, it may be necessary to
+     * know if a particular bad wifi is preferred to such a cell network. This method computes
+     * and returns this.
+     *
+     * @param candidate a bad wifi to evaluate
+     * @return whether this candidate is preferred to cell with POLICY_YIELD_TO_BAD_WIFI
+     */
+    private <T extends Scoreable> boolean isPreferredBadWiFi(@NonNull final T candidate) {
         final FullScore score = candidate.getScore();
-        return score.hasPolicy(POLICY_EVER_VALIDATED)
-                && !score.hasPolicy(POLICY_AVOIDED_WHEN_UNVALIDATED)
-                && candidate.getCapsNoCopy().hasTransport(TRANSPORT_WIFI);
+        final NetworkCapabilities caps = candidate.getCapsNoCopy();
+
+        // Whatever the policy, only WiFis can be preferred bad WiFis.
+        if (!caps.hasTransport(TRANSPORT_WIFI)) return false;
+        // Validated networks aren't bad networks, so a fortiori can't be preferred bad WiFis.
+        if (score.hasPolicy(POLICY_IS_VALIDATED)) return false;
+        // A WiFi that the user explicitly wanted to avoid in UI is never a preferred bad WiFi.
+        if (score.hasPolicy(POLICY_AVOIDED_WHEN_UNVALIDATED)) return false;
+
+        if (mConf.activelyPreferBadWifi()) {
+            // If a network is still evaluating, don't prefer it.
+            if (!score.hasPolicy(POLICY_EVER_EVALUATED)) return false;
+
+            // If a network is not a captive portal, then prefer it.
+            if (!caps.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL)) return true;
+
+            // If it's a captive portal, prefer it if it previously validated but is no longer
+            // validated (i.e., the user logged in in the past, but later the portal closed).
+            return score.hasPolicy(POLICY_EVER_VALIDATED);
+        } else {
+            // Under the original "prefer bad WiFi" policy, only networks that have ever validated
+            // are preferred.
+            return score.hasPolicy(POLICY_EVER_VALIDATED);
+        }
     }
 
     /**
@@ -169,7 +201,7 @@
             // No network with the policy : do nothing.
             return;
         }
-        if (!CollectionUtils.any(rejected, n -> isBadWiFi(n))) {
+        if (!CollectionUtils.any(rejected, n -> isPreferredBadWiFi(n))) {
             // No bad WiFi : do nothing.
             return;
         }
@@ -179,7 +211,7 @@
             // wifis by the following policies (e.g. exiting).
             final ArrayList<T> acceptedYielders = new ArrayList<>(accepted);
             final ArrayList<T> rejectedWithBadWiFis = new ArrayList<>(rejected);
-            partitionInto(rejectedWithBadWiFis, n -> isBadWiFi(n), accepted, rejected);
+            partitionInto(rejectedWithBadWiFis, n -> isPreferredBadWiFi(n), accepted, rejected);
             accepted.addAll(acceptedYielders);
             return;
         }
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 2c3397e..8b7b7c6 100755
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -29,6 +29,7 @@
 import static android.Manifest.permission.NETWORK_SETTINGS;
 import static android.Manifest.permission.NETWORK_STACK;
 import static android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD;
+import static android.Manifest.permission.READ_DEVICE_CONFIG;
 import static android.app.PendingIntent.FLAG_IMMUTABLE;
 import static android.content.Intent.ACTION_PACKAGE_ADDED;
 import static android.content.Intent.ACTION_PACKAGE_REMOVED;
@@ -3530,12 +3531,16 @@
 
     /** Expects the specified notification and returns the notification ID. */
     private int expectNotification(TestNetworkAgentWrapper agent, NotificationType type) {
-        verify(mNotificationManager).notify(
+        verify(mNotificationManager, timeout(TIMEOUT_MS)).notify(
                 eq(NetworkNotificationManager.tagFor(agent.getNetwork().netId)),
                 eq(type.eventId), any());
         return type.eventId;
     }
 
+    private void expectNoNotification(@NonNull final TestNetworkAgentWrapper agent) {
+        verify(mNotificationManager, never()).notifyAsUser(anyString(), anyInt(), any(), any());
+    }
+
     /**
      * Expects the specified notification happens when the unvalidated prompt message arrives
      *
@@ -3554,9 +3559,9 @@
      * This generally happens when the network disconnects or when the newtwork validates. During
      * normal usage the notification is also cleared by the system when the notification is tapped.
      */
-    private void expectClearNotification(TestNetworkAgentWrapper agent, int expectedId) {
-        verify(mNotificationManager).cancel(
-                eq(NetworkNotificationManager.tagFor(agent.getNetwork().netId)), eq(expectedId));
+    private void expectClearNotification(TestNetworkAgentWrapper agent, NotificationType type) {
+        verify(mNotificationManager, timeout(TIMEOUT_MS)).cancel(
+                eq(NetworkNotificationManager.tagFor(agent.getNetwork().netId)), eq(type.eventId));
     }
 
     /**
@@ -3567,13 +3572,13 @@
     private void expectUnvalidationCheckWillNotNotify(TestNetworkAgentWrapper agent) {
         mService.scheduleEvaluationTimeout(agent.getNetwork(), 0 /*delayMs */);
         waitForIdle();
-        verify(mNotificationManager, never()).notifyAsUser(anyString(), anyInt(), any(), any());
+        expectNoNotification(agent);
     }
 
     private void expectDisconnectAndClearNotifications(TestNetworkCallback callback,
-            TestNetworkAgentWrapper agent, int id) {
+            TestNetworkAgentWrapper agent, NotificationType type) {
         callback.expectCallback(CallbackEntry.LOST, agent);
-        expectClearNotification(agent, id);
+        expectClearNotification(agent, type);
     }
 
     private NativeNetworkConfig nativeNetworkConfigPhysical(int netId, int permission) {
@@ -3717,7 +3722,7 @@
         // Disconnect wifi, and then reconnect, again with explicitlySelected=true.
         mWiFiNetworkAgent.disconnect();
         expectDisconnectAndClearNotifications(callback, mWiFiNetworkAgent,
-                NotificationType.NO_INTERNET.eventId);
+                NotificationType.NO_INTERNET);
 
         mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
         mWiFiNetworkAgent.explicitlySelected(true, false);
@@ -3731,7 +3736,7 @@
         // network to disconnect.
         mCm.setAcceptUnvalidated(mWiFiNetworkAgent.getNetwork(), false, false);
         expectDisconnectAndClearNotifications(callback, mWiFiNetworkAgent,
-                NotificationType.NO_INTERNET.eventId);
+                NotificationType.NO_INTERNET);
         reset(mNotificationManager);
 
         // Reconnect, again with explicitlySelected=true, but this time validate.
@@ -4231,7 +4236,7 @@
         assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
 
         // Once the network validates, the notification disappears.
-        expectClearNotification(mWiFiNetworkAgent, NotificationType.PARTIAL_CONNECTIVITY.eventId);
+        expectClearNotification(mWiFiNetworkAgent, NotificationType.PARTIAL_CONNECTIVITY);
 
         // Disconnect and reconnect wifi with partial connectivity again.
         mWiFiNetworkAgent.disconnect();
@@ -4254,7 +4259,7 @@
         mCm.setAcceptPartialConnectivity(mWiFiNetworkAgent.getNetwork(), false /* accept */,
                 false /* always */);
         callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
-        expectClearNotification(mWiFiNetworkAgent, NotificationType.PARTIAL_CONNECTIVITY.eventId);
+        expectClearNotification(mWiFiNetworkAgent, NotificationType.PARTIAL_CONNECTIVITY);
         reset(mNotificationManager);
 
         // If the user accepted partial connectivity before, and the device connects to that network
@@ -4330,10 +4335,11 @@
 
     @Test
     public void testCaptivePortalOnPartialConnectivity() throws Exception {
-        final TestNetworkCallback captivePortalCallback = new TestNetworkCallback();
-        final NetworkRequest captivePortalRequest = new NetworkRequest.Builder()
-                .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL).build();
-        mCm.registerNetworkCallback(captivePortalRequest, captivePortalCallback);
+        final TestNetworkCallback wifiCallback = new TestNetworkCallback();
+        final NetworkRequest wifiRequest = new NetworkRequest.Builder()
+                .addTransportType(TRANSPORT_WIFI)
+                .build();
+        mCm.registerNetworkCallback(wifiRequest, wifiCallback);
 
         final TestNetworkCallback validatedCallback = new TestNetworkCallback();
         final NetworkRequest validatedRequest = new NetworkRequest.Builder()
@@ -4345,21 +4351,28 @@
         mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
         String redirectUrl = "http://android.com/path";
         mWiFiNetworkAgent.connectWithCaptivePortal(redirectUrl, false /* isStrictMode */);
-        captivePortalCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
+        wifiCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         assertEquals(mWiFiNetworkAgent.waitForRedirectUrl(), redirectUrl);
 
+        // This is necessary because of b/245893397, the same bug that happens where we use
+        // expectAvailableDoubleValidatedCallbacks.
+        // TODO : fix b/245893397 and remove this.
+        wifiCallback.expectCapabilitiesWith(NET_CAPABILITY_CAPTIVE_PORTAL, mWiFiNetworkAgent);
+
         // Check that startCaptivePortalApp sends the expected command to NetworkMonitor.
         mCm.startCaptivePortalApp(mWiFiNetworkAgent.getNetwork());
         verify(mWiFiNetworkAgent.mNetworkMonitor, timeout(TIMEOUT_MS).times(1))
                 .launchCaptivePortalApp();
 
         // Report that the captive portal is dismissed with partial connectivity, and check that
-        // callbacks are fired.
+        // callbacks are fired with PARTIAL and without CAPTIVE_PORTAL.
         mWiFiNetworkAgent.setNetworkPartial();
         mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
         waitForIdle();
-        captivePortalCallback.expectCapabilitiesWith(NET_CAPABILITY_PARTIAL_CONNECTIVITY,
-                mWiFiNetworkAgent);
+        wifiCallback.expectCapabilitiesThat(
+                mWiFiNetworkAgent, nc ->
+                nc.hasCapability(NET_CAPABILITY_PARTIAL_CONNECTIVITY)
+                && !nc.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL));
 
         // Report partial connectivity is accepted.
         mWiFiNetworkAgent.setNetworkPartialValid(false /* isStrictMode */);
@@ -4367,13 +4380,12 @@
                 false /* always */);
         waitForIdle();
         mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
-        captivePortalCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
+        wifiCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
         validatedCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent);
-        NetworkCapabilities nc =
-                validatedCallback.expectCapabilitiesWith(NET_CAPABILITY_PARTIAL_CONNECTIVITY,
+        validatedCallback.expectCapabilitiesWith(NET_CAPABILITY_PARTIAL_CONNECTIVITY,
                 mWiFiNetworkAgent);
 
-        mCm.unregisterNetworkCallback(captivePortalCallback);
+        mCm.unregisterNetworkCallback(wifiCallback);
         mCm.unregisterNetworkCallback(validatedCallback);
     }
 
@@ -4456,6 +4468,11 @@
         mCm.reportNetworkConnectivity(wifiNetwork, false);
         captivePortalCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         validatedCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
+        // This is necessary because of b/245893397, the same bug that happens where we use
+        // expectAvailableDoubleValidatedCallbacks.
+        // TODO : fix b/245893397 and remove this.
+        captivePortalCallback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED,
+                mWiFiNetworkAgent);
 
         // Check that startCaptivePortalApp sends the expected command to NetworkMonitor.
         mCm.startCaptivePortalApp(wifiNetwork);
@@ -5759,6 +5776,56 @@
         wifiCallback.assertNoCallback();
     }
 
+    public void doTestPreferBadWifi(final boolean preferBadWifi) 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(preferBadWifi ? 1 : 0).when(mResources)
+                .getInteger(R.integer.config_activelyPreferBadWifi);
+        mPolicyTracker.reevaluate();
+
+        registerDefaultNetworkCallbacks();
+        final NetworkRequest wifiRequest = new NetworkRequest.Builder()
+                .clearCapabilities()
+                .addTransportType(TRANSPORT_WIFI)
+                .build();
+        final TestableNetworkCallback wifiCallback = new TestableNetworkCallback();
+        mCm.registerNetworkCallback(wifiRequest, wifiCallback);
+
+        // Bring up validated cell and unvalidated wifi.
+        mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
+        mCellNetworkAgent.connect(true);
+        mDefaultNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent);
+
+        mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
+        mWiFiNetworkAgent.connect(false);
+        wifiCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
+
+        if (preferBadWifi) {
+            expectUnvalidationCheckWillNotify(mWiFiNetworkAgent, NotificationType.LOST_INTERNET);
+            mDefaultNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
+        } else {
+            expectUnvalidationCheckWillNotNotify(mWiFiNetworkAgent);
+            mDefaultNetworkCallback.assertNoCallback();
+        }
+    }
+
+    @Test
+    public void testPreferBadWifi_doNotPrefer() throws Exception {
+        // Starting with U this mode is no longer supported and can't actually be tested
+        assumeFalse(SdkLevel.isAtLeastU());
+        runAsShell(READ_DEVICE_CONFIG, () -> {
+            doTestPreferBadWifi(false /* preferBadWifi */);
+        });
+    }
+
+    @Test
+    public void testPreferBadWifi_doPrefer() throws Exception {
+        runAsShell(READ_DEVICE_CONFIG, () -> {
+            doTestPreferBadWifi(true /* preferBadWifi */);
+        });
+    }
+
     @Test
     public void testAvoidBadWifi() throws Exception {
         final ContentResolver cr = mServiceContext.getContentResolver();
@@ -5782,7 +5849,8 @@
         TestNetworkCallback validatedWifiCallback = new TestNetworkCallback();
         mCm.registerNetworkCallback(validatedWifiRequest, validatedWifiCallback);
 
-        Settings.Global.putInt(cr, ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI, 0);
+        // Prompt mode, so notifications can be tested
+        Settings.Global.putString(cr, ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI, null);
         mPolicyTracker.reevaluate();
 
         // Bring up validated cell.
@@ -5804,6 +5872,7 @@
         mCm.reportNetworkConnectivity(wifiNetwork, false);
         defaultCallback.expectCapabilitiesWithout(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
         validatedWifiCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
+        expectNotification(mWiFiNetworkAgent, NotificationType.LOST_INTERNET);
 
         // Because avoid bad wifi is off, we don't switch to cellular.
         defaultCallback.assertNoCallback();
@@ -5819,14 +5888,20 @@
         mPolicyTracker.reevaluate();
         defaultCallback.expectAvailableCallbacksValidated(mCellNetworkAgent);
         assertEquals(mCm.getActiveNetwork(), cellNetwork);
+        expectClearNotification(mWiFiNetworkAgent, NotificationType.LOST_INTERNET);
 
         // Switch back to a restrictive carrier.
         doReturn(0).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi);
         mPolicyTracker.reevaluate();
         defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         assertEquals(mCm.getActiveNetwork(), wifiNetwork);
+        // A notification was already shown for this very network.
+        expectNoNotification(mWiFiNetworkAgent);
 
         // Simulate the user selecting "switch" on the dialog, and check that we switch to cell.
+        // In principle this is a little bit unrealistic because the switch to a less restrictive
+        // carrier above should have remove the notification but this doesn't matter for the
+        // purposes of this test.
         mCm.setAvoidUnvalidated(wifiNetwork);
         defaultCallback.expectAvailableCallbacksValidated(mCellNetworkAgent);
         assertFalse(mCm.getNetworkCapabilities(wifiNetwork).hasCapability(
@@ -5848,6 +5923,7 @@
         mCm.reportNetworkConnectivity(wifiNetwork, false);
         defaultCallback.expectCapabilitiesWithout(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
         validatedWifiCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
+        expectNotification(mWiFiNetworkAgent, NotificationType.LOST_INTERNET);
 
         // Simulate the user selecting "switch" and checking the don't ask again checkbox.
         Settings.Global.putInt(cr, ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI, 1);
@@ -5860,6 +5936,7 @@
         assertTrue(mCm.getNetworkCapabilities(cellNetwork).hasCapability(
                 NET_CAPABILITY_VALIDATED));
         assertEquals(mCm.getActiveNetwork(), cellNetwork);
+        expectClearNotification(mWiFiNetworkAgent, NotificationType.LOST_INTERNET);
 
         // Simulate the user turning the cellular fallback setting off and then on.
         // We switch to wifi and then to cell.
@@ -5867,6 +5944,9 @@
         mPolicyTracker.reevaluate();
         defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         assertEquals(mCm.getActiveNetwork(), wifiNetwork);
+        // Notification is cleared again because CS doesn't particularly remember that it has
+        // cleared it before, and if it hasn't cleared it before then it should do so now.
+        expectClearNotification(mWiFiNetworkAgent, NotificationType.LOST_INTERNET);
         Settings.Global.putInt(cr, ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI, 1);
         mPolicyTracker.reevaluate();
         defaultCallback.expectAvailableCallbacksValidated(mCellNetworkAgent);
@@ -5877,6 +5957,8 @@
         defaultCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent);
         defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         validatedWifiCallback.assertNoCallback();
+        // Notification is cleared yet again because the device switched to wifi.
+        expectClearNotification(mWiFiNetworkAgent, NotificationType.LOST_INTERNET);
 
         mCm.unregisterNetworkCallback(cellNetworkCallback);
         mCm.unregisterNetworkCallback(validatedWifiCallback);
@@ -12980,6 +13062,12 @@
         if (null != mTestPackageDefaultNetworkCallback2) {
             mCm.unregisterNetworkCallback(mTestPackageDefaultNetworkCallback2);
         }
+        mSystemDefaultNetworkCallback = null;
+        mDefaultNetworkCallback = null;
+        mProfileDefaultNetworkCallback = null;
+        mTestPackageDefaultNetworkCallback = null;
+        mProfileDefaultNetworkCallbackAsAppUid2 = null;
+        mTestPackageDefaultNetworkCallback2 = null;
     }
 
     private void setupMultipleDefaultNetworksForOemNetworkPreferenceNotCurrentUidTest(
diff --git a/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt b/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt
index 05c648d..ae8b438 100644
--- a/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/NetworkRankerTest.kt
@@ -17,6 +17,7 @@
 package com.android.server.connectivity
 
 import android.net.NetworkCapabilities
+import android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL
 import android.net.NetworkCapabilities.TRANSPORT_CELLULAR
 import android.net.NetworkCapabilities.TRANSPORT_WIFI
 import android.net.NetworkScore.KEEP_CONNECTED_NONE
@@ -25,25 +26,29 @@
 import android.net.NetworkScore.POLICY_YIELD_TO_BAD_WIFI
 import android.os.Build
 import androidx.test.filters.SmallTest
+import com.android.connectivity.resources.R
 import com.android.server.connectivity.FullScore.POLICY_AVOIDED_WHEN_UNVALIDATED
+import com.android.server.connectivity.FullScore.POLICY_EVER_EVALUATED
 import com.android.server.connectivity.FullScore.POLICY_EVER_VALIDATED
 import com.android.server.connectivity.FullScore.POLICY_IS_VALIDATED
 import com.android.testutils.DevSdkIgnoreRule
-import com.android.testutils.DevSdkIgnoreRunner
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
 import kotlin.test.assertEquals
 
 private fun score(vararg policies: Int) = FullScore(
         policies.fold(0L) { acc, e -> acc or (1L shl e) }, KEEP_CONNECTED_NONE)
-private fun caps(transport: Int) = NetworkCapabilities.Builder().addTransportType(transport).build()
+private fun caps(transport: Int, vararg capabilities: Int) =
+        NetworkCapabilities.Builder().addTransportType(transport).apply {
+            capabilities.forEach { addCapability(it) }
+        }.build()
 
 @SmallTest
-@RunWith(DevSdkIgnoreRunner::class)
-@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.R)
-class NetworkRankerTest {
-    private val mRanker = NetworkRanker(NetworkRanker.Configuration(
-            false /* activelyPreferBadWifi */))
+@RunWith(Parameterized::class)
+class NetworkRankerTest(private val activelyPreferBadWifi: Boolean) {
+    private val mRanker = NetworkRanker(NetworkRanker.Configuration(activelyPreferBadWifi))
 
     private class TestScore(private val sc: FullScore, private val nc: NetworkCapabilities)
             : NetworkRanker.Scoreable {
@@ -51,35 +56,91 @@
         override fun getCapsNoCopy(): NetworkCapabilities = nc
     }
 
+    @get:Rule
+    val mIgnoreRule: DevSdkIgnoreRule = DevSdkIgnoreRule(ignoreClassUpTo = Build.VERSION_CODES.R)
+
+    companion object {
+        @JvmStatic
+        @Parameterized.Parameters
+        fun ranker() = listOf(true, false)
+    }
+
     @Test
     fun testYieldToBadWiFiOneCell() {
         // Only cell, it wins
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED, POLICY_EVER_EVALUATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(winner)
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
     @Test
+    fun testPreferBadWifiOneCellOneEvaluatingWifi() {
+        // TODO : refactor the tests to name each network like this test
+        val wifi = TestScore(score(), caps(TRANSPORT_WIFI))
+        val cell = TestScore(
+                score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED, POLICY_EVER_EVALUATED),
+                caps(TRANSPORT_CELLULAR))
+        assertEquals(cell, mRanker.getBestNetworkByPolicy(listOf(wifi, cell), null))
+    }
+
+    @Test
     fun testYieldToBadWiFiOneCellOneBadWiFi() {
         // Bad wifi wins against yielding validated cell
-        val winner = TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI))
+        val winner = TestScore(score(POLICY_EVER_VALIDATED, POLICY_EVER_EVALUATED),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED, POLICY_EVER_EVALUATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
     @Test
+    fun testPreferBadWifiOneCellOneBadWifi() {
+        val wifi = TestScore(score(POLICY_EVER_EVALUATED), caps(TRANSPORT_WIFI))
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        val scores = listOf(wifi, cell)
+        val winner = if (activelyPreferBadWifi) wifi else cell
+        assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
+    }
+
+    @Test
+    fun testPreferBadWifiOneCellOneCaptivePortalWifi() {
+        val wifi = TestScore(score(POLICY_EVER_EVALUATED),
+                caps(TRANSPORT_WIFI, NET_CAPABILITY_CAPTIVE_PORTAL))
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        assertEquals(cell, mRanker.getBestNetworkByPolicy(listOf(wifi, cell), null))
+    }
+
+    @Test
+    fun testYieldToBadWifiOneCellOneCaptivePortalWifiThatClosed() {
+        val wifi = TestScore(score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED),
+                caps(TRANSPORT_WIFI, NET_CAPABILITY_CAPTIVE_PORTAL))
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        assertEquals(wifi, mRanker.getBestNetworkByPolicy(listOf(wifi, cell), null))
+    }
+
+    @Test
     fun testYieldToBadWifiAvoidUnvalidated() {
         // Bad wifi avoided when unvalidated loses against yielding validated cell
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_AVOIDED_WHEN_UNVALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_AVOIDED_WHEN_UNVALIDATED),
                         caps(TRANSPORT_WIFI))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -88,12 +149,16 @@
     @Test
     fun testYieldToBadWiFiOneCellTwoBadWiFi() {
         // Bad wifi wins against yielding validated cell. Prefer the one that's primary.
-        val winner = TestScore(score(POLICY_EVER_VALIDATED,
-                POLICY_TRANSPORT_PRIMARY), caps(TRANSPORT_WIFI))
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED),
+                        caps(TRANSPORT_WIFI)),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -103,11 +168,13 @@
     fun testYieldToBadWiFiOneCellTwoBadWiFiOneNotAvoided() {
         // Bad wifi ever validated wins against bad wifi that never was validated (or was
         // avoided when bad).
-        val winner = TestScore(score(POLICY_EVER_VALIDATED), caps(TRANSPORT_WIFI))
+        val winner = TestScore(score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(), caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(score(POLICY_EVER_EVALUATED), caps(TRANSPORT_WIFI)),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -116,27 +183,49 @@
     @Test
     fun testYieldToBadWiFiOneCellOneBadWiFiOneGoodWiFi() {
         // Good wifi wins
-        val winner = TestScore(score(POLICY_EVER_VALIDATED, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_TRANSPORT_PRIMARY),
                         caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
 
     @Test
+    fun testPreferBadWifiOneCellOneBadWifiOneEvaluatingWifi() {
+        val cell = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
+        val badWifi = TestScore(score(POLICY_EVER_EVALUATED), caps(TRANSPORT_WIFI))
+        val evaluatingWifi = TestScore(score(), caps(TRANSPORT_WIFI))
+        val winner = if (activelyPreferBadWifi) badWifi else cell
+        assertEquals(winner,
+                mRanker.getBestNetworkByPolicy(listOf(cell, badWifi, evaluatingWifi), null))
+    }
+
+    @Test
     fun testYieldToBadWiFiTwoCellsOneBadWiFi() {
         // Cell that doesn't yield wins over cell that yields and bad wifi
-        val winner = TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR))
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_TRANSPORT_PRIMARY),
                         caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI,
+                                POLICY_EVER_VALIDATED, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -145,13 +234,20 @@
     @Test
     fun testYieldToBadWiFiTwoCellsOneBadWiFiOneGoodWiFi() {
         // Good wifi wins over cell that doesn't yield and cell that yields
-        val winner = TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_WIFI))
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED),
+                caps(TRANSPORT_WIFI))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_TRANSPORT_PRIMARY),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED,
+                                POLICY_TRANSPORT_PRIMARY),
                         caps(TRANSPORT_WIFI)),
-                TestScore(score(POLICY_IS_VALIDATED), caps(TRANSPORT_CELLULAR)),
-                TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED),
+                        caps(TRANSPORT_CELLULAR)),
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                         caps(TRANSPORT_CELLULAR))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
@@ -160,11 +256,14 @@
     @Test
     fun testYieldToBadWiFiOneExitingGoodWiFi() {
         // Yielding cell wins over good exiting wifi
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_IS_VALIDATED, POLICY_EXITING), caps(TRANSPORT_WIFI))
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_IS_VALIDATED, POLICY_EXITING),
+                        caps(TRANSPORT_WIFI))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }
@@ -172,11 +271,14 @@
     @Test
     fun testYieldToBadWiFiOneExitingBadWiFi() {
         // Yielding cell wins over bad exiting wifi
-        val winner = TestScore(score(POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
+        val winner = TestScore(
+                score(POLICY_EVER_EVALUATED, POLICY_YIELD_TO_BAD_WIFI, POLICY_IS_VALIDATED),
                 caps(TRANSPORT_CELLULAR))
         val scores = listOf(
                 winner,
-                TestScore(score(POLICY_EVER_VALIDATED, POLICY_EXITING), caps(TRANSPORT_WIFI))
+                TestScore(
+                        score(POLICY_EVER_EVALUATED, POLICY_EVER_VALIDATED, POLICY_EXITING),
+                        caps(TRANSPORT_WIFI))
         )
         assertEquals(winner, mRanker.getBestNetworkByPolicy(scores, null))
     }