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

Commit bb47f320 authored by Chaohui Wang's avatar Chaohui Wang
Browse files

Fix bluetooth settings pairing page stuck

There is heavy work to do when add device to list in the
DeviceListPreferenceFragment, off load these work from main thread
to solve the issue.

Make devicePreferenceMap a ConcurrentHashMap to avoid potential race
condition.

Also no longer use getCachedPreference(key) since we not put anything
into the cache, the fallback flow is always used.

Also in BluetoothDevicePreference.onPreferenceAttributesChanged(), move
more heavy work to background thread.

Using System.currentTimeMillis() to sort devices could cause flaky
because System.currentTimeMillis() could be same for different device,
use AtomicInteger instead.

Fix: 286628533
Test: Following the step in bug
Change-Id: Ia9750adb6b4c1424d084381e9d7c2ca8e7912391
parent 61659799
Loading
Loading
Loading
Loading
+1 −11
Original line number Diff line number Diff line
@@ -128,7 +128,7 @@ public abstract class BluetoothDevicePairingDetailBase extends DeviceListPrefere
            if (device != null && mSelectedList.contains(device)) {
                setResult(RESULT_OK);
                finish();
            } else if (mDevicePreferenceMap.containsKey(cachedDevice)) {
            } else {
                onDeviceDeleted(cachedDevice);
            }
        }
@@ -175,8 +175,6 @@ public abstract class BluetoothDevicePairingDetailBase extends DeviceListPrefere
    public void updateContent(int bluetoothState) {
        switch (bluetoothState) {
            case BluetoothAdapter.STATE_ON:
                mDevicePreferenceMap.clear();
                clearPreferenceGroupCache();
                mBluetoothAdapter.enable();
                enableScanning();
                break;
@@ -187,14 +185,6 @@ public abstract class BluetoothDevicePairingDetailBase extends DeviceListPrefere
        }
    }

    /**
     * Clears all cached preferences in {@code preferenceGroup}.
     */
    private void clearPreferenceGroupCache() {
        cacheRemoveAllPrefs(mAvailableDevicesCategory);
        removeCachedPrefs(mAvailableDevicesCategory);
    }

    @VisibleForTesting
    void showBluetoothTurnedOnToast() {
        Toast.makeText(getContext(), R.string.connected_device_bluetooth_turned_on_toast,
+38 −26
Original line number Diff line number Diff line
/*
 * Copyright (C) 2008 The Android Open Source Project
 * Copyright (C) 2023 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
@@ -35,6 +35,8 @@ import android.view.View;
import android.widget.ImageView;

import androidx.annotation.IntDef;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import androidx.appcompat.app.AlertDialog;
import androidx.preference.Preference;
@@ -52,6 +54,7 @@ import java.lang.annotation.RetentionPolicy;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.atomic.AtomicInteger;

/**
 * BluetoothDevicePreference is the preference type used to display each remote
@@ -79,7 +82,9 @@ public final class BluetoothDevicePreference extends GearPreference {
    @VisibleForTesting
    BluetoothAdapter mBluetoothAdapter;
    private final boolean mShowDevicesWithoutNames;
    private final long mCurrentTime;
    @NonNull
    private static final AtomicInteger sNextId = new AtomicInteger();
    private final int mId;
    private final int mType;

    private AlertDialog mDisconnectDialog;
@@ -127,8 +132,9 @@ public final class BluetoothDevicePreference extends GearPreference {

        mCachedDevice = cachedDevice;
        mCallback = new BluetoothDevicePreferenceCallback();
        mCurrentTime = System.currentTimeMillis();
        mId = sNextId.getAndIncrement();
        mType = type;
        setVisible(false);

        onPreferenceAttributesChanged();
    }
@@ -229,35 +235,41 @@ public final class BluetoothDevicePreference extends GearPreference {

    @SuppressWarnings("FutureReturnValueIgnored")
    void onPreferenceAttributesChanged() {
        Pair<Drawable, String> pair = mCachedDevice.getDrawableWithDescription();
        setIcon(pair.first);
        contentDescription = pair.second;

        try {
            ThreadUtils.postOnBackgroundThread(() -> {
                @Nullable String name = mCachedDevice.getName();
                // Null check is done at the framework
                @Nullable String connectionSummary = getConnectionSummary();
                @NonNull Pair<Drawable, String> pair = mCachedDevice.getDrawableWithDescription();
                boolean isBusy = mCachedDevice.isBusy();
                // Device is only visible in the UI if it has a valid name besides MAC address or
                // when user allows showing devices without user-friendly name in developer settings
                boolean isVisible =
                        mShowDevicesWithoutNames || mCachedDevice.hasHumanReadableName();

                ThreadUtils.postOnMainThread(() -> {
                    /*
                     * The preference framework takes care of making sure the value has
                     * changed before proceeding. It will also call notifyChanged() if
                     * any preference info has changed from the previous value.
                     */
        setTitle(mCachedDevice.getName());
        try {
            ThreadUtils.postOnBackgroundThread(() -> {
                // Null check is done at the framework
                ThreadUtils.postOnMainThread(() -> setSummary(getConnectionSummary()));
            });
        } catch (RejectedExecutionException e) {
            Log.w(TAG, "Handler thread unavailable, skipping getConnectionSummary!");
        }
                    setTitle(name);
                    setSummary(connectionSummary);
                    setIcon(pair.first);
                    contentDescription = pair.second;
                    // Used to gray out the item
        setEnabled(!mCachedDevice.isBusy());

        // Device is only visible in the UI if it has a valid name besides MAC address or when user
        // allows showing devices without user-friendly name in developer settings
        setVisible(mShowDevicesWithoutNames || mCachedDevice.hasHumanReadableName());
                    setEnabled(!isBusy);
                    setVisible(isVisible);

                    // This could affect ordering, so notify that
                    if (mNeedNotifyHierarchyChanged) {
                        notifyHierarchyChanged();
                    }
                });
            });
        } catch (RejectedExecutionException e) {
            Log.w(TAG, "Handler thread unavailable, skipping getConnectionSummary!");
        }
    }

    @Override
@@ -311,7 +323,7 @@ public final class BluetoothDevicePreference extends GearPreference {
                return mCachedDevice
                        .compareTo(((BluetoothDevicePreference) another).mCachedDevice);
            case SortType.TYPE_FIFO:
                return mCurrentTime > ((BluetoothDevicePreference) another).mCurrentTime ? 1 : -1;
                return mId > ((BluetoothDevicePreference) another).mId ? 1 : -1;
            default:
                return super.compareTo(another);
        }
+0 −351
Original line number Diff line number Diff line
/*
 * Copyright (C) 2011 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.settings.bluetooth;

import android.bluetooth.BluetoothAdapter;
import android.bluetooth.BluetoothDevice;
import android.bluetooth.le.BluetoothLeScanner;
import android.bluetooth.le.ScanCallback;
import android.bluetooth.le.ScanFilter;
import android.bluetooth.le.ScanResult;
import android.bluetooth.le.ScanSettings;
import android.os.Bundle;
import android.os.SystemProperties;
import android.text.BidiFormatter;
import android.util.Log;

import androidx.annotation.VisibleForTesting;
import androidx.preference.Preference;
import androidx.preference.PreferenceCategory;
import androidx.preference.PreferenceGroup;

import com.android.settings.R;
import com.android.settings.dashboard.RestrictedDashboardFragment;
import com.android.settingslib.bluetooth.BluetoothCallback;
import com.android.settingslib.bluetooth.BluetoothDeviceFilter;
import com.android.settingslib.bluetooth.CachedBluetoothDevice;
import com.android.settingslib.bluetooth.CachedBluetoothDeviceManager;
import com.android.settingslib.bluetooth.LocalBluetoothManager;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;

/**
 * Parent class for settings fragments that contain a list of Bluetooth
 * devices.
 *
 * @see DevicePickerFragment
 */
// TODO: Refactor this fragment
public abstract class DeviceListPreferenceFragment extends
        RestrictedDashboardFragment implements BluetoothCallback {

    private static final String TAG = "DeviceListPreferenceFragment";

    private static final String KEY_BT_SCAN = "bt_scan";

    // Copied from BluetoothDeviceNoNamePreferenceController.java
    private static final String BLUETOOTH_SHOW_DEVICES_WITHOUT_NAMES_PROPERTY =
            "persist.bluetooth.showdeviceswithoutnames";

    private BluetoothDeviceFilter.Filter mFilter;
    private List<ScanFilter> mLeScanFilters;
    private ScanCallback mScanCallback;

    @VisibleForTesting
    protected boolean mScanEnabled;

    protected BluetoothDevice mSelectedDevice;

    protected BluetoothAdapter mBluetoothAdapter;
    protected LocalBluetoothManager mLocalManager;
    protected CachedBluetoothDeviceManager mCachedDeviceManager;

    @VisibleForTesting
    protected PreferenceGroup mDeviceListGroup;

    protected final HashMap<CachedBluetoothDevice, BluetoothDevicePreference> mDevicePreferenceMap =
            new HashMap<>();
    protected final List<BluetoothDevice> mSelectedList = new ArrayList<>();

    protected boolean mShowDevicesWithoutNames;

    public DeviceListPreferenceFragment(String restrictedKey) {
        super(restrictedKey);
        mFilter = BluetoothDeviceFilter.ALL_FILTER;
    }

    protected final void setFilter(BluetoothDeviceFilter.Filter filter) {
        mFilter = filter;
    }

    protected final void setFilter(int filterType) {
        mFilter = BluetoothDeviceFilter.getFilter(filterType);
    }

    /**
     * Sets the bluetooth device scanning filter with {@link ScanFilter}s. It will change to start
     * {@link BluetoothLeScanner} which will scan BLE device only.
     *
     * @param leScanFilters list of settings to filter scan result
     */
    protected void setFilter(List<ScanFilter> leScanFilters) {
        mFilter = null;
        mLeScanFilters = leScanFilters;
    }

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);

        mLocalManager = Utils.getLocalBtManager(getActivity());
        if (mLocalManager == null) {
            Log.e(TAG, "Bluetooth is not supported on this device");
            return;
        }
        mBluetoothAdapter = BluetoothAdapter.getDefaultAdapter();
        mCachedDeviceManager = mLocalManager.getCachedDeviceManager();
        mShowDevicesWithoutNames = SystemProperties.getBoolean(
                BLUETOOTH_SHOW_DEVICES_WITHOUT_NAMES_PROPERTY, false);

        initPreferencesFromPreferenceScreen();

        mDeviceListGroup = (PreferenceCategory) findPreference(getDeviceListKey());
    }

    /** find and update preference that already existed in preference screen */
    protected abstract void initPreferencesFromPreferenceScreen();

    @Override
    public void onStart() {
        super.onStart();
        if (mLocalManager == null || isUiRestricted()) return;

        mLocalManager.setForegroundActivity(getActivity());
        mLocalManager.getEventManager().registerCallback(this);
    }

    @Override
    public void onStop() {
        super.onStop();
        if (mLocalManager == null || isUiRestricted()) {
            return;
        }

        removeAllDevices();
        mLocalManager.setForegroundActivity(null);
        mLocalManager.getEventManager().unregisterCallback(this);
    }

    void removeAllDevices() {
        mDevicePreferenceMap.clear();
        mDeviceListGroup.removeAll();
    }

    void addCachedDevices() {
        Collection<CachedBluetoothDevice> cachedDevices =
                mCachedDeviceManager.getCachedDevicesCopy();
        for (CachedBluetoothDevice cachedDevice : cachedDevices) {
            onDeviceAdded(cachedDevice);
        }
    }

    @Override
    public boolean onPreferenceTreeClick(Preference preference) {
        if (KEY_BT_SCAN.equals(preference.getKey())) {
            startScanning();
            return true;
        }

        if (preference instanceof BluetoothDevicePreference) {
            BluetoothDevicePreference btPreference = (BluetoothDevicePreference) preference;
            CachedBluetoothDevice device = btPreference.getCachedDevice();
            mSelectedDevice = device.getDevice();
            mSelectedList.add(mSelectedDevice);
            onDevicePreferenceClick(btPreference);
            return true;
        }

        return super.onPreferenceTreeClick(preference);
    }

    protected void onDevicePreferenceClick(BluetoothDevicePreference btPreference) {
        btPreference.onClicked();
    }

    @Override
    public void onDeviceAdded(CachedBluetoothDevice cachedDevice) {
        if (mDevicePreferenceMap.get(cachedDevice) != null) {
            return;
        }

        // Prevent updates while the list shows one of the state messages
        if (mBluetoothAdapter.getState() != BluetoothAdapter.STATE_ON) {
            return;
        }

        if (mFilter != null && mFilter.matches(cachedDevice.getDevice())) {
            createDevicePreference(cachedDevice);
        }
    }

    void createDevicePreference(CachedBluetoothDevice cachedDevice) {
        if (mDeviceListGroup == null) {
            Log.w(TAG, "Trying to create a device preference before the list group/category "
                    + "exists!");
            return;
        }

        String key = cachedDevice.getDevice().getAddress();
        BluetoothDevicePreference preference = (BluetoothDevicePreference) getCachedPreference(key);

        if (preference == null) {
            preference = new BluetoothDevicePreference(getPrefContext(), cachedDevice,
                    mShowDevicesWithoutNames, BluetoothDevicePreference.SortType.TYPE_FIFO);
            preference.setKey(key);
            //Set hideSecondTarget is true if it's bonded device.
            preference.hideSecondTarget(true);
            mDeviceListGroup.addPreference(preference);
        }

        initDevicePreference(preference);
        mDevicePreferenceMap.put(cachedDevice, preference);
    }

    protected void initDevicePreference(BluetoothDevicePreference preference) {
        // Does nothing by default
    }

    @VisibleForTesting
    void updateFooterPreference(Preference myDevicePreference) {
        final BidiFormatter bidiFormatter = BidiFormatter.getInstance();

        myDevicePreference.setTitle(getString(
                R.string.bluetooth_footer_mac_message,
                bidiFormatter.unicodeWrap(mBluetoothAdapter.getAddress())));
    }

    @Override
    public void onDeviceDeleted(CachedBluetoothDevice cachedDevice) {
        BluetoothDevicePreference preference = mDevicePreferenceMap.remove(cachedDevice);
        if (preference != null) {
            mDeviceListGroup.removePreference(preference);
        }
    }

    @VisibleForTesting
    protected void enableScanning() {
        // BluetoothAdapter already handles repeated scan requests
        if (!mScanEnabled) {
            startScanning();
            mScanEnabled = true;
        }
    }

    @VisibleForTesting
    protected void disableScanning() {
        if (mScanEnabled) {
            stopScanning();
            mScanEnabled = false;
        }
    }

    @Override
    public void onScanningStateChanged(boolean started) {
        if (!started && mScanEnabled) {
            startScanning();
        }
    }

    /**
     * Return the key of the {@link PreferenceGroup} that contains the bluetooth devices
     */
    public abstract String getDeviceListKey();

    public boolean shouldShowDevicesWithoutNames() {
        return mShowDevicesWithoutNames;
    }

    @VisibleForTesting
    void startScanning() {
        if (mFilter != null) {
            startClassicScanning();
        } else if (mLeScanFilters != null) {
            startLeScanning();
        }

    }

    @VisibleForTesting
    void stopScanning() {
        if (mFilter != null) {
            stopClassicScanning();
        } else if (mLeScanFilters != null) {
            stopLeScanning();
        }
    }

    private void startClassicScanning() {
        if (!mBluetoothAdapter.isDiscovering()) {
            mBluetoothAdapter.startDiscovery();
        }
    }

    private void stopClassicScanning() {
        if (mBluetoothAdapter.isDiscovering()) {
            mBluetoothAdapter.cancelDiscovery();
        }
    }

    private void startLeScanning() {
        final BluetoothLeScanner scanner = mBluetoothAdapter.getBluetoothLeScanner();
        final ScanSettings settings = new ScanSettings.Builder()
                .setScanMode(ScanSettings.SCAN_MODE_LOW_LATENCY)
                .build();
        mScanCallback = new ScanCallback() {
            @Override
            public void onScanResult(int callbackType, ScanResult result) {
                final BluetoothDevice device = result.getDevice();
                CachedBluetoothDevice cachedDevice = mCachedDeviceManager.findDevice(device);
                if (cachedDevice == null) {
                    cachedDevice = mCachedDeviceManager.addDevice(device);
                }
                // Only add device preference when it's not found in the map and there's no other
                // state message showing in the list
                if (mDevicePreferenceMap.get(cachedDevice) == null
                        && mBluetoothAdapter.getState() == BluetoothAdapter.STATE_ON) {
                    createDevicePreference(cachedDevice);
                }
            }

            @Override
            public void onScanFailed(int errorCode) {
                Log.w(TAG, "BLE Scan failed with error code " + errorCode);
            }
        };
        scanner.startScan(mLeScanFilters, settings, mScanCallback);
    }

    private void stopLeScanning() {
        final BluetoothLeScanner scanner = mBluetoothAdapter.getBluetoothLeScanner();
        if (scanner != null) {
            scanner.stopScan(mScanCallback);
        }
    }
}
+348 −0

File added.

Preview size limit exceeded, changes collapsed.

+3 −3
Original line number Diff line number Diff line
@@ -202,7 +202,7 @@ public class BluetoothDevicePairingDetailBaseTest {
                new BluetoothDevicePreference(mContext, mCachedBluetoothDevice,
                        true, BluetoothDevicePreference.SortType.TYPE_FIFO);
        final BluetoothDevice device = mBluetoothAdapter.getRemoteDevice(TEST_DEVICE_ADDRESS);
        mFragment.mDevicePreferenceMap.put(mCachedBluetoothDevice, preference);
        mFragment.getDevicePreferenceMap().put(mCachedBluetoothDevice, preference);

        when(mCachedBluetoothDevice.isConnected()).thenReturn(true);
        when(mCachedBluetoothDevice.getDevice()).thenReturn(device);
@@ -210,7 +210,7 @@ public class BluetoothDevicePairingDetailBaseTest {
        mFragment.onProfileConnectionStateChanged(mCachedBluetoothDevice,
                BluetoothProfile.A2DP, BluetoothAdapter.STATE_CONNECTED);

        assertThat(mFragment.mDevicePreferenceMap.size()).isEqualTo(0);
        assertThat(mFragment.getDevicePreferenceMap().size()).isEqualTo(0);
    }

    @Test
@@ -221,7 +221,7 @@ public class BluetoothDevicePairingDetailBaseTest {
                        true, BluetoothDevicePreference.SortType.TYPE_FIFO);
        final BluetoothDevice device = mBluetoothAdapter.getRemoteDevice(TEST_DEVICE_ADDRESS);
        final BluetoothDevice device2 = mBluetoothAdapter.getRemoteDevice(TEST_DEVICE_ADDRESS_B);
        mFragment.mDevicePreferenceMap.put(mCachedBluetoothDevice, preference);
        mFragment.getDevicePreferenceMap().put(mCachedBluetoothDevice, preference);

        when(mCachedBluetoothDevice.isConnected()).thenReturn(true);
        when(mCachedBluetoothDevice.getDevice()).thenReturn(device);
Loading