[MP03] Refactor probing class

Refactor probing class to allow sending probe via thread class
in legacy send parallel probes function and also refactor for
follow up commit to send multiple probes.

Bug: 139034276
Test: atest NetworkStackTests NetworkStackNextTests
Test: manually test with resource configuration
Change-Id: Ia25bfe58b10b0a1a641a2be535ee0d602ffd8cd6
Merged-In: Ia25bfe58b10b0a1a641a2be535ee0d602ffd8cd6
(cherry picked from commit c17b3996f428ac80a2f8b2c4f361b0e18b0b50fa)

6 files changed, 249 insertions, 93 deletions

diff --git a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
index ff743c8..2ba1dcc 100755
--- a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
+++ b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java
@@ -16,14 +16,20 @@
 
 package android.net.captiveportal;
 
+import static android.net.metrics.ValidationProbeEvent.PROBE_HTTP;
+import static android.net.metrics.ValidationProbeEvent.PROBE_HTTPS;
+
+import androidx.annotation.IntDef;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 /**
  * Result of calling isCaptivePortal().
  * @hide
  */
-public final class CaptivePortalProbeResult {
+public class CaptivePortalProbeResult {
     public static final int SUCCESS_CODE = 204;
     public static final int FAILED_CODE = 599;
     public static final int PORTAL_CODE = 302;
@@ -42,18 +48,17 @@ public final class CaptivePortalProbeResult {
     // be FAILED if one of the probes returns this status.
     // This logic is only used if the config_force_dns_probe_private_ip_no_internet flag is set.
     public static final int DNS_PRIVATE_IP_RESPONSE_CODE = -2;
-
-    @NonNull
-    public static final CaptivePortalProbeResult FAILED = new CaptivePortalProbeResult(FAILED_CODE);
-    @NonNull
-    public static final CaptivePortalProbeResult SUCCESS =
-            new CaptivePortalProbeResult(SUCCESS_CODE);
-    public static final CaptivePortalProbeResult PARTIAL =
-            new CaptivePortalProbeResult(PARTIAL_CODE);
+    // The private IP logic only applies to the HTTP probe.
     public static final CaptivePortalProbeResult PRIVATE_IP =
-            new CaptivePortalProbeResult(DNS_PRIVATE_IP_RESPONSE_CODE);
+            new CaptivePortalProbeResult(DNS_PRIVATE_IP_RESPONSE_CODE, 1 << PROBE_HTTP);
+    // Partial connectivity should be concluded from both HTTP and HTTPS probes.
+    @NonNull
+    public static final CaptivePortalProbeResult PARTIAL = new CaptivePortalProbeResult(
+            PARTIAL_CODE, 1 << PROBE_HTTP | 1 << PROBE_HTTPS);
+    // Probe type that is used for unspecified probe result.
+    public static final int PROBE_UNKNOWN = 0;
 
-    private final int mHttpResponseCode;  // HTTP response code returned from Internet probe.
+    final int mHttpResponseCode;  // HTTP response code returned from Internet probe.
     @Nullable
     public final String redirectUrl;      // Redirect destination returned from Internet probe.
     @Nullable
@@ -62,21 +67,39 @@ public final class CaptivePortalProbeResult {
     @Nullable
     public final CaptivePortalProbeSpec probeSpec;
 
-    public CaptivePortalProbeResult(int httpResponseCode) {
-        this(httpResponseCode, null, null);
+    // Indicate what kind of probe this is. This is a bitmask constructed by
+    // bitwise-or-ing(i.e. {@code |}) the {@code ValidationProbeEvent.PROBE_HTTP} or
+    // {@code ValidationProbeEvent.PROBE_HTTPS} values. Set it as {@code PROBE_UNKNOWN} if the probe
+    // type is unspecified.
+    @ProbeType
+    public final int probeType;
+
+    @IntDef(value = {
+        PROBE_UNKNOWN,
+        1 << PROBE_HTTP,
+        1 << PROBE_HTTPS,
+    })
+    @Retention(RetentionPolicy.SOURCE)
+    public @interface ProbeType {
+    }
+
+    public CaptivePortalProbeResult(int httpResponseCode, @ProbeType int probeType) {
+        this(httpResponseCode, null, null, null, probeType);
     }
 
     public CaptivePortalProbeResult(int httpResponseCode, @Nullable String redirectUrl,
-            @Nullable String detectUrl) {
-        this(httpResponseCode, redirectUrl, detectUrl, null);
+            @Nullable String detectUrl, @ProbeType int probeType) {
+        this(httpResponseCode, redirectUrl, detectUrl, null, probeType);
     }
 
     public CaptivePortalProbeResult(int httpResponseCode, @Nullable String redirectUrl,
-            @Nullable String detectUrl, @Nullable CaptivePortalProbeSpec probeSpec) {
+            @Nullable String detectUrl, @Nullable CaptivePortalProbeSpec probeSpec,
+            @ProbeType int probeType) {
         mHttpResponseCode = httpResponseCode;
         this.redirectUrl = redirectUrl;
         this.detectUrl = detectUrl;
         this.probeSpec = probeSpec;
+        this.probeType = probeType;
     }
 
     public boolean isSuccessful() {
@@ -98,4 +121,40 @@ public final class CaptivePortalProbeResult {
     public boolean isDnsPrivateIpResponse() {
         return mHttpResponseCode == DNS_PRIVATE_IP_RESPONSE_CODE;
     }
+
+    /**
+     *  Make a failed {@code this} for either HTTPS or HTTP determined by {@param isHttps}.
+     *  @param probeType probe type of the result.
+     *  @return a failed {@link CaptivePortalProbeResult}
+     */
+    public static CaptivePortalProbeResult failed(@ProbeType int probeType) {
+        return new CaptivePortalProbeResult(FAILED_CODE, probeType);
+    }
+
+    /**
+     *  Make a success {@code this} for either HTTPS or HTTP determined by {@param isHttps}.
+     *  @param probeType probe type of the result.
+     *  @return a success {@link CaptivePortalProbeResult}
+     */
+    public static CaptivePortalProbeResult success(@ProbeType int probeType) {
+        return new CaptivePortalProbeResult(SUCCESS_CODE, probeType);
+    }
+
+    /**
+     * As {@code this} is the result of calling isCaptivePortal(), the result may be determined from
+     * one or more probes depending on the configuration of detection probes. Return if HTTPS probe
+     * is one of the probes that concludes it.
+     */
+    public boolean isConcludedFromHttps() {
+        return (probeType & (1 << PROBE_HTTPS)) != 0;
+    }
+
+    /**
+     * As {@code this} is the result of calling isCaptivePortal(), the result may be determined from
+     * one or more probes depending on the configuration of detection probes. Return if HTTP probe
+     * is one of the probes that concludes it.
+     */
+    public boolean isConcludedFromHttp() {
+        return (probeType & (1 << PROBE_HTTP)) != 0;
+    }
 }
diff --git a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java
index bf983a5..f58f50b 100644
--- a/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java
+++ b/common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeSpec.java
@@ -18,6 +18,7 @@ package android.net.captiveportal;
 
 import static android.net.captiveportal.CaptivePortalProbeResult.PORTAL_CODE;
 import static android.net.captiveportal.CaptivePortalProbeResult.SUCCESS_CODE;
+import static android.net.metrics.ValidationProbeEvent.PROBE_HTTP;
 
 import android.text.TextUtils;
 import android.util.Log;
@@ -40,6 +41,7 @@ public abstract class CaptivePortalProbeSpec {
     private static final String TAG = CaptivePortalProbeSpec.class.getSimpleName();
     private static final String REGEX_SEPARATOR = "@@/@@";
     private static final String SPEC_SEPARATOR = "@@,@@";
+    private static final String PROTOCOL_HTTP = "http";
 
     private final String mEncodedSpec;
     private final URL mUrl;
@@ -138,7 +140,7 @@ public abstract class CaptivePortalProbeSpec {
     }
 
     /**
-     * Get the probe result from HTTP status and location header.
+     * Get the HTTP probe result from HTTP status and location header.
      */
     @NonNull
     public abstract CaptivePortalProbeResult getResult(int status, @Nullable String locationHeader);
@@ -157,6 +159,7 @@ public abstract class CaptivePortalProbeSpec {
      * Implementation of {@link CaptivePortalProbeSpec} that is based on configurable regular
      * expressions for the HTTP status code and location header (if any). Matches indicate that
      * the page is not a portal.
+     * @throws IllegalArgumentException The protocol of the url is not http.
      * This probe cannot fail: it always returns SUCCESS_CODE or PORTAL_CODE
      */
     private static class RegexMatchProbeSpec extends CaptivePortalProbeSpec {
@@ -165,9 +168,17 @@ public abstract class CaptivePortalProbeSpec {
         @Nullable
         final Pattern mLocationHeaderRegex;
 
-        RegexMatchProbeSpec(
-                String spec, URL url, Pattern statusRegex, Pattern locationHeaderRegex) {
+        RegexMatchProbeSpec(@NonNull String spec, @NonNull URL url, @Nullable Pattern statusRegex,
+                @Nullable Pattern locationHeaderRegex) throws ParseException {
             super(spec, url);
+            final String protocol = url.getProtocol();
+            if (!PROTOCOL_HTTP.equals(protocol)) {
+                // The probe type taken in the result is hard-coded as PROBE_HTTP currently, so the
+                // probe type taken into the result of {@code getResult} have to change if other
+                // kind of probes are used.
+                throw new IllegalArgumentException("Protocol for probe spec should be http but was"
+                        + protocol);
+            }
             mStatusRegex = statusRegex;
             mLocationHeaderRegex = locationHeaderRegex;
         }
@@ -178,7 +189,7 @@ public abstract class CaptivePortalProbeSpec {
             final boolean locationMatch = safeMatch(locationHeader, mLocationHeaderRegex);
             final int returnCode = statusMatch && locationMatch ? SUCCESS_CODE : PORTAL_CODE;
             return new CaptivePortalProbeResult(
-                    returnCode, locationHeader, getUrl().toString(), this);
+                    returnCode, locationHeader, getUrl().toString(), this, PROBE_HTTP);
         }
     }
 
diff --git a/src/android/net/captiveportal/CapportApiProbeResult.java b/src/android/net/captiveportal/CapportApiProbeResult.java
new file mode 100644
index 0000000..3ef07ce
--- /dev/null
+++ b/src/android/net/captiveportal/CapportApiProbeResult.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package android.net.captiveportal;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+
+import com.android.networkstack.apishim.CaptivePortalDataShim;
+
+/**
+ * Captive portal probe detection result including capport API detection result.
+ * @hide
+ */
+public class CapportApiProbeResult extends CaptivePortalProbeResult {
+    @NonNull
+    private final CaptivePortalDataShim mCapportData;
+
+    public CapportApiProbeResult(@NonNull CaptivePortalProbeResult result,
+            @NonNull CaptivePortalDataShim capportData) {
+        this(result.mHttpResponseCode, result.redirectUrl, result.detectUrl, capportData,
+                result.probeType);
+    }
+
+    public CapportApiProbeResult(int httpResponseCode, @Nullable String redirectUrl,
+            @Nullable String detectUrl, @Nullable CaptivePortalDataShim capportData,
+            int probeType) {
+        super(httpResponseCode, redirectUrl, detectUrl, probeType);
+        mCapportData = capportData;
+    }
+
+    public CaptivePortalDataShim getCaptivePortalData() {
+        return mCapportData;
+    }
+}
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index eed63e6..40ad87d 100755
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -103,6 +103,7 @@ import android.net.NetworkCapabilities;
 import android.net.ProxyInfo;
 import android.net.TrafficStats;
 import android.net.Uri;
+import android.net.captiveportal.CapportApiProbeResult;
 import android.net.captiveportal.CaptivePortalProbeResult;
 import android.net.captiveportal.CaptivePortalProbeSpec;
 import android.net.metrics.IpConnectivityLog;
@@ -406,7 +407,8 @@ public class NetworkMonitor extends StateMachine {
     private final Stopwatch mEvaluationTimer = new Stopwatch();
 
     // This variable is set before transitioning to the mCaptivePortalState.
-    private CaptivePortalProbeResult mLastPortalProbeResult = CaptivePortalProbeResult.FAILED;
+    private CaptivePortalProbeResult mLastPortalProbeResult =
+            CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN);
 
     // Random generator to select fallback URL index
     private final Random mRandom;
@@ -1840,7 +1842,7 @@ public class NetworkMonitor extends StateMachine {
     protected CaptivePortalProbeResult isCaptivePortal() {
         if (!mIsCaptivePortalCheckEnabled) {
             validationLog("Validation disabled.");
-            return CaptivePortalProbeResult.SUCCESS;
+            return CaptivePortalProbeResult.success(CaptivePortalProbeResult.PROBE_UNKNOWN);
         }
 
         URL pacUrl = null;
@@ -1868,13 +1870,13 @@ public class NetworkMonitor extends StateMachine {
         if (proxyInfo != null && !Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) {
             pacUrl = makeURL(proxyInfo.getPacFileUrl().toString());
             if (pacUrl == null) {
-                return CaptivePortalProbeResult.FAILED;
+                return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN);
             }
         }
 
         if ((pacUrl == null) && (httpUrls.length == 0 || httpsUrls.length == 0
                 || httpUrls[0] == null || httpsUrls[0] == null)) {
-            return CaptivePortalProbeResult.FAILED;
+            return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN);
         }
 
         long startTime = SystemClock.elapsedRealtime();
@@ -2073,7 +2075,8 @@ public class NetworkMonitor extends StateMachine {
         logValidationProbe(probeTimer.stop(), probeType, httpResponseCode);
 
         if (probeSpec == null) {
-            return new CaptivePortalProbeResult(httpResponseCode, redirectUrl, url.toString());
+            return new CaptivePortalProbeResult(httpResponseCode, redirectUrl, url.toString(),
+                    1 << probeType);
         } else {
             return probeSpec.getResult(httpResponseCode, redirectUrl);
         }
@@ -2162,33 +2165,29 @@ public class NetworkMonitor extends StateMachine {
         }
     }
 
-    private abstract static class ProbeThread extends Thread {
+    private class ProbeThread extends Thread {
         private final CountDownLatch mLatch;
-        private final ProxyInfo mProxy;
-        private final URL mUrl;
-        protected final Uri mCaptivePortalApiUrl;
+        private final Probe mProbe;
 
-        protected ProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url,
+        ProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, int probeType,
                 Uri captivePortalApiUrl) {
             mLatch = latch;
-            mProxy = proxy;
-            mUrl = url;
-            mCaptivePortalApiUrl = captivePortalApiUrl;
+            mProbe = (probeType == ValidationProbeEvent.PROBE_HTTPS)
+                    ? new HttpsProbe(proxy, url, captivePortalApiUrl)
+                    : new HttpProbe(proxy, url, captivePortalApiUrl);
+            mResult = CaptivePortalProbeResult.failed(probeType);
         }
 
-        private volatile CaptivePortalProbeResult mResult = CaptivePortalProbeResult.FAILED;
+        private volatile CaptivePortalProbeResult mResult;
 
         public CaptivePortalProbeResult result() {
             return mResult;
         }
 
-        protected abstract CaptivePortalProbeResult sendProbe(ProxyInfo proxy, URL url);
-        public abstract boolean isConclusiveResult(CaptivePortalProbeResult result);
-
         @Override
         public void run() {
-            mResult = sendProbe(mProxy, mUrl);
-            if (isConclusiveResult(mResult)) {
+            mResult = mProbe.sendProbe();
+            if (isConclusiveResult(mResult, mProbe.mCaptivePortalApiUrl)) {
                 // Stop waiting immediately if any probe is conclusive.
                 while (mLatch.getCount() > 0) {
                     mLatch.countDown();
@@ -2199,36 +2198,34 @@ public class NetworkMonitor extends StateMachine {
         }
     }
 
-    final class HttpsProbeThread extends ProbeThread {
-        HttpsProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
-            super(latch, proxy, url, captivePortalApiUrl);
+    private abstract static class Probe {
+        protected final ProxyInfo mProxy;
+        protected final URL mUrl;
+        protected final Uri mCaptivePortalApiUrl;
+
+        protected Probe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
+            mProxy = proxy;
+            mUrl = url;
+            mCaptivePortalApiUrl = captivePortalApiUrl;
         }
+        // sendProbe() is synchronous and blocks until it has the result.
+        protected abstract CaptivePortalProbeResult sendProbe();
+    }
 
-        @Override
-        protected CaptivePortalProbeResult sendProbe(ProxyInfo proxy, URL url) {
-            return sendDnsAndHttpProbes(proxy, url, ValidationProbeEvent.PROBE_HTTPS);
+    final class HttpsProbe extends Probe {
+        HttpsProbe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
+            super(proxy, url, captivePortalApiUrl);
         }
 
         @Override
-        public boolean isConclusiveResult(CaptivePortalProbeResult result) {
-            // isPortal() is not expected on the HTTPS probe, but check it nonetheless.
-            // In case the capport API is available, the API is authoritative on whether there is
-            // a portal, so the HTTPS probe is not enough to conclude there is connectivity,
-            // and a determination will be made once the capport API probe returns. Note that the
-            // API can only force the system to detect a portal even if the HTTPS probe succeeds.
-            // It cannot force the system to detect no portal if the HTTPS probe fails.
-            return (result.isPortal() || result.isSuccessful()) && mCaptivePortalApiUrl == null;
+        protected CaptivePortalProbeResult sendProbe() {
+            return sendDnsAndHttpProbes(mProxy, mUrl, ValidationProbeEvent.PROBE_HTTPS);
         }
     }
 
-    final class HttpProbeThread extends ProbeThread {
-        private volatile CaptivePortalDataShim mCapportData;
-        HttpProbeThread(CountDownLatch latch, ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
-            super(latch, proxy, url, captivePortalApiUrl);
-        }
-
-        CaptivePortalDataShim getCaptivePortalData() {
-            return mCapportData;
+    final class HttpProbe extends Probe {
+        HttpProbe(ProxyInfo proxy, URL url, Uri captivePortalApiUrl) {
+            super(proxy, url, captivePortalApiUrl);
         }
 
         private CaptivePortalDataShim tryCapportApiProbe() {
@@ -2277,33 +2274,61 @@ public class NetworkMonitor extends StateMachine {
         }
 
         @Override
-        protected CaptivePortalProbeResult sendProbe(ProxyInfo proxy, URL url) {
-            mCapportData = tryCapportApiProbe();
-            if (mCapportData != null && mCapportData.isCaptive()) {
-                if (mCapportData.getUserPortalUrl() == null) {
+        protected CaptivePortalProbeResult sendProbe() {
+            final CaptivePortalDataShim capportData = tryCapportApiProbe();
+            if (capportData != null && capportData.isCaptive()) {
+                if (capportData.getUserPortalUrl() == null) {
                     validationLog("Missing user-portal-url from capport response");
-                    return sendDnsAndHttpProbes(proxy, url, ValidationProbeEvent.PROBE_HTTP);
+                    return sendDnsAndHttpProbes(mProxy, mUrl, ValidationProbeEvent.PROBE_HTTP);
                 }
-                final String loginUrlString = mCapportData.getUserPortalUrl().toString();
+                final String loginUrlString = capportData.getUserPortalUrl().toString();
                 // Starting from R (where CaptivePortalData was introduced), the captive portal app
                 // delegates to NetworkMonitor for verifying when the network validates instead of
                 // probing the detectUrl. So pass the detectUrl to have the portal open on that,
                 // page; CaptivePortalLogin will not use it for probing.
-                return new CaptivePortalProbeResult(
+                return new CapportApiProbeResult(
                         CaptivePortalProbeResult.PORTAL_CODE,
                         loginUrlString /* redirectUrl */,
-                        loginUrlString /* detectUrl */);
+                        loginUrlString /* detectUrl */,
+                        capportData,
+                        1 << ValidationProbeEvent.PROBE_HTTP);
             }
 
             // If the API says it's not captive, still check for HTTP connectivity. This helps
             // with partial connectivity detection, and a broken API saying that there is no
             // redirect when there is one.
-            return sendDnsAndHttpProbes(proxy, url, ValidationProbeEvent.PROBE_HTTP);
+            final CaptivePortalProbeResult res =
+                    sendDnsAndHttpProbes(mProxy, mUrl, ValidationProbeEvent.PROBE_HTTP);
+            return capportData == null ? res : new CapportApiProbeResult(res, capportData);
         }
+    }
 
-        @Override
-        public boolean isConclusiveResult(CaptivePortalProbeResult result) {
-            return result.isPortal();
+    private static boolean isConclusiveResult(@NonNull CaptivePortalProbeResult result,
+            @Nullable Uri captivePortalApiUrl) {
+        // isPortal() is not expected on the HTTPS probe, but treat the network as portal would make
+        // sense if the probe reports portal. In case the capport API is available, the API is
+        // authoritative on whether there is a portal, so the HTTPS probe is not enough to conclude
+        // there is connectivity, and a determination will be made once the capport API probe
+        // returns. Note that the API can only force the system to detect a portal even if the HTTPS
+        // probe succeeds. It cannot force the system to detect no portal if the HTTPS probe fails.
+        return result.isPortal()
+                || (result.isConcludedFromHttps() && result.isSuccessful()
+                        && captivePortalApiUrl == null);
+    }
+
+    private void reportProbeResult(@NonNull CaptivePortalProbeResult res) {
+        if (res instanceof CapportApiProbeResult) {
+            maybeReportCaptivePortalData(((CapportApiProbeResult) res).getCaptivePortalData());
+        }
+
+        // This is not a if-else case since partial connectivity will concluded from both HTTP and
+        // HTTPS probe. Both HTTP and HTTPS result should be reported.
+        if (res.isConcludedFromHttps()) {
+            reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTPS, res);
+        }
+
+        if (res.isConcludedFromHttp()) {
+            reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, res);
         }
     }
 
@@ -2314,9 +2339,10 @@ public class NetworkMonitor extends StateMachine {
         final CountDownLatch latch = new CountDownLatch(2);
 
         final Uri capportApiUrl = getCaptivePortalApiUrl(mLinkProperties);
-        final HttpsProbeThread httpsProbe = new HttpsProbeThread(latch, proxy, httpsUrl,
-                capportApiUrl);
-        final HttpProbeThread httpProbe = new HttpProbeThread(latch, proxy, httpUrl, capportApiUrl);
+        final ProbeThread httpsProbe = new ProbeThread(latch, proxy, httpsUrl,
+                ValidationProbeEvent.PROBE_HTTPS, capportApiUrl);
+        final ProbeThread httpProbe = new ProbeThread(latch, proxy, httpUrl,
+                ValidationProbeEvent.PROBE_HTTP, capportApiUrl);
 
         try {
             httpsProbe.start();
@@ -2324,22 +2350,20 @@ public class NetworkMonitor extends StateMachine {
             latch.await(PROBE_TIMEOUT_MS, TimeUnit.MILLISECONDS);
         } catch (InterruptedException e) {
             validationLog("Error: probes wait interrupted!");
-            return CaptivePortalProbeResult.FAILED;
+            return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN);
         }
 
         final CaptivePortalProbeResult httpsResult = httpsProbe.result();
         final CaptivePortalProbeResult httpResult = httpProbe.result();
 
         // Look for a conclusive probe result first.
-        if (httpProbe.isConclusiveResult(httpResult)) {
-            maybeReportCaptivePortalData(httpProbe.getCaptivePortalData());
-            reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, httpResult);
+        if (isConclusiveResult(httpResult, capportApiUrl)) {
+            reportProbeResult(httpProbe.result());
             return httpResult;
         }
 
-        if (httpsProbe.isConclusiveResult(httpsResult)) {
-            maybeReportCaptivePortalData(httpProbe.getCaptivePortalData());
-            reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTPS, httpsResult);
+        if (isConclusiveResult(httpsResult, capportApiUrl)) {
+            reportProbeResult(httpsProbe.result());
             return httpsResult;
         }
         // Consider a DNS response with a private IP address on the HTTP probe as an indication that
@@ -2350,7 +2374,7 @@ public class NetworkMonitor extends StateMachine {
         // probe should not be delayed by this check.
         if (mPrivateIpNoInternetEnabled && (httpResult.isDnsPrivateIpResponse())) {
             validationLog("DNS response to the URL is private IP");
-            return CaptivePortalProbeResult.FAILED;
+            return CaptivePortalProbeResult.failed(1 << ValidationProbeEvent.PROBE_HTTP);
         }
         // If a fallback method exists, use it to retry portal detection.
         // If we have new-style probe specs, use those. Otherwise, use the fallback URLs.
@@ -2367,8 +2391,7 @@ public class NetworkMonitor extends StateMachine {
         // Otherwise wait until http and https probes completes and use their results.
         try {
             httpProbe.join();
-            maybeReportCaptivePortalData(httpProbe.getCaptivePortalData());
-            reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, httpProbe.result());
+            reportProbeResult(httpProbe.result());
 
             if (httpProbe.result().isPortal()) {
                 return httpProbe.result();
@@ -2386,7 +2409,7 @@ public class NetworkMonitor extends StateMachine {
             return httpsProbe.result();
         } catch (InterruptedException e) {
             validationLog("Error: http or https probe wait interrupted!");
-            return CaptivePortalProbeResult.FAILED;
+            return CaptivePortalProbeResult.failed(CaptivePortalProbeResult.PROBE_UNKNOWN);
         }
     }
 
diff --git a/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt b/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt
index 3820165..4c62071 100644
--- a/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt
+++ b/tests/unit/src/com/android/networkstack/metrics/DataStallStatsUtilsTest.kt
@@ -23,6 +23,7 @@ import com.android.server.connectivity.nano.DataStallEventProto
 import org.junit.Assert.assertEquals
 import org.junit.Test
 import org.junit.runner.RunWith
+import android.net.metrics.ValidationProbeEvent
 
 @RunWith(AndroidJUnit4::class)
 @SmallTest
@@ -30,14 +31,26 @@ class DataStallStatsUtilsTest {
     @Test
     fun testProbeResultToEnum() {
         assertEquals(DataStallStatsUtils.probeResultToEnum(null), DataStallEventProto.INVALID)
-        assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult.FAILED),
+        // Metrics cares only http response code.
+        assertEquals(DataStallStatsUtils.probeResultToEnum(
+                CaptivePortalProbeResult.failed(ValidationProbeEvent.PROBE_HTTP)),
+                DataStallEventProto.INVALID)
+        assertEquals(DataStallStatsUtils.probeResultToEnum(
+                CaptivePortalProbeResult.failed(ValidationProbeEvent.PROBE_HTTPS)),
                 DataStallEventProto.INVALID)
-        assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult.SUCCESS),
+        assertEquals(DataStallStatsUtils.probeResultToEnum(
+                CaptivePortalProbeResult.success(ValidationProbeEvent.PROBE_HTTP)),
+                DataStallEventProto.VALID)
+        assertEquals(DataStallStatsUtils.probeResultToEnum(
+                CaptivePortalProbeResult.success(ValidationProbeEvent.PROBE_HTTP)),
                 DataStallEventProto.VALID)
         assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult.PARTIAL),
                 DataStallEventProto.PARTIAL)
-        assertEquals(DataStallStatsUtils.probeResultToEnum(
-                CaptivePortalProbeResult(CaptivePortalProbeResult.PORTAL_CODE)),
+        assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult(
+                CaptivePortalProbeResult.PORTAL_CODE, ValidationProbeEvent.PROBE_HTTP)),
+                DataStallEventProto.PORTAL)
+        assertEquals(DataStallStatsUtils.probeResultToEnum(CaptivePortalProbeResult(
+                CaptivePortalProbeResult.PORTAL_CODE, ValidationProbeEvent.PROBE_HTTPS)),
                 DataStallEventProto.PORTAL)
     }
 }
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index 6a0aa8a..64c106d 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -27,6 +27,7 @@ import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_PRIVDNS;
 import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL;
 import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
+import static android.net.metrics.ValidationProbeEvent.PROBE_HTTP;
 import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_CONSECUTIVE_DNS_TIMEOUT_THRESHOLD;
 import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_EVALUATION_TYPE;
 import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_MIN_EVALUATE_INTERVAL;
@@ -1731,13 +1732,15 @@ public class NetworkMonitorTest {
 
         resetCallbacks();
 
-        nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.SUCCESS);
+        nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP,
+                CaptivePortalProbeResult.success(PROBE_HTTP));
         // Verify result should be appended and notifyNetworkTestedWithExtras callback is triggered
         // once.
         assertEquals(nm.getEvaluationState().getNetworkTestResult(),
                 VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP);
 
-        nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.FAILED);
+        nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP,
+                CaptivePortalProbeResult.failed(PROBE_HTTP));
         // Verify DNS probe result should not be cleared.
         assertTrue((nm.getEvaluationState().getNetworkTestResult() & NETWORK_VALIDATION_PROBE_DNS)
                 == NETWORK_VALIDATION_PROBE_DNS);