Merge "Address comments on NetworkStack AIDL v6" am: 0e93810453

Change-Id: I755d14877a65b359c0837cab1488875d7238440e

services/core/java/com/android/server/ConnectivityService.java

@@ -18,6 +18,14 @@ package com.android.server;
 
 import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
+import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_PROBES_ATTEMPTED_BITMASK;
+import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_PROBES_SUCCEEDED_BITMASK;
+import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_VALIDATION_RESULT;
+import static android.net.ConnectivityDiagnosticsManager.DataStallReport.DETECTION_METHOD_DNS_EVENTS;
+import static android.net.ConnectivityDiagnosticsManager.DataStallReport.DETECTION_METHOD_TCP_METRICS;
+import static android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_DNS_CONSECUTIVE_TIMEOUTS;
+import static android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS;
+import static android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_TCP_PACKET_FAIL_RATE;
 import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;
 import static android.net.ConnectivityManager.NETID_UNSET;
 import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC;
@@ -72,6 +80,7 @@ import android.net.ConnectionInfo;
 import android.net.ConnectivityDiagnosticsManager.ConnectivityReport;
 import android.net.ConnectivityDiagnosticsManager.DataStallReport;
 import android.net.ConnectivityManager;
+import android.net.DataStallReportParcelable;
 import android.net.ICaptivePortal;
 import android.net.IConnectivityDiagnosticsCallback;
 import android.net.IConnectivityManager;
@@ -108,6 +117,7 @@ import android.net.NetworkSpecifier;
 import android.net.NetworkStack;
 import android.net.NetworkStackClient;
 import android.net.NetworkState;
+import android.net.NetworkTestResultParcelable;
 import android.net.NetworkUtils;
 import android.net.NetworkWatchlistManager;
 import android.net.PrivateDnsConfigParcel;
@@ -2812,14 +2822,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
                     handleNetworkTested(nai, results.mTestResult,
                             (results.mRedirectUrl == null) ? "" : results.mRedirectUrl);
-
-                    // Invoke ConnectivityReport generation for this Network test event.
-                    final Message m =
-                            mConnectivityDiagnosticsHandler.obtainMessage(
-                                    ConnectivityDiagnosticsHandler.EVENT_NETWORK_TESTED,
-                                    new ConnectivityReportEvent(results.mTimestampMillis, nai));
-                    m.setData(msg.getData());
-                    mConnectivityDiagnosticsHandler.sendMessage(m);
                     break;
                 }
                 case EVENT_PROVISIONING_NOTIFICATION: {
@@ -2998,23 +3000,33 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
         @Override
         public void notifyNetworkTested(int testResult, @Nullable String redirectUrl) {
-            notifyNetworkTestedWithExtras(testResult, redirectUrl, SystemClock.elapsedRealtime(),
+            // Legacy version of notifyNetworkTestedWithExtras.
-                    PersistableBundle.EMPTY);
+            // Would only be called if the system has a NetworkStack module older than the
+            // framework, which does not happen in practice.
         }
 
         @Override
-        public void notifyNetworkTestedWithExtras(
+        public void notifyNetworkTestedWithExtras(NetworkTestResultParcelable p) {
-                int testResult,
+            final Message msg = mTrackerHandler.obtainMessage(
-                @Nullable String redirectUrl,
+                    EVENT_NETWORK_TESTED,
-                long timestampMillis,
+                    new NetworkTestedResults(
-                @NonNull PersistableBundle extras) {
+                            mNetId, p.result, p.timestampMillis, p.redirectUrl));
-            final Message msg =
-                    mTrackerHandler.obtainMessage(
-                            EVENT_NETWORK_TESTED,
-                            new NetworkTestedResults(
-                                    mNetId, testResult, timestampMillis, redirectUrl));
-            msg.setData(new Bundle(extras));
             mTrackerHandler.sendMessage(msg);
+
+            // Invoke ConnectivityReport generation for this Network test event.
+            final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(mNetId);
+            if (nai == null) return;
+            final Message m = mConnectivityDiagnosticsHandler.obtainMessage(
+                    ConnectivityDiagnosticsHandler.EVENT_NETWORK_TESTED,
+                    new ConnectivityReportEvent(p.timestampMillis, nai));
+
+            final PersistableBundle extras = new PersistableBundle();
+            extras.putInt(KEY_NETWORK_VALIDATION_RESULT, p.result);
+            extras.putInt(KEY_NETWORK_PROBES_SUCCEEDED_BITMASK, p.probesSucceeded);
+            extras.putInt(KEY_NETWORK_PROBES_ATTEMPTED_BITMASK, p.probesAttempted);
+
+            m.setData(new Bundle(extras));
+            mConnectivityDiagnosticsHandler.sendMessage(m);
         }
 
         @Override
@@ -3063,12 +3075,25 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
 
         @Override
-        public void notifyDataStallSuspected(
+        public void notifyDataStallSuspected(DataStallReportParcelable p) {
-                long timestampMillis, int detectionMethod, PersistableBundle extras) {
+            final Message msg = mConnectivityDiagnosticsHandler.obtainMessage(
-            final Message msg =
+                    ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED,
-                    mConnectivityDiagnosticsHandler.obtainMessage(
+                    p.detectionMethod, mNetId, p.timestampMillis);
-                            ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED,
+
-                            detectionMethod, mNetId, timestampMillis);
+            final PersistableBundle extras = new PersistableBundle();
+            switch (p.detectionMethod) {
+                case DETECTION_METHOD_DNS_EVENTS:
+                    extras.putInt(KEY_DNS_CONSECUTIVE_TIMEOUTS, p.dnsConsecutiveTimeouts);
+                    break;
+                case DETECTION_METHOD_TCP_METRICS:
+                    extras.putInt(KEY_TCP_PACKET_FAIL_RATE, p.tcpPacketFailRate);
+                    extras.putInt(KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS,
+                            p.tcpMetricsCollectionPeriodMillis);
+                    break;
+                default:
+                    log("Unknown data stall detection method, ignoring: " + p.detectionMethod);
+                    return;
+            }
             msg.setData(new Bundle(extras));
 
             // NetworkStateTrackerHandler currently doesn't take any actions based on data

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -144,6 +144,7 @@ import android.net.ConnectivityManager.PacketKeepalive;
 import android.net.ConnectivityManager.PacketKeepaliveCallback;
 import android.net.ConnectivityManager.TooManyRequestsException;
 import android.net.ConnectivityThread;
+import android.net.DataStallReportParcelable;
 import android.net.IConnectivityDiagnosticsCallback;
 import android.net.IDnsResolver;
 import android.net.IIpConnectivityMetrics;
@@ -170,6 +171,7 @@ import android.net.NetworkSpecifier;
 import android.net.NetworkStack;
 import android.net.NetworkStackClient;
 import android.net.NetworkState;
+import android.net.NetworkTestResultParcelable;
 import android.net.NetworkUtils;
 import android.net.ProxyInfo;
 import android.net.ResolverParamsParcel;
@@ -196,7 +198,6 @@ import android.os.Looper;
 import android.os.Parcel;
 import android.os.ParcelFileDescriptor;
 import android.os.Parcelable;
-import android.os.PersistableBundle;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.SystemClock;
@@ -580,14 +581,6 @@ public class ConnectivityServiceTest {
     }
 
     private class TestNetworkAgentWrapper extends NetworkAgentWrapper {
-        private static final int VALIDATION_RESULT_BASE = NETWORK_VALIDATION_PROBE_DNS
-                | NETWORK_VALIDATION_PROBE_HTTP
-                | NETWORK_VALIDATION_PROBE_HTTPS;
-        private static final int VALIDATION_RESULT_VALID = VALIDATION_RESULT_BASE
-                | NETWORK_VALIDATION_RESULT_VALID;
-        private static final int VALIDATION_RESULT_PARTIAL = VALIDATION_RESULT_BASE
-                | NETWORK_VALIDATION_PROBE_FALLBACK
-                | NETWORK_VALIDATION_RESULT_PARTIAL;
         private static final int VALIDATION_RESULT_INVALID = 0;
 
         private static final long DATA_STALL_TIMESTAMP = 10L;
@@ -595,12 +588,10 @@ public class ConnectivityServiceTest {
 
         private INetworkMonitor mNetworkMonitor;
         private INetworkMonitorCallbacks mNmCallbacks;
-        private int mNmValidationResult = VALIDATION_RESULT_BASE;
+        private int mNmValidationResult = VALIDATION_RESULT_INVALID;
         private int mProbesCompleted;
         private int mProbesSucceeded;
         private String mNmValidationRedirectUrl = null;
-        private PersistableBundle mValidationExtras = PersistableBundle.EMPTY;
-        private PersistableBundle mDataStallExtras = PersistableBundle.EMPTY;
         private boolean mNmProvNotificationRequested = false;
 
         private final ConditionVariable mNetworkStatusReceived = new ConditionVariable();
@@ -668,8 +659,13 @@ public class ConnectivityServiceTest {
             }
 
             mNmCallbacks.notifyProbeStatusChanged(mProbesCompleted, mProbesSucceeded);
-            mNmCallbacks.notifyNetworkTestedWithExtras(
+            final NetworkTestResultParcelable p = new NetworkTestResultParcelable();
-                    mNmValidationResult, mNmValidationRedirectUrl, TIMESTAMP, mValidationExtras);
+            p.result = mNmValidationResult;
+            p.probesAttempted = mProbesCompleted;
+            p.probesSucceeded = mProbesSucceeded;
+            p.redirectUrl = mNmValidationRedirectUrl;
+            p.timestampMillis = TIMESTAMP;
+            mNmCallbacks.notifyNetworkTestedWithExtras(p);
 
             if (mNmValidationRedirectUrl != null) {
                 mNmCallbacks.showProvisioningNotification(
@@ -751,9 +747,9 @@ public class ConnectivityServiceTest {
         }
 
         void setNetworkValid(boolean isStrictMode) {
-            mNmValidationResult = VALIDATION_RESULT_VALID;
+            mNmValidationResult = NETWORK_VALIDATION_RESULT_VALID;
             mNmValidationRedirectUrl = null;
-            int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTP;
+            int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS;
             if (isStrictMode) {
                 probesSucceeded |= NETWORK_VALIDATION_PROBE_PRIVDNS;
             }
@@ -765,8 +761,9 @@ public class ConnectivityServiceTest {
         void setNetworkInvalid(boolean isStrictMode) {
             mNmValidationResult = VALIDATION_RESULT_INVALID;
             mNmValidationRedirectUrl = null;
-            int probesCompleted = VALIDATION_RESULT_BASE;
+            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS
-            int probesSucceeded = VALIDATION_RESULT_INVALID;
+                    | NETWORK_VALIDATION_PROBE_HTTP;
+            int probesSucceeded = 0;
             // If the isStrictMode is true, it means the network is invalid when NetworkMonitor
             // tried to validate the private DNS but failed.
             if (isStrictMode) {
@@ -782,7 +779,7 @@ public class ConnectivityServiceTest {
             mNmValidationRedirectUrl = redirectUrl;
             // Suppose the portal is found when NetworkMonitor probes NETWORK_VALIDATION_PROBE_HTTP
             // in the beginning, so the NETWORK_VALIDATION_PROBE_HTTPS hasn't probed yet.
-            int probesCompleted = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;
+            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP;
             int probesSucceeded = VALIDATION_RESULT_INVALID;
             if (isStrictMode) {
                 probesCompleted |= NETWORK_VALIDATION_PROBE_PRIVDNS;
@@ -791,18 +788,20 @@ public class ConnectivityServiceTest {
         }
 
         void setNetworkPartial() {
-            mNmValidationResult = VALIDATION_RESULT_PARTIAL;
+            mNmValidationResult = NETWORK_VALIDATION_RESULT_PARTIAL;
             mNmValidationRedirectUrl = null;
-            int probesCompleted = VALIDATION_RESULT_BASE;
+            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS
-            int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;
+                    | NETWORK_VALIDATION_PROBE_FALLBACK;
+            int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK;
             setProbesStatus(probesCompleted, probesSucceeded);
         }
 
         void setNetworkPartialValid(boolean isStrictMode) {
             setNetworkPartial();
-            mNmValidationResult |= VALIDATION_RESULT_VALID;
+            mNmValidationResult |= NETWORK_VALIDATION_RESULT_VALID;
-            int probesCompleted = VALIDATION_RESULT_BASE;
+            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS
-            int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;
+                    | NETWORK_VALIDATION_PROBE_HTTP;
+            int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP;
             // Suppose the partial network cannot pass the private DNS validation as well, so only
             // add NETWORK_VALIDATION_PROBE_DNS in probesCompleted but not probesSucceeded.
             if (isStrictMode) {
@@ -838,8 +837,10 @@ public class ConnectivityServiceTest {
         }
 
         void notifyDataStallSuspected() throws Exception {
-            mNmCallbacks.notifyDataStallSuspected(
+            final DataStallReportParcelable p = new DataStallReportParcelable();
-                    DATA_STALL_TIMESTAMP, DATA_STALL_DETECTION_METHOD, mDataStallExtras);
+            p.detectionMethod = DATA_STALL_DETECTION_METHOD;
+            p.timestampMillis = DATA_STALL_TIMESTAMP;
+            mNmCallbacks.notifyDataStallSuspected(p);
         }
     }