Tethering: ignore duplicate upstream changed event
Different network objects with the same network ID should be
treated as the same network.
chooseUpstreamType compares the current upstream and new upstream
by object (comparison operator) instead of network id
(Network#equals). This implies that different objects with the
same network id still trigger upstream changed report.
Since this commit, reportUpstreamChanged is only called when upsteam
has really changed (connected, switched or disconnected) in
chooseUpstreamType. In previous code, reportUpstreamChanged is also
called if EVENT_ON_CAPABILITIES is called when the upstream is the
same but its capabilities changed.
The NotificationUpdater#onUpstreamCapabilitiesChanged method only
needs to be called by chooseUpstreamType when it chooses a new
upstream. If the upstream remains the same but its capabilities
change, the EVENT_ON_CAPABILITIES will call
onUpstreamCapabilitiesChanged.
Bug: 243516306
Test: atest TetheringTest
Change-Id: I009383a61a5fabd249ba78fcffd524a5bbe4602e
diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index e5f644e..b371178 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -157,6 +157,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executor;
@@ -1845,11 +1846,12 @@
setUpstreamNetwork(ns);
final Network newUpstream = (ns != null) ? ns.network : null;
- if (mTetherUpstream != newUpstream) {
+ if (!Objects.equals(mTetherUpstream, newUpstream)) {
mTetherUpstream = newUpstream;
reportUpstreamChanged(mTetherUpstream);
- // Need to notify capabilities change after upstream network changed because new
- // network's capabilities should be checked every time.
+ // Need to notify capabilities change after upstream network changed because
+ // upstream may switch to existing network which don't have
+ // UpstreamNetworkMonitor.EVENT_ON_CAPABILITIES callback.
mNotificationUpdater.onUpstreamCapabilitiesChanged(
(ns != null) ? ns.networkCapabilities : null);
}
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index 687171b..2280e38 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -2685,10 +2685,9 @@
final UpstreamNetworkState upstreamState2 = buildMobileIPv4UpstreamState();
initTetheringUpstream(upstreamState2);
stateMachine.chooseUpstreamType(true);
- // Bug: duplicated upstream change event.
- mTetheringEventCallback.expectUpstreamChanged(upstreamState2.network);
- inOrder.verify(mNotificationUpdater)
- .onUpstreamCapabilitiesChanged(upstreamState2.networkCapabilities);
+ // Expect that no upstream change event and capabilities changed event.
+ mTetheringEventCallback.assertNoUpstreamChangeCallback();
+ inOrder.verify(mNotificationUpdater, never()).onUpstreamCapabilitiesChanged(any());
// Set the upstream with the same network ID but different object and different capability.
final UpstreamNetworkState upstreamState3 = buildMobileIPv4UpstreamState();
@@ -2696,11 +2695,13 @@
upstreamState3.networkCapabilities.addCapability(NET_CAPABILITY_VALIDATED);
initTetheringUpstream(upstreamState3);
stateMachine.chooseUpstreamType(true);
- // Bug: duplicated upstream change event.
- mTetheringEventCallback.expectUpstreamChanged(upstreamState3.network);
+ // Expect that no upstream change event and capabilities changed event.
+ mTetheringEventCallback.assertNoUpstreamChangeCallback();
+ stateMachine.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState3);
inOrder.verify(mNotificationUpdater)
.onUpstreamCapabilitiesChanged(upstreamState3.networkCapabilities);
+
// Lose upstream.
initTetheringUpstream(null);
stateMachine.chooseUpstreamType(true);