Define callback for suspected data stall.

Currently, NetworkStack does not notify ConnectivityService explicitly
when it detects a potential data stall. This adds a new method
notifyDataStallSuspected() to INetworkMonitorCallbacks for notifying
ConnectivityService.

ConstantsShim is defined for using constants from the Android platform
that are introduced after the NetworkStack mainline module was defined.

Bug: 143187964
Test: compiles
Test: atest NetworkStackTests
Change-Id: Ie3b5b4e71e4a4b15a5a84f761eaead3acf6fc807
Merged-In: Ie3b5b4e71e4a4b15a5a84f761eaead3acf6fc807

5 files changed, 96 insertions, 5 deletions

diff --git a/apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java b/apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java
new file mode 100644
index 0000000..fe2691e
--- /dev/null
+++ b/apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java
@@ -0,0 +1,26 @@
+/*
+ * 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 com.android.networkstack.apishim.api29;
+
+/**
+ * Utility class for defining and importing constants from the Android platform.
+ */
+public class ConstantsShim {
+    // Constants defined in android.net.ConnectivityDiagnosticsManager.DataStallReport.
+    public static final int DETECTION_METHOD_DNS_EVENTS = 1;
+    public static final int DETECTION_METHOD_TCP_METRICS = 2;
+}
diff --git a/apishim/30/com/android/networkstack/apishim/ConstantsShim.java b/apishim/30/com/android/networkstack/apishim/ConstantsShim.java
new file mode 100644
index 0000000..77e7be2
--- /dev/null
+++ b/apishim/30/com/android/networkstack/apishim/ConstantsShim.java
@@ -0,0 +1,29 @@
+/*
+ * 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 com.android.networkstack.apishim;
+
+import static android.net.ConnectivityDiagnosticsManager.DataStallReport;
+
+/**
+ * Utility class for defining and importing constants from the Android platform.
+ */
+public class ConstantsShim extends com.android.networkstack.apishim.api29.ConstantsShim {
+    public static final int DETECTION_METHOD_DNS_EVENTS =
+            DataStallReport.DETECTION_METHOD_DNS_EVENTS;
+    public static final int DETECTION_METHOD_TCP_METRICS =
+            DataStallReport.DETECTION_METHOD_TCP_METRICS;
+}
diff --git a/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl b/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl
index 62c3f29..e2901dd 100644
--- a/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl
+++ b/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl
@@ -31,4 +31,6 @@ oneway interface INetworkMonitorCallbacks {
     void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded);
     void notifyNetworkTestedWithExtras(int testResult, @nullable String redirectUrl,
             long timestampMillis, in PersistableBundle extras);
+    void notifyDataStallSuspected(long timestampMillis, int detectionMethod,
+            in PersistableBundle extras);
 }
\ No newline at end of file
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index a4f934f..3a3f95c 100644
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -68,6 +68,8 @@ import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS;
 import static android.net.util.NetworkStackUtils.isEmpty;
 import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;
 
+import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;
+import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_TCP_METRICS;
 import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX;
 import static com.android.networkstack.util.DnsUtils.TYPE_ADDRCONFIG;
 
@@ -103,6 +105,7 @@ import android.net.wifi.WifiManager;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.Message;
+import android.os.PersistableBundle;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.SystemClock;
@@ -613,6 +616,15 @@ public class NetworkMonitor extends StateMachine {
         }
     }
 
+    private void notifyDataStallSuspected(int detectionMethod, PersistableBundle extras) {
+        try {
+            mCallback.notifyDataStallSuspected(
+                    SystemClock.elapsedRealtime(), detectionMethod, extras);
+        } catch (RemoteException e) {
+            Log.e(TAG, "Error sending notification for suspected data stall", e);
+        }
+    }
+
     // DefaultState is the parent of all States.  It exists only to handle CMD_* messages but
     // does not entail any real state (hence no enter() or exit() routines).
     private class DefaultState extends State {
@@ -2275,6 +2287,9 @@ public class NetworkMonitor extends StateMachine {
                 result = false;
             } else if (tst.isDataStallSuspected()) {
                 result = true;
+
+                // TODO(b/147249364): add metrics to PersistableBundle once keys are defined
+                notifyDataStallSuspected(DETECTION_METHOD_TCP_METRICS, PersistableBundle.EMPTY);
             }
             if (DBG || VDBG_STALL) {
                 msg.add("tcp packets received=" + tst.getLatestReceivedCount())
@@ -2292,6 +2307,9 @@ public class NetworkMonitor extends StateMachine {
                     mDataStallValidDnsTimeThreshold)) {
                 result = true;
                 logNetworkEvent(NetworkEvent.NETWORK_CONSECUTIVE_DNS_TIMEOUT_FOUND);
+
+                // TODO(b/147249364): add metrics to PersistableBundle once keys are defined
+                notifyDataStallSuspected(DETECTION_METHOD_DNS_EVENTS, PersistableBundle.EMPTY);
             }
             if (DBG || VDBG_STALL) {
                 msg.add("consecutive dns timeout count=" + dsd.getConsecutiveTimeoutCount());
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index 469284b..4383347 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -38,6 +38,8 @@ import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_FALLBACK_PROBE_S
 import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS;
 import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS;
 
+import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;
+import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_TCP_METRICS;
 import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX;
 
 import static junit.framework.Assert.assertEquals;
@@ -48,6 +50,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
@@ -88,6 +91,7 @@ import android.os.Bundle;
 import android.os.ConditionVariable;
 import android.os.Handler;
 import android.os.Looper;
+import android.os.PersistableBundle;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.SystemClock;
@@ -771,15 +775,17 @@ public class NetworkMonitorTest {
     }
 
     @Test
-    public void testIsDataStall_EvaluationDnsOnNotMeteredNetwork() {
+    public void testIsDataStall_EvaluationDnsOnNotMeteredNetwork() throws Exception {
         WrappedNetworkMonitor wrappedMonitor = makeNotMeteredNetworkMonitor();
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 100);
         makeDnsTimeoutEvent(wrappedMonitor, DEFAULT_DNS_TIMEOUT_THRESHOLD);
         assertTrue(wrappedMonitor.isDataStall());
+        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),
+                any(PersistableBundle.class));
     }
 
     @Test
-    public void testIsDataStall_EvaluationDnsOnMeteredNetwork() {
+    public void testIsDataStall_EvaluationDnsOnMeteredNetwork() throws Exception {
         WrappedNetworkMonitor wrappedMonitor = makeMeteredNetworkMonitor();
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 100);
         assertFalse(wrappedMonitor.isDataStall());
@@ -787,10 +793,12 @@ public class NetworkMonitorTest {
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);
         makeDnsTimeoutEvent(wrappedMonitor, DEFAULT_DNS_TIMEOUT_THRESHOLD);
         assertTrue(wrappedMonitor.isDataStall());
+        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),
+                any(PersistableBundle.class));
     }
 
     @Test
-    public void testIsDataStall_EvaluationDnsWithDnsTimeoutCount() {
+    public void testIsDataStall_EvaluationDnsWithDnsTimeoutCount() throws Exception {
         WrappedNetworkMonitor wrappedMonitor = makeMeteredNetworkMonitor();
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);
         makeDnsTimeoutEvent(wrappedMonitor, 3);
@@ -802,6 +810,8 @@ public class NetworkMonitorTest {
 
         makeDnsTimeoutEvent(wrappedMonitor, 3);
         assertTrue(wrappedMonitor.isDataStall());
+        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),
+                any(PersistableBundle.class));
 
         // Set the value to larger than the default dns log size.
         setConsecutiveDnsTimeoutThreshold(51);
@@ -812,6 +822,8 @@ public class NetworkMonitorTest {
 
         makeDnsTimeoutEvent(wrappedMonitor, 1);
         assertTrue(wrappedMonitor.isDataStall());
+        verify(mCallbacks, times(2)).notifyDataStallSuspected(anyLong(),
+                eq(DETECTION_METHOD_DNS_EVENTS), any(PersistableBundle.class));
     }
 
     @Test
@@ -828,7 +840,7 @@ public class NetworkMonitorTest {
     }
 
     @Test
-    public void testIsDataStall_EvaluationDnsWithDnsTimeThreshold() {
+    public void testIsDataStall_EvaluationDnsWithDnsTimeThreshold() throws Exception {
         // Test dns events happened in valid dns time threshold.
         WrappedNetworkMonitor wrappedMonitor = makeMeteredNetworkMonitor();
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 100);
@@ -836,6 +848,8 @@ public class NetworkMonitorTest {
         assertFalse(wrappedMonitor.isDataStall());
         wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);
         assertTrue(wrappedMonitor.isDataStall());
+        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),
+                any(PersistableBundle.class));
 
         // Test dns events happened before valid dns time threshold.
         setValidDataStallDnsTimeThreshold(0);
@@ -848,7 +862,7 @@ public class NetworkMonitorTest {
     }
 
     @Test
-    public void testIsDataStall_EvaluationTcp() {
+    public void testIsDataStall_EvaluationTcp() throws Exception {
         // Evaluate TCP only. Expect ignoring DNS signal.
         setDataStallEvaluationType(DATA_STALL_EVALUATION_TYPE_TCP);
         WrappedNetworkMonitor wrappedMonitor = makeMonitor(METERED_CAPABILITIES);
@@ -869,6 +883,8 @@ public class NetworkMonitorTest {
         wrappedMonitor.sendTcpPollingEvent();
         HandlerUtilsKt.waitForIdle(wrappedMonitor.getHandler(), HANDLER_TIMEOUT_MS);
         assertTrue(wrappedMonitor.isDataStall());
+        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_TCP_METRICS),
+                any(PersistableBundle.class));
     }
 
     @Test