Don't report probe status to ConnectivityService. (9a868738) · Commits · e / os / android_packages_modules_NetworkStack

src/com/android/server/connectivity/NetworkMonitor.java

+16 −7

Original line number	Diff line number	Diff line
		@@ -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);
		}
		}

tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java

+11 −41

Original line number	Diff line number	Diff line
		@@ -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();
		}

		}