Commit 84b56aac authored by Martin Kreichgauer's avatar Martin Kreichgauer Committed by Commit Bot

fido: work around WinRT Bluetooth weirdness in FidoCableDiscovery

Windows appears to send two BluetoothAdapter::StateChanged events
whenever a Bluetooth adapter is powered up. At least the first such
event races against Windows-internal adapter initialization,
such that calls to BluetoothLEAdvertisementWatcher::Start can fail with
"device not ready for use" for some time after the first StateChanged.

Additionally, calling BluetoothAdapter::RegisterAdvertisement() twice
in a row appears to send things into a wonky state where callbacks stop
firing.

In combination, these quirks do not cause noticeable issues in most
environments: When the first StateChanged event following an adapter
power-on arrives, starting BLE discovery fails because starting the
advertisement watcher fails. By the time the second StateChange event
arrives, sufficient time has passed for discovery to start
successfully. As a result, RegisterAdvertisement() is only called a
single time.

On some machines however, the first attempt to start discovering does
*not* fail (perhaps because the machine is slow, or because post-power-
on adapter initialization is fast), resulting in two calls to
RegisterAdvertisement().

Work around all this by doing the following:

(1) Make FidoCableDiscovery remember the adapter power state and ignore
AdapterPoweredChanged() calls that don't really change state, in
order to avoid calling RegisterAdvertisement() multiple times.

(2) Hold off on starting discovery for 500ms after the first power-on
event.

This is admittedly a bit of a hack. The 500ms are somewhat arbitrary and
I don't really know if adapter initialization perhaps takes even longer
in other environments. A more sound strategy would perhaps be to
retry the calls that are known to fail spuriously, preferably at the
//device/bluetooth layer; and also to fix whatever makes multiple
calls to RegisterAdvertisement() break everything.

Bug: b/145224971
Change-Id: If5ec45ec2b964c28f480d2e24a41a4c9a292c0a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2018498Reviewed-by: default avatarAdam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735154}
parent 0f55c487
......@@ -381,21 +381,43 @@ void FidoCableDiscovery::DeviceRemoved(BluetoothAdapter* adapter,
void FidoCableDiscovery::AdapterPoweredChanged(BluetoothAdapter* adapter,
bool powered) {
// If Bluetooth adapter is powered on, resume scanning for nearby Cable
// devices and start advertising client EIDs.
if (powered) {
StartCableDiscovery();
} else {
// Windows is causing multiple of these callbacks for a single power-on
// event, and calling RegisterAdvertisement() more than once
// breaks it. Hence ignore all but the first event.
if (is_powered_ == powered) {
return;
}
is_powered_ = powered;
if (!is_powered_) {
// In order to prevent duplicate client EIDs from being advertised when
// BluetoothAdapter is powered back on, unregister all existing client
// EIDs.
StopAdvertisements(base::DoNothing());
return;
}
#if defined(OS_WIN)
// On Windows, the (first) power-on event appears to race against
// initialization of the adapter, such that one of the WinRT API calls inside
// BluetoothAdapter::StartDiscoverySessionWithFilter() can fail with "Device
// not ready for use". So wait for things to actually be ready.
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&FidoCableDiscovery::StartCableDiscovery,
weak_factory_.GetWeakPtr()),
base::TimeDelta::FromMilliseconds(500));
#else
StartCableDiscovery();
#endif // defined(OS_WIN)
}
void FidoCableDiscovery::OnSetPowered() {
DCHECK(adapter());
if (is_powered_) {
return;
}
is_powered_ = true;
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&FidoCableDiscovery::StartCableDiscovery,
weak_factory_.GetWeakPtr()));
......
......@@ -146,6 +146,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
base::RepeatingCallback<void(std::unique_ptr<CableDiscoveryData>)>>
pairing_callback_;
bool is_powered_ = false;
// observed_devices_ caches the information from observed caBLE devices so
// that the device-log isn't spammed.
mutable base::flat_map<std::string, std::unique_ptr<ObservedDeviceData>>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment