[Audiosharing] Avoid concurrent issue when adding preference

Bug: 394685943
Flag: EXEMPT bug fix
Test: atest
Change-Id: I0d50dee9a36fd719236b52ab633a18818c3e10bb

src/com/android/settings/connecteddevice/audiosharing/AudioSharingDeviceVolumeGroupController.java

@@ -51,10 +51,9 @@ import com.android.settingslib.bluetooth.LocalBluetoothManager;
 import com.android.settingslib.bluetooth.LocalBluetoothProfileManager;
 import com.android.settingslib.bluetooth.VolumeControlProfile;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -73,8 +72,9 @@ public class AudioSharingDeviceVolumeGroupController extends AudioSharingBasePre
     private final Executor mExecutor;
     private final ContentObserver mSettingsObserver;
     @Nullable private PreferenceGroup mPreferenceGroup;
-    private List<AudioSharingDeviceVolumePreference> mVolumePreferences = new ArrayList<>();
+    private CopyOnWriteArraySet<AudioSharingDeviceVolumePreference> mVolumePreferences =
-    private Map<Integer, Integer> mValueMap = new HashMap<Integer, Integer>();
+            new CopyOnWriteArraySet<>();
+    private ConcurrentHashMap<Integer, Integer> mValueMap = new ConcurrentHashMap<>();
     private AtomicBoolean mCallbacksRegistered = new AtomicBoolean(false);
 
     @VisibleForTesting
@@ -104,8 +104,8 @@ public class AudioSharingDeviceVolumeGroupController extends AudioSharingBasePre
                                             + finalVolume
                                             + " for "
                                             + device.getAnonymizedAddress());
-                            mContext.getMainExecutor()
+                            AudioSharingUtils.postOnMainThread(mContext,
-                                    .execute(() -> preference.setProgress(finalVolume));
+                                    () -> preference.setProgress(finalVolume));
                             break;
                         }
                     }
@@ -196,7 +196,9 @@ public class AudioSharingDeviceVolumeGroupController extends AudioSharingBasePre
         public void onChange(boolean selfChange) {
             Log.d(TAG, "onChange, fallback device group id has been changed");
             for (AudioSharingDeviceVolumePreference preference : mVolumePreferences) {
-                preference.setOrder(getPreferenceOrderForDevice(preference.getCachedDevice()));
+                int order = getPreferenceOrderForDevice(preference.getCachedDevice());
+                Log.d(TAG, "onChange: set order to " + order + " for " + preference);
+                AudioSharingUtils.postOnMainThread(mContext, () -> preference.setOrder(order));
             }
         }
     }
@@ -240,53 +242,55 @@ public class AudioSharingDeviceVolumeGroupController extends AudioSharingBasePre
 
     @Override
     public void onDeviceAdded(Preference preference) {
+        if (!(preference instanceof AudioSharingDeviceVolumePreference)) {
+            Log.d(TAG, "Skip onDeviceAdded, invalid preference type");
+            return;
+        }
+        var volumePref = (AudioSharingDeviceVolumePreference) preference;
+        mVolumePreferences.add(volumePref);
+        AudioSharingUtils.postOnMainThread(mContext, () -> {
             if (mPreferenceGroup != null) {
                 if (mPreferenceGroup.getPreferenceCount() == 0) {
                     mPreferenceGroup.setVisible(true);
                 }
-            mPreferenceGroup.addPreference(preference);
+                mPreferenceGroup.addPreference(volumePref);
             }
-        if (preference instanceof AudioSharingDeviceVolumePreference) {
+        });
-            var volumePref = (AudioSharingDeviceVolumePreference) preference;
         CachedBluetoothDevice cachedDevice = volumePref.getCachedDevice();
-            volumePref.setOrder(getPreferenceOrderForDevice(cachedDevice));
+        String address = cachedDevice.getDevice() == null ? "null"
-            mVolumePreferences.add(volumePref);
+                : cachedDevice.getDevice().getAnonymizedAddress();
-            if (volumePref.getProgress() > 0) return;
+        int order = getPreferenceOrderForDevice(cachedDevice);
+        Log.d(TAG, "onDeviceAdded: set order to " + order + " for " + address);
+        AudioSharingUtils.postOnMainThread(mContext, () -> volumePref.setOrder(order));
         int volume = mValueMap.getOrDefault(BluetoothUtils.getGroupId(cachedDevice), -1);
         // If the volume is invalid, try to get the volume from AudioManager.STREAM_MUSIC
         int finalVolume = getAudioVolumeIfNeeded(volume);
-            Log.d(
+        Log.d(TAG, "onDeviceAdded: set volume to " + finalVolume + " for " + address);
-                    TAG,
-                    "onDeviceAdded: set volume to "
-                            + finalVolume
-                            + " for "
-                            + cachedDevice.getDevice().getAnonymizedAddress());
         AudioSharingUtils.postOnMainThread(mContext, () -> volumePref.setProgress(finalVolume));
     }
-    }
 
     @Override
     public void onDeviceRemoved(Preference preference) {
-        if (mPreferenceGroup != null) {
+        if (!(preference instanceof AudioSharingDeviceVolumePreference)) {
-            mPreferenceGroup.removePreference(preference);
+            Log.d(TAG, "Skip onDeviceRemoved, invalid preference type");
-            if (mPreferenceGroup.getPreferenceCount() == 0) {
+            return;
-                mPreferenceGroup.setVisible(false);
         }
-        }
-        if (preference instanceof AudioSharingDeviceVolumePreference) {
         var volumePref = (AudioSharingDeviceVolumePreference) preference;
         if (mVolumePreferences.contains(volumePref)) {
             mVolumePreferences.remove(volumePref);
         }
-            CachedBluetoothDevice device = volumePref.getCachedDevice();
+        String address = volumePref.getCachedDevice().getDevice() == null ? "null"
-            Log.d(
+                : volumePref.getCachedDevice().getDevice().getAnonymizedAddress();
-                    TAG,
+        Log.d(TAG, "onDeviceRemoved: " + address);
-                    "onDeviceRemoved: "
+        AudioSharingUtils.postOnMainThread(mContext, () -> {
-                            + (device == null
+            if (mPreferenceGroup != null) {
-                                    ? "null"
+                mPreferenceGroup.removePreference(volumePref);
-                                    : device.getDevice().getAnonymizedAddress()));
+                if (mPreferenceGroup.getPreferenceCount() == 0) {
+                    mPreferenceGroup.setVisible(false);
                 }
             }
+        });
+    }
 
     @Override
     public void updateVisibility() {

tests/robotests/src/com/android/settings/connecteddevice/audiosharing/AudioSharingDeviceVolumeGroupControllerTest.java

@@ -309,6 +309,8 @@ public class AudioSharingDeviceVolumeGroupControllerTest {
         when(mPreference1.getProgress()).thenReturn(TEST_VOLUME_VALUE);
         mController.setPreferenceGroup(mPreferenceGroup);
         mController.onDeviceAdded(mPreference1);
+        shadowOf(Looper.getMainLooper()).idle();
+
         verify(mPreferenceGroup).setVisible(true);
         assertThat(mPreferenceGroup.isVisible()).isTrue();
     }
@@ -365,6 +367,8 @@ public class AudioSharingDeviceVolumeGroupControllerTest {
         mPreferenceGroup.addPreference(mPreference1);
         mController.setPreferenceGroup(mPreferenceGroup);
         mController.onDeviceRemoved(mPreference1);
+        shadowOf(Looper.getMainLooper()).idle();
+
         verify(mPreferenceGroup).setVisible(false);
         assertThat(mPreferenceGroup.isVisible()).isFalse();
     }