Commit 3c39ed30 authored by Martin Kreichgauer's avatar Martin Kreichgauer Committed by Commit Bot

fido: count BLE scanning failures during caBLEv1 requests

This adds to new events to the WebAuthentication.CableV1DiscoveryEvent
UMA histogram.

kStartScanningFailed (12) indicates that
BluetoothAdapter::StartDiscoverySession() was called but failed.
kScanningStoppedUnexpectedly (13) indicates that after
StartDiscoverySession() succeeded, the BluetoothAdapter notified the
FidoCableDiscovery that the discovery session stopped due to some
error.

Both of those scenarios currently cannot be recovered from without the
user manually retrying the entire request.

Change-Id: Idfd4b9c2f45cb2eb9549caccdeea6c0463e77a5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2258468Reviewed-by: default avatarAdam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#781428}
parent ab6ce390
...@@ -115,12 +115,14 @@ enum class FidoCableDiscovery::CableV1DiscoveryEvent : int { ...@@ -115,12 +115,14 @@ enum class FidoCableDiscovery::CableV1DiscoveryEvent : int {
kAdapterManuallyPowered = 4, kAdapterManuallyPowered = 4,
kAdapterPoweredOff = 5, kAdapterPoweredOff = 5,
kScanningStarted = 6, kScanningStarted = 6,
kStartScanningFailed = 12,
kScanningStoppedUnexpectedly = 13,
kAdvertisementRegistered = 7, kAdvertisementRegistered = 7,
kFirstCableDeviceFound = 8, kFirstCableDeviceFound = 8,
kFirstCableDeviceGATTConnected = 9, kFirstCableDeviceGATTConnected = 9,
kFirstCableHandshakeSucceeded = 10, kFirstCableHandshakeSucceeded = 10,
kFirstCableDeviceTimeout = 11, kFirstCableDeviceTimeout = 11,
kMaxValue = kFirstCableDeviceTimeout, kMaxValue = kScanningStoppedUnexpectedly,
}; };
// FidoCableDiscovery::Result ------------------------------------------------- // FidoCableDiscovery::Result -------------------------------------------------
...@@ -257,10 +259,6 @@ void FidoCableDiscovery::OnGetAdapter(scoped_refptr<BluetoothAdapter> adapter) { ...@@ -257,10 +259,6 @@ void FidoCableDiscovery::OnGetAdapter(scoped_refptr<BluetoothAdapter> adapter) {
if (has_v1_discovery_data_) { if (has_v1_discovery_data_) {
RecordCableV1DiscoveryEventOnce(CableV1DiscoveryEvent::kAdapterPresent); RecordCableV1DiscoveryEventOnce(CableV1DiscoveryEvent::kAdapterPresent);
if (adapter->IsPowered()) {
RecordCableV1DiscoveryEventOnce(
CableV1DiscoveryEvent::kAdapterAlreadyPowered);
}
} }
DCHECK(!adapter_); DCHECK(!adapter_);
...@@ -270,6 +268,10 @@ void FidoCableDiscovery::OnGetAdapter(scoped_refptr<BluetoothAdapter> adapter) { ...@@ -270,6 +268,10 @@ void FidoCableDiscovery::OnGetAdapter(scoped_refptr<BluetoothAdapter> adapter) {
adapter_->AddObserver(this); adapter_->AddObserver(this);
if (adapter_->IsPowered()) { if (adapter_->IsPowered()) {
if (has_v1_discovery_data_) {
RecordCableV1DiscoveryEventOnce(
CableV1DiscoveryEvent::kAdapterAlreadyPowered);
}
OnSetPowered(); OnSetPowered();
} }
...@@ -348,6 +350,22 @@ void FidoCableDiscovery::AdapterPoweredChanged(BluetoothAdapter* adapter, ...@@ -348,6 +350,22 @@ void FidoCableDiscovery::AdapterPoweredChanged(BluetoothAdapter* adapter,
#endif // defined(OS_WIN) #endif // defined(OS_WIN)
} }
void FidoCableDiscovery::AdapterDiscoveringChanged(BluetoothAdapter* adapter,
bool is_scanning) {
FIDO_LOG(DEBUG) << "AdapterDiscoveringChanged() is_scanning=" << is_scanning;
// Ignore updates while we're not scanning for caBLE devices ourselves. Other
// things in Chrome may start or stop scans at any time.
if (!discovery_session_) {
return;
}
if (has_v1_discovery_data_ && !is_scanning) {
RecordCableV1DiscoveryEventOnce(
CableV1DiscoveryEvent::kScanningStoppedUnexpectedly);
}
}
void FidoCableDiscovery::FidoCableDeviceConnected(FidoCableDevice* device, void FidoCableDiscovery::FidoCableDeviceConnected(FidoCableDevice* device,
bool success) { bool success) {
if (!success || !IsObservedV1Device(device->GetAddress())) { if (!success || !IsObservedV1Device(device->GetAddress())) {
...@@ -399,6 +417,10 @@ void FidoCableDiscovery::OnStartDiscoverySession( ...@@ -399,6 +417,10 @@ void FidoCableDiscovery::OnStartDiscoverySession(
void FidoCableDiscovery::OnStartDiscoverySessionError() { void FidoCableDiscovery::OnStartDiscoverySessionError() {
FIDO_LOG(ERROR) << "Failed to start caBLE discovery"; FIDO_LOG(ERROR) << "Failed to start caBLE discovery";
if (has_v1_discovery_data_) {
RecordCableV1DiscoveryEventOnce(
CableV1DiscoveryEvent::kStartScanningFailed);
}
} }
void FidoCableDiscovery::StartAdvertisement() { void FidoCableDiscovery::StartAdvertisement() {
......
...@@ -148,6 +148,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery ...@@ -148,6 +148,8 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
void DeviceRemoved(BluetoothAdapter* adapter, void DeviceRemoved(BluetoothAdapter* adapter,
BluetoothDevice* device) override; BluetoothDevice* device) override;
void AdapterPoweredChanged(BluetoothAdapter* adapter, bool powered) override; void AdapterPoweredChanged(BluetoothAdapter* adapter, bool powered) override;
void AdapterDiscoveringChanged(BluetoothAdapter* adapter,
bool discovering) override;
// FidoCableDevice::Observer: // FidoCableDevice::Observer:
void FidoCableDeviceConnected(FidoCableDevice* device, bool success) override; void FidoCableDeviceConnected(FidoCableDevice* device, bool success) override;
...@@ -157,10 +159,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery ...@@ -157,10 +159,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
std::unique_ptr<BluetoothDiscoverySession> discovery_session_; std::unique_ptr<BluetoothDiscoverySession> discovery_session_;
std::vector<CableDiscoveryData> discovery_data_; std::vector<CableDiscoveryData> discovery_data_;
// active_authenticator_eids_ contains authenticator EIDs for which a // active_authenticator_eids_ contains authenticator EIDs for which a
// handshake is currently running. Further advertisements for the same EIDs // handshake is currently running. Further advertisements for the same EIDs
// will be ignored. // will be ignored.
std::set<CableEidArray> active_authenticator_eids_; std::set<CableEidArray> active_authenticator_eids_;
// active_devices_ contains the BLE addresses of devices for which a handshake // active_devices_ contains the BLE addresses of devices for which a handshake
// is already running. Further advertisements from these devices will be // is already running. Further advertisements from these devices will be
// ignored. However, devices may rotate their BLE address at will so this is // ignored. However, devices may rotate their BLE address at will so this is
...@@ -184,6 +188,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery ...@@ -184,6 +188,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
// that the device-log isn't spammed. // that the device-log isn't spammed.
mutable base::flat_map<std::string, std::unique_ptr<ObservedDeviceData>> mutable base::flat_map<std::string, std::unique_ptr<ObservedDeviceData>>
observed_devices_; observed_devices_;
// noted_obsolete_eids_ remembers QR-code EIDs that have been logged as // noted_obsolete_eids_ remembers QR-code EIDs that have been logged as
// valid-but-expired in order to avoid spamming the device-log. // valid-but-expired in order to avoid spamming the device-log.
mutable base::flat_set<CableEidArray> noted_obsolete_eids_; mutable base::flat_set<CableEidArray> noted_obsolete_eids_;
......
...@@ -71772,6 +71772,8 @@ Full version information for the fingerprint enum values: ...@@ -71772,6 +71772,8 @@ Full version information for the fingerprint enum values:
<int value="9" label="GATT connection established"/> <int value="9" label="GATT connection established"/>
<int value="10" label="caBLE handshake completed"/> <int value="10" label="caBLE handshake completed"/>
<int value="11" label="caBLE device timed out"/> <int value="11" label="caBLE device timed out"/>
<int value="12" label="Scanning for BLE advertisements failed"/>
<int value="13" label="Scanning for BLE advertisements stopped unexpectedly"/>
</enum> </enum>
<enum name="WebAuthenticationCableV1Event"> <enum name="WebAuthenticationCableV1Event">
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