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

9 files changed, 167 insertions, 22 deletions

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;
 }