Send the probe status from NetworkMonitor to ConnectivityService

Provide a new callback for NetworkMonitor to notify
ConnectivityService that which probes are completed.

Bug: 113242081
Test: atest NetworkStackTests
Change-Id: I81f22002bc9a59c7c49d471e15e67722613d61a3

4 files changed, 113 insertions, 11 deletions

diff --git a/common/networkstackclient/Android.bp b/common/networkstackclient/Android.bp
index ccb3f45..c82f751 100644
--- a/common/networkstackclient/Android.bp
+++ b/common/networkstackclient/Android.bp
@@ -87,6 +87,6 @@ java_library {
     ],
     static_libs: [
         "ipmemorystore-aidl-interfaces-V3-java",
-        "networkstack-aidl-interfaces-V3-java",
+        "networkstack-aidl-interfaces-java",
     ],
 }
diff --git a/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl b/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl
index 2c61511..f8dcd6c 100644
--- a/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl
+++ b/common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl
@@ -26,4 +26,5 @@ oneway interface INetworkMonitorCallbacks {
     void notifyPrivateDnsConfigResolved(in PrivateDnsConfigParcel config);
     void showProvisioningNotification(String action, String packageName);
     void hideProvisioningNotification();
+    void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded);
 }
\ No newline at end of file
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 585e38e..7cabb1e 100644
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -368,7 +368,13 @@ public class NetworkMonitor extends StateMachine {
         } catch (RemoteException e) {
             version = 0;
         }
-        if (version == Build.VERSION_CODES.CUR_DEVELOPMENT) version = 0;
+        // The AIDL was freezed from Q beta 5 but it's unfreezing from R before releasing. In order
+        // to distinguish the behavior between R and Q beta 5 and before Q beta 5, add SDK and
+        // CODENAME check here. Basically, it's only expected to return 0 for Q beta 4 and below
+        // because the test result has changed.
+        if (Build.VERSION.SDK_INT == Build.VERSION_CODES.Q
+                && Build.VERSION.CODENAME.equals("REL")
+                && version == Build.VERSION_CODES.CUR_DEVELOPMENT) version = 0;
         return version;
     }
 
@@ -555,6 +561,14 @@ public class NetworkMonitor extends StateMachine {
         }
     }
 
+    private void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded) {
+        try {
+            mCallback.notifyProbeStatusChanged(probesCompleted, probesSucceeded);
+        } catch (RemoteException e) {
+            Log.e(TAG, "Error sending probe status", e);
+        }
+    }
+
     private void showProvisioningNotification(String action) {
         try {
             mCallback.showProvisioningNotification(action, mContext.getPackageName());
@@ -1020,6 +1034,9 @@ public class NetworkMonitor extends StateMachine {
                             handlePrivateDnsEvaluationFailure();
                             break;
                         }
+                        handlePrivateDnsEvaluationSuccess();
+                    } else {
+                        mEvaluationState.removeProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS);
                     }
 
                     // All good!
@@ -1051,8 +1068,6 @@ public class NetworkMonitor extends StateMachine {
             } catch (UnknownHostException uhe) {
                 mPrivateDnsConfig = null;
             }
-            mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS,
-                    (mPrivateDnsConfig != null) /* succeeded */);
         }
 
         private void notifyPrivateDnsConfigResolved() {
@@ -1063,7 +1078,14 @@ public class NetworkMonitor extends StateMachine {
             }
         }
 
+        private void handlePrivateDnsEvaluationSuccess() {
+            mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS,
+                    true /* succeeded */);
+        }
+
         private void handlePrivateDnsEvaluationFailure() {
+            mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS,
+                    false /* succeeded */);
             mEvaluationState.reportEvaluationResult(NETWORK_VALIDATION_RESULT_INVALID,
                     null /* redirectUrl */);
             // Queue up a re-evaluation with backoff.
@@ -1101,7 +1123,6 @@ public class NetworkMonitor extends StateMachine {
                         String.format("%dms - Error: %s", time, uhe.getMessage()));
             }
             logValidationProbe(time, PROBE_PRIVDNS, success ? DNS_SUCCESS : DNS_FAILURE);
-            mEvaluationState.noteProbeResult(NETWORK_VALIDATION_PROBE_PRIVDNS, success);
             return success;
         }
     }
@@ -2105,22 +2126,43 @@ public class NetworkMonitor extends StateMachine {
         // Indicates which probes have completed 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.
+        private int mProbeCompleted = 0;
         // The latest redirect URL.
         private String mRedirectUrl;
 
         protected void clearProbeResults() {
             mProbeResults = 0;
+            mProbeCompleted = 0;
         }
 
-        // Probe result for http probe should be updated from reportHttpProbeResult().
-        protected void noteProbeResult(int probeResult, boolean succeeded) {
-            if (succeeded) {
-                mProbeResults |= probeResult;
-            } else {
-                mProbeResults &= ~probeResult;
+        private void maybeNotifyProbeResults(@NonNull final Runnable modif) {
+            final int oldCompleted = mProbeCompleted;
+            final int oldResults = mProbeResults;
+            modif.run();
+            if (oldCompleted != mProbeCompleted || oldResults != mProbeResults) {
+                notifyProbeStatusChanged(mProbeCompleted, mProbeResults);
             }
         }
 
+        protected void removeProbeResult(final int probeResult) {
+            maybeNotifyProbeResults(() -> {
+                mProbeCompleted &= ~probeResult;
+                mProbeResults &= ~probeResult;
+            });
+        }
+
+        protected void noteProbeResult(final int probeResult, final boolean succeeded) {
+            maybeNotifyProbeResults(() -> {
+                mProbeCompleted |= probeResult;
+                if (succeeded) {
+                    mProbeResults |= probeResult;
+                } else {
+                    mProbeResults &= ~probeResult;
+                }
+            });
+        }
+
         protected void reportEvaluationResult(int result, @Nullable String redirectUrl) {
             mEvaluationResult = result;
             mRedirectUrl = redirectUrl;
@@ -2139,6 +2181,11 @@ public class NetworkMonitor extends StateMachine {
             }
             return mEvaluationResult | mProbeResults;
         }
+
+        @VisibleForTesting
+        protected int getProbeCompletedResult() {
+            return mProbeCompleted;
+        }
     }
 
     @VisibleForTesting
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
index e8bc8d2..6eeadf5 100644
--- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -173,6 +173,8 @@ public class NetworkMonitorTest {
     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_PRIVDNS_VALID = NETWORK_VALIDATION_PROBE_DNS
+            | NETWORK_VALIDATION_PROBE_HTTPS | NETWORK_VALIDATION_PROBE_PRIVDNS;
 
     private static final int RETURN_CODE_DNS_SUCCESS = 0;
     private static final int RETURN_CODE_DNS_TIMEOUT = 255;
@@ -800,6 +802,8 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
                 .notifyNetworkTested(eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS),
                         eq(null));
+        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID));
 
         // Verify dns query only get v4 address.
         resetCallbacks();
@@ -809,6 +813,10 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
                 .notifyNetworkTested(eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS),
                         eq(null));
+        // 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));
 
         // Verify dns query get both v4 and v6 address.
         resetCallbacks();
@@ -818,6 +826,37 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
                 .notifyNetworkTested(eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS),
                         eq(null));
+        verify(mCallbacks, never()).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID));
+    }
+
+    @Test
+    public void testProbeStatusChanged() throws Exception {
+        // Set no record in FakeDns and expect validation to fail.
+        setStatus(mHttpsConnection, 204);
+        setStatus(mHttpConnection, 204);
+
+        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)).notifyNetworkTested(
+                eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS), eq(null));
+        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS
+                | NETWORK_VALIDATION_PROBE_HTTPS));
+        // Fix DNS and retry, expect validation to succeed.
+        resetCallbacks();
+        mFakeDns.setAnswer("dns.google", new String[]{"2001:db8::1"}, TYPE_AAAA);
+
+        wnm.forceReevaluation(Process.myUid());
+        // 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))
+                .notifyNetworkTested(eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS),
+                        eq(null));
+        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID));
     }
 
     @Test
@@ -833,6 +872,9 @@ public class NetworkMonitorTest {
                 .notifyNetworkTested(
                         eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS),
                         eq(null));
+        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS
+                | NETWORK_VALIDATION_PROBE_HTTPS));
 
         // Fix DNS and retry, expect validation to succeed.
         resetCallbacks();
@@ -842,6 +884,8 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce())
                 .notifyNetworkTested(eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS),
                         eq(null));
+        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID));
 
         // Change configuration to an invalid DNS name, expect validation to fail.
         resetCallbacks();
@@ -853,6 +897,9 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS))
                 .notifyNetworkTested(eq(NETWORK_VALIDATION_PROBE_DNS
                         | NETWORK_VALIDATION_PROBE_HTTPS), eq(null));
+        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(NETWORK_VALIDATION_PROBE_DNS
+                | NETWORK_VALIDATION_PROBE_HTTPS));
 
         // Change configuration back to working again, but make private DNS not work.
         // Expect validation to fail.
@@ -864,6 +911,11 @@ public class NetworkMonitorTest {
                 .notifyNetworkTested(
                         eq(NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS),
                         eq(null));
+        // 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
+                | NETWORK_VALIDATION_PROBE_HTTPS));
 
         // Make private DNS work again. Expect validation to succeed.
         resetCallbacks();
@@ -872,6 +924,8 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce())
                 .notifyNetworkTested(
                         eq(VALIDATION_RESULT_VALID | NETWORK_VALIDATION_PROBE_PRIVDNS), eq(null));
+        verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)).notifyProbeStatusChanged(
+                eq(VALIDATION_RESULT_PRIVDNS_VALID), eq(VALIDATION_RESULT_PRIVDNS_VALID));
     }
 
     @Test