Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit efe27387 authored by Yiyi Shen's avatar Yiyi Shen
Browse files

Avoid unintended pref removal after CSIP main/member switch 

Test: atest
Bug: 394765052
Flag: EXEMPT small fix
Change-Id: I5b4646ff9ee092b851d3f8d5cc3ac2030f189430
parent c8d9ab2c

src/com/android/settings/bluetooth/BluetoothDeviceUpdater.java

+20 −6
Original line number Diff line number Diff line
@@ -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<BluetoothDevice, Preference> mPreferenceMap;

    protected final ConcurrentHashMap<BluetoothDevice, Preference> 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,10 +279,23 @@ public abstract class BluetoothDeviceUpdater implements BluetoothCallback, 


    private void removePreference(BluetoothDevice device) {

        if (mPreferenceMap.containsKey(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);

            }

        }

    }



    /**

     * Get {@link CachedBluetoothDevice} from {@link Preference} and it is used to init

tests/robotests/src/com/android/settings/bluetooth/BluetoothDeviceUpdaterTest.java

+43 −1
Original line number Diff line number Diff line
@@ -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,