diff options
| author | Remi NGUYEN VAN <reminv@google.com> | 2020-04-24 12:19:37 +0000 |
|---|---|---|
| committer | Remi NGUYEN VAN <reminv@google.com> | 2020-04-28 08:10:04 +0000 |
| commit | 812baf3e3a5bd0a7cfdaf4b96c8ae190a36b7d30 (patch) | |
| tree | 1a96fb09ca1564271d15cbb9130a372aa8b46251 | |
| parent | 90b9ebe5f6cf77435373ebab0e14770a8bcc0d4d (diff) | |
Address comments on NetworkStack AIDL v6
Address issues found during AIDL review:
- Rename clientAddr to singleClientAddr
- Do not use a ParcelableBundle for notifyNetworkTested or
notifyDataStallSuspected; instead use AIDL parcelables for stronger
backwards compatibility guarantees.
As part of moving notifyNetworkTested to using a parcelable the test
result int is split into two: the actual evaluation result, and the
probesSucceeded int. It used to contain both as a bit mask, which does
not make sense if probesAttempted is in a separate int itself.
Test: atest NetworkMonitorTest ConnectivityServiceTest
ConnectivityServiceIntegrationTest, manual
Bug: 153500847
Merged-In: I4aac6ff7432472f8a9345fb5785c6314ec8946e4
Change-Id: I4aac6ff7432472f8a9345fb5785c6314ec8946e4
16 files changed, 458 insertions, 323 deletions
diff --git a/apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java b/apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java index eb914d6..b655858 100644 --- a/apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java +++ b/apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java @@ -34,11 +34,4 @@ public class ConstantsShim { // Constants defined in android.net.ConnectivityDiagnosticsManager. public static final int DETECTION_METHOD_DNS_EVENTS = 1; public static final int DETECTION_METHOD_TCP_METRICS = 2; - public static final String KEY_DNS_CONSECUTIVE_TIMEOUTS = "dnsConsecutiveTimeouts"; - public static final String KEY_NETWORK_PROBES_ATTEMPTED_BITMASK = "networkProbesAttempted"; - public static final String KEY_NETWORK_PROBES_SUCCEEDED_BITMASK = "networkProbesSucceeded"; - public static final String KEY_NETWORK_VALIDATION_RESULT = "networkValidationResult"; - public static final String KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS = - "tcpMetricsCollectionPeriodMillis"; - public static final String KEY_TCP_PACKET_FAIL_RATE = "tcpPacketFailRate"; } diff --git a/apishim/30/com/android/networkstack/apishim/ConstantsShim.java b/apishim/30/com/android/networkstack/apishim/ConstantsShim.java index 38a1640..cbbb3a7 100644 --- a/apishim/30/com/android/networkstack/apishim/ConstantsShim.java +++ b/apishim/30/com/android/networkstack/apishim/ConstantsShim.java @@ -16,7 +16,6 @@ package com.android.networkstack.apishim; -import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import static android.net.ConnectivityDiagnosticsManager.DataStallReport; import androidx.annotation.VisibleForTesting; @@ -38,15 +37,4 @@ public class ConstantsShim extends com.android.networkstack.apishim.api29.Consta DataStallReport.DETECTION_METHOD_DNS_EVENTS; public static final int DETECTION_METHOD_TCP_METRICS = DataStallReport.DETECTION_METHOD_TCP_METRICS; - public static final String KEY_DNS_CONSECUTIVE_TIMEOUTS = - DataStallReport.KEY_DNS_CONSECUTIVE_TIMEOUTS; - public static final String KEY_NETWORK_PROBES_ATTEMPTED_BITMASK = - ConnectivityReport.KEY_NETWORK_PROBES_ATTEMPTED_BITMASK; - public static final String KEY_NETWORK_PROBES_SUCCEEDED_BITMASK = - ConnectivityReport.KEY_NETWORK_PROBES_SUCCEEDED_BITMASK; - public static final String KEY_NETWORK_VALIDATION_RESULT = - ConnectivityReport.KEY_NETWORK_VALIDATION_RESULT; - public static final String KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS = - DataStallReport.KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS; - public static final String KEY_TCP_PACKET_FAIL_RATE = DataStallReport.KEY_TCP_PACKET_FAIL_RATE; } diff --git a/common/networkstackclient/Android.bp b/common/networkstackclient/Android.bp index 7e69d7a..93d9ca8 100644 --- a/common/networkstackclient/Android.bp +++ b/common/networkstackclient/Android.bp @@ -58,6 +58,7 @@ aidl_interface { "frameworks/base/wifi/aidl-export", // For wifi parcelables. ], srcs: [ + "src/android/net/DataStallReportParcelable.aidl", "src/android/net/DhcpResultsParcelable.aidl", "src/android/net/INetworkMonitor.aidl", "src/android/net/INetworkMonitorCallbacks.aidl", @@ -68,6 +69,7 @@ aidl_interface { "src/android/net/Layer2InformationParcelable.aidl", "src/android/net/Layer2PacketParcelable.aidl", "src/android/net/NattKeepalivePacketDataParcelable.aidl", + "src/android/net/NetworkTestResultParcelable.aidl", "src/android/net/PrivateDnsConfigParcel.aidl", "src/android/net/ProvisioningConfigurationParcelable.aidl", "src/android/net/ScanResultInfoParcelable.aidl", diff --git a/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/DataStallReportParcelable.aidl b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/DataStallReportParcelable.aidl new file mode 100644 index 0000000..69ff31f --- /dev/null +++ b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/DataStallReportParcelable.aidl @@ -0,0 +1,25 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL interface (or parcelable). Do not try to +// edit this file. It looks like you are doing that because you have modified +// an AIDL interface in a backward-incompatible way, e.g., deleting a function +// from an interface or a field from a parcelable and it broke the build. That +// breakage is intended. +// +// You must not make a backward incompatible changes to the AIDL files built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +parcelable DataStallReportParcelable { + long timestampMillis = 0; + int detectionMethod = 1; + int tcpPacketFailRate = 2; + int tcpMetricsCollectionPeriodMillis = 3; + int dnsConsecutiveTimeouts = 4; +} diff --git a/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/INetworkMonitorCallbacks.aidl b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/INetworkMonitorCallbacks.aidl index f418fe4..b7ddad9 100644 --- a/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/INetworkMonitorCallbacks.aidl +++ b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/INetworkMonitorCallbacks.aidl @@ -18,13 +18,13 @@ package android.net; /* @hide */ interface INetworkMonitorCallbacks { - oneway void onNetworkMonitorCreated(in android.net.INetworkMonitor networkMonitor); - oneway void notifyNetworkTested(int testResult, @nullable String redirectUrl); - oneway void notifyPrivateDnsConfigResolved(in android.net.PrivateDnsConfigParcel config); - oneway void showProvisioningNotification(String action, String packageName); - oneway void hideProvisioningNotification(); - oneway void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded); - oneway void notifyNetworkTestedWithExtras(int testResult, @nullable String redirectUrl, long timestampMillis, in android.os.PersistableBundle extras); - oneway void notifyDataStallSuspected(long timestampMillis, int detectionMethod, in android.os.PersistableBundle extras); - oneway void notifyCaptivePortalDataChanged(in android.net.CaptivePortalData data); + oneway void onNetworkMonitorCreated(in android.net.INetworkMonitor networkMonitor) = 0; + oneway void notifyNetworkTested(int testResult, @nullable String redirectUrl) = 1; + oneway void notifyPrivateDnsConfigResolved(in android.net.PrivateDnsConfigParcel config) = 2; + oneway void showProvisioningNotification(String action, String packageName) = 3; + oneway void hideProvisioningNotification() = 4; + oneway void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded) = 5; + oneway void notifyNetworkTestedWithExtras(in android.net.NetworkTestResultParcelable result) = 6; + oneway void notifyDataStallSuspected(in android.net.DataStallReportParcelable report) = 7; + oneway void notifyCaptivePortalDataChanged(in android.net.CaptivePortalData data) = 8; } diff --git a/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/NetworkTestResultParcelable.aidl b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/NetworkTestResultParcelable.aidl new file mode 100644 index 0000000..f31a669 --- /dev/null +++ b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/NetworkTestResultParcelable.aidl @@ -0,0 +1,25 @@ +/////////////////////////////////////////////////////////////////////////////// +// THIS FILE IS IMMUTABLE. DO NOT EDIT IN ANY CASE. // +/////////////////////////////////////////////////////////////////////////////// + +// This file is a snapshot of an AIDL interface (or parcelable). Do not try to +// edit this file. It looks like you are doing that because you have modified +// an AIDL interface in a backward-incompatible way, e.g., deleting a function +// from an interface or a field from a parcelable and it broke the build. That +// breakage is intended. +// +// You must not make a backward incompatible changes to the AIDL files built +// with the aidl_interface module type with versions property set. The module +// type is used to build AIDL files in a way that they can be used across +// independently updatable components of the system. If a device is shipped +// with such a backward incompatible change, it has a high risk of breaking +// later when a module using the interface is updated, e.g., Mainline modules. + +package android.net; +parcelable NetworkTestResultParcelable { + long timestampMillis; + int result; + int probesSucceeded; + int probesAttempted; + String redirectUrl; +} diff --git a/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/dhcp/DhcpServingParamsParcel.aidl b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/dhcp/DhcpServingParamsParcel.aidl index 657b83c..2570995 100644 --- a/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/dhcp/DhcpServingParamsParcel.aidl +++ b/common/networkstackclient/aidl_api/networkstack-aidl-interfaces/current/android/net/dhcp/DhcpServingParamsParcel.aidl @@ -25,6 +25,6 @@ parcelable DhcpServingParamsParcel { long dhcpLeaseTimeSecs; int linkMtu; boolean metered; - int clientAddr; + int singleClientAddr; boolean changePrefixOnDecline; } diff --git a/common/networkstackclient/src/android/net/DataStallReportParcelable.aidl b/common/networkstackclient/src/android/net/DataStallReportParcelable.aidl new file mode 100644 index 0000000..c5ba19d --- /dev/null +++ b/common/networkstackclient/src/android/net/DataStallReportParcelable.aidl @@ -0,0 +1,48 @@ +/** + * 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 perNmissions and + * limitations under the License. + */ + +package android.net; + +parcelable DataStallReportParcelable { + /** + * Timestamp of the report, relative to SystemClock.elapsedRealtime(). + */ + long timestampMillis = 0; + + /** + * Detection method of the data stall, one of DataStallReport.DETECTION_METHOD_*. + */ + int detectionMethod = 1; + + /** + * @see android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_TCP_PACKET_FAIL_RATE. + * Only set if the detection method is TCP, otherwise 0. + */ + int tcpPacketFailRate = 2; + + /** + * @see android.net.ConnectivityDiagnosticsManager.DataStallReport + * .KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS. + * Only set if the detection method is TCP, otherwise 0. + */ + int tcpMetricsCollectionPeriodMillis = 3; + + /** + * @see android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_DNS_CONSECUTIVE_TIMEOUTS. + * Only set if the detection method is DNS, otherwise 0. + */ + int dnsConsecutiveTimeouts = 4; +}
\ No newline at end of file diff --git a/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl b/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl index f57a8f7..79eb95a 100644 --- a/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl +++ b/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl @@ -17,22 +17,22 @@ package android.net; import android.net.CaptivePortalData; +import android.net.DataStallReportParcelable; import android.net.INetworkMonitor; +import android.net.NetworkTestResultParcelable; import android.net.PrivateDnsConfigParcel; /** @hide */ oneway interface INetworkMonitorCallbacks { - void onNetworkMonitorCreated(in INetworkMonitor networkMonitor); + void onNetworkMonitorCreated(in INetworkMonitor networkMonitor) = 0; // Deprecated. Use notifyNetworkTestedWithExtras() instead. - void notifyNetworkTested(int testResult, @nullable String redirectUrl); - void notifyPrivateDnsConfigResolved(in PrivateDnsConfigParcel config); - void showProvisioningNotification(String action, String packageName); - void hideProvisioningNotification(); - 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); - void notifyCaptivePortalDataChanged(in CaptivePortalData data); + void notifyNetworkTested(int testResult, @nullable String redirectUrl) = 1; + void notifyPrivateDnsConfigResolved(in PrivateDnsConfigParcel config) = 2; + void showProvisioningNotification(String action, String packageName) = 3; + void hideProvisioningNotification() = 4; + void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded) = 5; + void notifyNetworkTestedWithExtras(in NetworkTestResultParcelable result) = 6; + void notifyDataStallSuspected(in DataStallReportParcelable report) = 7; + void notifyCaptivePortalDataChanged(in CaptivePortalData data) = 8; }
\ No newline at end of file diff --git a/common/networkstackclient/src/android/net/NetworkTestResultParcelable.aidl b/common/networkstackclient/src/android/net/NetworkTestResultParcelable.aidl new file mode 100644 index 0000000..93efd73 --- /dev/null +++ b/common/networkstackclient/src/android/net/NetworkTestResultParcelable.aidl @@ -0,0 +1,45 @@ +/** + * 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 perNmissions and + * limitations under the License. + */ + +package android.net; + +parcelable NetworkTestResultParcelable { + /** + * Timestamp of the evaluation, as determined by to SystemClock.elapsedRealtime(). + */ + long timestampMillis; + + /** + * Result of the evaluation, as a bitmask of INetworkMonitor.NETWORK_VALIDATION_RESULT_*. + */ + int result; + + /** + * List of succeeded probes, as a bitmask of INetworkMonitor.NETWORK_VALIDATION_PROBE_* flags. + */ + int probesSucceeded; + + /** + * List of attempted probes, as a bitmask of INetworkMonitor.NETWORK_VALIDATION_PROBE_* flags. + */ + int probesAttempted; + + /** + * If the evaluation detected a captive portal, the URL that can be used to login to that + * portal. Otherwise null. + */ + String redirectUrl; +}
\ No newline at end of file diff --git a/common/networkstackclient/src/android/net/dhcp/DhcpServingParamsParcel.aidl b/common/networkstackclient/src/android/net/dhcp/DhcpServingParamsParcel.aidl index 1ba0195..503ccaa 100644 --- a/common/networkstackclient/src/android/net/dhcp/DhcpServingParamsParcel.aidl +++ b/common/networkstackclient/src/android/net/dhcp/DhcpServingParamsParcel.aidl @@ -26,6 +26,6 @@ parcelable DhcpServingParamsParcel { long dhcpLeaseTimeSecs; int linkMtu; boolean metered; - int clientAddr; + int singleClientAddr; boolean changePrefixOnDecline; } diff --git a/src/android/net/dhcp/DhcpServer.java b/src/android/net/dhcp/DhcpServer.java index 429b10c..1bc7c65 100644 --- a/src/android/net/dhcp/DhcpServer.java +++ b/src/android/net/dhcp/DhcpServer.java @@ -206,7 +206,7 @@ public class DhcpServer extends IDhcpServer.Stub { return new DhcpLeaseRepository( DhcpServingParams.makeIpPrefix(servingParams.serverAddr), servingParams.excludedAddrs, servingParams.dhcpLeaseTimeSecs * 1000, - servingParams.clientAddr, log.forSubComponent(REPO_TAG), clock); + servingParams.singleClientAddr, log.forSubComponent(REPO_TAG), clock); } @Override @@ -352,7 +352,7 @@ public class DhcpServer extends IDhcpServer.Stub { DhcpServingParams.makeIpPrefix(mServingParams.serverAddr), params.excludedAddrs, params.dhcpLeaseTimeSecs, - params.clientAddr); + params.singleClientAddr); cb = pair.second; break; diff --git a/src/android/net/dhcp/DhcpServingParams.java b/src/android/net/dhcp/DhcpServingParams.java index c52cae9..77cdf7a 100644 --- a/src/android/net/dhcp/DhcpServingParams.java +++ b/src/android/net/dhcp/DhcpServingParams.java @@ -88,7 +88,7 @@ public class DhcpServingParams { * Client inet address. This will be the only address offered by DhcpServer if set. */ @Nullable - public final Inet4Address clientAddr; + public final Inet4Address singleClientAddr; /** * Indicates whether the DHCP server should request a new prefix from IpServer when receiving @@ -110,7 +110,7 @@ public class DhcpServingParams { private DhcpServingParams(@NonNull LinkAddress serverAddr, @NonNull Set<Inet4Address> defaultRouters, @NonNull Set<Inet4Address> dnsServers, @NonNull Set<Inet4Address> excludedAddrs, - long dhcpLeaseTimeSecs, int linkMtu, boolean metered, Inet4Address clientAddr, + long dhcpLeaseTimeSecs, int linkMtu, boolean metered, Inet4Address singleClientAddr, boolean changePrefixOnDecline) { this.serverAddr = serverAddr; this.defaultRouters = defaultRouters; @@ -119,7 +119,7 @@ public class DhcpServingParams { this.dhcpLeaseTimeSecs = dhcpLeaseTimeSecs; this.linkMtu = linkMtu; this.metered = metered; - this.clientAddr = clientAddr; + this.singleClientAddr = singleClientAddr; this.changePrefixOnDecline = changePrefixOnDecline; } @@ -136,8 +136,8 @@ public class DhcpServingParams { intToInet4AddressHTH(parcel.serverAddr), parcel.serverAddrPrefixLength); Inet4Address clientAddr = null; - if (parcel.clientAddr != 0) { - clientAddr = intToInet4AddressHTH(parcel.clientAddr); + if (parcel.singleClientAddr != 0) { + clientAddr = intToInet4AddressHTH(parcel.singleClientAddr); } return new Builder() @@ -148,7 +148,7 @@ public class DhcpServingParams { .setDhcpLeaseTimeSecs(parcel.dhcpLeaseTimeSecs) .setLinkMtu(parcel.linkMtu) .setMetered(parcel.metered) - .setClientAddr(clientAddr) + .setSingleClientAddr(clientAddr) .setChangePrefixOnDecline(parcel.changePrefixOnDecline) .build(); } @@ -334,7 +334,7 @@ public class DhcpServingParams { * * <p>If not set, the default value is null. */ - public Builder setClientAddr(@Nullable Inet4Address clientAddr) { + public Builder setSingleClientAddr(@Nullable Inet4Address clientAddr) { this.mClientAddr = clientAddr; return this; } diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index 5826d14..b6387ab 100755 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java @@ -77,12 +77,6 @@ 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.apishim.ConstantsShim.KEY_DNS_CONSECUTIVE_TIMEOUTS; -import static com.android.networkstack.apishim.ConstantsShim.KEY_NETWORK_PROBES_ATTEMPTED_BITMASK; -import static com.android.networkstack.apishim.ConstantsShim.KEY_NETWORK_PROBES_SUCCEEDED_BITMASK; -import static com.android.networkstack.apishim.ConstantsShim.KEY_NETWORK_VALIDATION_RESULT; -import static com.android.networkstack.apishim.ConstantsShim.KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS; -import static com.android.networkstack.apishim.ConstantsShim.KEY_TCP_PACKET_FAIL_RATE; import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX; import static com.android.networkstack.util.DnsUtils.TYPE_ADDRCONFIG; @@ -95,11 +89,13 @@ import android.content.pm.PackageManager; import android.content.res.Configuration; import android.content.res.Resources; import android.net.ConnectivityManager; +import android.net.DataStallReportParcelable; import android.net.DnsResolver; import android.net.INetworkMonitorCallbacks; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; +import android.net.NetworkTestResultParcelable; import android.net.ProxyInfo; import android.net.TrafficStats; import android.net.Uri; @@ -119,7 +115,6 @@ 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; @@ -659,20 +654,43 @@ public class NetworkMonitor extends StateMachine { return NetworkMonitorUtils.isPrivateDnsValidationRequired(mNetworkCapabilities); } - private void notifyNetworkTested( - int result, @Nullable String redirectUrl, PersistableBundle extras) { + private void notifyNetworkTested(NetworkTestResultParcelable result) { try { if (mCallbackVersion <= 5) { - mCallback.notifyNetworkTested(result, redirectUrl); + mCallback.notifyNetworkTested( + getLegacyTestResult(result.result, result.probesSucceeded), + result.redirectUrl); } else { - mCallback.notifyNetworkTestedWithExtras( - result, redirectUrl, SystemClock.elapsedRealtime(), extras); + mCallback.notifyNetworkTestedWithExtras(result); } } catch (RemoteException e) { Log.e(TAG, "Error sending network test result", e); } } + /** + * Get the test result that was used as an int up to interface version 5. + * + * <p>For callback version < 3 (only used in Q beta preview builds), the int represented one of + * the NETWORK_TEST_RESULT_* constants. + * + * <p>Q released with version 3, which used a single int for both the evaluation result bitmask, + * and the probesSucceeded bitmask. + */ + protected int getLegacyTestResult(int evaluationResult, int probesSucceeded) { + if (mCallbackVersion < 3) { + if ((evaluationResult & NETWORK_VALIDATION_RESULT_VALID) != 0) { + return NETWORK_TEST_RESULT_VALID; + } + if ((evaluationResult & NETWORK_VALIDATION_RESULT_PARTIAL) != 0) { + return NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY; + } + return NETWORK_TEST_RESULT_INVALID; + } + + return evaluationResult | probesSucceeded; + } + private void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded) { try { mCallback.notifyProbeStatusChanged(probesCompleted, probesSucceeded); @@ -697,10 +715,9 @@ public class NetworkMonitor extends StateMachine { } } - private void notifyDataStallSuspected(int detectionMethod, PersistableBundle extras) { + private void notifyDataStallSuspected(@NonNull DataStallReportParcelable p) { try { - mCallback.notifyDataStallSuspected( - SystemClock.elapsedRealtime(), detectionMethod, extras); + mCallback.notifyDataStallSuspected(p); } catch (RemoteException e) { Log.e(TAG, "Error sending notification for suspected data stall", e); } @@ -2941,10 +2958,13 @@ public class NetworkMonitor extends StateMachine { } else if (tst.isDataStallSuspected()) { result = true; - final PersistableBundle extras = new PersistableBundle(); - extras.putInt(KEY_TCP_PACKET_FAIL_RATE, tst.getLatestPacketFailPercentage()); - extras.putInt(KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS, getTcpPollingInterval()); - notifyDataStallSuspected(DETECTION_METHOD_TCP_METRICS, extras); + final DataStallReportParcelable p = new DataStallReportParcelable(); + p.detectionMethod = DETECTION_METHOD_TCP_METRICS; + p.timestampMillis = SystemClock.elapsedRealtime(); + p.tcpPacketFailRate = tst.getLatestPacketFailPercentage(); + p.tcpMetricsCollectionPeriodMillis = getTcpPollingInterval(); + + notifyDataStallSuspected(p); } if (DBG || VDBG_STALL) { msg.add("tcp packets received=" + tst.getLatestReceivedCount()) @@ -2963,10 +2983,11 @@ public class NetworkMonitor extends StateMachine { result = true; logNetworkEvent(NetworkEvent.NETWORK_CONSECUTIVE_DNS_TIMEOUT_FOUND); - final PersistableBundle extras = new PersistableBundle(); - extras.putInt(KEY_DNS_CONSECUTIVE_TIMEOUTS, - mDnsStallDetector.getConsecutiveTimeoutCount()); - notifyDataStallSuspected(DETECTION_METHOD_DNS_EVENTS, extras); + final DataStallReportParcelable p = new DataStallReportParcelable(); + p.detectionMethod = DETECTION_METHOD_DNS_EVENTS; + p.timestampMillis = SystemClock.elapsedRealtime(); + p.dnsConsecutiveTimeouts = mDnsStallDetector.getConsecutiveTimeoutCount(); + notifyDataStallSuspected(p); } if (DBG || VDBG_STALL) { msg.add("consecutive dns timeout count=" + dsd.getConsecutiveTimeoutCount()); @@ -3001,7 +3022,7 @@ public class NetworkMonitor extends StateMachine { // The latest validation result for this network. This is a bitmask of // INetworkMonitor.NETWORK_VALIDATION_RESULT_* constants. private int mEvaluationResult = NETWORK_VALIDATION_RESULT_INVALID; - // Indicates which probes have completed since clearProbeResults was called. + // Indicates which probes have succeeded since clearProbeResults was called. // This is a bitmask of INetworkMonitor.NETWORK_VALIDATION_PROBE_* constants. private int mProbeResults = 0; // A bitmask to record which probes are completed. @@ -3044,25 +3065,23 @@ public class NetworkMonitor extends StateMachine { protected void reportEvaluationResult(int result, @Nullable String redirectUrl) { mEvaluationResult = result; mRedirectUrl = redirectUrl; - final PersistableBundle extras = new PersistableBundle(); + final NetworkTestResultParcelable p = new NetworkTestResultParcelable(); + p.result = result; + p.probesSucceeded = mProbeResults; + p.probesAttempted = mProbeCompleted; + p.redirectUrl = redirectUrl; + p.timestampMillis = SystemClock.elapsedRealtime(); + notifyNetworkTested(p); + } - extras.putInt(KEY_NETWORK_VALIDATION_RESULT, result); - extras.putInt(KEY_NETWORK_PROBES_SUCCEEDED_BITMASK, mProbeResults); - extras.putInt(KEY_NETWORK_PROBES_ATTEMPTED_BITMASK, mProbeCompleted); - notifyNetworkTested(getNetworkTestResult(), mRedirectUrl, extras); + @VisibleForTesting + protected int getEvaluationResult() { + return mEvaluationResult; } - protected int getNetworkTestResult() { - if (mCallbackVersion < 3) { - if ((mEvaluationResult & NETWORK_VALIDATION_RESULT_VALID) != 0) { - return NETWORK_TEST_RESULT_VALID; - } - if ((mEvaluationResult & NETWORK_VALIDATION_RESULT_PARTIAL) != 0) { - return NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY; - } - return NETWORK_TEST_RESULT_INVALID; - } - return mEvaluationResult | mProbeResults; + @VisibleForTesting + protected int getProbeResults() { + return mProbeResults; } @VisibleForTesting @@ -3080,7 +3099,7 @@ public class NetworkMonitor extends StateMachine { mAcceptPartialConnectivity = acceptPartial; // Ignore https probe in next validation if user accept partial connectivity on a partial // connectivity network. - if (((mEvaluationState.getNetworkTestResult() & NETWORK_VALIDATION_RESULT_PARTIAL) != 0) + if (((mEvaluationState.getEvaluationResult() & NETWORK_VALIDATION_RESULT_PARTIAL) != 0) && mAcceptPartialConnectivity) { mUseHttps = false; } diff --git a/tests/unit/src/android/net/dhcp/DhcpServingParamsTest.java b/tests/unit/src/android/net/dhcp/DhcpServingParamsTest.java index 5962696..1ce2f82 100644 --- a/tests/unit/src/android/net/dhcp/DhcpServingParamsTest.java +++ b/tests/unit/src/android/net/dhcp/DhcpServingParamsTest.java @@ -75,7 +75,7 @@ public class DhcpServingParamsTest { .setLinkMtu(TEST_MTU) .setExcludedAddrs(TEST_EXCLUDED_ADDRS) .setMetered(TEST_METERED) - .setClientAddr(TEST_CLIENT_ADDR) + .setSingleClientAddr(TEST_CLIENT_ADDR) .setChangePrefixOnDecline(TEST_CHANGE_PREFIX_ON_DECLINE); } @@ -184,7 +184,7 @@ public class DhcpServingParamsTest { parcel.linkMtu = TEST_MTU; parcel.excludedAddrs = toIntArray(TEST_EXCLUDED_ADDRS); parcel.metered = TEST_METERED; - parcel.clientAddr = inet4AddressToIntHTH(TEST_CLIENT_ADDR); + parcel.singleClientAddr = inet4AddressToIntHTH(TEST_CLIENT_ADDR); parcel.changePrefixOnDecline = TEST_CHANGE_PREFIX_ON_DECLINE; final DhcpServingParams parceled = DhcpServingParams.fromParcelableObject(parcel); @@ -195,7 +195,7 @@ public class DhcpServingParamsTest { assertEquals(params.linkMtu, parceled.linkMtu); assertEquals(params.excludedAddrs, parceled.excludedAddrs); assertEquals(params.metered, parceled.metered); - assertEquals(params.clientAddr, parceled.clientAddr); + assertEquals(params.singleClientAddr, parceled.singleClientAddr); assertEquals(params.changePrefixOnDecline, parceled.changePrefixOnDecline); MiscAssertsKt.assertFieldCountEquals(10, DhcpServingParamsParcel.class); diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index 669a14e..145c89c 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java @@ -41,14 +41,6 @@ import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS; import static android.net.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK; import static android.net.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION; -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.apishim.ConstantsShim.KEY_DNS_CONSECUTIVE_TIMEOUTS; -import static com.android.networkstack.apishim.ConstantsShim.KEY_NETWORK_PROBES_ATTEMPTED_BITMASK; -import static com.android.networkstack.apishim.ConstantsShim.KEY_NETWORK_PROBES_SUCCEEDED_BITMASK; -import static com.android.networkstack.apishim.ConstantsShim.KEY_NETWORK_VALIDATION_RESULT; -import static com.android.networkstack.apishim.ConstantsShim.KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS; -import static com.android.networkstack.apishim.ConstantsShim.KEY_TCP_PACKET_FAIL_RATE; import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX; import static com.android.server.connectivity.NetworkMonitor.extractCharset; @@ -64,7 +56,6 @@ import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; @@ -95,6 +86,7 @@ import android.content.res.Configuration; import android.content.res.Resources; import android.net.CaptivePortalData; import android.net.ConnectivityManager; +import android.net.DataStallReportParcelable; import android.net.DnsResolver; import android.net.INetd; import android.net.INetworkMonitorCallbacks; @@ -102,6 +94,7 @@ import android.net.InetAddresses; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; +import android.net.NetworkTestResultParcelable; import android.net.Uri; import android.net.captiveportal.CaptivePortalProbeResult; import android.net.metrics.IpConnectivityLog; @@ -115,7 +108,6 @@ import android.os.ConditionVariable; import android.os.Handler; import android.os.IBinder; import android.os.Looper; -import android.os.PersistableBundle; import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; @@ -135,6 +127,7 @@ import androidx.test.runner.AndroidJUnit4; import com.android.networkstack.NetworkStackNotifier; import com.android.networkstack.R; import com.android.networkstack.apishim.CaptivePortalDataShimImpl; +import com.android.networkstack.apishim.ConstantsShim; import com.android.networkstack.apishim.ShimUtils; import com.android.networkstack.metrics.DataStallDetectionStats; import com.android.networkstack.metrics.DataStallStatsUtils; @@ -144,13 +137,14 @@ import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; import com.android.testutils.HandlerUtilsKt; +import junit.framework.AssertionFailedError; + import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.Spy; @@ -175,6 +169,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Random; import java.util.concurrent.Executor; @@ -219,7 +214,6 @@ public class NetworkMonitorTest { private @Mock Network mNetwork; private @Mock DataStallStatsUtils mDataStallStatsUtils; private @Mock WifiInfo mWifiInfo; - private @Captor ArgumentCaptor<String> mNetworkTestedRedirectUrlCaptor; private @Mock TcpSocketTracker.Dependencies mTstDependencies; private @Mock INetd mNetd; private @Mock TcpSocketTracker mTst; @@ -248,18 +242,7 @@ public class NetworkMonitorTest { private static final int VALIDATION_RESULT_INVALID = 0; private static final int VALIDATION_RESULT_PORTAL = 0; private static final String TEST_REDIRECT_URL = "android.com"; - private static final int VALIDATION_RESULT_PARTIAL = NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTP - | NETWORK_VALIDATION_RESULT_PARTIAL; - private static final int VALIDATION_RESULT_FALLBACK_PARTIAL = NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_FALLBACK - | NETWORK_VALIDATION_RESULT_PARTIAL; - private static final int VALIDATION_RESULT_VALID = NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTPS - | NETWORK_VALIDATION_RESULT_VALID; - private static final int VALIDATION_RESULT_VALID_ALL_PROBES = - VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP; - private static final int VALIDATION_RESULT_PRIVDNS_VALID = NETWORK_VALIDATION_PROBE_DNS + private static final int PROBES_PRIVDNS_VALID = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS | NETWORK_VALIDATION_PROBE_PRIVDNS; private static final int RETURN_CODE_DNS_SUCCESS = 0; @@ -290,9 +273,6 @@ public class NetworkMonitorTest { private static final NetworkCapabilities NO_INTERNET_CAPABILITIES = new NetworkCapabilities() .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR); - private static final int NOTIFY_NETWORK_TESTED_VALIDATION_RESULT_MASK = 0x3; - private static final int NOTIFY_NETWORK_TESTED_SUCCESSFUL_PROBES_MASK = 0xFFFC; - /** * Fakes DNS responses. * @@ -883,10 +863,10 @@ public class NetworkMonitorTest { } @Test - public void testIsCaptivePortal_HttpProbeIsPortal() throws IOException { + public void testIsCaptivePortal_HttpProbeIsPortal() throws Exception { setSslException(mHttpsConnection); setPortal302(mHttpConnection); - runPortalNetworkTest(VALIDATION_RESULT_PORTAL); + runPortalNetworkTest(); } private void setupPrivateIpResponse(String privateAddr) throws Exception { @@ -918,27 +898,27 @@ public class NetworkMonitorTest { @Test public void testIsCaptivePortal_PrivateIpNotPortal_Disabled() throws Exception { setupPrivateIpResponse("192.168.1.1"); - runPortalNetworkTest(VALIDATION_RESULT_PORTAL); + runPortalNetworkTest(); } @Test - public void testIsCaptivePortal_HttpsProbeIsNotPortal() throws IOException { + public void testIsCaptivePortal_HttpsProbeIsNotPortal() throws Exception { setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 500); - runNotPortalNetworkTest(); + runValidatedNetworkTest(); } @Test - public void testIsCaptivePortal_FallbackProbeIsPortal() throws IOException { + public void testIsCaptivePortal_FallbackProbeIsPortal() throws Exception { setSslException(mHttpsConnection); setStatus(mHttpConnection, 500); setPortal302(mFallbackConnection); - runPortalNetworkTest(VALIDATION_RESULT_INVALID); + runPortalNetworkTest(); } @Test - public void testIsCaptivePortal_FallbackProbeIsNotPortal() throws IOException { + public void testIsCaptivePortal_FallbackProbeIsNotPortal() throws Exception { setSslException(mHttpsConnection); setStatus(mHttpConnection, 500); setStatus(mFallbackConnection, 500); @@ -948,7 +928,7 @@ public class NetworkMonitorTest { } @Test - public void testIsCaptivePortal_OtherFallbackProbeIsPortal() throws IOException { + public void testIsCaptivePortal_OtherFallbackProbeIsPortal() throws Exception { // Set all fallback probes but one to invalid URLs to verify they are being skipped setFallbackUrl(TEST_FALLBACK_URL); setOtherFallbackUrls(TEST_FALLBACK_URL + "," + TEST_OTHER_FALLBACK_URL); @@ -962,8 +942,7 @@ public class NetworkMonitorTest { when(mRandom.nextInt()).thenReturn(2); // First check always uses the first fallback URL: inconclusive - final NetworkMonitor monitor = runNetworkTest(VALIDATION_RESULT_INVALID); - assertNull(mNetworkTestedRedirectUrlCaptor.getValue()); + final NetworkMonitor monitor = runFailedNetworkTest(); verify(mFallbackConnection, times(1)).getResponseCode(); verify(mOtherFallbackConnection, never()).getResponseCode(); @@ -974,7 +953,7 @@ public class NetworkMonitorTest { } @Test - public void testIsCaptivePortal_AllProbesFailed() throws IOException { + public void testIsCaptivePortal_AllProbesFailed() throws Exception { setSslException(mHttpsConnection); setStatus(mHttpConnection, 500); setStatus(mFallbackConnection, 404); @@ -985,14 +964,14 @@ public class NetworkMonitorTest { } @Test - public void testIsCaptivePortal_InvalidUrlSkipped() throws IOException { + public void testIsCaptivePortal_InvalidUrlSkipped() throws Exception { setFallbackUrl("invalid"); setOtherFallbackUrls("otherinvalid," + TEST_OTHER_FALLBACK_URL + ",yetanotherinvalid"); setSslException(mHttpsConnection); setStatus(mHttpConnection, 500); setPortal302(mOtherFallbackConnection); - runPortalNetworkTest(VALIDATION_RESULT_INVALID); + runPortalNetworkTest(); verify(mOtherFallbackConnection, times(1)).getResponseCode(); verify(mFallbackConnection, never()).getResponseCode(); } @@ -1010,11 +989,11 @@ public class NetworkMonitorTest { + "'bytes-remaining': " + bytesRemaining + "," + "'seconds-remaining': " + secondsRemaining + "}"); - runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL); + runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL, + 0 /* probesSucceeded*/, TEST_LOGIN_URL); verify(mHttpConnection, never()).getResponseCode(); verify(mCapportApiConnection).getResponseCode(); - assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue()); final ArgumentCaptor<CaptivePortalData> capportDataCaptor = ArgumentCaptor.forClass(CaptivePortalData.class); @@ -1033,18 +1012,19 @@ public class NetworkMonitorTest { @Test public void testIsCaptivePortal_CapportApiRevalidation() throws Exception { assumeTrue(CaptivePortalDataShimImpl.isSupported()); + setValidProbes(); final NetworkMonitor nm = runValidatedNetworkTest(); setApiContent(mCapportApiConnection, "{'captive': true, " + "'user-portal-url': '" + TEST_LOGIN_URL + "'}"); nm.notifyLinkPropertiesChanged(makeCapportLPs()); - verifyNetworkTested(VALIDATION_RESULT_PORTAL); + verifyNetworkTested(VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, + TEST_LOGIN_URL); final ArgumentCaptor<CaptivePortalData> capportCaptor = ArgumentCaptor.forClass( CaptivePortalData.class); verify(mCallbacks).notifyCaptivePortalDataChanged(capportCaptor.capture()); assertEquals(Uri.parse(TEST_LOGIN_URL), capportCaptor.getValue().getUserPortalUrl()); - assertEquals(TEST_LOGIN_URL, mNetworkTestedRedirectUrlCaptor.getValue()); // HTTP probe was sent on first validation but not re-sent when there was a portal URL. verify(mHttpConnection, times(1)).getResponseCode(); @@ -1058,7 +1038,8 @@ public class NetworkMonitorTest { setStatus(mHttpConnection, 500); setApiContent(mCapportApiConnection, "{'captive': false," + "'venue-info-url': '" + TEST_VENUE_INFO_URL + "'}"); - runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_INVALID); + runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_INVALID, + 0 /* probesSucceeded */, null /* redirectUrl */); final ArgumentCaptor<CaptivePortalData> capportCaptor = ArgumentCaptor.forClass( CaptivePortalData.class); @@ -1073,7 +1054,10 @@ public class NetworkMonitorTest { setStatus(mHttpConnection, 204); setApiContent(mCapportApiConnection, "{'captive': false," + "'venue-info-url': '" + TEST_VENUE_INFO_URL + "'}"); - runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_PARTIAL); + runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, + NETWORK_VALIDATION_RESULT_PARTIAL, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP, + null /* redirectUrl */); final ArgumentCaptor<CaptivePortalData> capportCaptor = ArgumentCaptor.forClass( CaptivePortalData.class); @@ -1088,7 +1072,11 @@ public class NetworkMonitorTest { setStatus(mHttpConnection, 204); setApiContent(mCapportApiConnection, "{'captive': false," + "'venue-info-url': '" + TEST_VENUE_INFO_URL + "'}"); - runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_VALID_ALL_PROBES); + runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, + NETWORK_VALIDATION_RESULT_VALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP + | NETWORK_VALIDATION_PROBE_HTTPS, + null /* redirectUrl */); final ArgumentCaptor<CaptivePortalData> capportCaptor = ArgumentCaptor.forClass( CaptivePortalData.class); @@ -1102,7 +1090,9 @@ public class NetworkMonitorTest { setStatus(mHttpsConnection, 204); setPortal302(mHttpConnection); setApiContent(mCapportApiConnection, "{SomeInvalidText"); - runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL); + runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, + VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, + TEST_LOGIN_URL); verify(mCallbacks, never()).notifyCaptivePortalDataChanged(any()); verify(mHttpConnection).getResponseCode(); @@ -1117,7 +1107,9 @@ public class NetworkMonitorTest { setPortal302(mHttpConnection); setApiContent(mCapportApiConnection, "{'captive': false," + "'venue-info-url': '" + TEST_VENUE_INFO_URL + "'}"); - runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL); + runNetworkTest(makeCapportLPs(), METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL, + 0 /* probesSucceeded */, + TEST_LOGIN_URL); verify(mCallbacks, never()).notifyCaptivePortalDataChanged(any()); verify(mHttpConnection).getResponseCode(); @@ -1152,7 +1144,8 @@ public class NetworkMonitorTest { R.integer.config_max_matches_http_content_length); doReturn("te.t").when(mResources).getString( R.string.config_network_validation_success_content_regexp); - runPartialConnectivityNetworkTest(VALIDATION_RESULT_PARTIAL); + runPartialConnectivityNetworkTest( + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP); } private void setupFallbackSpec() throws IOException { @@ -1168,25 +1161,26 @@ public class NetworkMonitorTest { } @Test - public void testIsCaptivePortal_FallbackSpecIsPartial() throws IOException { + public void testIsCaptivePortal_FallbackSpecIsPartial() throws Exception { setupFallbackSpec(); set302(mOtherFallbackConnection, "https://www.google.com/test?q=3"); // HTTPS failed, fallback spec went through -> partial connectivity - runPartialConnectivityNetworkTest(VALIDATION_RESULT_FALLBACK_PARTIAL); + runPartialConnectivityNetworkTest( + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK); verify(mOtherFallbackConnection, times(1)).getResponseCode(); verify(mFallbackConnection, never()).getResponseCode(); } @Test - public void testIsCaptivePortal_FallbackSpecIsPortal() throws IOException { + public void testIsCaptivePortal_FallbackSpecIsPortal() throws Exception { setupFallbackSpec(); - set302(mOtherFallbackConnection, "http://login.portal.example.com"); - runPortalNetworkTest(VALIDATION_RESULT_INVALID); + setPortal302(mOtherFallbackConnection); + runPortalNetworkTest(); } @Test - public void testIsCaptivePortal_IgnorePortals() throws IOException { + public void testIsCaptivePortal_IgnorePortals() throws Exception { setCaptivePortalMode(Settings.Global.CAPTIVE_PORTAL_MODE_IGNORE); setSslException(mHttpsConnection); setPortal302(mHttpConnection); @@ -1208,8 +1202,8 @@ public class NetworkMonitorTest { wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 100); makeDnsTimeoutEvent(wrappedMonitor, DEFAULT_DNS_TIMEOUT_THRESHOLD); assertTrue(wrappedMonitor.isDataStall()); - verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS), - bundleForDnsDataStall(DEFAULT_DNS_TIMEOUT_THRESHOLD)); + verify(mCallbacks).notifyDataStallSuspected( + matchDnsDataStallParcelable(DEFAULT_DNS_TIMEOUT_THRESHOLD)); } @Test @@ -1221,8 +1215,8 @@ 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), - bundleForDnsDataStall(DEFAULT_DNS_TIMEOUT_THRESHOLD)); + verify(mCallbacks).notifyDataStallSuspected( + matchDnsDataStallParcelable(DEFAULT_DNS_TIMEOUT_THRESHOLD)); } @Test @@ -1240,8 +1234,8 @@ public class NetworkMonitorTest { assertTrue(wrappedMonitor.isDataStall()); // The expected timeout count is the previous 2 DNS timeouts + the most recent 3 timeouts - verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS), - bundleForDnsDataStall(5)); + verify(mCallbacks).notifyDataStallSuspected( + matchDnsDataStallParcelable(5 /* timeoutCount */)); // Set the value to larger than the default dns log size. setConsecutiveDnsTimeoutThreshold(51); @@ -1254,8 +1248,8 @@ public class NetworkMonitorTest { assertTrue(wrappedMonitor.isDataStall()); // The expected timeout count is the previous 50 DNS timeouts + the most recent timeout - verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS), - bundleForDnsDataStall(51)); + verify(mCallbacks).notifyDataStallSuspected( + matchDnsDataStallParcelable(51 /* timeoutCount */)); } @Test @@ -1280,8 +1274,8 @@ public class NetworkMonitorTest { assertFalse(wrappedMonitor.isDataStall()); wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000); assertTrue(wrappedMonitor.isDataStall()); - verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS), - bundleForDnsDataStall(DEFAULT_DNS_TIMEOUT_THRESHOLD)); + verify(mCallbacks).notifyDataStallSuspected( + matchDnsDataStallParcelable(DEFAULT_DNS_TIMEOUT_THRESHOLD)); // Test dns events happened before valid dns time threshold. setValidDataStallDnsTimeThreshold(0); @@ -1315,8 +1309,7 @@ public class NetworkMonitorTest { wrappedMonitor.sendTcpPollingEvent(); HandlerUtilsKt.waitForIdle(wrappedMonitor.getHandler(), HANDLER_TIMEOUT_MS); assertTrue(wrappedMonitor.isDataStall()); - verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_TCP_METRICS), - bundleForTcpDataStall()); + verify(mCallbacks).notifyDataStallSuspected(matchTcpDataStallParcelable()); } @Test @@ -1344,7 +1337,7 @@ public class NetworkMonitorTest { @Test public void testNoInternetCapabilityValidated() throws Exception { runNetworkTest(TEST_LINK_PROPERTIES, NO_INTERNET_CAPABILITIES, - NETWORK_VALIDATION_RESULT_VALID); + NETWORK_VALIDATION_RESULT_VALID, 0 /* probesSucceeded */, null /* redirectUrl */); verify(mCleartextDnsNetwork, never()).openConnection(any()); } @@ -1384,13 +1377,12 @@ public class NetworkMonitorTest { setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); - final int expectedResult = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP - | NETWORK_VALIDATION_RESULT_VALID; resetCallbacks(); nm.notifyCaptivePortalAppFinished(APP_RETURN_DISMISSED); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce()) - .notifyNetworkTestedWithExtras(eq(expectedResult), any(), anyLong(), - bundleForNotifyNetworkTested(expectedResult)); + .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable( + NETWORK_VALIDATION_RESULT_VALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP)); assertEquals(0, mRegisteredReceivers.size()); } @@ -1399,43 +1391,35 @@ public class NetworkMonitorTest { setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); - final int expectedResult = VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS; - // Verify dns query only get v6 address. mFakeDns.setAnswer("dns6.google", new String[]{"2001:db8::53"}, TYPE_AAAA); WrappedNetworkMonitor wnm = makeNotMeteredNetworkMonitor(); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns6.google", new InetAddress[0])); notifyNetworkConnected(wnm, NOT_METERED_CAPABILITIES); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) - .notifyNetworkTestedWithExtras(eq(expectedResult), eq(null), anyLong(), - bundleForNotifyNetworkTested(expectedResult)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, PROBES_PRIVDNS_VALID); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID)); + eq(PROBES_PRIVDNS_VALID), eq(PROBES_PRIVDNS_VALID)); // Verify dns query only get v4 address. resetCallbacks(); mFakeDns.setAnswer("dns4.google", new String[]{"192.0.2.1"}, TYPE_A); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns4.google", new InetAddress[0])); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) - .notifyNetworkTestedWithExtras(eq(expectedResult), eq(null), anyLong(), - bundleForNotifyNetworkTested(expectedResult)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, PROBES_PRIVDNS_VALID); // NetworkMonitor will check if the probes has changed or not, if the probes has not // changed, the callback won't be fired. verify(mCallbacks, never()).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID)); + eq(PROBES_PRIVDNS_VALID), eq(PROBES_PRIVDNS_VALID)); // Verify dns query get both v4 and v6 address. resetCallbacks(); mFakeDns.setAnswer("dns.google", new String[]{"2001:db8::54"}, TYPE_AAAA); mFakeDns.setAnswer("dns.google", new String[]{"192.0.2.3"}, TYPE_A); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.google", new InetAddress[0])); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) - .notifyNetworkTestedWithExtras(eq(expectedResult), eq(null), anyLong(), - bundleForNotifyNetworkTested(expectedResult)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, PROBES_PRIVDNS_VALID); verify(mCallbacks, never()).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID)); + eq(PROBES_PRIVDNS_VALID), eq(PROBES_PRIVDNS_VALID)); } @Test @@ -1447,12 +1431,10 @@ public class NetworkMonitorTest { WrappedNetworkMonitor wnm = makeNotMeteredNetworkMonitor(); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.google", new InetAddress[0])); wnm.notifyNetworkConnected(TEST_LINK_PROPERTIES, NOT_METERED_CAPABILITIES); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTestedWithExtras( - eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), eq(null), - anyLong(), bundleForNotifyNetworkTested(NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTPS)); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS + verifyNetworkTested(VALIDATION_RESULT_INVALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS); + verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)).notifyProbeStatusChanged( + eq(PROBES_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS)); // Fix DNS and retry, expect validation to succeed. resetCallbacks(); @@ -1462,13 +1444,9 @@ public class NetworkMonitorTest { // ProbeCompleted should be reset to 0 HandlerUtilsKt.waitForIdle(wnm.getHandler(), HANDLER_TIMEOUT_MS); assertEquals(wnm.getEvaluationState().getProbeCompletedResult(), 0); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) - .notifyNetworkTestedWithExtras(eq(VALIDATION_RESULT_VALID - | NETWORK_VALIDATION_PROBE_PRIVDNS), eq(null), anyLong(), - bundleForNotifyNetworkTested(VALIDATION_RESULT_VALID - | NETWORK_VALIDATION_PROBE_PRIVDNS)); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, PROBES_PRIVDNS_VALID); + verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)).notifyProbeStatusChanged( + eq(PROBES_PRIVDNS_VALID), eq(PROBES_PRIVDNS_VALID)); } @Test @@ -1480,12 +1458,10 @@ public class NetworkMonitorTest { WrappedNetworkMonitor wnm = makeNotMeteredNetworkMonitor(); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.google", new InetAddress[0])); wnm.notifyNetworkConnected(TEST_LINK_PROPERTIES, NOT_METERED_CAPABILITIES); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)).notifyNetworkTestedWithExtras( - eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), eq(null), - anyLong(), bundleForNotifyNetworkTested(NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTPS)); + verifyNetworkTested(VALIDATION_RESULT_INVALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS + eq(PROBES_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS)); // Fix DNS and retry, expect validation to succeed. @@ -1494,12 +1470,10 @@ public class NetworkMonitorTest { wnm.forceReevaluation(Process.myUid()); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce()) - .notifyNetworkTestedWithExtras(eq(VALIDATION_RESULT_VALID - | NETWORK_VALIDATION_PROBE_PRIVDNS), eq(null), anyLong(), - bundleForNotifyNetworkTested(VALIDATION_RESULT_VALID - | NETWORK_VALIDATION_PROBE_PRIVDNS)); + .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable( + NETWORK_VALIDATION_RESULT_VALID, PROBES_PRIVDNS_VALID)); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID)); + eq(PROBES_PRIVDNS_VALID), eq(PROBES_PRIVDNS_VALID)); // Change configuration to an invalid DNS name, expect validation to fail. resetCallbacks(); @@ -1508,13 +1482,10 @@ public class NetworkMonitorTest { wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.bad", new InetAddress[0])); // Strict mode hostname resolve fail. Expect only notification for evaluation fail. No probe // notification. - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) - .notifyNetworkTestedWithExtras(eq(NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTPS), eq(null), anyLong(), - bundleForNotifyNetworkTested(NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTPS)); + verifyNetworkTested(VALIDATION_RESULT_INVALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS + eq(PROBES_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS)); // Change configuration back to working again, but make private DNS not work. @@ -1523,37 +1494,32 @@ public class NetworkMonitorTest { mFakeDns.setNonBypassPrivateDnsWorking(false); wnm.notifyPrivateDnsSettingsChanged(new PrivateDnsConfig("dns.google", new InetAddress[0])); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)).notifyNetworkTestedWithExtras( - eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), eq(null), - anyLong(), bundleForNotifyNetworkTested(NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTPS)); + verifyNetworkTested(VALIDATION_RESULT_INVALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS); // NetworkMonitor will check if the probes has changed or not, if the probes has not // changed, the callback won't be fired. verify(mCallbacks, never()).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS + eq(PROBES_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS)); // Make private DNS work again. Expect validation to succeed. resetCallbacks(); mFakeDns.setNonBypassPrivateDnsWorking(true); wnm.forceReevaluation(Process.myUid()); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce()) - .notifyNetworkTestedWithExtras( - eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS), eq(null), - anyLong(), bundleForNotifyNetworkTested(VALIDATION_RESULT_VALID - | NETWORK_VALIDATION_PROBE_PRIVDNS)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, PROBES_PRIVDNS_VALID); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged( - eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID)); + eq(PROBES_PRIVDNS_VALID), eq(PROBES_PRIVDNS_VALID)); } @Test - public void testDataStall_StallDnsSuspectedAndSendMetrics() throws IOException { + public void testDataStall_StallDnsSuspectedAndSendMetrics() throws Exception { // 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); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS); // Setup dns data stall signal. wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000); makeDnsTimeoutEvent(wrappedMonitor, 5); @@ -1575,13 +1541,14 @@ public class NetworkMonitorTest { } @Test - public void testDataStall_NoStallSuspectedAndSendMetrics() throws IOException { + public void testDataStall_NoStallSuspectedAndSendMetrics() throws Exception { // 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); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS); // Setup no data stall dns signal. wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000); makeDnsTimeoutEvent(wrappedMonitor, 3); @@ -1631,29 +1598,31 @@ public class NetworkMonitorTest { setSslException(mHttpsConnection); setStatus(mHttpConnection, 204); // Expect to send HTTP, HTTPS, FALLBACK probe and evaluation result notifications to CS. - final NetworkMonitor nm = runNetworkTest(VALIDATION_RESULT_PARTIAL); + final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_PARTIAL, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP, + null /* redirectUrl */); - final int expectedResult = VALIDATION_RESULT_PARTIAL | NETWORK_VALIDATION_RESULT_VALID; resetCallbacks(); nm.setAcceptPartialConnectivity(); // Expect to update evaluation result notifications to CS. - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTestedWithExtras( - eq(expectedResult), eq(null), anyLong(), bundleForNotifyNetworkTested( - expectedResult)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_PARTIAL | NETWORK_VALIDATION_RESULT_VALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP); } @Test - public void testIsPartialConnectivity() throws IOException { + public void testIsPartialConnectivity() throws Exception { setStatus(mHttpsConnection, 500); setStatus(mHttpConnection, 204); setStatus(mFallbackConnection, 500); - runPartialConnectivityNetworkTest(VALIDATION_RESULT_PARTIAL); + runPartialConnectivityNetworkTest( + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP); resetCallbacks(); setStatus(mHttpsConnection, 500); setStatus(mHttpConnection, 500); setStatus(mFallbackConnection, 204); - runPartialConnectivityNetworkTest(VALIDATION_RESULT_FALLBACK_PARTIAL); + runPartialConnectivityNetworkTest( + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK); } private void assertIpAddressArrayEquals(String[] expected, InetAddress[] actual) { @@ -1703,11 +1672,9 @@ public class NetworkMonitorTest { @Test public void testNotifyNetwork_WithforceReevaluation() throws Exception { - final int expectedResult = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK - | NETWORK_VALIDATION_RESULT_PARTIAL; - + setValidProbes(); final NetworkMonitor nm = runValidatedNetworkTest(); - // Verify forceReevalution will not reset the validation result but only probe result until + // Verify forceReevaluation will not reset the validation result but only probe result until // getting the validation result. resetCallbacks(); setSslException(mHttpsConnection); @@ -1716,9 +1683,8 @@ public class NetworkMonitorTest { nm.forceReevaluation(Process.myUid()); final ArgumentCaptor<Integer> intCaptor = ArgumentCaptor.forClass(Integer.class); // Expect to send HTTP, HTTPs, FALLBACK and evaluation results. - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) - .notifyNetworkTestedWithExtras(eq(expectedResult), any(), anyLong(), - bundleForNotifyNetworkTested(expectedResult)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_PARTIAL, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK); } @Test @@ -1734,7 +1700,9 @@ public class NetworkMonitorTest { nm.notifyNetworkConnected(TEST_LINK_PROPERTIES, METERED_CAPABILITIES); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) - .notifyNetworkTested(eq(VALIDATION_RESULT_VALID), any()); + .notifyNetworkTested(eq(NETWORK_VALIDATION_RESULT_VALID + | NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), + eq(null) /* redirectUrl */); } @Test @@ -1783,15 +1751,18 @@ public class NetworkMonitorTest { @Test public void testEvaluationState_clearProbeResults() throws Exception { + setValidProbes(); final NetworkMonitor nm = runValidatedNetworkTest(); nm.getEvaluationState().clearProbeResults(); // Verify probe results are all reset and only evaluation result left. assertEquals(NETWORK_VALIDATION_RESULT_VALID, - nm.getEvaluationState().getNetworkTestResult()); + nm.getEvaluationState().getEvaluationResult()); + assertEquals(0, nm.getEvaluationState().getProbeResults()); } @Test public void testEvaluationState_reportProbeResult() throws Exception { + setValidProbes(); final NetworkMonitor nm = runValidatedNetworkTest(); resetCallbacks(); @@ -1800,42 +1771,43 @@ public class NetworkMonitorTest { CaptivePortalProbeResult.success(1 << PROBE_HTTP)); // Verify result should be appended and notifyNetworkTestedWithExtras callback is triggered // once. - assertEquals(nm.getEvaluationState().getNetworkTestResult(), - VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_HTTP); + assertEquals(NETWORK_VALIDATION_RESULT_VALID, + nm.getEvaluationState().getEvaluationResult()); + assertEquals(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS + | NETWORK_VALIDATION_PROBE_HTTP, nm.getEvaluationState().getProbeResults()); nm.reportHttpProbeResult(NETWORK_VALIDATION_PROBE_HTTP, CaptivePortalProbeResult.failed(1 << PROBE_HTTP)); // Verify DNS probe result should not be cleared. - assertTrue((nm.getEvaluationState().getNetworkTestResult() & NETWORK_VALIDATION_PROBE_DNS) - == NETWORK_VALIDATION_PROBE_DNS); + assertEquals(NETWORK_VALIDATION_PROBE_DNS, + nm.getEvaluationState().getProbeResults() & NETWORK_VALIDATION_PROBE_DNS); } @Test public void testEvaluationState_reportEvaluationResult() throws Exception { - final NetworkMonitor nm = runValidatedNetworkTest(); + setStatus(mHttpsConnection, 500); + setStatus(mHttpConnection, 204); + final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_PARTIAL, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP, + null /* redirectUrl */); - nm.getEvaluationState().reportEvaluationResult(NETWORK_VALIDATION_RESULT_PARTIAL, + nm.getEvaluationState().reportEvaluationResult(NETWORK_VALIDATION_RESULT_VALID, null /* redirectUrl */); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTestedWithExtras( - eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS - | NETWORK_VALIDATION_RESULT_PARTIAL), eq(null), anyLong(), - bundleForNotifyNetworkTested(NETWORK_VALIDATION_PROBE_DNS - | NETWORK_VALIDATION_PROBE_HTTPS | NETWORK_VALIDATION_RESULT_PARTIAL)); + verifyNetworkTested(NETWORK_VALIDATION_RESULT_VALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP); nm.getEvaluationState().reportEvaluationResult( NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_PARTIAL, null /* redirectUrl */); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTestedWithExtras( - eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_PARTIAL), eq(null), - anyLong(), bundleForNotifyNetworkTested(VALIDATION_RESULT_VALID - | NETWORK_VALIDATION_RESULT_PARTIAL)); + verifyNetworkTested( + NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_PARTIAL, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP); nm.getEvaluationState().reportEvaluationResult(VALIDATION_RESULT_INVALID, TEST_REDIRECT_URL); - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyNetworkTestedWithExtras( - eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), - eq(TEST_REDIRECT_URL), anyLong(), bundleForNotifyNetworkTested( - NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS)); + verifyNetworkTested(VALIDATION_RESULT_INVALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP, + TEST_REDIRECT_URL); } @Test @@ -1897,7 +1869,7 @@ public class NetworkMonitorTest { setupResourceForMultipleProbes(); // One of the http probes is portal, then result is portal. setPortal302(mOtherHttpConnection1); - runPortalNetworkTest(VALIDATION_RESULT_INVALID); + runPortalNetworkTest(); // Get conclusive result from one of the HTTP probe. Expect to create 2 HTTP and 2 HTTPS // probes as resource configuration. verify(mCleartextDnsNetwork, times(4)).openConnection(any()); @@ -1908,7 +1880,7 @@ public class NetworkMonitorTest { setupResourceForMultipleProbes(); // One of the https probes succeeds, then it's validated. setStatus(mOtherHttpsConnection2, 204); - runNetworkTest(VALIDATION_RESULT_VALID); + runValidatedNetworkTest(); // Get conclusive result from one of the HTTPS probe. Expect to create 2 HTTP and 2 HTTPS // probes as resource configuration. verify(mCleartextDnsNetwork, times(4)).openConnection(any()); @@ -1922,16 +1894,16 @@ public class NetworkMonitorTest { // mOtherHttpConnection2, mOtherHttpConnection2 will affect the result.) // Configure mHttpsConnection is no-op. setStatus(mHttpsConnection, 204); - runNetworkTest(VALIDATION_RESULT_INVALID); + runFailedNetworkTest(); // No conclusive result from both HTTP and HTTPS probes. Expect to create 2 HTTP and 2 HTTPS // probes as resource configuration. verify(mCleartextDnsNetwork, times(4)).openConnection(any()); } @Test - public void testMultipleProbesOnInValiadNetwork() throws Exception { + public void testMultipleProbesOnInValidNetwork() throws Exception { setupResourceForMultipleProbes(); - runNetworkTest(VALIDATION_RESULT_INVALID); + runFailedNetworkTest(); // No conclusive result from both HTTP and HTTPS probes. Expect to create 2 HTTP and 2 HTTPS // probes as resource configuration. verify(mCleartextDnsNetwork, times(4)).openConnection(any()); @@ -2013,65 +1985,85 @@ public class NetworkMonitorTest { eq(DISMISS_PORTAL_IN_VALIDATED_NETWORK), anyBoolean())).thenReturn(enabled); } - private void runPortalNetworkTest(int result) { - runNetworkTest(result); + private NetworkMonitor runPortalNetworkTest() throws RemoteException { + final NetworkMonitor nm = runNetworkTest(VALIDATION_RESULT_PORTAL, + 0 /* probesSucceeded */, TEST_LOGIN_URL); assertEquals(1, mRegisteredReceivers.size()); - assertNotNull(mNetworkTestedRedirectUrlCaptor.getValue()); - } - - private void runNotPortalNetworkTest() { - runNetworkTest(VALIDATION_RESULT_VALID); - assertEquals(0, mRegisteredReceivers.size()); - assertNull(mNetworkTestedRedirectUrlCaptor.getValue()); + return nm; } - private void runNoValidationNetworkTest() { - runNetworkTest(NETWORK_VALIDATION_RESULT_VALID); + private NetworkMonitor runNoValidationNetworkTest() throws RemoteException { + final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_VALID, + 0 /* probesSucceeded */, null /* redirectUrl */); assertEquals(0, mRegisteredReceivers.size()); - assertNull(mNetworkTestedRedirectUrlCaptor.getValue()); + return nm; } - private void runFailedNetworkTest() { - runNetworkTest(VALIDATION_RESULT_INVALID); + private NetworkMonitor runFailedNetworkTest() throws RemoteException { + final NetworkMonitor nm = runNetworkTest( + VALIDATION_RESULT_INVALID, 0 /* probesSucceeded */, null /* redirectUrl */); assertEquals(0, mRegisteredReceivers.size()); - assertNull(mNetworkTestedRedirectUrlCaptor.getValue()); + return nm; } - private void runPartialConnectivityNetworkTest(int result) { - runNetworkTest(result); + private NetworkMonitor runPartialConnectivityNetworkTest(int probesSucceeded) + throws RemoteException { + final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_PARTIAL, + probesSucceeded, null /* redirectUrl */); assertEquals(0, mRegisteredReceivers.size()); - assertNull(mNetworkTestedRedirectUrlCaptor.getValue()); + return nm; } - private NetworkMonitor runValidatedNetworkTest() throws IOException { - setStatus(mHttpsConnection, 204); - setStatus(mHttpConnection, 204); - // Expect to send HTTPs and evaluation results. - return runNetworkTest(VALIDATION_RESULT_VALID); + private NetworkMonitor runValidatedNetworkTest() throws RemoteException { + // Expect to send HTTPS and evaluation results. + return runNetworkTest(NETWORK_VALIDATION_RESULT_VALID, + NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS, + null /* redirectUrl */); } - private NetworkMonitor runNetworkTest(int testResult) { - return runNetworkTest(TEST_LINK_PROPERTIES, METERED_CAPABILITIES, testResult); + private NetworkMonitor runNetworkTest(int testResult, int probesSucceeded, String redirectUrl) + throws RemoteException { + return runNetworkTest(TEST_LINK_PROPERTIES, METERED_CAPABILITIES, testResult, + probesSucceeded, redirectUrl); } private NetworkMonitor runNetworkTest(LinkProperties lp, NetworkCapabilities nc, - int testResult) { + int testResult, int probesSucceeded, String redirectUrl) throws RemoteException { final NetworkMonitor monitor = makeMonitor(nc); monitor.notifyNetworkConnected(lp, nc); - verifyNetworkTested(testResult); + verifyNetworkTested(testResult, probesSucceeded, redirectUrl); HandlerUtilsKt.waitForIdle(monitor.getHandler(), HANDLER_TIMEOUT_MS); return monitor; } - private void verifyNetworkTested(int testResult) { + private void verifyNetworkTested(int testResult, int probesSucceeded) throws RemoteException { + verifyNetworkTested(testResult, probesSucceeded, null /* redirectUrl */); + } + + private void verifyNetworkTested(int testResult, int probesSucceeded, String redirectUrl) + throws RemoteException { try { - verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)) - .notifyNetworkTestedWithExtras(eq(testResult), - mNetworkTestedRedirectUrlCaptor.capture(), anyLong(), - bundleForNotifyNetworkTested(testResult)); - } catch (RemoteException e) { - fail("Unexpected exception: " + e); + verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS)).notifyNetworkTestedWithExtras( + matchNetworkTestResultParcelable(testResult, probesSucceeded, redirectUrl)); + } catch (AssertionFailedError e) { + // Capture the callbacks up to now to give a better error message + final ArgumentCaptor<NetworkTestResultParcelable> captor = + ArgumentCaptor.forClass(NetworkTestResultParcelable.class); + + // Call verify() again to verify the same method call verified by the previous verify + // call which failed, but this time use a captor to log the exact parcel sent by + // NetworkMonitor. + // This assertion will fail if notifyNetworkTested was not called at all. + verify(mCallbacks).notifyNetworkTestedWithExtras(captor.capture()); + + final NetworkTestResultParcelable lastResult = captor.getValue(); + fail(String.format("notifyNetworkTestedWithExtras was not called with the " + + "expected result within timeout. " + + "Expected result %d, probes succeeded %d, redirect URL %s, " + + "last result was (%d, %d, %s).", + testResult, probesSucceeded, redirectUrl, + lastResult.result, lastResult.probesSucceeded, lastResult.redirectUrl)); } } @@ -2083,13 +2075,18 @@ public class NetworkMonitorTest { doThrow(new SSLHandshakeException("Invalid cert")).when(connection).getResponseCode(); } + private void setValidProbes() throws IOException { + setStatus(mHttpsConnection, 204); + setStatus(mHttpConnection, 204); + } + private void set302(HttpURLConnection connection, String location) throws IOException { setStatus(connection, 302); doReturn(location).when(connection).getHeaderField(LOCATION_HEADER); } private void setPortal302(HttpURLConnection connection) throws IOException { - set302(connection, "http://login.example.com"); + set302(connection, TEST_LOGIN_URL); } private void setApiContent(HttpURLConnection connection, String content) throws IOException { @@ -2112,32 +2109,25 @@ public class NetworkMonitorTest { } } - private PersistableBundle bundleForNotifyNetworkTested(final int result) { - // result = KEY_NETWORK_PROBES_SUCCEEDED_BITMASK | KEY_NETWORK_VALIDATION_RESULT - // See NetworkMonitor.EvaluationState#getNetworkTestResult - final int validationResult = result & NOTIFY_NETWORK_TESTED_VALIDATION_RESULT_MASK; - final int probesSucceeded = result & NOTIFY_NETWORK_TESTED_SUCCESSFUL_PROBES_MASK; + private NetworkTestResultParcelable matchNetworkTestResultParcelable(final int result, + final int probesSucceeded) { + return matchNetworkTestResultParcelable(result, probesSucceeded, null /* redirectUrl */); + } - return argThat(bundle -> - bundle.containsKey(KEY_NETWORK_PROBES_ATTEMPTED_BITMASK) - && bundle.containsKey(KEY_NETWORK_PROBES_SUCCEEDED_BITMASK) - && (bundle.getInt(KEY_NETWORK_PROBES_SUCCEEDED_BITMASK) & probesSucceeded) - == probesSucceeded - && bundle.containsKey(KEY_NETWORK_VALIDATION_RESULT) - && (bundle.getInt(KEY_NETWORK_VALIDATION_RESULT) & validationResult) - == validationResult); + private NetworkTestResultParcelable matchNetworkTestResultParcelable(final int result, + final int probesSucceeded, String redirectUrl) { + // TODO: also verify probesAttempted + return argThat(p -> p.result == result && p.probesSucceeded == probesSucceeded + && Objects.equals(p.redirectUrl, redirectUrl)); } - private PersistableBundle bundleForDnsDataStall(final int timeoutCount) { - return argThat(bundle -> - bundle.containsKey(KEY_DNS_CONSECUTIVE_TIMEOUTS) - && bundle.getInt(KEY_DNS_CONSECUTIVE_TIMEOUTS) == timeoutCount); + private DataStallReportParcelable matchDnsDataStallParcelable(final int timeoutCount) { + return argThat(p -> p.detectionMethod == ConstantsShim.DETECTION_METHOD_DNS_EVENTS + && p.dnsConsecutiveTimeouts == timeoutCount); } - private PersistableBundle bundleForTcpDataStall() { - return argThat(bundle -> - bundle.containsKey(KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS) - && bundle.containsKey(KEY_TCP_PACKET_FAIL_RATE)); + private DataStallReportParcelable matchTcpDataStallParcelable() { + return argThat(p -> p.detectionMethod == ConstantsShim.DETECTION_METHOD_TCP_METRICS); } } |