2 files changed, 47 insertions, 10 deletions

diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index fcdd507..d16fcf5 100644
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -355,7 +355,6 @@ public class NetworkMonitor extends StateMachine {
     private final NetworkStackNotifier mNotifier;
     private final IpConnectivityLog mMetricsLog;
     private final Dependencies mDependencies;
-    private final DataStallStatsUtils mDetectionStatsUtils;
     private final TcpSocketTracker mTcpTracker;
     // Configuration values for captive portal detection probes.
     private final String mCaptivePortalUserAgent;
@@ -438,15 +437,14 @@ public class NetworkMonitor extends StateMachine {
     public NetworkMonitor(Context context, INetworkMonitorCallbacks cb, Network network,
             SharedLog validationLog, @NonNull NetworkStackServiceManager serviceManager) {
         this(context, cb, network, new IpConnectivityLog(), validationLog, serviceManager,
-                Dependencies.DEFAULT, new DataStallStatsUtils(),
-                getTcpSocketTrackerOrNull(context, network));
+                Dependencies.DEFAULT, getTcpSocketTrackerOrNull(context, network));
     }
 
     @VisibleForTesting
     public NetworkMonitor(Context context, INetworkMonitorCallbacks cb, Network network,
             IpConnectivityLog logger, SharedLog validationLogs,
             @NonNull NetworkStackServiceManager serviceManager, Dependencies deps,
-            DataStallStatsUtils detectionStatsUtils, @Nullable TcpSocketTracker tst) {
+            @Nullable TcpSocketTracker tst) {
         // Add suffix indicating which NetworkMonitor we're talking about.
         super(TAG + "/" + network.toString());
 
@@ -460,7 +458,6 @@ public class NetworkMonitor extends StateMachine {
         mCallback = cb;
         mCallbackVersion = getCallbackVersion(cb);
         mDependencies = deps;
-        mDetectionStatsUtils = detectionStatsUtils;
         mNetwork = network;
         mCleartextDnsNetwork = deps.getPrivateDnsBypassNetwork(network);
         mTelephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE);
@@ -897,7 +894,8 @@ public class NetworkMonitor extends StateMachine {
         final int[] transports = mNetworkCapabilities.getTransportTypes();
 
         for (int i = 0; i < transports.length; i++) {
-            DataStallStatsUtils.write(buildDataStallDetectionStats(transports[i]), result);
+            final DataStallDetectionStats stats = buildDataStallDetectionStats(transports[i]);
+            mDependencies.writeDataStallDetectionStats(stats, result);
         }
         mCollectDataStallMetrics = false;
     }
@@ -2419,6 +2417,18 @@ public class NetworkMonitor extends StateMachine {
             return NetworkStackUtils.isFeatureEnabled(context, namespace, name, defaultEnabled);
         }
 
+        /**
+         * Collect data stall detection level information for each transport type. Write metrics
+         * data to statsd pipeline.
+         * @param stats a {@link DataStallDetectionStats} that contains the detection level
+         *              information.
+         * @para result the network reevaluation result.
+         */
+        public void writeDataStallDetectionStats(@NonNull final DataStallDetectionStats stats,
+                @NonNull final CaptivePortalProbeResult result) {
+            DataStallStatsUtils.write(stats, result);
+        }
+
         public static final Dependencies DEFAULT = new Dependencies();
     }
 
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index bbedf59..ab1ebd6 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -524,7 +524,7 @@ public class NetworkMonitorTest {
 
         WrappedNetworkMonitor() {
             super(mContext, mCallbacks, mNetwork, mLogger, mValidationLogger, mServiceManager,
-                    mDependencies, mDataStallStatsUtils, mTst);
+                    mDependencies, mTst);
         }
 
         @Override
@@ -1332,21 +1332,48 @@ public class NetworkMonitorTest {
     }
 
     @Test
-    public void testDataStall_StallSuspectedAndSendMetrics() throws IOException {
+    public void testDataStall_StallDnsSuspectedAndSendMetrics() throws IOException {
+        // Connect a VALID network to simulate the data stall detection because data stall
+        // evaluation will only start from validated state.
+        setStatus(mHttpsConnection, 204);
         WrappedNetworkMonitor wrappedMonitor = makeNotMeteredNetworkMonitor();
+        wrappedMonitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, METERED_CAPABILITIES);
+        verifyNetworkTested(VALIDATION_RESULT_VALID);
+        // Setup dns data stall signal.
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);
         makeDnsTimeoutEvent(wrappedMonitor, 5);
         assertTrue(wrappedMonitor.isDataStall());
-        verify(mDataStallStatsUtils, times(1)).write(makeEmptyDataStallDetectionStats(), any());
+        // Trigger a dns signal to start evaluate data stall and upload metrics.
+        wrappedMonitor.notifyDnsResponse(RETURN_CODE_DNS_TIMEOUT);
+        // Setup information to prevent null data and cause NPE during testing.
+        when(mTelephony.getDataNetworkType()).thenReturn(TelephonyManager.NETWORK_TYPE_LTE);
+        when(mTelephony.getNetworkOperator()).thenReturn(TEST_MCCMNC);
+        when(mTelephony.getSimOperator()).thenReturn(TEST_MCCMNC);
+        // Verify data sent as expectation.
+        final DataStallDetectionStats stats = wrappedMonitor.buildDataStallDetectionStats(
+                NetworkCapabilities.TRANSPORT_CELLULAR);
+        final ArgumentCaptor<CaptivePortalProbeResult> probeResultCaptor =
+                ArgumentCaptor.forClass(CaptivePortalProbeResult.class);
+        verify(mDependencies, timeout(HANDLER_TIMEOUT_MS).times(1))
+                .writeDataStallDetectionStats(eq(stats), probeResultCaptor.capture());
+        assertTrue(probeResultCaptor.getValue().isSuccessful());
     }
 
     @Test
     public void testDataStall_NoStallSuspectedAndSendMetrics() throws IOException {
+        // Connect a VALID network to simulate the data stall detection because data stall
+        // evaluation will only start from validated state.
+        setStatus(mHttpsConnection, 204);
         WrappedNetworkMonitor wrappedMonitor = makeNotMeteredNetworkMonitor();
+        wrappedMonitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, METERED_CAPABILITIES);
+        verifyNetworkTested(VALIDATION_RESULT_VALID);
+        // Setup no data stall dns signal.
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);
         makeDnsTimeoutEvent(wrappedMonitor, 3);
         assertFalse(wrappedMonitor.isDataStall());
-        verify(mDataStallStatsUtils, never()).write(makeEmptyDataStallDetectionStats(), any());
+        // Trigger a dns signal to start evaluate data stall.
+        wrappedMonitor.notifyDnsResponse(RETURN_CODE_DNS_SUCCESS);
+        verify(mDependencies, never()).writeDataStallDetectionStats(any(), any());
     }
 
     @Test