Do not revalidate before network is ready

ConnectivityService sends notifyLinkPropertiesChanged before
notifyNetworkConnected. When a captive portal URL is present, this
causes NetworkMonitor to revalidate, even though the network is not
ready to start validating (DNS servers have not been set yet in
particular). ConnectivityService does this because the sending new
LinkProperties to NetworkMonitor is part of the standard LinkProperties
update flow; NetworkMonitor should be resilient to such behavior.

Test: atest NetworkMonitorTest, manual (flashed, wifi working)
Bug: 156697983
Original-Change: https://android-review.googlesource.com/1315220
Merged-In: I0619d37a3374726b77e162d174c5b12659db3bbb
Change-Id: I0619d37a3374726b77e162d174c5b12659db3bbb

2 files changed, 32 insertions, 0 deletions

diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index d0f62b2..8493816 100755
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -792,6 +792,11 @@ public class NetworkMonitor extends StateMachine {
                     return HANDLED;
                 case CMD_FORCE_REEVALUATION:
                 case CMD_CAPTIVE_PORTAL_RECHECK:
+                    if (getCurrentState() == mDefaultState) {
+                        // Before receiving CMD_NETWORK_CONNECTED (when still in mDefaultState),
+                        // requests to reevaluate are not valid: drop them.
+                        return HANDLED;
+                    }
                     String msg = "Forcing reevaluation for UID " + message.arg1;
                     final DnsStallDetector dsd = getDnsStallDetector();
                     if (dsd != null) {
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index 2422b77..00fe17c 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -69,6 +69,7 @@ import static org.junit.Assume.assumeTrue;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.after;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
 import static org.mockito.Mockito.atLeastOnce;
@@ -1206,6 +1207,32 @@ public class NetworkMonitorTest {
     }
 
     @Test
+    public void testIsCaptivePortal_NoRevalidationBeforeNetworkConnected() throws Exception {
+        assumeTrue(CaptivePortalDataShimImpl.isSupported());
+
+        final NetworkMonitor nm = makeCellMeteredNetworkMonitor();
+
+        final LinkProperties lp = makeCapportLPs();
+
+        // LinkProperties changed, but NM should not revalidate before notifyNetworkConnected
+        nm.notifyLinkPropertiesChanged(lp);
+        verify(mHttpConnection, after(100).never()).getResponseCode();
+        verify(mHttpsConnection, never()).getResponseCode();
+        verify(mCapportApiConnection, never()).getResponseCode();
+
+        setValidProbes();
+        setApiContent(mCapportApiConnection, "{'captive': true, "
+                + "'user-portal-url': '" + TEST_LOGIN_URL + "'}");
+
+        // After notifyNetworkConnected, validation uses the capport API contents
+        nm.notifyNetworkConnected(lp, CELL_METERED_CAPABILITIES);
+        verifyNetworkTested(VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, TEST_LOGIN_URL);
+
+        verify(mHttpConnection, never()).getResponseCode();
+        verify(mCapportApiConnection).getResponseCode();
+    }
+
+    @Test
     public void testIsCaptivePortal_CapportApiNotPortalNotValidated() throws Exception {
         assumeTrue(CaptivePortalDataShimImpl.isSupported());
         setSslException(mHttpsConnection);