diff options
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 |