From 812baf3e3a5bd0a7cfdaf4b96c8ae190a36b7d30 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Fri, 24 Apr 2020 12:19:37 +0000 Subject: 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 --- .../src/android/net/DataStallReportParcelable.aidl | 48 ++++++++++++++++++++++ .../src/android/net/INetworkMonitorCallbacks.aidl | 22 +++++----- .../android/net/NetworkTestResultParcelable.aidl | 45 ++++++++++++++++++++ .../android/net/dhcp/DhcpServingParamsParcel.aidl | 2 +- 4 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 common/networkstackclient/src/android/net/DataStallReportParcelable.aidl create mode 100644 common/networkstackclient/src/android/net/NetworkTestResultParcelable.aidl (limited to 'common/networkstackclient/src') 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; } -- cgit v1.2.3