NetworkMonitor: send one DNS probe per web probe

This patch changes sligthly the two web probes mechanism for captive
portal detection and network validation so that DNS resolution is always
done for both probes.

In general the target web servers of the two parallel HTTP and HTTPS probes
are now different. This introduces a bias in the latency measurement of
th HTTPS probe since this latency will also include DNS resolution in
general.

Test: manual verification + $ runtest frameworks-net
Bug: 32198726
Change-Id: I71ed6927f5d9cdab0d0236cc147fe1c5735a1245

1 files changed, 52 insertions, 44 deletions

diff --git a/services/core/java/com/android/server/connectivity/NetworkMonitor.java b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
index 209ea647922c..5e9885914169 100644
--- a/services/core/java/com/android/server/connectivity/NetworkMonitor.java
+++ b/services/core/java/com/android/server/connectivity/NetworkMonitor.java
@@ -707,48 +707,13 @@ public class NetworkMonitor extends StateMachine {
 
         long startTime = SystemClock.elapsedRealtime();
 
-        // Pre-resolve the captive portal server host so we can log it.
-        // Only do this if HttpURLConnection is about to, to avoid any potentially
-        // unnecessary resolution.
-        String hostToResolve = null;
-        if (pacUrl != null) {
-            hostToResolve = pacUrl.getHost();
-        } else if (proxyInfo != null) {
-            hostToResolve = proxyInfo.getHost();
-        } else {
-            hostToResolve = httpUrl.getHost();
-        }
-
-        if (!TextUtils.isEmpty(hostToResolve)) {
-            String probeName = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS);
-            final Stopwatch dnsTimer = new Stopwatch().start();
-            int dnsResult;
-            long dnsLatency;
-            try {
-                InetAddress[] addresses = mNetworkAgentInfo.network.getAllByName(hostToResolve);
-                dnsResult = ValidationProbeEvent.DNS_SUCCESS;
-                dnsLatency = dnsTimer.stop();
-                final StringBuffer connectInfo = new StringBuffer(", " + hostToResolve + "=");
-                for (InetAddress address : addresses) {
-                    connectInfo.append(address.getHostAddress());
-                    if (address != addresses[addresses.length-1]) connectInfo.append(",");
-                }
-                validationLog(probeName + " OK " + dnsLatency + "ms" + connectInfo);
-            } catch (UnknownHostException e) {
-                dnsResult = ValidationProbeEvent.DNS_FAILURE;
-                dnsLatency = dnsTimer.stop();
-                validationLog(probeName + " FAIL " + dnsLatency + "ms, " + hostToResolve);
-            }
-            logValidationProbe(dnsLatency, ValidationProbeEvent.PROBE_DNS, dnsResult);
-        }
-
-        CaptivePortalProbeResult result;
+        final CaptivePortalProbeResult result;
         if (pacUrl != null) {
-            result = sendHttpProbe(pacUrl, ValidationProbeEvent.PROBE_PAC);
+            result = sendDnsAndHttpProbes(null, pacUrl, ValidationProbeEvent.PROBE_PAC);
         } else if (mUseHttps) {
-            result = sendParallelHttpProbes(httpsUrl, httpUrl, fallbackUrl);
+            result = sendParallelHttpProbes(proxyInfo, httpsUrl, httpUrl, fallbackUrl);
         } else {
-            result = sendHttpProbe(httpUrl, ValidationProbeEvent.PROBE_HTTP);
+            result = sendDnsAndHttpProbes(proxyInfo, httpUrl, ValidationProbeEvent.PROBE_HTTP);
         }
 
         long endTime = SystemClock.elapsedRealtime();
@@ -761,8 +726,50 @@ public class NetworkMonitor extends StateMachine {
     }
 
     /**
-     * Do a URL fetch on a known server to see if we get the data we expect.
-     * Returns HTTP response code.
+     * Do a DNS resolution and URL fetch on a known web server to see if we get the data we expect.
+     * @return a CaptivePortalProbeResult inferred from the HTTP response.
+     */
+    private CaptivePortalProbeResult sendDnsAndHttpProbes(ProxyInfo proxy, URL url, int probeType) {
+        // Pre-resolve the captive portal server host so we can log it.
+        // Only do this if HttpURLConnection is about to, to avoid any potentially
+        // unnecessary resolution.
+        final String host = (proxy != null) ? proxy.getHost() : url.getHost();
+        sendDnsProbe(host);
+        return sendHttpProbe(url, probeType);
+    }
+
+    /** Do a DNS resolution of the given server. */
+    private void sendDnsProbe(String host) {
+        if (TextUtils.isEmpty(host)) {
+            return;
+        }
+
+        final String name = ValidationProbeEvent.getProbeName(ValidationProbeEvent.PROBE_DNS);
+        final Stopwatch watch = new Stopwatch().start();
+        int result;
+        String connectInfo;
+        try {
+            InetAddress[] addresses = mNetworkAgentInfo.network.getAllByName(host);
+            result = ValidationProbeEvent.DNS_SUCCESS;
+            StringBuffer buffer = new StringBuffer(host).append("=");
+            for (InetAddress address : addresses) {
+                buffer.append(address.getHostAddress());
+                if (address != addresses[addresses.length-1]) buffer.append(",");
+            }
+            connectInfo = buffer.toString();
+        } catch (UnknownHostException e) {
+            result = ValidationProbeEvent.DNS_FAILURE;
+            connectInfo = host;
+        }
+        final long latency = watch.stop();
+        String resultString = (ValidationProbeEvent.DNS_SUCCESS == result) ? "OK" : "FAIL";
+        validationLog(String.format("%s %s %dms, %s", name, resultString, latency, connectInfo));
+        logValidationProbe(latency, ValidationProbeEvent.PROBE_DNS, result);
+    }
+
+    /**
+     * Do a URL fetch on a known web server to see if we get the data we expect.
+     * @return a CaptivePortalProbeResult inferred from the HTTP response.
      */
     @VisibleForTesting
     protected CaptivePortalProbeResult sendHttpProbe(URL url, int probeType) {
@@ -829,7 +836,7 @@ public class NetworkMonitor extends StateMachine {
     }
 
     private CaptivePortalProbeResult sendParallelHttpProbes(
-            URL httpsUrl, URL httpUrl, URL fallbackUrl) {
+            ProxyInfo proxy, URL httpsUrl, URL httpUrl, URL fallbackUrl) {
         // Number of probes to wait for. If a probe completes with a conclusive answer
         // it shortcuts the latch immediately by forcing the count to 0.
         final CountDownLatch latch = new CountDownLatch(2);
@@ -849,9 +856,10 @@ public class NetworkMonitor extends StateMachine {
             @Override
             public void run() {
                 if (mIsHttps) {
-                    mResult = sendHttpProbe(httpsUrl, ValidationProbeEvent.PROBE_HTTPS);
+                    mResult =
+                            sendDnsAndHttpProbes(proxy, httpsUrl, ValidationProbeEvent.PROBE_HTTPS);
                 } else {
-                    mResult = sendHttpProbe(httpUrl, ValidationProbeEvent.PROBE_HTTP);
+                    mResult = sendDnsAndHttpProbes(proxy, httpUrl, ValidationProbeEvent.PROBE_HTTP);
                 }
                 if ((mIsHttps && mResult.isSuccessful()) || (!mIsHttps && mResult.isPortal())) {
                     // Stop waiting immediately if https succeeds or if http finds a portal.