summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/com/android/server/connectivity/NetworkMonitor.java23
-rw-r--r--tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java52
2 files changed, 27 insertions, 48 deletions
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 4e40ba4..9a8b304 100644
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -1034,7 +1034,7 @@ public class NetworkMonitor extends StateMachine {
mPrivateDnsConfig = null;
validationLog("Strict mode hostname resolution failed: " + uhe.getMessage());
}
- mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS,
+ mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS,
(mPrivateDnsConfig != null) /* succeeded */);
}
@@ -1086,7 +1086,7 @@ public class NetworkMonitor extends StateMachine {
String.format("%dms - Error: %s", time, uhe.getMessage()));
}
logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE);
- mEvaluationState.reportProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success);
+ mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success);
return success;
}
}
@@ -2067,11 +2067,21 @@ public class NetworkMonitor extends StateMachine {
}
// Class to keep state of evaluation results and probe results.
- // The main purpose is to ensure NetworkMonitor can notify ConnectivityService of probe results
+ //
+ // The main purpose was to ensure NetworkMonitor can notify ConnectivityService of probe results
// as soon as they happen, without triggering any other changes. This requires keeping state on
- // the most recent evaluation result. Calling reportProbeResult will ensure that the results
+ // the most recent evaluation result. Calling noteProbeResult will ensure that the results
// reported to ConnectivityService contain the previous evaluation result, and thus won't
// trigger a validation or partial connectivity state change.
+ //
+ // Note that this class is not currently being used for this purpose. The reason is that some
+ // of the system behaviour triggered by reporting network validation - notably, NetworkAgent
+ // behaviour - depends not only on the value passed by notifyNetworkTested, but also on the
+ // fact that notifyNetworkTested was called. For example, telephony triggers network recovery
+ // any time it is told that validation failed, i.e., if the result does not contain
+ // NETWORK_VALIDATION_RESULT_VALID. But with this scheme, the first two or three validation
+ // reports are all failures, because they are "HTTP succeeded but validation not yet passed",
+ // "HTTP and HTTPS succeeded but validation not yet passed", etc.
@VisibleForTesting
protected class EvaluationState {
// The latest validation result for this network. This is a bitmask of
@@ -2088,13 +2098,12 @@ public class NetworkMonitor extends StateMachine {
}
// Probe result for http probe should be updated from reportHttpProbeResult().
- protected void reportProbeResult(int probeResult, boolean succeeded) {
+ protected void noteProbeResult(int probeResult, boolean succeeded) {
if (succeeded) {
mProbeResults |= probeResult;
} else {
mProbeResults &= ~probeResult;
}
- notifyNetworkTested(getNetworkTestResult(), mRedirectUrl);
}
protected void reportEvaluationResult(int result, @Nullable String redirectUrl) {
@@ -2138,6 +2147,6 @@ public class NetworkMonitor extends StateMachine {
if (succeeded) {
probeResult |= NETWORK_VALIDATION_PROBE_DNS;
}
- mEvaluationState.reportProbeResult(probeResult, succeeded);
+ mEvaluationState.noteProbeResult(probeResult, succeeded);
}
}
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index 262641d..287d598 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -691,8 +691,7 @@ public class NetworkMonitorTest {
@Test
public void testNoInternetCapabilityValidated() throws Exception {
- runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID,
- getGeneralVerification());
+ runNetworkTest(NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID);
verify(mCleartextDnsNetwork, never()).openConnection(any());
}
@@ -780,8 +779,9 @@ public class NetworkMonitorTest {
wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.bad", new InetAddress[0]));
// Strict mode hostname resolve fail. Expect only notification for evaluation fail. No probe
// notification.
- verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
- .notifyNetworkTested(eq(VALIDATION_RESULT_VALID), eq(null));
+ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS))
+ .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS
+ | NETWORK_VALIDATION_PROBE_HTTPS), eq(null));
// Change configuration back to working again, but make private DNS not work.
// Expect validation to fail.
@@ -937,21 +937,9 @@ public class NetworkMonitorTest {
nm.forceReevaluation(Process.myUid());
final ArgumentCaptor<Integer> intCaptor = ArgumentCaptor.forClass(Integer.class);
// Expect to send HTTP, HTTPs, FALLBACK and evaluation results.
- verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(4))
- .notifyNetworkTested(intCaptor.capture(), any());
- List<Integer> intArgs = intCaptor.getAllValues();
-
- // None of these exact values can be known in advance except for intArgs.get(0) because the
- // HTTP and HTTPS probes race and the order in which they complete is non-deterministic.
- // Thus, check only exact value for intArgs.get(0) and only check the validation result for
- // the rest ones.
- assertEquals(Integer.valueOf(NETWORK_VALIDATION_PROBE_DNS
- | NETWORK_VALIDATION_PROBE_FALLBACK | NETWORK_VALIDATION_RESULT_VALID),
- intArgs.get(0));
- assertTrue((intArgs.get(1) & NETWORK_VALIDATION_RESULT_VALID) != 0);
- assertTrue((intArgs.get(2) & NETWORK_VALIDATION_RESULT_VALID) != 0);
- assertTrue((intArgs.get(3) & NETWORK_VALIDATION_RESULT_PARTIAL) != 0);
- assertTrue((intArgs.get(3) & NETWORK_VALIDATION_RESULT_VALID) == 0);
+ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS))
+ .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS |
+ NETWORK_VALIDATION_PROBE_FALLBACK | NETWORK_VALIDATION_RESULT_PARTIAL), any());
}
@Test
@@ -973,8 +961,6 @@ public class NetworkMonitorTest {
// Verify result should be appended and notifyNetworkTested callback is triggered once.
assertEquals(nm.getEvaluationState().getNetworkTestResult(),
VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP);
- verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTested(
- eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP), any());
nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.FAILED);
// Verify DNS probe result should not be cleared.
@@ -1064,14 +1050,7 @@ public class NetworkMonitorTest {
}
private void runPortalNetworkTest(int result) {
- // The network test event will be triggered twice with the same result. Expect to capture
- // the second one with direct url.
- runPortalNetworkTest(result,
- (VerificationWithTimeout) timeout(HANDLER_TIMEOUT_MS).times(2));
- }
-
- private void runPortalNetworkTest(int result, VerificationWithTimeout mode) {
- runNetworkTest(result, mode);
+ runNetworkTest(result);
assertEquals(1, mRegisteredReceivers.size());
assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue());
}
@@ -1108,19 +1087,14 @@ public class NetworkMonitorTest {
}
private NetworkMonitor runNetworkTest(int testResult) {
- return runNetworkTest(METERED_CAPABILITIES, testResult, getGeneralVerification());
+ return runNetworkTest(METERED_CAPABILITIES, testResult);
}
- private NetworkMonitor runNetworkTest(int testResult, VerificationWithTimeout mode) {
- return runNetworkTest(METERED_CAPABILITIES, testResult, mode);
- }
-
- private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult,
- VerificationWithTimeout mode) {
+ private NetworkMonitor runNetworkTest(NetworkCapabilities nc, int testResult) {
final NetworkMonitor monitor = makeMonitor(nc);
monitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, nc);
try {
- verify(mCallbacks, mode)
+ verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS))
.notifyNetworkTested(eq(testResult), mNetworkTestedRedirectUrlCaptor.capture());
} catch (RemoteException e) {
fail("Unexpected exception: " + e);
@@ -1153,9 +1127,5 @@ public class NetworkMonitorTest {
}
}
- private VerificationWithTimeout getGeneralVerification() {
- return (VerificationWithTimeout) timeout(HANDLER_TIMEOUT_MS).atLeastOnce();
- }
-
}
generated by cgit v1.2.3 (git 2.25.1) at 2025-10-04 16:49:13 +0000