WifiConnectivityManager: stop caching ClientModeManagers
Instead, get it from ActiveModeWarden directly,
so that they don't accidentally go out of sync.
Also, call onActiveModeManagerRemoved callbacks
when a mode manager fails to start (onStartFailure).
If a failure occurs while switching roles, the
mode manager would have already been added, so
need to notify listeners of the removal.
Bug: 169772923
Test: atest FrameworksWifiTests
Test: toggle wifi a few times
Change-Id: I67c07f6cb3ab69d92302b7aaa623357c59486bbf
diff --git a/service/java/com/android/server/wifi/ActiveModeWarden.java b/service/java/com/android/server/wifi/ActiveModeWarden.java
index ec42313..89cbc24 100644
--- a/service/java/com/android/server/wifi/ActiveModeWarden.java
+++ b/service/java/com/android/server/wifi/ActiveModeWarden.java
@@ -243,18 +243,27 @@
}
private void invokeOnAddedCallbacks(@NonNull ActiveModeManager activeModeManager) {
+ if (mVerboseLoggingEnabled) {
+ Log.v(TAG, "ModeManager added " + activeModeManager);
+ }
for (ModeChangeCallback callback : mCallbacks) {
callback.onActiveModeManagerAdded(activeModeManager);
}
}
private void invokeOnRemovedCallbacks(@NonNull ActiveModeManager activeModeManager) {
+ if (mVerboseLoggingEnabled) {
+ Log.v(TAG, "ModeManager removed " + activeModeManager);
+ }
for (ModeChangeCallback callback : mCallbacks) {
callback.onActiveModeManagerRemoved(activeModeManager);
}
}
private void invokeOnRoleChangedCallbacks(@NonNull ActiveModeManager activeModeManager) {
+ if (mVerboseLoggingEnabled) {
+ Log.v(TAG, "ModeManager role changed " + activeModeManager);
+ }
for (ModeChangeCallback callback : mCallbacks) {
callback.onActiveModeManagerRoleChanged(activeModeManager);
}
@@ -880,6 +889,10 @@
mGraveyard.inter(softApManager);
updateBatteryStats();
mWifiController.sendMessage(WifiController.CMD_AP_START_FAILURE);
+ // onStartFailure can be called when switching between roles. So, remove
+ // update listeners.
+ Log.e(TAG, "SoftApManager start failed!" + softApManager);
+ invokeOnRemovedCallbacks(softApManager);
}
}
@@ -930,6 +943,10 @@
updateClientScanMode();
updateBatteryStats();
mWifiController.sendMessage(WifiController.CMD_STA_START_FAILURE);
+ // onStartFailure can be called when switching between roles. So, remove
+ // update listeners.
+ Log.e(TAG, "ClientModeManager start failed!" + clientModeManager);
+ invokeOnRemovedCallbacks(clientModeManager);
}
}
diff --git a/service/java/com/android/server/wifi/ConcreteClientModeManager.java b/service/java/com/android/server/wifi/ConcreteClientModeManager.java
index 21f0177..213ce06 100644
--- a/service/java/com/android/server/wifi/ConcreteClientModeManager.java
+++ b/service/java/com/android/server/wifi/ConcreteClientModeManager.java
@@ -1128,6 +1128,9 @@
@Override
public String toString() {
- return "ClientModeManager[" + mClientInterfaceName + "](" + mRole + ":" + mId + ")";
+ return "ConcreteClientModeManager{id=" + getId()
+ + " iface=" + getInterfaceName()
+ + " role=" + getRole()
+ + "}";
}
}
diff --git a/service/java/com/android/server/wifi/DefaultClientModeManager.java b/service/java/com/android/server/wifi/DefaultClientModeManager.java
index 4fd0ecf..c3e210e 100644
--- a/service/java/com/android/server/wifi/DefaultClientModeManager.java
+++ b/service/java/com/android/server/wifi/DefaultClientModeManager.java
@@ -64,6 +64,9 @@
@Override
public String toString() {
- return "ClientModeManager(Default)";
+ return "DefaultClientModeManager{id=" + getId()
+ + " iface=" + getInterfaceName()
+ + " role=" + getRole()
+ + "}";
}
}
diff --git a/service/java/com/android/server/wifi/SoftApManager.java b/service/java/com/android/server/wifi/SoftApManager.java
index 363ec9e..3c0206e 100644
--- a/service/java/com/android/server/wifi/SoftApManager.java
+++ b/service/java/com/android/server/wifi/SoftApManager.java
@@ -329,7 +329,10 @@
@Override
public String toString() {
- return "SoftApManager[" + mApInterfaceName + "](" + mRole + ":" + mId + ")";
+ return "SoftApManager{id=" + getId()
+ + " iface=" + getInterfaceName()
+ + " role=" + getRole()
+ + "}";
}
private String getCurrentStateName() {
diff --git a/service/java/com/android/server/wifi/WifiConnectivityManager.java b/service/java/com/android/server/wifi/WifiConnectivityManager.java
index 4920840..295dbf1 100644
--- a/service/java/com/android/server/wifi/WifiConnectivityManager.java
+++ b/service/java/com/android/server/wifi/WifiConnectivityManager.java
@@ -16,7 +16,6 @@
package com.android.server.wifi;
-import static com.android.server.wifi.ActiveModeManager.ClientInternetConnectivityRole;
import static com.android.server.wifi.ClientModeImpl.WIFI_WORK_SOURCE;
import android.annotation.NonNull;
@@ -42,7 +41,6 @@
import android.os.Process;
import android.os.WorkSource;
import android.util.ArrayMap;
-import android.util.ArraySet;
import android.util.LocalLog;
import android.util.Log;
@@ -210,10 +208,6 @@
private @DeviceMobilityState int mDeviceMobilityState =
WifiManager.DEVICE_MOBILITY_STATE_UNKNOWN;
- // Set of active client mode managers. This would be more than 1 only in case of
- // STA + STA use-cases.
- private final ArraySet<ClientModeManager> mClientModeManagers = new ArraySet<>();
-
// A helper to log debugging information in the local log buffer, which can
// be retrieved in bugreport.
private void localLog(String log) {
@@ -780,39 +774,23 @@
private class ModeChangeCallback implements ActiveModeWarden.ModeChangeCallback {
@Override
public void onActiveModeManagerAdded(@NonNull ActiveModeManager activeModeManager) {
- if (!(activeModeManager instanceof ClientModeManager)) return;
- if (mVerboseLoggingEnabled) {
- Log.v(TAG, "ModeManager added " + activeModeManager);
- }
- if (activeModeManager.getRole() instanceof ClientInternetConnectivityRole) {
- mClientModeManagers.add((ClientModeManager) activeModeManager);
- }
- setWifiEnabled(!mClientModeManagers.isEmpty());
+ update();
}
@Override
public void onActiveModeManagerRemoved(@NonNull ActiveModeManager activeModeManager) {
- if (!(activeModeManager instanceof ClientModeManager)) return;
- if (mVerboseLoggingEnabled) {
- Log.v(TAG, "ModeManager removed " + activeModeManager);
- }
- // No need to check for role when mode manager is removed.
- mClientModeManagers.remove(activeModeManager);
- setWifiEnabled(!mClientModeManagers.isEmpty());
+ update();
}
@Override
public void onActiveModeManagerRoleChanged(@NonNull ActiveModeManager activeModeManager) {
- if (!(activeModeManager instanceof ClientModeManager)) return;
- if (mVerboseLoggingEnabled) {
- Log.v(TAG, "ModeManager role changed " + activeModeManager);
- }
- if (activeModeManager.getRole() instanceof ClientInternetConnectivityRole) {
- mClientModeManagers.add((ClientModeManager) activeModeManager);
- } else {
- mClientModeManagers.remove(activeModeManager);
- }
- setWifiEnabled(!mClientModeManagers.isEmpty());
+ update();
+ }
+
+ private void update() {
+ List<ClientModeManager> primaryManagers =
+ mActiveModeWarden.getInternetConnectivityClientModeManagers();
+ setWifiEnabled(!primaryManagers.isEmpty());
}
}
@@ -893,10 +871,7 @@
private ClientModeManager getPrimaryClientModeManager() {
// There should only be 1 primary client mode manager at any point of time.
- return mClientModeManagers.stream()
- .filter(cm -> cm.getRole() == ActiveModeManager.ROLE_CLIENT_PRIMARY)
- .findFirst()
- .get();
+ return mActiveModeWarden.getPrimaryClientModeManager();
}
/** Initialize single scanning schedules, and validate them */
@@ -1803,9 +1778,11 @@
* Handler for WiFi state (connected/disconnected) changes
*/
public void handleConnectionStateChanged(ActiveModeManager activeModeManager, int state) {
- if (!mClientModeManagers.contains(activeModeManager)) {
- Log.w(TAG, "Ignoring call from non primary Mode Manager "
- + activeModeManager.getRole(), new Throwable());
+ List<ClientModeManager> primaryManagers =
+ mActiveModeWarden.getInternetConnectivityClientModeManagers();
+ if (!(primaryManagers.contains(activeModeManager))) {
+ Log.w(TAG, "Ignoring call from non primary Mode Manager " + activeModeManager,
+ new Throwable());
return;
}
localLog("handleConnectionStateChanged: state=" + stateToString(state));
@@ -1859,9 +1836,11 @@
*/
public void handleConnectionAttemptEnded(@NonNull ActiveModeManager activeModeManager,
int failureCode, @NonNull String bssid, @NonNull String ssid) {
- if (!mClientModeManagers.contains(activeModeManager)) {
- Log.w(TAG, "Ignoring call from non primary Mode Manager"
- + activeModeManager.getRole(), new Throwable());
+ List<ClientModeManager> primaryManagers =
+ mActiveModeWarden.getInternetConnectivityClientModeManagers();
+ if (!(primaryManagers.contains(activeModeManager))) {
+ Log.w(TAG, "Ignoring call from non primary Mode Manager " + activeModeManager,
+ new Throwable());
return;
}
if (failureCode == WifiMetrics.ConnectionEvent.FAILURE_NONE) {
@@ -2114,9 +2093,5 @@
pw.println("WifiConnectivityManager - Log End ----");
mOpenNetworkNotifier.dump(fd, pw, args);
mBssidBlocklistMonitor.dump(fd, pw, args);
- pw.println("mClientModeManager IDs: "
- + mClientModeManagers.stream()
- .map(m -> String.valueOf(m.getId()))
- .collect(Collectors.joining(", ")));
}
}
diff --git a/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java b/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java
index 9e049ec..519edf5 100644
--- a/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java
+++ b/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java
@@ -692,6 +692,7 @@
// now inject failure through the SoftApManager.Listener
mSoftApListener.onStartFailure();
mLooper.dispatchAll();
+ verify(mModeChangeCallback).onActiveModeManagerRemoved(mSoftApManager);
assertInDisabledState();
// clear the first call to start SoftApManager
reset(mSoftApManager, mBatteryStats, mModeChangeCallback);
@@ -709,6 +710,7 @@
// now inject a failure through the ScanOnlyModeManager.Listener
mClientListener.onStartFailure();
mLooper.dispatchAll();
+ verify(mModeChangeCallback).onActiveModeManagerRemoved(mClientModeManager);
assertInDisabledState();
verify(mBatteryStats).reportWifiOff();
}
@@ -723,6 +725,7 @@
// now inject failure through the SoftApManager.Listener
mSoftApListener.onStartFailure();
mLooper.dispatchAll();
+ verify(mModeChangeCallback).onActiveModeManagerRemoved(mSoftApManager);
verify(mBatteryStats).reportWifiOff();
}
@@ -752,6 +755,7 @@
// now inject failure through the SoftApManager.Listener
mSoftApListener.onStartFailure();
mLooper.dispatchAll();
+ verify(mModeChangeCallback).onActiveModeManagerRemoved(mSoftApManager);
verify(mBatteryStats).reportWifiOff();
verifyNoMoreInteractions(mWifiNative);
}
diff --git a/service/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java b/service/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
index 4020cf9..8e2eaf1 100644
--- a/service/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
+++ b/service/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
@@ -127,6 +127,7 @@
when(mContext.getSystemService(PowerManager.class)).thenReturn(powerManager);
when(powerManager.isInteractive()).thenReturn(false);
when(mPrimaryClientModeManager.getRole()).thenReturn(ActiveModeManager.ROLE_CLIENT_PRIMARY);
+ when(mActiveModeWarden.getPrimaryClientModeManager()).thenReturn(mPrimaryClientModeManager);
mWifiConnectivityManager = createConnectivityManager();
mWifiConnectivityManager.setTrustedConnectionAllowed(true);
@@ -3397,8 +3398,12 @@
mModeChangeCallbackCaptor.getValue();
assertNotNull(modeChangeCallback);
if (enable) {
+ when(mActiveModeWarden.getInternetConnectivityClientModeManagers())
+ .thenReturn(Arrays.asList(mPrimaryClientModeManager));
modeChangeCallback.onActiveModeManagerAdded(mPrimaryClientModeManager);
} else {
+ when(mActiveModeWarden.getInternetConnectivityClientModeManagers())
+ .thenReturn(Arrays.asList());
modeChangeCallback.onActiveModeManagerRemoved(mPrimaryClientModeManager);
}
}
