Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2423)

Unified Diff: device/bluetooth/bluetooth_adapter_chromeos.cc

Issue 933823003: bluetooth: Modify DCHECK to conditionals in several BluetoothAdapterChromeOS methods. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: device/bluetooth/bluetooth_adapter_chromeos.cc
diff --git a/device/bluetooth/bluetooth_adapter_chromeos.cc b/device/bluetooth/bluetooth_adapter_chromeos.cc
index e6c628322d349b629b507b17f753d8d541b35955..028c55fcb34fa843526b71f3255cf2e7c4d677a2 100644
--- a/device/bluetooth/bluetooth_adapter_chromeos.cc
+++ b/device/bluetooth/bluetooth_adapter_chromeos.cc
@@ -316,15 +316,14 @@ void BluetoothAdapterChromeOS::RegisterAudioSink(
scoped_refptr<BluetoothAudioSinkChromeOS> audio_sink(
new BluetoothAudioSinkChromeOS(this));
audio_sink->Register(
- options,
- base::Bind(&BluetoothAdapterChromeOS::OnRegisterAudioSink,
- weak_ptr_factory_.GetWeakPtr(), callback, audio_sink),
+ options, base::Bind(&BluetoothAdapterChromeOS::OnRegisterAudioSink,
+ weak_ptr_factory_.GetWeakPtr(), callback,
+ error_callback, audio_sink),
error_callback);
}
void BluetoothAdapterChromeOS::RemovePairingDelegateInternal(
BluetoothDevice::PairingDelegate* pairing_delegate) {
- DCHECK(IsPresent());
// Before removing a pairing delegate make sure that there aren't any devices
// currently using it; if there are, clear the pairing context which will
// make any responses no-ops.
@@ -481,9 +480,10 @@ void BluetoothAdapterChromeOS::InputPropertyChanged(
}
void BluetoothAdapterChromeOS::Released() {
- DCHECK(IsPresent());
- DCHECK(agent_.get());
VLOG(1) << "Release";
+ if (!IsPresent())
+ return;
+ DCHECK(agent_.get());
// Called after we unregister the pairing agent, e.g. when changing I/O
// capabilities. Nothing much to be done right now.
@@ -624,7 +624,6 @@ void BluetoothAdapterChromeOS::Cancel() {
}
void BluetoothAdapterChromeOS::OnRegisterAgent() {
- DCHECK(IsPresent());
VLOG(1) << "Pairing agent registered, requesting to be made default";
DBusThreadManager::Get()->GetBluetoothAgentManagerClient()->
@@ -639,7 +638,6 @@ void BluetoothAdapterChromeOS::OnRegisterAgent() {
void BluetoothAdapterChromeOS::OnRegisterAgentError(
const std::string& error_name,
const std::string& error_message) {
- DCHECK(IsPresent());
// Our agent being already registered isn't an error.
if (error_name == bluetooth_agent_manager::kErrorAlreadyExists)
return;
@@ -649,21 +647,25 @@ void BluetoothAdapterChromeOS::OnRegisterAgentError(
}
void BluetoothAdapterChromeOS::OnRequestDefaultAgent() {
- DCHECK(IsPresent());
VLOG(1) << "Pairing agent now default";
}
void BluetoothAdapterChromeOS::OnRequestDefaultAgentError(
const std::string& error_name,
const std::string& error_message) {
- DCHECK(IsPresent());
LOG(WARNING) << ": Failed to make pairing agent default: "
<< error_name << ": " << error_message;
}
void BluetoothAdapterChromeOS::OnRegisterAudioSink(
const device::BluetoothAdapter::AcquiredCallback& callback,
+ const device::BluetoothAudioSink::ErrorCallback& error_callback,
scoped_refptr<BluetoothAudioSink> audio_sink) {
+ if (!IsPresent()) {
+ VLOG(1) << "Failed to register audio sink, adapter not present";
+ error_callback.Run(BluetoothAudioSink::ERROR_INVALID_ADAPTER);
+ return;
+ }
DCHECK(audio_sink.get());
callback.Run(audio_sink);
}
@@ -1019,7 +1021,11 @@ void BluetoothAdapterChromeOS::OnSetDiscoverable(
const base::Closure& callback,
const ErrorCallback& error_callback,
bool success) {
- DCHECK(IsPresent());
+ if (!IsPresent()) {
+ error_callback.Run();
+ return;
+ }
+
// Set the discoverable_timeout property to zero so the adapter remains
// discoverable forever.
DBusThreadManager::Get()->GetBluetoothAdapterClient()->
@@ -1035,8 +1041,7 @@ void BluetoothAdapterChromeOS::OnPropertyChangeCompleted(
const base::Closure& callback,
const ErrorCallback& error_callback,
bool success) {
- DCHECK(IsPresent());
- if (success)
+ if (IsPresent() && success)
callback.Run();
else
error_callback.Run();
@@ -1089,7 +1094,11 @@ void BluetoothAdapterChromeOS::AddDiscoverySession(
void BluetoothAdapterChromeOS::RemoveDiscoverySession(
const base::Closure& callback,
const ErrorCallback& error_callback) {
- DCHECK(IsPresent());
+ if (!IsPresent()) {
+ error_callback.Run();
+ return;
+ }
+
VLOG(1) << __func__;
// There are active sessions other than the one currently being removed.
if (num_discovery_sessions_ > 1) {
@@ -1152,7 +1161,10 @@ void BluetoothAdapterChromeOS::OnStartDiscoveryError(
const ErrorCallback& error_callback,
const std::string& error_name,
const std::string& error_message) {
- DCHECK(IsPresent());
+ if (!IsPresent()) {
+ error_callback.Run();
+ return;
+ }
LOG(WARNING) << object_path_.value() << ": Failed to start discovery: "
<< error_name << ": " << error_message;
@@ -1194,13 +1206,17 @@ void BluetoothAdapterChromeOS::OnStopDiscoveryError(
const ErrorCallback& error_callback,
const std::string& error_name,
const std::string& error_message) {
- DCHECK(IsPresent());
LOG(WARNING) << object_path_.value() << ": Failed to stop discovery: "
<< error_name << ": " << error_message;
+ if (IsPresent()) {
+ // Failed to stop discovery. This can only happen if the count is at 1.
+ DCHECK(discovery_request_pending_);
+ DCHECK_EQ(num_discovery_sessions_, 1);
+ }
+
+ // Both when IsPresent and after Shutdown, run the error callbacks and empty
+ // the queue.
armansito 2015/02/19 21:40:49 Sorry for the push back, can you add a unit test f
scheib 2015/02/20 00:41:17 Done. I've added 4 more unit tests to verify that
- // Failed to stop discovery. This can only happen if the count is at 1.
- DCHECK(discovery_request_pending_);
- DCHECK_EQ(num_discovery_sessions_, 1);
discovery_request_pending_ = false;
error_callback.Run();

Powered by Google App Engine
This is Rietveld 408576698