diff options
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 |