Skip to content

Commit

Permalink
Bluetooth: fix metrics collection
Browse files Browse the repository at this point in the history 
Bluetooth.PairingMethod was always sent with UMA_PAIRING_METHOD_NONE
even when another method was used because the pairing_delegate_used_
flag was never updated.

Bluetooth.PairingResult was never called with
UMA_PAIRING_RESULT_SUCCESS; add this add the end of OnConnect(), adding
a after_pairing flag to the connect chain to ensure we only record this
as part of a paring attempt.

Bluetooth.PairingResult failures were recorded for ordinary Connect()
calls, use the added after_pairing flag to only record them when
pairing.

BUG=237340
TEST=chrome:///histograms
R=youngki@chromium.org

Review URL: https://codereview.chromium.org/14889002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@198171 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
keybuk@chromium.org committed May 3, 2013
1 parent a2fed5a commit 75eb9b8
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 52 deletions.
98 changes: 54 additions & 44 deletions device/bluetooth/bluetooth_device_experimental_chromeos.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,39 @@ void ParseModalias(const dbus::ObjectPath& object_path,
}
}

void RecordPairingResult(BluetoothDevice::ConnectErrorCode error_code) {
UMAPairingResult pairing_result;
switch (error_code) {
case BluetoothDevice::ERROR_INPROGRESS:
pairing_result = UMA_PAIRING_RESULT_INPROGRESS;
break;
case BluetoothDevice::ERROR_FAILED:
pairing_result = UMA_PAIRING_RESULT_FAILED;
break;
case BluetoothDevice::ERROR_AUTH_FAILED:
pairing_result = UMA_PAIRING_RESULT_AUTH_FAILED;
break;
case BluetoothDevice::ERROR_AUTH_CANCELED:
pairing_result = UMA_PAIRING_RESULT_AUTH_CANCELED;
break;
case BluetoothDevice::ERROR_AUTH_REJECTED:
pairing_result = UMA_PAIRING_RESULT_AUTH_REJECTED;
break;
case BluetoothDevice::ERROR_AUTH_TIMEOUT:
pairing_result = UMA_PAIRING_RESULT_AUTH_TIMEOUT;
break;
case BluetoothDevice::ERROR_UNSUPPORTED_DEVICE:
pairing_result = UMA_PAIRING_RESULT_UNSUPPORTED_DEVICE;
break;
default:
pairing_result = UMA_PAIRING_RESULT_UNKNOWN_ERROR;
}

UMA_HISTOGRAM_ENUMERATION("Bluetooth.PairingResult",
pairing_result,
UMA_PAIRING_RESULT_COUNT);
}

} // namespace

namespace chromeos {
Expand Down Expand Up @@ -237,7 +270,7 @@ void BluetoothDeviceExperimentalChromeOS::Connect(

if (IsPaired() || !pairing_delegate || !IsPairable()) {
// No need to pair, or unable to, skip straight to connection.
ConnectInternal(callback, error_callback);
ConnectInternal(false, callback, error_callback);
} else {
// Initiate high-security connection with pairing.
DCHECK(!pairing_delegate_);
Expand Down Expand Up @@ -406,6 +439,7 @@ void BluetoothDeviceExperimentalChromeOS::RequestPinCode(
DCHECK(pincode_callback_.is_null());
pincode_callback_ = callback;
pairing_delegate_->RequestPinCode(this);
pairing_delegate_used_ = true;
}

void BluetoothDeviceExperimentalChromeOS::DisplayPinCode(
Expand All @@ -421,6 +455,7 @@ void BluetoothDeviceExperimentalChromeOS::DisplayPinCode(

DCHECK(pairing_delegate_);
pairing_delegate_->DisplayPinCode(this, pincode);
pairing_delegate_used_ = true;
}

void BluetoothDeviceExperimentalChromeOS::RequestPasskey(
Expand All @@ -438,6 +473,7 @@ void BluetoothDeviceExperimentalChromeOS::RequestPasskey(
DCHECK(passkey_callback_.is_null());
passkey_callback_ = callback;
pairing_delegate_->RequestPasskey(this);
pairing_delegate_used_ = true;
}

void BluetoothDeviceExperimentalChromeOS::DisplayPasskey(
Expand All @@ -458,6 +494,7 @@ void BluetoothDeviceExperimentalChromeOS::DisplayPasskey(
if (entered == 0)
pairing_delegate_->DisplayPasskey(this, passkey);
pairing_delegate_->KeysEntered(this, entered);
pairing_delegate_used_ = true;
}

void BluetoothDeviceExperimentalChromeOS::RequestConfirmation(
Expand All @@ -476,6 +513,7 @@ void BluetoothDeviceExperimentalChromeOS::RequestConfirmation(
DCHECK(confirmation_callback_.is_null());
confirmation_callback_ = callback;
pairing_delegate_->ConfirmPasskey(this, passkey);
pairing_delegate_used_ = true;
}

void BluetoothDeviceExperimentalChromeOS::RequestAuthorization(
Expand All @@ -502,6 +540,7 @@ void BluetoothDeviceExperimentalChromeOS::Cancel() {
}

void BluetoothDeviceExperimentalChromeOS::ConnectInternal(
bool after_pairing,
const base::Closure& callback,
const ConnectErrorCallback& error_callback) {
VLOG(1) << object_path_.value() << ": Connecting";
Expand All @@ -511,14 +550,17 @@ void BluetoothDeviceExperimentalChromeOS::ConnectInternal(
base::Bind(
&BluetoothDeviceExperimentalChromeOS::OnConnect,
weak_ptr_factory_.GetWeakPtr(),
after_pairing,
callback),
base::Bind(
&BluetoothDeviceExperimentalChromeOS::OnConnectError,
weak_ptr_factory_.GetWeakPtr(),
after_pairing,
error_callback));
}

void BluetoothDeviceExperimentalChromeOS::OnConnect(
bool after_pairing,
const base::Closure& callback) {
if (--num_connecting_calls_ == 0)
adapter_->NotifyDeviceChanged(this);
Expand All @@ -529,10 +571,16 @@ void BluetoothDeviceExperimentalChromeOS::OnConnect(

SetTrusted();

if (after_pairing)
UMA_HISTOGRAM_ENUMERATION("Bluetooth.PairingResult",
UMA_PAIRING_RESULT_SUCCESS,
UMA_PAIRING_RESULT_COUNT);

callback.Run();
}

void BluetoothDeviceExperimentalChromeOS::OnConnectError(
bool after_pairing,
const ConnectErrorCallback& error_callback,
const std::string& error_name,
const std::string& error_message) {
Expand All @@ -555,7 +603,8 @@ void BluetoothDeviceExperimentalChromeOS::OnConnectError(
error_code = ERROR_UNSUPPORTED_DEVICE;
}

RecordPairingResult(false, error_code);
if (after_pairing)
RecordPairingResult(error_code);
error_callback.Run(error_code);
}

Expand Down Expand Up @@ -596,7 +645,7 @@ void BluetoothDeviceExperimentalChromeOS::OnRegisterAgentError(
if (error_name == bluetooth_adapter::kErrorAlreadyExists)
error_code = ERROR_INPROGRESS;

RecordPairingResult(false, error_code);
RecordPairingResult(error_code);
error_callback.Run(error_code);
}

Expand All @@ -611,7 +660,7 @@ void BluetoothDeviceExperimentalChromeOS::OnPair(
UMA_PAIRING_METHOD_COUNT);
UnregisterAgent();
SetTrusted();
ConnectInternal(callback, error_callback);
ConnectInternal(true, callback, error_callback);
}

void BluetoothDeviceExperimentalChromeOS::OnPairError(
Expand Down Expand Up @@ -643,7 +692,7 @@ void BluetoothDeviceExperimentalChromeOS::OnPairError(
error_code = ERROR_AUTH_TIMEOUT;
}

RecordPairingResult(false, error_code);
RecordPairingResult(error_code);
error_callback.Run(error_code);
}

Expand Down Expand Up @@ -755,43 +804,4 @@ bool BluetoothDeviceExperimentalChromeOS::RunPairingCallbacks(Status status) {
return callback_run;
}

void BluetoothDeviceExperimentalChromeOS::RecordPairingResult(
bool success,
ConnectErrorCode error_code) {
UMAPairingResult pairing_result;
if (success) {
pairing_result = UMA_PAIRING_RESULT_SUCCESS;
} else {
switch (error_code) {
case ERROR_INPROGRESS:
pairing_result = UMA_PAIRING_RESULT_INPROGRESS;
break;
case ERROR_FAILED:
pairing_result = UMA_PAIRING_RESULT_FAILED;
break;
case ERROR_AUTH_FAILED:
pairing_result = UMA_PAIRING_RESULT_AUTH_FAILED;
break;
case ERROR_AUTH_CANCELED:
pairing_result = UMA_PAIRING_RESULT_AUTH_CANCELED;
break;
case ERROR_AUTH_REJECTED:
pairing_result = UMA_PAIRING_RESULT_AUTH_REJECTED;
break;
case ERROR_AUTH_TIMEOUT:
pairing_result = UMA_PAIRING_RESULT_AUTH_TIMEOUT;
break;
case ERROR_UNSUPPORTED_DEVICE:
pairing_result = UMA_PAIRING_RESULT_UNSUPPORTED_DEVICE;
break;
default:
pairing_result = UMA_PAIRING_RESULT_UNKNOWN_ERROR;
}
}

UMA_HISTOGRAM_ENUMERATION("Bluetooth.PairingResult",
pairing_result,
UMA_PAIRING_RESULT_COUNT);
}

} // namespace chromeos
14 changes: 6 additions & 8 deletions device/bluetooth/bluetooth_device_experimental_chromeos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,13 @@ class BluetoothDeviceExperimentalChromeOS

// Internal method to initiate a connection to this device, and methods called
// by dbus:: on completion of the D-Bus method call.
void ConnectInternal(const base::Closure& callback,
void ConnectInternal(bool after_pairing,
const base::Closure& callback,
const ConnectErrorCallback& error_callback);
void OnConnect(const base::Closure& callback);
void OnConnectError(const ConnectErrorCallback& error_callback,
void OnConnect(bool after_pairing,
const base::Closure& callback);
void OnConnectError(bool after_pairing,
const ConnectErrorCallback& error_callback,
const std::string& error_name,
const std::string& error_message);

Expand Down Expand Up @@ -170,11 +173,6 @@ class BluetoothDeviceExperimentalChromeOS
// pairing. Returns true if any callbacks were run, false if not.
bool RunPairingCallbacks(Status status);

// Record the result of pairing as a UMA histogram metric; |success| should
// be true if pairing succeeded, and |false| if not - in which case
// |error_code| specifies the reason for failure.
void RecordPairingResult(bool success, ConnectErrorCode error_code);

// Return the object path of the device; used by
// BluetoothAdapterExperimentalChromeOS
const dbus::ObjectPath& object_path() const { return object_path_; }
Expand Down

0 comments on commit 75eb9b8

Please sign in to comment.