From efe2738735a99bfff9b3243ee976a5090c51f502 Mon Sep 17 00:00:00 2001 From: Yiyi Shen Date: Thu, 13 Mar 2025 17:32:38 +0800 Subject: [PATCH] Avoid unintended pref removal after CSIP main/member switch Test: atest Bug: 394765052 Flag: EXEMPT small fix Change-Id: I5b4646ff9ee092b851d3f8d5cc3ac2030f189430 --- .../bluetooth/BluetoothDeviceUpdater.java | 26 ++++++++--- .../bluetooth/BluetoothDeviceUpdaterTest.java | 44 ++++++++++++++++++- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/com/android/settings/bluetooth/BluetoothDeviceUpdater.java b/src/com/android/settings/bluetooth/BluetoothDeviceUpdater.java index 965bb94f255..6cbb9c20b79 100644 --- a/src/com/android/settings/bluetooth/BluetoothDeviceUpdater.java +++ b/src/com/android/settings/bluetooth/BluetoothDeviceUpdater.java @@ -37,9 +37,8 @@ import com.android.settingslib.core.instrumentation.MetricsFeatureProvider; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.List; -import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Update the bluetooth devices. It gets bluetooth event from {@link LocalBluetoothManager} using @@ -53,7 +52,7 @@ public abstract class BluetoothDeviceUpdater implements BluetoothCallback, LocalBluetoothProfileManager.ServiceListener { protected final MetricsFeatureProvider mMetricsFeatureProvider; protected final DevicePreferenceCallback mDevicePreferenceCallback; - protected final Map mPreferenceMap; + protected final ConcurrentHashMap mPreferenceMap; protected Context mContext; protected Context mPrefContext; @VisibleForTesting @@ -79,7 +78,7 @@ public abstract class BluetoothDeviceUpdater implements BluetoothCallback, int metricsCategory) { mContext = context; mDevicePreferenceCallback = devicePreferenceCallback; - mPreferenceMap = new HashMap<>(); + mPreferenceMap = new ConcurrentHashMap<>(); mLocalManager = localManager; mMetricsCategory = metricsCategory; mMetricsFeatureProvider = FeatureFactory.getFeatureFactory().getMetricsFeatureProvider(); @@ -155,11 +154,13 @@ public abstract class BluetoothDeviceUpdater implements BluetoothCallback, @Override public void onDeviceAdded(CachedBluetoothDevice cachedDevice) { + Log.d(getLogTag(), "onDeviceAdded() device: " + cachedDevice.getName()); update(cachedDevice); } @Override public void onDeviceDeleted(CachedBluetoothDevice cachedDevice) { + Log.d(getLogTag(), "onDeviceDeleted() device: " + cachedDevice.getName()); // Used to combine the hearing aid entries just after pairing. Once both the hearing aids // get connected and their hiSyncId gets populated, this gets called for one of the // 2 hearing aids so that only one entry in the connected devices list will be seen. @@ -278,8 +279,21 @@ public abstract class BluetoothDeviceUpdater implements BluetoothCallback, private void removePreference(BluetoothDevice device) { if (mPreferenceMap.containsKey(device)) { - mDevicePreferenceCallback.onDeviceRemoved(mPreferenceMap.get(device)); - mPreferenceMap.remove(device); + if (mPreferenceMap.get(device) instanceof BluetoothDevicePreference preference) { + BluetoothDevice prefDevice = preference.getBluetoothDevice().getDevice(); + // For CSIP device, when it {@link CachedBluetoothDevice}#switchMemberDeviceContent, + // it will change its mDevice and lead to the hashcode change for this preference. + // This will cause unintended remove preference, see b/394765052 + if (device.equals(prefDevice) || !mPreferenceMap.containsKey(prefDevice)) { + mDevicePreferenceCallback.onDeviceRemoved(preference); + } else { + Log.w(getLogTag(), "Inconsistent key and preference when removePreference"); + } + mPreferenceMap.remove(device); + } else { + mDevicePreferenceCallback.onDeviceRemoved(mPreferenceMap.get(device)); + mPreferenceMap.remove(device); + } } } diff --git a/tests/robotests/src/com/android/settings/bluetooth/BluetoothDeviceUpdaterTest.java b/tests/robotests/src/com/android/settings/bluetooth/BluetoothDeviceUpdaterTest.java index 4bfe3359255..afb51239d7c 100644 --- a/tests/robotests/src/com/android/settings/bluetooth/BluetoothDeviceUpdaterTest.java +++ b/tests/robotests/src/com/android/settings/bluetooth/BluetoothDeviceUpdaterTest.java @@ -39,6 +39,8 @@ import com.android.settingslib.bluetooth.CachedBluetoothDevice; import com.android.settingslib.bluetooth.CachedBluetoothDeviceManager; import com.android.settingslib.bluetooth.LocalBluetoothManager; +import com.google.common.collect.ImmutableList; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -99,6 +101,7 @@ public class BluetoothDeviceUpdaterTest { when(mLocalManager.getCachedDeviceManager()).thenReturn(mCachedDeviceManager); when(mCachedDeviceManager.getCachedDevicesCopy()).thenReturn(mCachedDevices); when(mCachedBluetoothDevice.getAddress()).thenReturn(MAC_ADDRESS); + when(mBluetoothDevice.getAddress()).thenReturn(MAC_ADDRESS); when(mSubBluetoothDevice.getAddress()).thenReturn(SUB_MAC_ADDRESS); when(mCachedBluetoothDevice.getDrawableWithDescription()).thenReturn(pairs); @@ -252,7 +255,7 @@ public class BluetoothDeviceUpdaterTest { } @Test - public void havePreference_refreshPreference() { + public void refreshPreference_havePreference_refresh() { mBluetoothDeviceUpdater.mPreferenceMap.put(mBluetoothDevice, mPreference); mPreference.setTitle("fake_name"); @@ -262,6 +265,45 @@ public class BluetoothDeviceUpdaterTest { assertThat(mPreference.getTitle()).isEqualTo(TEST_NAME); } + @Test + public void refreshPreference_staledPreference_remove() { + mBluetoothDeviceUpdater.mPreferenceMap.put(mBluetoothDevice, mPreference); + mPreference.setTitle("fake_name"); + + when(mCachedBluetoothDevice.getName()).thenReturn(TEST_NAME); + when(mCachedDeviceManager.getCachedDevicesCopy()).thenReturn(ImmutableList.of()); + mBluetoothDeviceUpdater.refreshPreference(); + + verify(mDevicePreferenceCallback).onDeviceRemoved(mPreference); + assertThat(mBluetoothDeviceUpdater.mPreferenceMap.containsKey(mBluetoothDevice)).isFalse(); + } + + @Test + public void refreshPreference_inconsistentPreference_doNothing() { + when(mCachedBluetoothDevice.getDevice()).thenReturn(mSubBluetoothDevice); + mBluetoothDeviceUpdater.mPreferenceMap.put(mBluetoothDevice, mPreference); + mBluetoothDeviceUpdater.mPreferenceMap.put(mSubBluetoothDevice, mPreference); + + when(mCachedDeviceManager.getCachedDevicesCopy()).thenReturn( + ImmutableList.of(mSubCachedBluetoothDevice)); + mBluetoothDeviceUpdater.refreshPreference(); + + verify(mDevicePreferenceCallback, never()).onDeviceRemoved(mPreference); + assertThat(mBluetoothDeviceUpdater.mPreferenceMap.containsKey(mBluetoothDevice)).isFalse(); + } + + @Test + public void refreshPreference_staledInconsistentPreference_remove() { + when(mCachedBluetoothDevice.getDevice()).thenReturn(mSubBluetoothDevice); + mBluetoothDeviceUpdater.mPreferenceMap.put(mBluetoothDevice, mPreference); + + when(mCachedDeviceManager.getCachedDevicesCopy()).thenReturn(ImmutableList.of()); + mBluetoothDeviceUpdater.refreshPreference(); + + verify(mDevicePreferenceCallback).onDeviceRemoved(mPreference); + assertThat(mBluetoothDeviceUpdater.mPreferenceMap.containsKey(mBluetoothDevice)).isFalse(); + } + public static class TestBluetoothDeviceUpdater extends BluetoothDeviceUpdater { public TestBluetoothDeviceUpdater(Context context, DevicePreferenceCallback devicePreferenceCallback,