[Bluetooth] Combine characteristic read callbacks.
Combine the Bluetooth characteristic read value/descriptor
success and error callbacks into a single read result callback.
This change makes the C++ implementation consistent with the
Mojo API (ReadValueForCharacteristic), and eliminates repeating
callbacks.
Bug: 730593
Change-Id: Ia186f5dd017897b133a9e7d414d47f209b7c0fc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2818972
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Josh Horwich <jhorwich@chromium.org>
Reviewed-by: James Hollyer <jameshollyer@chromium.org>
Commit-Queue: Chris Mumford <cmumford@google.com>
Cr-Commit-Position: refs/heads/master@{#878493}
diff --git a/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.cc b/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.cc
index 6cf47b5..1213d896 100644
--- a/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.cc
+++ b/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.cc
@@ -302,36 +302,37 @@
error_code, /* is_read_operation = */ false));
}
-// Common success callback for ReadGattCharacteristic and ReadGattDescriptor
-void OnGattReadDone(GattReadCallback callback,
- const std::vector<uint8_t>& result) {
+// Common callback (success and error) for ReadGattCharacteristic and
+// ReadGattDescriptor.
+void OnGattRead(
+ GattReadCallback callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& result) {
arc::mojom::BluetoothGattValuePtr gattValue =
arc::mojom::BluetoothGattValue::New();
- gattValue->status = arc::mojom::BluetoothGattStatus::GATT_SUCCESS;
- gattValue->value = result;
- std::move(callback).Run(std::move(gattValue));
-}
-// Common error callback for ReadGattCharacteristic and ReadGattDescriptor
-void OnGattReadError(GattReadCallback callback,
- BluetoothGattService::GattErrorCode error_code) {
- arc::mojom::BluetoothGattValuePtr gattValue =
- arc::mojom::BluetoothGattValue::New();
- gattValue->status =
- ConvertGattErrorCodeToStatus(error_code, /* is_read_operation = */ true);
+ if (error_code.has_value()) {
+ gattValue->status = ConvertGattErrorCodeToStatus(
+ error_code.value(), /*is_read_operation=*/true);
+ } else {
+ gattValue->status = arc::mojom::BluetoothGattStatus::GATT_SUCCESS;
+ }
+ gattValue->value = result;
std::move(callback).Run(std::move(gattValue));
}
// Callback function for mojom::BluetoothInstance::RequestGattRead
void OnGattServerRead(
- BluetoothLocalGattService::Delegate::ValueCallback success_callback,
- BluetoothLocalGattService::Delegate::ErrorCallback error_callback,
+ BluetoothLocalGattService::Delegate::ValueCallback callback,
arc::mojom::BluetoothGattStatus status,
const std::vector<uint8_t>& value) {
- if (status == arc::mojom::BluetoothGattStatus::GATT_SUCCESS)
- std::move(success_callback).Run(value);
- else
- std::move(error_callback).Run();
+ if (status == arc::mojom::BluetoothGattStatus::GATT_SUCCESS) {
+ std::move(callback).Run(/*error_code=*/base::nullopt, value);
+ } else {
+ std::move(callback).Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
+ }
}
// Callback function for mojom::BluetoothInstance::RequestGattWrite
@@ -878,13 +879,13 @@
const LocalGattAttribute* attribute,
int offset,
mojom::BluetoothGattDBAttributeType attribute_type,
- ValueCallback success_callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
auto* bluetooth_instance = ARC_GET_INSTANCE_FOR_METHOD(
arc_bridge_service_->bluetooth(), RequestGattRead);
if (!bluetooth_instance || !IsGattOffsetValid(offset)) {
- std::move(error_callback).Run();
+ std::move(callback).Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -893,9 +894,7 @@
bluetooth_instance->RequestGattRead(
mojom::BluetoothAddress::From(device->GetAddress()),
gatt_handle_[attribute->GetIdentifier()], offset, false /* is_long */,
- attribute_type,
- base::BindOnce(&OnGattServerRead, std::move(success_callback),
- std::move(error_callback)));
+ attribute_type, base::BindOnce(&OnGattServerRead, std::move(callback)));
}
void ArcBluetoothBridge::OnGattServerPrepareWrite(
@@ -967,12 +966,11 @@
const BluetoothDevice* device,
const BluetoothLocalGattCharacteristic* characteristic,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
OnGattAttributeReadRequest(
device, characteristic, offset,
mojom::BluetoothGattDBAttributeType::BTGATT_DB_CHARACTERISTIC,
- std::move(callback), std::move(error_callback));
+ std::move(callback));
}
void ArcBluetoothBridge::OnCharacteristicWriteRequest(
@@ -1008,12 +1006,11 @@
const BluetoothDevice* device,
const BluetoothLocalGattDescriptor* descriptor,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
OnGattAttributeReadRequest(
device, descriptor, offset,
mojom::BluetoothGattDBAttributeType::BTGATT_DB_DESCRIPTOR,
- std::move(callback), std::move(error_callback));
+ std::move(callback));
}
void ArcBluetoothBridge::OnDescriptorWriteRequest(
@@ -1731,10 +1728,8 @@
DCHECK(characteristic);
DCHECK(characteristic->GetPermissions() & kGattReadPermission);
- auto split_callback = base::SplitOnceCallback(std::move(callback));
characteristic->ReadRemoteCharacteristic(
- base::BindOnce(&OnGattReadDone, std::move(split_callback.first)),
- base::BindOnce(&OnGattReadError, std::move(split_callback.second)));
+ base::BindOnce(&OnGattRead, std::move(callback)));
}
void ArcBluetoothBridge::WriteGattCharacteristic(
@@ -1777,10 +1772,8 @@
DCHECK(descriptor);
DCHECK(descriptor->GetPermissions() & kGattReadPermission);
- auto split_callback = base::SplitOnceCallback(std::move(callback));
descriptor->ReadRemoteDescriptor(
- base::BindOnce(&OnGattReadDone, std::move(split_callback.first)),
- base::BindOnce(&OnGattReadError, std::move(split_callback.second)));
+ base::BindOnce(&OnGattRead, std::move(callback)));
}
void ArcBluetoothBridge::WriteGattDescriptor(
diff --git a/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.h b/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.h
index 9a24e479..6ce5a52 100644
--- a/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.h
+++ b/chrome/browser/ash/arc/bluetooth/arc_bluetooth_bridge.h
@@ -160,8 +160,7 @@
const device::BluetoothDevice* device,
const device::BluetoothLocalGattCharacteristic* characteristic,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) override;
+ ValueCallback callback) override;
void OnCharacteristicWriteRequest(
const device::BluetoothDevice* device,
@@ -184,8 +183,7 @@
const device::BluetoothDevice* device,
const device::BluetoothLocalGattDescriptor* descriptor,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) override;
+ ValueCallback callback) override;
void OnDescriptorWriteRequest(
const device::BluetoothDevice* device,
@@ -467,8 +465,7 @@
const LocalGattAttribute* attribute,
int offset,
mojom::BluetoothGattDBAttributeType attribute_type,
- ValueCallback success_callback,
- ErrorCallback error_callback);
+ ValueCallback callback);
// Common code for OnCharacteristicWriteRequest and OnDescriptorWriteRequest
// |is_prepare| is only set when a local characteristic receives a prepare
diff --git a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc b/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc
index b43678d..7868c6d 100644
--- a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc
+++ b/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc
@@ -224,6 +224,12 @@
std::move(std::get<k>(args)).Run(p0);
}
+ACTION_TEMPLATE(InvokeCallbackArgument,
+ HAS_1_TEMPLATE_PARAMS(int, k),
+ AND_2_VALUE_PARAMS(p0, p1)) {
+ std::move(std::get<k>(args)).Run(p0, p1);
+}
+
ACTION_TEMPLATE(InvokeCallbackWithScopedPtrArg,
HAS_2_TEMPLATE_PARAMS(int, k, typename, T),
AND_1_VALUE_PARAMS(p0)) {
@@ -703,11 +709,12 @@
.WillRepeatedly(Return(chrc0_.get()));
std::vector<uint8_t> value;
- EXPECT_CALL(*chrc0_, ReadRemoteCharacteristic_(_, _))
+ EXPECT_CALL(*chrc0_, ReadRemoteCharacteristic_(_))
.Times(2)
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_FAILED))
- .WillOnce(InvokeCallbackArgument<0>(value));
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(base::nullopt, value));
ExtensionTestMessageListener listener("ready", true);
listener.set_failure_message("fail");
@@ -968,23 +975,30 @@
.WillRepeatedly(Return(desc0_.get()));
std::vector<uint8_t> value;
- EXPECT_CALL(*desc0_, ReadRemoteDescriptor_(_, _))
+ EXPECT_CALL(*desc0_, ReadRemoteDescriptor_(_))
.Times(8)
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_FAILED))
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_INVALID_LENGTH))
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED))
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_NOT_AUTHORIZED))
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_NOT_PAIRED))
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED))
- .WillOnce(InvokeCallbackArgument<1>(
- BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS))
- .WillOnce(InvokeCallbackArgument<0>(value));
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_INVALID_LENGTH,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_NOT_AUTHORIZED,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_NOT_PAIRED,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(
+ BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ /*value=*/std::vector<uint8_t>()))
+ .WillOnce(InvokeCallbackArgument<0>(/*error_code=*/base::nullopt, value));
ExtensionTestMessageListener listener("ready", true);
listener.set_failure_message("fail");
diff --git a/chromeos/services/secure_channel/ble_characteristics_finder.cc b/chromeos/services/secure_channel/ble_characteristics_finder.cc
index f81ba43..5ec4edb 100644
--- a/chromeos/services/secure_channel/ble_characteristics_finder.cc
+++ b/chromeos/services/secure_channel/ble_characteristics_finder.cc
@@ -165,19 +165,15 @@
void BluetoothLowEnergyCharacteristicsFinder::TryToVerifyEid(
device::BluetoothRemoteGattCharacteristic* eid_char) {
- eid_char->ReadRemoteCharacteristic(
- base::BindOnce(
- &BluetoothLowEnergyCharacteristicsFinder::OnRemoteCharacteristicRead,
- weak_ptr_factory_.GetWeakPtr(),
- eid_char->GetService()->GetIdentifier()),
- base::BindOnce(&BluetoothLowEnergyCharacteristicsFinder::
- OnReadRemoteCharacteristicError,
- weak_ptr_factory_.GetWeakPtr(),
- eid_char->GetService()->GetIdentifier()));
+ eid_char->ReadRemoteCharacteristic(base::BindOnce(
+ &BluetoothLowEnergyCharacteristicsFinder::OnRemoteCharacteristicRead,
+ weak_ptr_factory_.GetWeakPtr(), eid_char->GetService()->GetIdentifier()));
}
void BluetoothLowEnergyCharacteristicsFinder::OnRemoteCharacteristicRead(
const std::string& service_id,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
const std::vector<uint8_t>& value) {
auto it = service_ids_pending_eid_read_.find(service_id);
if (it == service_ids_pending_eid_read_.end()) {
@@ -187,6 +183,15 @@
service_ids_pending_eid_read_.erase(it);
+ if (error_code.has_value()) {
+ PA_LOG(ERROR) << "OnWriteRemoteCharacteristicError() Error code: "
+ << error_code.value();
+ service_ids_pending_eid_read_.erase(service_id);
+ if (!has_callback_been_invoked_)
+ NotifyFailureIfNoPendingEidCharReads();
+ return;
+ }
+
if (has_callback_been_invoked_) {
PA_LOG(VERBOSE) << "Characteristic read after callback was invoked.";
return;
@@ -216,23 +221,6 @@
rx_chars.front()->GetIdentifier());
}
-void BluetoothLowEnergyCharacteristicsFinder::OnReadRemoteCharacteristicError(
- const std::string& service_id,
- device::BluetoothRemoteGattService::GattErrorCode error) {
- auto it = service_ids_pending_eid_read_.find(service_id);
- if (it == service_ids_pending_eid_read_.end()) {
- PA_LOG(WARNING) << "No request entry for " << service_id;
- return;
- }
-
- service_ids_pending_eid_read_.erase(it);
-
- PA_LOG(ERROR) << "OnWriteRemoteCharacteristicError() Error code: " << error;
- service_ids_pending_eid_read_.erase(service_id);
- if (!has_callback_been_invoked_)
- NotifyFailureIfNoPendingEidCharReads();
-}
-
bool BluetoothLowEnergyCharacteristicsFinder::DoesEidMatchExpectedDevice(
const std::vector<uint8_t>& eid_value_read) {
// Convert the char data from a std::vector<uint8_t> to a std::string.
diff --git a/chromeos/services/secure_channel/ble_characteristics_finder.h b/chromeos/services/secure_channel/ble_characteristics_finder.h
index 521d319..d5d7773 100644
--- a/chromeos/services/secure_channel/ble_characteristics_finder.h
+++ b/chromeos/services/secure_channel/ble_characteristics_finder.h
@@ -96,11 +96,11 @@
void NotifyFailureIfNoPendingEidCharReads();
void TryToVerifyEid(device::BluetoothRemoteGattCharacteristic* eid_char);
- void OnRemoteCharacteristicRead(const std::string& service_id,
- const std::vector<uint8_t>& value);
- void OnReadRemoteCharacteristicError(
+ void OnRemoteCharacteristicRead(
const std::string& service_id,
- device::BluetoothRemoteGattService::GattErrorCode error);
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value);
bool DoesEidMatchExpectedDevice(const std::vector<uint8_t>& eid_value_read);
// The Bluetooth adapter where the connection was established.
diff --git a/chromeos/services/secure_channel/ble_characteristics_finder_unittest.cc b/chromeos/services/secure_channel/ble_characteristics_finder_unittest.cc
index 4dc6947..8c40004 100644
--- a/chromeos/services/secure_channel/ble_characteristics_finder_unittest.cc
+++ b/chromeos/services/secure_channel/ble_characteristics_finder_unittest.cc
@@ -172,21 +172,23 @@
// running only on one thread. Calls to
// |task_environment_.RunUntilIdle()| in tests will process any
// pending callbacks.
- ON_CALL(*characteristic.get(), ReadRemoteCharacteristic_(_, _))
+ ON_CALL(*characteristic.get(), ReadRemoteCharacteristic_(_))
.WillByDefault(Invoke(
[read_success, correct_eid](
- BluetoothRemoteGattCharacteristic::ValueCallback& callback,
- BluetoothRemoteGattCharacteristic::ErrorCallback&
- error_callback) {
+ BluetoothRemoteGattCharacteristic::ValueCallback& callback) {
+ base::Optional<BluetoothRemoteGattService::GattErrorCode>
+ error_code;
+ std::vector<uint8_t> value;
+ if (read_success) {
+ error_code = base::nullopt;
+ value =
+ correct_eid ? GetCorrectEidValue() : GetIncorrectEidValue();
+ } else {
+ error_code = device::BluetoothGattService::GATT_ERROR_FAILED;
+ }
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- read_success
- ? base::BindOnce(std::move(callback),
- correct_eid ? GetCorrectEidValue()
- : GetIncorrectEidValue())
- : base::BindOnce(
- std::move(error_callback),
- BluetoothGattService::GATT_ERROR_FAILED));
+ base::BindOnce(std::move(callback), error_code, value));
}));
return characteristic;
}
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.cc b/content/browser/bluetooth/web_bluetooth_service_impl.cc
index 953aefe8..ef657ad3 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.cc
@@ -1136,14 +1136,9 @@
return;
}
- // TODO(crbug.com/730593): Remove AdaptCallbackForRepeating() by updating
- // the callee interface.
- auto copyable_callback = AdaptCallbackForRepeating(std::move(callback));
query_result.characteristic->ReadRemoteCharacteristic(
- base::BindOnce(&WebBluetoothServiceImpl::OnCharacteristicReadValueSuccess,
- weak_ptr_factory_.GetWeakPtr(), copyable_callback),
- base::BindOnce(&WebBluetoothServiceImpl::OnCharacteristicReadValueFailed,
- weak_ptr_factory_.GetWeakPtr(), copyable_callback));
+ base::BindOnce(&WebBluetoothServiceImpl::OnCharacteristicReadValue,
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
void WebBluetoothServiceImpl::RemoteCharacteristicWriteValue(
@@ -1313,14 +1308,9 @@
return;
}
- // TODO(crbug.com/730593): Remove AdaptCallbackForRepeating() by updating
- // the callee interface.
- auto copyable_callback = base::AdaptCallbackForRepeating(std::move(callback));
query_result.descriptor->ReadRemoteDescriptor(
- base::BindOnce(&WebBluetoothServiceImpl::OnDescriptorReadValueSuccess,
- weak_ptr_factory_.GetWeakPtr(), copyable_callback),
- base::BindOnce(&WebBluetoothServiceImpl::OnDescriptorReadValueFailed,
- weak_ptr_factory_.GetWeakPtr(), copyable_callback));
+ base::BindOnce(&WebBluetoothServiceImpl::OnDescriptorReadValue,
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
void WebBluetoothServiceImpl::RemoteDescriptorWriteValue(
@@ -1887,21 +1877,19 @@
std::move(callback).Run(TranslateConnectErrorAndRecord(error_code));
}
-void WebBluetoothServiceImpl::OnCharacteristicReadValueSuccess(
+void WebBluetoothServiceImpl::OnCharacteristicReadValue(
RemoteCharacteristicReadValueCallback callback,
+ base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
const std::vector<uint8_t>& value) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ if (error_code.has_value()) {
+ std::move(callback).Run(TranslateGATTError(error_code.value()),
+ /*value=*/base::nullopt);
+ return;
+ }
std::move(callback).Run(blink::mojom::WebBluetoothResult::SUCCESS, value);
}
-void WebBluetoothServiceImpl::OnCharacteristicReadValueFailed(
- RemoteCharacteristicReadValueCallback callback,
- BluetoothRemoteGattService::GattErrorCode error_code) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
- std::move(callback).Run(TranslateGATTError(error_code),
- /*value=*/base::nullopt);
-}
-
void WebBluetoothServiceImpl::OnCharacteristicWriteValueSuccess(
RemoteCharacteristicWriteValueCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -1949,21 +1937,19 @@
std::move(callback).Run();
}
-void WebBluetoothServiceImpl::OnDescriptorReadValueSuccess(
+void WebBluetoothServiceImpl::OnDescriptorReadValue(
RemoteDescriptorReadValueCallback callback,
+ base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
const std::vector<uint8_t>& value) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ if (error_code.has_value()) {
+ std::move(callback).Run(TranslateGATTError(error_code.value()),
+ /*value=*/base::nullopt);
+ return;
+ }
std::move(callback).Run(blink::mojom::WebBluetoothResult::SUCCESS, value);
}
-void WebBluetoothServiceImpl::OnDescriptorReadValueFailed(
- RemoteDescriptorReadValueCallback callback,
- BluetoothRemoteGattService::GattErrorCode error_code) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
- std::move(callback).Run(TranslateGATTError(error_code),
- /*value=*/base::nullopt);
-}
-
void WebBluetoothServiceImpl::OnDescriptorWriteValueSuccess(
RemoteDescriptorWriteValueCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.h b/content/browser/bluetooth/web_bluetooth_service_impl.h
index 436183d..c353346 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.h
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.h
@@ -115,6 +115,8 @@
BluetoothScanningPermissionRevokedWhenBlocked);
FRIEND_TEST_ALL_PREFIXES(WebBluetoothServiceImplTest,
BluetoothScanningPermissionRevokedWhenFocusIsLost);
+ FRIEND_TEST_ALL_PREFIXES(WebBluetoothServiceImplTest,
+ ReadCharacteristicValueErrorWithValueIgnored);
friend class FrameConnectedBluetoothDevicesTest;
friend class WebBluetoothServiceImplTest;
using PrimaryServicesRequestCallback =
@@ -294,12 +296,11 @@
device::BluetoothDevice::ConnectErrorCode error_code);
// Callbacks for BluetoothRemoteGattCharacteristic::ReadRemoteCharacteristic.
- void OnCharacteristicReadValueSuccess(
+ void OnCharacteristicReadValue(
RemoteCharacteristicReadValueCallback callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
const std::vector<uint8_t>& value);
- void OnCharacteristicReadValueFailed(
- RemoteCharacteristicReadValueCallback callback,
- device::BluetoothRemoteGattService::GattErrorCode error_code);
// Callbacks for BluetoothRemoteGattCharacteristic::WriteRemoteCharacteristic.
void OnCharacteristicWriteValueSuccess(
@@ -324,11 +325,11 @@
RemoteCharacteristicStopNotificationsCallback callback);
// Callbacks for BluetoothRemoteGattDescriptor::ReadRemoteDescriptor.
- void OnDescriptorReadValueSuccess(RemoteDescriptorReadValueCallback callback,
- const std::vector<uint8_t>& value);
- void OnDescriptorReadValueFailed(
+ void OnDescriptorReadValue(
RemoteDescriptorReadValueCallback callback,
- device::BluetoothRemoteGattService::GattErrorCode error_code);
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value);
// Callbacks for BluetoothRemoteGattDescriptor::WriteRemoteDescriptor.
void OnDescriptorWriteValueSuccess(
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl_unittest.cc b/content/browser/bluetooth/web_bluetooth_service_impl_unittest.cc
index b69e5161..f002398a 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl_unittest.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl_unittest.cc
@@ -5,6 +5,7 @@
#include "content/browser/bluetooth/web_bluetooth_service_impl.h"
#include <utility>
+#include <vector>
#include "base/macros.h"
#include "base/test/bind.h"
@@ -20,6 +21,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/mojom/bluetooth/web_bluetooth.mojom.h"
+using testing::Mock;
using testing::Return;
namespace content {
@@ -228,10 +230,10 @@
RenderViewHostImplTestHarness::SetUp();
// Set up an adapter.
- scoped_refptr<FakeBluetoothAdapter> adapter(new FakeBluetoothAdapter());
- EXPECT_CALL(*adapter, IsPresent()).WillRepeatedly(Return(true));
+ adapter_ = new FakeBluetoothAdapter();
+ EXPECT_CALL(*adapter_, IsPresent()).WillRepeatedly(Return(true));
BluetoothAdapterFactoryWrapper::Get().SetBluetoothAdapterForTesting(
- adapter);
+ adapter_);
// Hook up the test bluetooth delegate.
old_browser_client_ = SetBrowserClientForTesting(&browser_client_);
@@ -244,6 +246,7 @@
}
void TearDown() override {
+ adapter_.reset();
service_ = nullptr;
SetBrowserClientForTesting(old_browser_client_);
RenderViewHostImplTestHarness::TearDown();
@@ -301,6 +304,7 @@
return result;
}
+ scoped_refptr<FakeBluetoothAdapter> adapter_;
WebBluetoothServiceImpl* service_;
TestContentBrowserClient browser_client_;
ContentBrowserClient* old_browser_client_ = nullptr;
@@ -501,4 +505,34 @@
EXPECT_TRUE(client_impl_3.on_connection_error_called());
}
+TEST_F(WebBluetoothServiceImplTest,
+ ReadCharacteristicValueErrorWithValueIgnored) {
+ // The contract for calls accepting a
+ // BluetoothRemoteGattCharacteristic::ValueCallback callback argument is that
+ // when an error occurs, value must be ignored. This test verifies that
+ // WebBluetoothServiceImpl::OnCharacteristicReadValue honors that contract
+ // and will not pass a value to it's callback
+ // (a RemoteCharacteristicReadValueCallback instance) when an error occurs
+ // with a non-empty value array.
+ const std::vector<uint8_t> read_error_value = {1, 2, 3};
+ bool callback_called = false;
+ service_->OnCharacteristicReadValue(
+ base::BindLambdaForTesting(
+ [&callback_called](
+ blink::mojom::WebBluetoothResult result,
+ const base::Optional<std::vector<uint8_t>>& value) {
+ callback_called = true;
+ EXPECT_EQ(
+ blink::mojom::WebBluetoothResult::GATT_OPERATION_IN_PROGRESS,
+ result);
+ EXPECT_FALSE(value.has_value());
+ }),
+ device::BluetoothGattService::GATT_ERROR_IN_PROGRESS, read_error_value);
+ EXPECT_TRUE(callback_called);
+
+ // This test doesn't invoke any methods of the mock adapter. Allow it to be
+ // leaked without producing errors.
+ Mock::AllowLeak(adapter_.get());
+}
+
} // namespace content
diff --git a/content/web_test/browser/web_test_bluetooth_adapter_provider.cc b/content/web_test/browser/web_test_bluetooth_adapter_provider.cc
index 82c4d8ff..126e70c 100644
--- a/content/web_test/browser/web_test_bluetooth_adapter_provider.cc
+++ b/content/web_test/browser/web_test_bluetooth_adapter_provider.cc
@@ -707,9 +707,9 @@
NiceMockBluetoothGattCharacteristic* measurement_ptr =
measurement_interval.get();
- ON_CALL(*measurement_interval, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(
- RunCallback<0 /* success_callback */>(std::vector<uint8_t>({1})));
+ ON_CALL(*measurement_interval, ReadRemoteCharacteristic_(_))
+ .WillByDefault(RunCallbackWithResult<0>(/*error_code=*/base::nullopt,
+ std::vector<uint8_t>({1})));
ON_CALL(*measurement_interval, WriteRemoteCharacteristic_(_, _, _, _))
.WillByDefault(RunCallback<2 /* success_callback */>());
@@ -730,14 +730,13 @@
BluetoothUUID(kUserDescriptionUUID),
device::BluetoothRemoteGattCharacteristic::PROPERTY_READ);
- ON_CALL(*user_description, ReadRemoteDescriptor_(_, _))
+ ON_CALL(*user_description, ReadRemoteDescriptor_(_))
.WillByDefault(
Invoke([descriptorName](
- BluetoothRemoteGattDescriptor::ValueCallback& callback,
- BluetoothRemoteGattDescriptor::ErrorCallback&) {
+ BluetoothRemoteGattDescriptor::ValueCallback& callback) {
std::vector<uint8_t> value(descriptorName.begin(),
descriptorName.end());
- std::move(callback).Run(value);
+ std::move(callback).Run(/*error_code=*/base::nullopt, value);
}));
ON_CALL(*user_description, WriteRemoteDescriptor_(_, _, _))
@@ -769,10 +768,9 @@
// because this is used in web tests that may not report a mock
// expectation
// error correctly as a web test failure.
- ON_CALL(*no_read_descriptor, ReadRemoteDescriptor_(_, _))
+ ON_CALL(*no_read_descriptor, ReadRemoteDescriptor_(_))
.WillByDefault(
- Invoke([](BluetoothRemoteGattDescriptor::ValueCallback&,
- BluetoothRemoteGattDescriptor::ErrorCallback&) {
+ Invoke([](BluetoothRemoteGattDescriptor::ValueCallback&) {
NOTREACHED();
}));
@@ -1016,20 +1014,20 @@
NiceMockBluetoothGattCharacteristic* measurement_ptr =
measurement_interval.get();
- ON_CALL(*measurement_interval, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(
- Invoke([adapter_ptr, device_ptr, disconnect, succeeds](
- BluetoothRemoteGattCharacteristic::ValueCallback& callback,
- BluetoothRemoteGattCharacteristic::ErrorCallback&
- error_callback) {
+ ON_CALL(*measurement_interval, ReadRemoteCharacteristic_(_))
+ .WillByDefault(Invoke(
+ [adapter_ptr, device_ptr, disconnect, succeeds](
+ BluetoothRemoteGattCharacteristic::ValueCallback& callback) {
base::OnceClosure pending;
if (succeeds) {
pending = base::BindOnce(std::move(callback),
+ /*error_code=*/base::nullopt,
std::vector<uint8_t>({1}));
} else {
pending =
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_FAILED);
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
device_ptr->PushPendingCallback(std::move(pending));
if (disconnect) {
@@ -1123,19 +1121,20 @@
BluetoothUUID(kUserDescriptionUUID),
device::BluetoothRemoteGattCharacteristic::PROPERTY_READ);
- ON_CALL(*user_descriptor, ReadRemoteDescriptor_(_, _))
- .WillByDefault(Invoke(
- [adapter_ptr, device_ptr, disconnect, succeeds](
- BluetoothRemoteGattDescriptor::ValueCallback& callback,
- BluetoothRemoteGattDescriptor::ErrorCallback& error_callback) {
+ ON_CALL(*user_descriptor, ReadRemoteDescriptor_(_))
+ .WillByDefault(
+ Invoke([adapter_ptr, device_ptr, disconnect, succeeds](
+ BluetoothRemoteGattDescriptor::ValueCallback& callback) {
base::OnceClosure pending;
if (succeeds) {
pending = base::BindOnce(std::move(callback),
+ /*error_code=*/base::nullopt,
std::vector<uint8_t>({1}));
} else {
pending =
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_FAILED);
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
device_ptr->PushPendingCallback(std::move(pending));
if (disconnect) {
@@ -1463,11 +1462,9 @@
// Crash if ReadRemoteCharacteristic called. Not using GoogleMock's Expect
// because this is used in web tests that may not report a mock expectation
// error correctly as a web test failure.
- ON_CALL(*blocklist_exclude_reads_characteristic,
- ReadRemoteCharacteristic_(_, _))
+ ON_CALL(*blocklist_exclude_reads_characteristic, ReadRemoteCharacteristic_(_))
.WillByDefault(
- Invoke([](BluetoothRemoteGattCharacteristic::ValueCallback&,
- BluetoothRemoteGattCharacteristic::ErrorCallback&) {
+ Invoke([](BluetoothRemoteGattCharacteristic::ValueCallback&) {
NOTREACHED();
}));
@@ -1501,10 +1498,9 @@
// Crash if ReadRemoteCharacteristic called. Not using GoogleMock's Expect
// because this is used in web tests that may not report a mock expectation
// error correctly as a web test failure.
- ON_CALL(*serial_number_string, ReadRemoteCharacteristic_(_, _))
+ ON_CALL(*serial_number_string, ReadRemoteCharacteristic_(_))
.WillByDefault(
- Invoke([](BluetoothRemoteGattCharacteristic::ValueCallback&,
- BluetoothRemoteGattCharacteristic::ErrorCallback&) {
+ Invoke([](BluetoothRemoteGattCharacteristic::ValueCallback&) {
NOTREACHED();
}));
@@ -1530,8 +1526,9 @@
std::vector<uint8_t> device_name_value;
if (base::Optional<std::string> name = device->GetName())
device_name_value.assign(name.value().begin(), name.value().end());
- ON_CALL(*device_name, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(RunCallback<0>(device_name_value));
+ ON_CALL(*device_name, ReadRemoteCharacteristic_(_))
+ .WillByDefault(RunCallbackWithResult<0>(/*error_code=*/base::nullopt,
+ device_name_value));
// Write response.
ON_CALL(*device_name, WriteRemoteCharacteristic_(_, _, _, _))
@@ -1556,8 +1553,9 @@
std::vector<uint8_t> value(1);
value[0] = false;
- ON_CALL(*peripheral_privacy_flag, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(RunCallback<0>(value));
+ ON_CALL(*peripheral_privacy_flag, ReadRemoteCharacteristic_(_))
+ .WillByDefault(
+ RunCallbackWithResult<0>(/*error_code=*/base::nullopt, value));
// Crash if WriteRemoteCharacteristic called. Not using GoogleMock's Expect
// because this is used in web tests that may not report a mock
@@ -1623,9 +1621,9 @@
"Body Sensor Location Chest", heart_rate.get(), kBodySensorLocation,
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
- ON_CALL(*body_sensor_location_chest, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(RunCallback<0 /* success_callback */>(
- std::vector<uint8_t>({1} /* Chest */)));
+ ON_CALL(*body_sensor_location_chest, ReadRemoteCharacteristic_(_))
+ .WillByDefault(RunCallbackWithResult<0>(
+ /*error_code=*/base::nullopt, std::vector<uint8_t>({1} /* Chest */)));
// Body Sensor Location Characteristic (Wrist)
std::unique_ptr<NiceMockBluetoothGattCharacteristic>
@@ -1633,9 +1631,9 @@
"Body Sensor Location Wrist", heart_rate.get(), kBodySensorLocation,
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
- ON_CALL(*body_sensor_location_wrist, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(RunCallback<0 /* success_callback */>(
- std::vector<uint8_t>({2} /* Wrist */)));
+ ON_CALL(*body_sensor_location_wrist, ReadRemoteCharacteristic_(_))
+ .WillByDefault(RunCallbackWithResult<0>(
+ /*error_code=*/base::nullopt, std::vector<uint8_t>({2} /* Wrist */)));
heart_rate->AddMockCharacteristic(std::move(heart_rate_measurement));
heart_rate->AddMockCharacteristic(std::move(body_sensor_location_chest));
@@ -1701,9 +1699,10 @@
service, identifier, BluetoothUUID(uuid), properties,
BluetoothGattCharacteristic::Permission::PERMISSION_NONE);
- ON_CALL(*characteristic, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(
- RunCallback<1>(BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED));
+ ON_CALL(*characteristic, ReadRemoteCharacteristic_(_))
+ .WillByDefault(RunCallbackWithResult<0>(
+ BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED,
+ /*value=*/std::vector<uint8_t>()));
ON_CALL(*characteristic, WriteRemoteCharacteristic_(_, _, _, _))
.WillByDefault(
@@ -1735,8 +1734,10 @@
BluetoothRemoteGattCharacteristic::PROPERTY_INDICATE));
// Read response.
- ON_CALL(*characteristic, ReadRemoteCharacteristic_(_, _))
- .WillByDefault(RunCallback<1 /* error_callback */>(error_code));
+ ON_CALL(*characteristic, ReadRemoteCharacteristic_(_))
+ .WillByDefault(
+ RunCallbackWithResult<0>(error_code,
+ /*value=*/std::vector<uint8_t>()));
// Write response.
ON_CALL(*characteristic, WriteRemoteCharacteristic_(_, _, _, _))
@@ -1756,8 +1757,10 @@
BluetoothUUID(kUserDescriptionUUID),
device::BluetoothRemoteGattCharacteristic::PROPERTY_READ);
- ON_CALL(*error_descriptor, ReadRemoteDescriptor_(_, _))
- .WillByDefault(RunCallback<1 /* error_callback */>(error_code));
+ ON_CALL(*error_descriptor, ReadRemoteDescriptor_(_))
+ .WillByDefault(
+ RunCallbackWithResult<0>(error_code,
+ /*value=*/std::vector<uint8_t>()));
ON_CALL(*error_descriptor, WriteRemoteDescriptor_(_, _, _))
.WillByDefault(RunCallback<2 /* error_callback */>(error_code));
diff --git a/device/BUILD.gn b/device/BUILD.gn
index 1255fe6..1197c3eb 100644
--- a/device/BUILD.gn
+++ b/device/BUILD.gn
@@ -252,6 +252,7 @@
"bluetooth/bluez/bluetooth_service_record_bluez_unittest.cc",
"bluetooth/bluez/bluetooth_socket_bluez_unittest.cc",
"bluetooth/dbus/bluetooth_gatt_application_service_provider_unittest.cc",
+ "bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_unittest.cc",
"bluetooth/test/bluetooth_test_bluez.cc",
"bluetooth/test/bluetooth_test_bluez.h",
]
diff --git a/device/bluetooth/bluetooth_adapter_mac_metrics_unittest.mm b/device/bluetooth/bluetooth_adapter_mac_metrics_unittest.mm
index 974e312..1bb09dda 100644
--- a/device/bluetooth/bluetooth_adapter_mac_metrics_unittest.mm
+++ b/device/bluetooth/bluetooth_adapter_mac_metrics_unittest.mm
@@ -141,8 +141,8 @@
EXPECT_EQ(1u, characteristic_->GetDescriptors().size());
BluetoothRemoteGattDescriptor* descriptor =
characteristic_->GetDescriptors()[0];
- descriptor->ReadRemoteDescriptor(GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ descriptor->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
SimulateGattDescriptorUpdateError(
descriptor, BluetoothRemoteGattService::GATT_ERROR_FAILED);
base::RunLoop().RunUntilIdle();
diff --git a/device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc b/device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc
index 78deac8..f9916bd01 100644
--- a/device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc
+++ b/device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc
@@ -63,8 +63,8 @@
MAYBE_ReadLocalCharacteristicValue) {
delegate_->value_to_write_ = 0x1337;
SimulateLocalGattCharacteristicValueReadRequest(
- device_, read_characteristic_.get(), GetReadValueCallback(Call::EXPECTED),
- GetCallback(Call::NOT_EXPECTED));
+ device_, read_characteristic_.get(),
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(delegate_->value_to_write_, GetInteger(last_read_value_));
EXPECT_EQ(device_->GetIdentifier(), delegate_->last_seen_device_);
@@ -129,7 +129,7 @@
delegate_->should_fail_ = true;
SimulateLocalGattCharacteristicValueReadRequest(
device_, read_characteristic_.get(),
- GetReadValueCallback(Call::NOT_EXPECTED), GetCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
EXPECT_NE(delegate_->value_to_write_, GetInteger(last_read_value_));
EXPECT_NE(device_->GetIdentifier(), delegate_->last_seen_device_);
@@ -147,7 +147,7 @@
delegate_->value_to_write_ = 0x1337;
SimulateLocalGattCharacteristicValueReadRequest(
device_, write_characteristic_.get(),
- GetReadValueCallback(Call::NOT_EXPECTED), GetCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
EXPECT_NE(delegate_->value_to_write_, GetInteger(last_read_value_));
EXPECT_NE(device_->GetIdentifier(), delegate_->last_seen_device_);
diff --git a/device/bluetooth/bluetooth_local_gatt_descriptor_unittest.cc b/device/bluetooth/bluetooth_local_gatt_descriptor_unittest.cc
index 2d1c02f..8273229e 100644
--- a/device/bluetooth/bluetooth_local_gatt_descriptor_unittest.cc
+++ b/device/bluetooth/bluetooth_local_gatt_descriptor_unittest.cc
@@ -54,8 +54,8 @@
TEST_F(BluetoothLocalGattDescriptorTest, MAYBE_ReadLocalDescriptorValue) {
delegate_->value_to_write_ = 0x1337;
SimulateLocalGattDescriptorValueReadRequest(
- device_, read_descriptor_.get(), GetReadValueCallback(Call::EXPECTED),
- GetCallback(Call::NOT_EXPECTED));
+ device_, read_descriptor_.get(),
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(delegate_->value_to_write_, GetInteger(last_read_value_));
EXPECT_EQ(device_->GetIdentifier(), delegate_->last_seen_device_);
@@ -85,8 +85,8 @@
delegate_->value_to_write_ = 0x1337;
delegate_->should_fail_ = true;
SimulateLocalGattDescriptorValueReadRequest(
- device_, read_descriptor_.get(), GetReadValueCallback(Call::NOT_EXPECTED),
- GetCallback(Call::EXPECTED));
+ device_, read_descriptor_.get(),
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
EXPECT_NE(delegate_->value_to_write_, GetInteger(last_read_value_));
EXPECT_NE(device_->GetIdentifier(), delegate_->last_seen_device_);
@@ -121,7 +121,7 @@
delegate_->value_to_write_ = 0x1337;
SimulateLocalGattDescriptorValueReadRequest(
device_, write_descriptor_.get(),
- GetReadValueCallback(Call::NOT_EXPECTED), GetCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
EXPECT_NE(delegate_->value_to_write_, GetInteger(last_read_value_));
EXPECT_NE(device_->GetIdentifier(), delegate_->last_seen_device_);
diff --git a/device/bluetooth/bluetooth_local_gatt_service.h b/device/bluetooth/bluetooth_local_gatt_service.h
index 24eb3b1..251c92c3 100644
--- a/device/bluetooth/bluetooth_local_gatt_service.h
+++ b/device/bluetooth/bluetooth_local_gatt_service.h
@@ -12,6 +12,7 @@
#include "base/callback_forward.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
#include "device/bluetooth/bluetooth_adapter.h"
#include "device/bluetooth/bluetooth_export.h"
#include "device/bluetooth/bluetooth_gatt_characteristic.h"
@@ -45,28 +46,28 @@
class Delegate {
public:
// Callbacks used for communicating GATT request responses.
- using ValueCallback = base::OnceCallback<void(const std::vector<uint8_t>&)>;
+ using ValueCallback = base::OnceCallback<void(
+ base::Optional<BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>&)>;
using ErrorCallback = base::OnceClosure;
// Called when a remote device |device| requests to read the value of the
- // characteristic |characteristic| starting at offset |offset|.
- // This method is only called if the characteristic was specified as
- // readable and any authentication and authorization challenges were
- // satisfied by the remote device.
+ // characteristic |characteristic| starting at offset |offset|. To respond
+ // to the request with failure (e.g. if an invalid offset was given),
+ // delegates must invoke |callback| with the appropriate error code. If
+ // |callback| is not invoked, the request will time out resulting in an
+ // error. Therefore, delegates MUST invoke |callback| regardless of success
+ // or failure.
//
// To respond to the request with success and return the requested value,
- // the delegate must invoke |callback| with the value. Doing so will
- // automatically update the value property of |characteristic|. To respond
- // to the request with failure (e.g. if an invalid offset was given),
- // delegates must invoke |error_callback|. If neither callback parameter is
- // invoked, the request will time out and result in an error. Therefore,
- // delegates MUST invoke either |callback| or |error_callback|.
+ // the delegate must invoke |callback| with the value (and without an error
+ // code). Doing so will automatically update the value property of
+ // |characteristic|.
virtual void OnCharacteristicReadRequest(
const BluetoothDevice* device,
const BluetoothLocalGattCharacteristic* characteristic,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) = 0;
+ ValueCallback callback) = 0;
// Called when a remote device |device| requests to write the value of the
// characteristic |characteristic| starting at offset |offset|.
@@ -113,24 +114,22 @@
ErrorCallback error_callback) = 0;
// Called when a remote device |device| requests to read the value of the
- // descriptor |descriptor| starting at offset |offset|.
- // This method is only called if the descriptor was specified as
- // readable and any authentication and authorization challenges were
- // satisfied by the remote device.
+ // descriptor |descriptor| starting at offset |offset|. To respond
+ // to the request with failure (e.g. if an invalid offset was given),
+ // delegates must invoke |callback| with the appropriate error code. If
+ // |callback| is not invoked, the request will time out resulting in an
+ // error. Therefore, delegates MUST invoke |callback| regardless of success
+ // or failure.
//
// To respond to the request with success and return the requested value,
- // the delegate must invoke |callback| with the value. Doing so will
- // automatically update the value property of |descriptor|. To respond
- // to the request with failure (e.g. if an invalid offset was given),
- // delegates must invoke |error_callback|. If neither callback parameter is
- // invoked, the request will time out and result in an error. Therefore,
- // delegates MUST invoke either |callback| or |error_callback|.
+ // the delegate must invoke |callback| with the value (and without an error
+ // code). Doing so will automatically update the value property of
+ // |descriptor|.
virtual void OnDescriptorReadRequest(
const BluetoothDevice* device,
const BluetoothLocalGattDescriptor* descriptor,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) = 0;
+ ValueCallback callback) = 0;
// Called when a remote device |devie| requests to write the value of the
// descriptor |descriptor| starting at offset |offset|.
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic.h b/device/bluetooth/bluetooth_remote_gatt_characteristic.h
index 873f950..64a3aed 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic.h
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic.h
@@ -51,8 +51,12 @@
};
// The ValueCallback is used to return the value of a remote characteristic
- // upon a read request.
- using ValueCallback = base::OnceCallback<void(const std::vector<uint8_t>&)>;
+ // upon a read request. Upon successful completion |error_code| will not
+ // have a value and |value| may be used. When unsuccessful |error_code| will
+ // have a value and |value| must be ignored.
+ using ValueCallback = base::OnceCallback<void(
+ base::Optional<BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value)>;
// The NotifySessionCallback is used to return sessions after they have
// been successfully started.
@@ -139,10 +143,8 @@
#endif
// Sends a read request to a remote characteristic to read its value.
- // |callback| is called to return the read value on success and
- // |error_callback| is called for failures.
- virtual void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) = 0;
+ // |callback| is called to return the read value or error.
+ virtual void ReadRemoteCharacteristic(ValueCallback callback) = 0;
// Sends a write request to a remote characteristic with the value |value|
// using the specified |write_type|. |callback| is called to signal success
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc
index 4fd63137..50ca90d 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc
@@ -55,10 +55,10 @@
~BluetoothRemoteGattCharacteristicAndroid() {
Java_ChromeBluetoothRemoteGattCharacteristic_onBluetoothRemoteGattCharacteristicAndroidDestruction(
AttachCurrentThread(), j_characteristic_);
- if (!read_callback_.is_null()) {
- DCHECK(!read_error_callback_.is_null());
- std::move(read_error_callback_)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ if (read_callback_) {
+ std::move(read_callback_)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
if (!write_callback_.is_null()) {
@@ -126,28 +126,27 @@
}
void BluetoothRemoteGattCharacteristicAndroid::ReadRemoteCharacteristic(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
if (read_pending_ || write_pending_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ /*value=*/std::vector<uint8_t>()));
return;
}
if (!Java_ChromeBluetoothRemoteGattCharacteristic_readRemoteCharacteristic(
AttachCurrentThread(), j_characteristic_)) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_FAILED));
+ FROM_HERE, base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
read_pending_ = true;
read_callback_ = std::move(callback);
- read_error_callback_ = std::move(error_callback);
}
void BluetoothRemoteGattCharacteristicAndroid::WriteRemoteCharacteristic(
@@ -234,15 +233,16 @@
// Clear callbacks before calling to avoid reentrancy issues.
ValueCallback read_callback = std::move(read_callback_);
- ErrorCallback read_error_callback = std::move(read_error_callback_);
+ if (!read_callback)
+ return;
- if (status == 0 // android.bluetooth.BluetoothGatt.GATT_SUCCESS
- && !read_callback.is_null()) {
+ if (status == 0) { // android.bluetooth.BluetoothGatt.GATT_SUCCESS
base::android::JavaByteArrayToByteVector(env, value, &value_);
- std::move(read_callback).Run(value_);
- } else if (!read_error_callback.is_null()) {
- std::move(read_error_callback)
- .Run(BluetoothRemoteGattServiceAndroid::GetGattErrorCode(status));
+ std::move(read_callback).Run(/*error_code=*/base::nullopt, value_);
+ } else {
+ std::move(read_callback)
+ .Run(BluetoothRemoteGattServiceAndroid::GetGattErrorCode(status),
+ /*value=*/std::vector<uint8_t>());
}
}
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.h b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.h
index 7710e77..89f5514f 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.h
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.h
@@ -63,8 +63,7 @@
const std::string& identifier) const override;
std::vector<BluetoothRemoteGattDescriptor*> GetDescriptorsByUUID(
const BluetoothUUID& uuid) const override;
- void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteCharacteristic(ValueCallback callback) override;
void WriteRemoteCharacteristic(const std::vector<uint8_t>& value,
WriteType write_type,
base::OnceClosure callback,
@@ -142,10 +141,9 @@
// Adapter unique instance ID.
std::string instance_id_;
- // ReadRemoteCharacteristic callbacks and pending state.
+ // ReadRemoteCharacteristic callback and pending state.
bool read_pending_ = false;
ValueCallback read_callback_;
- ErrorCallback read_error_callback_;
// WriteRemoteCharacteristic callbacks and pending state.
bool write_pending_ = false;
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h b/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h
index 71510e2..60164dd 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h
@@ -41,8 +41,7 @@
const std::vector<uint8_t>& GetValue() const override;
BluetoothRemoteGattService* GetService() const override;
bool IsNotifying() const override;
- void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteCharacteristic(ValueCallback callback) override;
void WriteRemoteCharacteristic(const std::vector<uint8_t>& value,
WriteType write_type,
base::OnceClosure callback,
@@ -102,7 +101,7 @@
BluetoothRemoteGattDescriptorMac* GetBluetoothRemoteGattDescriptorMac(
CBDescriptor* cb_descriptor) const;
bool HasPendingRead() const {
- return !read_characteristic_value_callbacks_.first.is_null();
+ return !read_characteristic_value_callback_.is_null();
}
bool HasPendingWrite() const {
return !write_characteristic_value_callbacks_.first.is_null();
@@ -123,8 +122,11 @@
BluetoothUUID uuid_;
// Characteristic value.
std::vector<uint8_t> value_;
- // ReadRemoteCharacteristic request callbacks.
- std::pair<ValueCallback, ErrorCallback> read_characteristic_value_callbacks_;
+ // The destructor runs callbacks. Methods can use |destructor_called_| to
+ // protect against reentrant calls to a partially deleted instance.
+ bool destructor_called_ = false;
+ // ReadRemoteCharacteristic request callback.
+ ValueCallback read_characteristic_value_callback_;
// WriteRemoteCharacteristic request callbacks.
std::pair<base::OnceClosure, ErrorCallback>
write_characteristic_value_callbacks_;
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.mm b/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.mm
index 8cf91ed..276afb6 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.mm
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_mac.mm
@@ -84,9 +84,11 @@
}
BluetoothRemoteGattCharacteristicMac::~BluetoothRemoteGattCharacteristicMac() {
+ destructor_called_ = true;
if (HasPendingRead()) {
- std::move(read_characteristic_value_callbacks_.second)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(read_characteristic_value_callback_)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
if (HasPendingWrite()) {
std::move(write_characteristic_value_callbacks_.second)
@@ -129,27 +131,27 @@
}
void BluetoothRemoteGattCharacteristicMac::ReadRemoteCharacteristic(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
if (!IsReadable()) {
DVLOG(1) << *this << ": Characteristic not readable.";
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
- if (HasPendingRead() || HasPendingWrite()) {
+ if (destructor_called_ || HasPendingRead() || HasPendingWrite()) {
DVLOG(1) << *this << ": Characteristic read already in progress.";
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ /*value=*/std::vector<uint8_t>()));
return;
}
DVLOG(1) << *this << ": Read characteristic.";
- read_characteristic_value_callbacks_ =
- std::make_pair(std::move(callback), std::move(error_callback));
+ read_characteristic_value_callback_ = std::move(callback);
[GetCBPeripheral() readValueForCharacteristic:cb_characteristic_];
}
@@ -158,7 +160,7 @@
WriteType write_type,
base::OnceClosure callback,
ErrorCallback error_callback) {
- if (HasPendingRead() || HasPendingWrite()) {
+ if (destructor_called_ || HasPendingRead() || HasPendingWrite()) {
DVLOG(1) << *this << ": Characteristic write already in progress.";
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
@@ -205,7 +207,7 @@
BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED));
return;
}
- if (HasPendingRead() || HasPendingWrite()) {
+ if (destructor_called_ || HasPendingRead() || HasPendingWrite()) {
DVLOG(1) << *this << ": Characteristic write already in progress.";
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
@@ -271,8 +273,8 @@
// notification is received.
RecordDidUpdateValueResult(error);
if (HasPendingRead()) {
- std::pair<ValueCallback, ErrorCallback> callbacks;
- callbacks.swap(read_characteristic_value_callbacks_);
+ ValueCallback read_callback =
+ std::move(read_characteristic_value_callback_);
if (error) {
BluetoothGattService::GattErrorCode error_code =
BluetoothDeviceMac::GetGattErrorCodeFromNSError(error);
@@ -280,12 +282,14 @@
<< ": Bluetooth error while reading for characteristic, domain: "
<< BluetoothAdapterMac::String(error)
<< ", error code: " << error_code;
- std::move(callbacks.second).Run(error_code);
+ std::move(read_callback)
+ .Run(error_code,
+ /*value=*/std::vector<uint8_t>());
return;
}
DVLOG(1) << *this << ": Read request arrived.";
UpdateValue();
- std::move(callbacks.first).Run(value_);
+ std::move(read_callback).Run(/*error_code=*/base::nullopt, value_);
} else if (IsNotifying()) {
DVLOG(1) << *this << ": Notification arrived.";
UpdateValue();
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
index f0cc2fb..8c7e4b91 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
@@ -384,8 +384,7 @@
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
std::vector<uint8_t> empty_vector;
SimulateGattCharacteristicRead(characteristic1_, empty_vector);
base::RunLoop().RunUntilIdle();
@@ -499,23 +498,20 @@
ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
- bool read_error_callback_called = false;
- characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- base::BindLambdaForTesting(
- [&](BluetoothRemoteGattService::GattErrorCode error_code) {
- EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_FAILED,
- error_code);
- read_error_callback_called = true;
- // Retrying Read should fail:
- characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
- }));
+ bool read_characteristic_failed = false;
+ characteristic1_->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>&) {
+ EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_FAILED, error_code);
+ read_characteristic_failed = true;
+ // Retrying Read should fail:
+ characteristic1_->ReadRemoteCharacteristic(
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
+ }));
DeleteDevice(device_); // TODO(576906) delete only the characteristic.
base::RunLoop().RunUntilIdle();
- EXPECT_TRUE(read_error_callback_called);
+ EXPECT_TRUE(read_characteristic_failed);
EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
last_gatt_error_code_);
}
@@ -641,8 +637,7 @@
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
RememberCharacteristicForSubsequentAction(characteristic1_);
DeleteDevice(device_); // TODO(576906) delete only the characteristic.
@@ -678,8 +673,7 @@
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
// Set up for receiving a read response after disconnection.
// On macOS or WinRT no events arrive after disconnection so there is no point
@@ -919,8 +913,7 @@
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
std::vector<uint8_t> test_vector = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
SimulateGattCharacteristicRead(characteristic1_, test_vector);
@@ -941,9 +934,10 @@
static void TestCallback(
BluetoothRemoteGattCharacteristic::ValueCallback callback,
const TestBluetoothAdapterObserver& callback_observer,
+ base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
const std::vector<uint8_t>& value) {
EXPECT_EQ(0, callback_observer.gatt_characteristic_value_changed_count());
- std::move(callback).Run(value);
+ std::move(callback).Run(error_code, value);
}
#if defined(OS_ANDROID) || defined(OS_MAC)
@@ -971,10 +965,9 @@
TestBluetoothAdapterObserver observer(adapter_);
- characteristic1_->ReadRemoteCharacteristic(
- base::BindOnce(TestCallback, GetReadValueCallback(Call::EXPECTED),
- std::cref(observer)),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ characteristic1_->ReadRemoteCharacteristic(base::BindOnce(
+ TestCallback, GetReadValueCallback(Call::EXPECTED, Result::SUCCESS),
+ std::cref(observer)));
std::vector<uint8_t> test_vector = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
SimulateGattCharacteristicRead(characteristic1_, test_vector);
@@ -1099,8 +1092,7 @@
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
uint8_t values[] = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
std::vector<uint8_t> test_vector(values, values + base::size(values));
@@ -1115,8 +1107,7 @@
// Read again, with different value:
ResetEventCounts();
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
std::vector<uint8_t> empty_vector;
SimulateGattCharacteristicRead(characteristic1_, empty_vector);
base::RunLoop().RunUntilIdle();
@@ -1259,11 +1250,9 @@
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
characteristic2_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(0, callback_count_);
EXPECT_EQ(0, error_callback_count_);
@@ -1437,9 +1426,11 @@
std::vector<uint8_t> test_vector_1 = {0, 1, 2, 3, 4};
std::vector<uint8_t> test_vector_2 = {0xf, 0xf0, 0xff};
- characteristic1_->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
- GetReadValueCallback(Call::EXPECTED).Run(data);
+ characteristic1_->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS)
+ .Run(error_code, data);
EXPECT_EQ(1, gatt_read_characteristic_attempts_);
EXPECT_EQ(1, callback_count_);
@@ -1448,11 +1439,9 @@
EXPECT_EQ(test_vector_1, characteristic1_->GetValue());
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
SimulateGattCharacteristicRead(characteristic1_, test_vector_2);
- }),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ }));
SimulateGattCharacteristicRead(characteristic1_, test_vector_1);
base::RunLoop().RunUntilIdle();
@@ -1599,8 +1588,11 @@
std::vector<uint8_t> test_vector_1 = {0, 1, 2, 3, 4};
std::vector<uint8_t> test_vector_2 = {0xf, 0xf0, 0xff};
- characteristic1_->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
+ characteristic1_->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ ASSERT_FALSE(error_code.has_value())
+ << "unexpected error: " << error_code.value();
EXPECT_EQ(1, gatt_read_characteristic_attempts_);
EXPECT_EQ(0, gatt_write_characteristic_attempts_);
EXPECT_EQ(test_vector_1, data);
@@ -1621,12 +1613,7 @@
}));
SimulateGattCharacteristicWrite(characteristic1_);
- }),
- base::BindLambdaForTesting(
- [&](BluetoothGattService::GattErrorCode error_code) {
- ADD_FAILURE() << "unexpected error: " << error_code;
- loop.Quit();
- }));
+ }));
SimulateGattCharacteristicRead(characteristic1_, test_vector_1);
loop.Run();
@@ -1659,9 +1646,11 @@
std::vector<uint8_t> test_vector_1 = {0, 1, 2, 3, 4};
std::vector<uint8_t> test_vector_2 = {0xf, 0xf0, 0xff};
- characteristic1_->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
- GetReadValueCallback(Call::EXPECTED).Run(data);
+ characteristic1_->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS)
+ .Run(error_code, data);
EXPECT_EQ(1, gatt_read_characteristic_attempts_);
EXPECT_EQ(0, gatt_write_characteristic_attempts_);
@@ -1674,8 +1663,7 @@
test_vector_2, GetCallback(Call::EXPECTED),
GetGattErrorCallback(Call::NOT_EXPECTED));
SimulateGattCharacteristicWrite(characteristic1_);
- }),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ }));
SimulateGattCharacteristicRead(characteristic1_, test_vector_1);
base::RunLoop().RunUntilIdle();
@@ -1720,19 +1708,17 @@
EXPECT_EQ(1, gatt_write_characteristic_attempts_);
EXPECT_EQ(test_vector_1, last_write_value_);
- characteristic1_->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
+ characteristic1_->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& data) {
+ EXPECT_EQ(error_code, base::nullopt);
EXPECT_EQ(1, gatt_read_characteristic_attempts_);
EXPECT_EQ(1, gatt_write_characteristic_attempts_);
EXPECT_EQ(test_vector_2, data);
EXPECT_EQ(test_vector_2, characteristic1_->GetValue());
loop.Quit();
- }),
- base::BindLambdaForTesting(
- [&](BluetoothGattService::GattErrorCode error_code) {
- ADD_FAILURE() << "unexpected error: " << error_code;
- loop.Quit();
- }));
+ }));
SimulateGattCharacteristicRead(characteristic1_, test_vector_2);
}),
base::BindLambdaForTesting(
@@ -1783,8 +1769,7 @@
EXPECT_EQ(test_vector_1, last_write_value_);
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
SimulateGattCharacteristicRead(characteristic1_, test_vector_2);
}),
GetGattErrorCallback(Call::NOT_EXPECTED));
@@ -1820,8 +1805,7 @@
TestBluetoothAdapterObserver observer(adapter_);
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
SimulateGattCharacteristicReadError(
characteristic1_, BluetoothRemoteGattService::GATT_ERROR_INVALID_LENGTH);
SimulateGattCharacteristicReadError(
@@ -1916,8 +1900,7 @@
SimulateGattCharacteristicReadWillFailSynchronouslyOnce(characteristic1_);
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
EXPECT_EQ(0, gatt_read_characteristic_attempts_);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0, callback_count_);
@@ -1928,8 +1911,7 @@
// After failing once, can succeed:
ResetEventCounts();
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_characteristic_attempts_);
std::vector<uint8_t> empty_vector;
SimulateGattCharacteristicRead(characteristic1_, empty_vector);
@@ -2042,11 +2024,9 @@
BluetoothRemoteGattCharacteristic::PROPERTY_READ));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
base::RunLoop().RunUntilIdle();
@@ -2202,17 +2182,13 @@
ADD_FAILURE() << "unexpected error: " << error_code;
loop1.Quit();
}));
- characteristic1_->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
- ADD_FAILURE() << "unexpected success";
+ characteristic1_->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ error_code);
loop2.Quit();
- }),
- base::BindLambdaForTesting(
- [&](BluetoothGattService::GattErrorCode error_code) {
- EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
- error_code);
- loop2.Quit();
- }));
+ }));
loop2.Run();
@@ -2250,8 +2226,7 @@
empty_vector, GetCallback(Call::EXPECTED),
GetGattErrorCallback(Call::NOT_EXPECTED));
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
base::RunLoop().RunUntilIdle();
@@ -2294,16 +2269,12 @@
base::RunLoop loop1;
base::RunLoop loop2;
std::vector<uint8_t> empty_vector;
- characteristic1_->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
- SUCCEED();
+ characteristic1_->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ EXPECT_FALSE(error_code.has_value()) << "unexpected failure";
loop1.Quit();
- }),
- base::BindLambdaForTesting(
- [&](BluetoothGattService::GattErrorCode error_code) {
- ADD_FAILURE() << "unexpected error: " << error_code;
- loop1.Quit();
- }));
+ }));
characteristic1_->WriteRemoteCharacteristic(
empty_vector, WriteType::kWithResponse, base::BindLambdaForTesting([&] {
ADD_FAILURE() << "unexpected success";
@@ -2349,8 +2320,7 @@
std::vector<uint8_t> empty_vector;
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
characteristic1_->DeprecatedWriteRemoteCharacteristic(
empty_vector, GetCallback(Call::NOT_EXPECTED),
GetGattErrorCallback(Call::EXPECTED));
@@ -2399,8 +2369,7 @@
TestBluetoothAdapterObserver observer(adapter_);
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
std::vector<uint8_t> notification_value = {111};
SimulateGattCharacteristicChanged(characteristic1_, notification_value);
@@ -4178,8 +4147,7 @@
SimulateGattDisconnection(device_);
// Do not yet call RunUntilIdle() to process the disconnect task.
characteristic1_->ReadRemoteCharacteristic(
- GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_FAILED,
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc
index 3572d0f..4527cd8 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_win.cc
@@ -56,10 +56,10 @@
}
parent_service_->GetWinAdapter()->NotifyGattCharacteristicRemoved(this);
- if (!read_characteristic_value_callbacks_.first.is_null()) {
- DCHECK(!read_characteristic_value_callbacks_.second.is_null());
- std::move(read_characteristic_value_callbacks_.second)
- .Run(BluetoothRemoteGattService::GATT_ERROR_FAILED);
+ if (read_characteristic_value_callback_) {
+ std::move(read_characteristic_value_callback_)
+ .Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
if (!write_characteristic_value_callbacks_.first.is_null()) {
@@ -131,25 +131,24 @@
}
void BluetoothRemoteGattCharacteristicWin::ReadRemoteCharacteristic(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
DCHECK(ui_task_runner_->RunsTasksInCurrentSequence());
if (!characteristic_info_.get()->IsReadable) {
- std::move(error_callback)
- .Run(BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED);
+ std::move(callback).Run(
+ BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED,
+ std::vector<uint8_t>());
return;
}
if (characteristic_value_read_or_write_in_progress_) {
- std::move(error_callback)
- .Run(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS);
+ std::move(callback).Run(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ std::vector<uint8_t>());
return;
}
characteristic_value_read_or_write_in_progress_ = true;
- read_characteristic_value_callbacks_ =
- std::make_pair(std::move(callback), std::move(error_callback));
+ read_characteristic_value_callback_ = std::move(callback);
task_manager_->PostReadGattCharacteristicValue(
parent_service_->GetServicePath(), characteristic_info_.get(),
base::BindOnce(&BluetoothRemoteGattCharacteristicWin::
@@ -363,16 +362,15 @@
DCHECK(ui_task_runner_->RunsTasksInCurrentSequence());
characteristic_value_read_or_write_in_progress_ = false;
- std::pair<ValueCallback, ErrorCallback> callbacks;
- callbacks.swap(read_characteristic_value_callbacks_);
+ ValueCallback callback = std::move(read_characteristic_value_callback_);
if (FAILED(hr)) {
- std::move(callbacks.second).Run(HRESULTToGattErrorCode(hr));
+ std::move(callback).Run(HRESULTToGattErrorCode(hr), std::vector<uint8_t>());
} else {
characteristic_value_.clear();
for (ULONG i = 0; i < value->DataSize; i++)
characteristic_value_.push_back(value->Data[i]);
- std::move(callbacks.first).Run(characteristic_value_);
+ std::move(callback).Run(base::nullopt, characteristic_value_);
}
}
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_win.h b/device/bluetooth/bluetooth_remote_gatt_characteristic_win.h
index 50fcaac8..e14b1384 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_win.h
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_win.h
@@ -42,8 +42,7 @@
Properties GetProperties() const override;
Permissions GetPermissions() const override;
bool IsNotifying() const override;
- void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteCharacteristic(ValueCallback callback) override;
void WriteRemoteCharacteristic(const std::vector<uint8_t>& value,
WriteType write_type,
base::OnceClosure callback,
@@ -113,8 +112,8 @@
// has been sent out to avoid duplicate notification.
bool characteristic_added_notified_;
- // ReadRemoteCharacteristic request callbacks.
- std::pair<ValueCallback, ErrorCallback> read_characteristic_value_callbacks_;
+ // ReadRemoteCharacteristic request callback.
+ ValueCallback read_characteristic_value_callback_;
// WriteRemoteCharacteristic request callbacks.
std::pair<base::OnceClosure, ErrorCallback>
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.cc
index 17cfeb2..0c84b10 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.cc
@@ -107,9 +107,11 @@
BluetoothRemoteGattCharacteristicWinrt::
~BluetoothRemoteGattCharacteristicWinrt() {
- if (pending_read_callbacks_) {
- std::move(pending_read_callbacks_->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ destructor_called_ = true;
+ if (pending_read_callback_) {
+ std::move(pending_read_callback_)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
if (pending_write_callbacks_) {
@@ -151,21 +153,23 @@
}
void BluetoothRemoteGattCharacteristicWinrt::ReadRemoteCharacteristic(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
if (!(GetProperties() & PROPERTY_READ)) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_NOT_PERMITTED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
- if (pending_read_callbacks_ || pending_write_callbacks_) {
+ if (destructor_called_ || pending_read_callback_ ||
+ pending_write_callbacks_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ /*value=*/std::vector<uint8_t>()));
return;
}
@@ -177,9 +181,9 @@
<< "GattCharacteristic::ReadValueWithCacheModeAsync failed: "
<< logging::SystemErrorCodeToString(hr);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_FAILED));
+ FROM_HERE, base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
@@ -192,14 +196,13 @@
BLUETOOTH_LOG(DEBUG) << "PostAsyncResults failed: "
<< logging::SystemErrorCodeToString(hr);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_FAILED));
+ FROM_HERE, base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
- pending_read_callbacks_ = std::make_unique<PendingReadCallbacks>(
- std::move(callback), std::move(error_callback));
+ pending_read_callback_ = std::move(callback);
}
void BluetoothRemoteGattCharacteristicWinrt::WriteRemoteCharacteristic(
@@ -207,7 +210,8 @@
WriteType write_type,
base::OnceClosure callback,
ErrorCallback error_callback) {
- if (pending_read_callbacks_ || pending_write_callbacks_) {
+ if (destructor_called_ || pending_read_callback_ ||
+ pending_write_callbacks_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(std::move(error_callback),
@@ -389,13 +393,6 @@
std::move(success_callback), std::move(split_error_callback.second));
}
-BluetoothRemoteGattCharacteristicWinrt::PendingReadCallbacks::
- PendingReadCallbacks(ValueCallback callback, ErrorCallback error_callback)
- : callback(std::move(callback)),
- error_callback(std::move(error_callback)) {}
-BluetoothRemoteGattCharacteristicWinrt::PendingReadCallbacks::
- ~PendingReadCallbacks() = default;
-
BluetoothRemoteGattCharacteristicWinrt::PendingWriteCallbacks::
PendingWriteCallbacks(base::OnceClosure callback,
ErrorCallback error_callback)
@@ -477,12 +474,13 @@
void BluetoothRemoteGattCharacteristicWinrt::OnReadValue(
ComPtr<IGattReadResult> read_result) {
- DCHECK(pending_read_callbacks_);
- auto pending_read_callbacks = std::move(pending_read_callbacks_);
+ DCHECK(pending_read_callback_);
+ auto pending_read_callback = std::move(pending_read_callback_);
if (!read_result) {
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -491,8 +489,9 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(DEBUG) << "Getting GATT Communication Status failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -503,14 +502,16 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(DEBUG) << "As IGattReadResult2 failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
- std::move(pending_read_callbacks->error_callback)
+ std::move(pending_read_callback)
.Run(BluetoothRemoteGattServiceWinrt::GetGattErrorCode(
- read_result_2.Get()));
+ read_result_2.Get()),
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -519,8 +520,9 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(DEBUG) << "Getting Characteristic Value failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -530,13 +532,14 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(DEBUG) << "Getting Pointer To Buffer Data failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
value_.assign(data, data + length);
- std::move(pending_read_callbacks->callback).Run(value_);
+ std::move(pending_read_callback).Run(/*error_code=*/base::nullopt, value_);
}
void BluetoothRemoteGattCharacteristicWinrt::OnWriteValueWithResultAndOption(
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.h b/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.h
index 1402fa2..4d9e9e7 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.h
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_winrt.h
@@ -45,8 +45,7 @@
// BluetoothRemoteGattCharacteristic:
const std::vector<uint8_t>& GetValue() const override;
BluetoothRemoteGattService* GetService() const override;
- void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteCharacteristic(ValueCallback callback) override;
void WriteRemoteCharacteristic(const std::vector<uint8_t>& value,
WriteType write_type,
base::OnceClosure callback,
@@ -73,14 +72,6 @@
ErrorCallback error_callback) override;
private:
- struct PendingReadCallbacks {
- PendingReadCallbacks(ValueCallback callback, ErrorCallback error_callback);
- ~PendingReadCallbacks();
-
- ValueCallback callback;
- ErrorCallback error_callback;
- };
-
struct PendingWriteCallbacks {
PendingWriteCallbacks(base::OnceClosure callback,
ErrorCallback error_callback);
@@ -144,10 +135,13 @@
uint16_t attribute_handle_;
std::string identifier_;
std::vector<uint8_t> value_;
- std::unique_ptr<PendingReadCallbacks> pending_read_callbacks_;
+ ValueCallback pending_read_callback_;
std::unique_ptr<PendingWriteCallbacks> pending_write_callbacks_;
std::unique_ptr<PendingNotificationCallbacks> pending_notification_callbacks_;
base::Optional<EventRegistrationToken> value_changed_token_;
+ // The destructor runs callbacks. Methods can use |destructor_called_| to
+ // protect against reentrant calls to a partially deleted instance.
+ bool destructor_called_ = false;
base::WeakPtrFactory<BluetoothRemoteGattCharacteristicWinrt>
weak_ptr_factory_{this};
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor.h b/device/bluetooth/bluetooth_remote_gatt_descriptor.h
index 25f0d7f..2c52d99 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor.h
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor.h
@@ -33,7 +33,13 @@
// The ValueCallback is used to return the value of a remote characteristic
// descriptor upon a read request.
- using ValueCallback = base::OnceCallback<void(const std::vector<uint8_t>&)>;
+ //
+ // This callback is called on both success and failure. On error |error_code|
+ // will contain a value and |value| should be ignored. When successful
+ // |error_code| will have no value and |value| may be used.
+ using ValueCallback = base::OnceCallback<void(
+ base::Optional<BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value)>;
// Returns the value of the descriptor. For remote descriptors, this is the
// most recently cached value of the remote descriptor. For local descriptors
@@ -46,10 +52,8 @@
virtual BluetoothRemoteGattCharacteristic* GetCharacteristic() const = 0;
// Sends a read request to a remote characteristic descriptor to read its
- // value. |callback| is called to return the read value on success and
- // |error_callback| is called for failures.
- virtual void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) = 0;
+ // value. |callback| is called to return the read value on success or error.
+ virtual void ReadRemoteDescriptor(ValueCallback callback) = 0;
// Sends a write request to a remote characteristic descriptor, to modify the
// value of the descriptor with the new value |new_value|. |callback| is
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_android.cc b/device/bluetooth/bluetooth_remote_gatt_descriptor_android.cc
index ecf9188..12fc03f 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_android.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_android.cc
@@ -80,28 +80,27 @@
}
void BluetoothRemoteGattDescriptorAndroid::ReadRemoteDescriptor(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
if (read_pending_ || write_pending_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ /*value=*/std::vector<uint8_t>()));
return;
}
if (!Java_ChromeBluetoothRemoteGattDescriptor_readRemoteDescriptor(
AttachCurrentThread(), j_descriptor_)) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED));
+ FROM_HERE, base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
read_pending_ = true;
read_callback_ = std::move(callback);
- read_error_callback_ = std::move(error_callback);
}
void BluetoothRemoteGattDescriptorAndroid::WriteRemoteDescriptor(
@@ -140,16 +139,17 @@
// Clear callbacks before calling to avoid reentrancy issues.
ValueCallback read_callback = std::move(read_callback_);
- ErrorCallback read_error_callback = std::move(read_error_callback_);
+ if (!read_callback)
+ return;
- if (status == 0 // android.bluetooth.BluetoothGatt.GATT_SUCCESS
- && !read_callback.is_null()) {
+ if (status == 0) { // android.bluetooth.BluetoothGatt.GATT_SUCCESS
base::android::JavaByteArrayToByteVector(env, value, &value_);
- std::move(read_callback).Run(value_);
+ std::move(read_callback).Run(/*error_code=*/base::nullopt, value_);
// TODO(https://crbug.com/584369): Call GattDescriptorValueChanged.
- } else if (!read_error_callback.is_null()) {
- std::move(read_error_callback)
- .Run(BluetoothRemoteGattServiceAndroid::GetGattErrorCode(status));
+ } else {
+ std::move(read_callback)
+ .Run(BluetoothRemoteGattServiceAndroid::GetGattErrorCode(status),
+ /*value=*/std::vector<uint8_t>());
}
}
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_android.h b/device/bluetooth/bluetooth_remote_gatt_descriptor_android.h
index b1e0672..de7d019 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_android.h
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_android.h
@@ -47,8 +47,7 @@
BluetoothRemoteGattCharacteristic* GetCharacteristic() const override;
BluetoothRemoteGattCharacteristic::Permissions GetPermissions()
const override;
- void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteDescriptor(ValueCallback callback) override;
void WriteRemoteDescriptor(const std::vector<uint8_t>& value,
base::OnceClosure callback,
ErrorCallback error_callback) override;
@@ -77,7 +76,6 @@
// ReadRemoteCharacteristic callbacks and pending state.
bool read_pending_ = false;
ValueCallback read_callback_;
- ErrorCallback read_error_callback_;
// WriteRemoteCharacteristic callbacks and pending state.
bool write_pending_ = false;
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.h b/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.h
index 7bb1a999..9131f0c 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.h
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.h
@@ -37,8 +37,7 @@
// BluetoothRemoteGattDescriptor
const std::vector<uint8_t>& GetValue() const override;
BluetoothRemoteGattCharacteristic* GetCharacteristic() const override;
- void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteDescriptor(ValueCallback callback) override;
void WriteRemoteDescriptor(const std::vector<uint8_t>& new_value,
base::OnceClosure callback,
ErrorCallback error_callback) override;
@@ -54,6 +53,10 @@
// Calls callbacks, when -[id<CBPeripheralDelegate>
// peripheral:didWriteValueForDescriptor:error:] is called.
void DidWriteValueForDescriptor(NSError* error);
+ bool HasPendingRead() const { return !read_value_callback_.is_null(); }
+ bool HasPendingWrite() const {
+ return !write_value_callbacks_.first.is_null();
+ }
// Returns CoreBluetooth peripheral.
CBPeripheral* GetCBPeripheral() const;
@@ -69,10 +72,11 @@
BluetoothUUID uuid_;
// Descriptor value.
std::vector<uint8_t> value_;
- // True if a gatt read or write request is in progress.
- bool value_read_or_write_in_progress_;
- // ReadRemoteDescriptor request callbacks.
- std::pair<ValueCallback, ErrorCallback> read_value_callbacks_;
+ // The destructor runs callbacks. Methods can use |destructor_called_| to
+ // protect against reentrant calls to a partially deleted instance.
+ bool destructor_called_ = false;
+ // ReadRemoteDescriptor request callback.
+ ValueCallback read_value_callback_;
// WriteRemoteDescriptor request callbacks.
std::pair<base::OnceClosure, ErrorCallback> write_value_callbacks_;
};
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.mm b/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.mm
index 03e5c93a..7ae6566 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.mm
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_mac.mm
@@ -47,8 +47,7 @@
BluetoothRemoteGattCharacteristicMac* characteristic,
CBDescriptor* descriptor)
: gatt_characteristic_(characteristic),
- cb_descriptor_(descriptor, base::scoped_policy::RETAIN),
- value_read_or_write_in_progress_(false) {
+ cb_descriptor_(descriptor, base::scoped_policy::RETAIN) {
uuid_ = BluetoothAdapterMac::BluetoothUUIDWithCBUUID([cb_descriptor_ UUID]);
identifier_ = base::SysNSStringToUTF8(
[NSString stringWithFormat:@"%s-%p", uuid_.canonical_value().c_str(),
@@ -74,11 +73,13 @@
}
BluetoothRemoteGattDescriptorMac::~BluetoothRemoteGattDescriptorMac() {
- if (!read_value_callbacks_.first.is_null()) {
- std::move(read_value_callbacks_)
- .second.Run(BluetoothGattService::GATT_ERROR_FAILED);
+ destructor_called_ = true;
+ if (HasPendingRead()) {
+ std::move(read_value_callback_)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
- if (!write_value_callbacks_.first.is_null()) {
+ if (HasPendingWrite()) {
std::move(write_value_callbacks_)
.second.Run(BluetoothGattService::GATT_ERROR_FAILED);
}
@@ -93,20 +94,18 @@
// value. |callback| is called to return the read value on success and
// |error_callback| is called for failures.
void BluetoothRemoteGattDescriptorMac::ReadRemoteDescriptor(
- ValueCallback callback,
- ErrorCallback error_callback) {
- if (value_read_or_write_in_progress_) {
+ ValueCallback callback) {
+ if (destructor_called_ || HasPendingRead() || HasPendingWrite()) {
DVLOG(1) << *this << ": Read failed, already in progress.";
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ /*value=*/std::vector<uint8_t>()));
return;
}
DVLOG(1) << *this << ": Read value.";
- value_read_or_write_in_progress_ = true;
- read_value_callbacks_ =
- std::make_pair(std::move(callback), std::move(error_callback));
+ read_value_callback_ = std::move(callback);
[GetCBPeripheral() readValueForDescriptor:cb_descriptor_];
}
@@ -114,7 +113,7 @@
const std::vector<uint8_t>& value,
base::OnceClosure callback,
ErrorCallback error_callback) {
- if (value_read_or_write_in_progress_) {
+ if (destructor_called_ || HasPendingRead() || HasPendingWrite()) {
DVLOG(1) << *this << ": Write failed, already in progress.";
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
@@ -123,7 +122,6 @@
return;
}
DVLOG(1) << *this << ": Write value.";
- value_read_or_write_in_progress_ = true;
write_value_callbacks_ =
std::make_pair(std::move(callback), std::move(error_callback));
base::scoped_nsobject<NSData> nsdata_value(
@@ -133,13 +131,10 @@
void BluetoothRemoteGattDescriptorMac::DidUpdateValueForDescriptor(
NSError* error) {
- if (!value_read_or_write_in_progress_) {
+ if (!HasPendingRead()) {
DVLOG(1) << *this << ": Value updated, no read in progress.";
return;
}
- std::pair<ValueCallback, ErrorCallback> callbacks;
- callbacks.swap(read_value_callbacks_);
- value_read_or_write_in_progress_ = false;
RecordDidUpdateValueForDescriptorResult(error);
if (error) {
BluetoothGattService::GattErrorCode error_code =
@@ -147,23 +142,24 @@
DVLOG(1) << *this << ": Read value failed with error: "
<< BluetoothAdapterMac::String(error)
<< ", converted to error code: " << error_code;
- std::move(callbacks.second).Run(error_code);
+ std::move(read_value_callback_)
+ .Run(error_code,
+ /*value=*/std::vector<uint8_t>());
return;
}
DVLOG(1) << *this << ": Value read.";
value_ = VectorValueFromObjC([cb_descriptor_ value]);
- std::move(callbacks.first).Run(value_);
+ std::move(read_value_callback_).Run(/*error_code=*/base::nullopt, value_);
}
void BluetoothRemoteGattDescriptorMac::DidWriteValueForDescriptor(
NSError* error) {
- if (!value_read_or_write_in_progress_) {
+ if (!HasPendingWrite()) {
DVLOG(1) << *this << ": Value written, no write in progress.";
return;
}
std::pair<base::OnceClosure, ErrorCallback> callbacks;
callbacks.swap(write_value_callbacks_);
- value_read_or_write_in_progress_ = false;
RecordDidWriteValueForDescriptorResult(error);
if (error) {
BluetoothGattService::GattErrorCode error_code =
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_unittest.cc b/device/bluetooth/bluetooth_remote_gatt_descriptor_unittest.cc
index 8b281d5b..6ea06e2 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_unittest.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_unittest.cc
@@ -240,8 +240,8 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_descriptor_attempts_);
std::vector<uint8_t> empty_vector;
SimulateGattDescriptorRead(descriptor1_, empty_vector);
@@ -305,8 +305,8 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::NOT_EXPECTED, Result::FAILURE));
RememberDescriptorForSubsequentAction(descriptor1_);
DeleteDevice(device_); // TODO(576906) delete only the descriptor.
@@ -366,8 +366,8 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_descriptor_attempts_);
uint8_t values[] = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
@@ -430,8 +430,8 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_descriptor_attempts_);
uint8_t values[] = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
@@ -445,8 +445,8 @@
// Read again, with different value:
ResetEventCounts();
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_descriptor_attempts_);
std::vector<uint8_t> empty_vector;
SimulateGattDescriptorRead(descriptor1_, empty_vector);
@@ -521,10 +521,10 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
- descriptor2_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
+ descriptor2_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(2, gatt_read_descriptor_attempts_);
EXPECT_EQ(0, callback_count_);
EXPECT_EQ(0, error_callback_count_);
@@ -609,8 +609,8 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
SimulateGattDescriptorReadError(
descriptor1_, BluetoothRemoteGattService::GATT_ERROR_INVALID_LENGTH);
SimulateGattDescriptorReadError(
@@ -667,8 +667,8 @@
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
SimulateGattDescriptorReadWillFailSynchronouslyOnce(descriptor1_);
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
EXPECT_EQ(0, gatt_read_descriptor_attempts_);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0, callback_count_);
@@ -678,8 +678,8 @@
// After failing once, can succeed:
ResetEventCounts();
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_descriptor_attempts_);
std::vector<uint8_t> empty_vector;
SimulateGattDescriptorRead(descriptor1_, empty_vector);
@@ -746,10 +746,10 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
base::RunLoop().RunUntilIdle();
@@ -834,8 +834,8 @@
std::vector<uint8_t> empty_vector;
descriptor1_->WriteRemoteDescriptor(empty_vector, GetCallback(Call::EXPECTED),
GetGattErrorCallback(Call::NOT_EXPECTED));
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::NOT_EXPECTED),
- GetGattErrorCallback(Call::EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::FAILURE));
base::RunLoop().RunUntilIdle();
@@ -874,8 +874,8 @@
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
std::vector<uint8_t> empty_vector;
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
descriptor1_->WriteRemoteDescriptor(empty_vector,
GetCallback(Call::NOT_EXPECTED),
GetGattErrorCallback(Call::EXPECTED));
@@ -913,9 +913,9 @@
SimulateGattDisconnection(device_);
// Don't run the disconnect task.
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::NOT_EXPECTED),
- // TODO(crbug.com/621901): Expect an error.
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ // TODO(crbug.com/621901): Expect an error.
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::NOT_EXPECTED, Result::FAILURE));
base::RunLoop().RunUntilIdle();
// TODO(crbug.com/621901): Test error callback was called.
@@ -959,8 +959,8 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_descriptor_attempts_);
std::string test_string = "Hello";
@@ -981,8 +981,8 @@
}
ASSERT_NO_FATAL_FAILURE(FakeDescriptorBoilerplate());
- descriptor1_->ReadRemoteDescriptor(GetReadValueCallback(Call::EXPECTED),
- GetGattErrorCallback(Call::NOT_EXPECTED));
+ descriptor1_->ReadRemoteDescriptor(
+ GetReadValueCallback(Call::EXPECTED, Result::SUCCESS));
EXPECT_EQ(1, gatt_read_descriptor_attempts_);
const short test_number = 0x1234;
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc b/device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc
index 9197c23..456c18b 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_win.cc
@@ -69,13 +69,12 @@
}
void BluetoothRemoteGattDescriptorWin::ReadRemoteDescriptor(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
DCHECK(ui_task_runner_->RunsTasksInCurrentSequence());
NOTIMPLEMENTED();
- std::move(error_callback)
- .Run(BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED);
+ std::move(callback).Run(BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED,
+ /*value=*/std::vector<uint8_t>());
}
void BluetoothRemoteGattDescriptorWin::WriteRemoteDescriptor(
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_win.h b/device/bluetooth/bluetooth_remote_gatt_descriptor_win.h
index ffa7b324..f99cde3 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_win.h
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_win.h
@@ -38,8 +38,7 @@
BluetoothRemoteGattCharacteristic* GetCharacteristic() const override;
BluetoothRemoteGattCharacteristic::Permissions GetPermissions()
const override;
- void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteDescriptor(ValueCallback callback) override;
void WriteRemoteDescriptor(const std::vector<uint8_t>& new_value,
base::OnceClosure callback,
ErrorCallback error_callback) override;
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.cc b/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.cc
index 7b845065..92d8b71 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.cc
@@ -100,13 +100,13 @@
}
void BluetoothRemoteGattDescriptorWinrt::ReadRemoteDescriptor(
- ValueCallback callback,
- ErrorCallback error_callback) {
- if (pending_read_callbacks_ || pending_write_callbacks_) {
+ ValueCallback callback) {
+ if (pending_read_callback_ || pending_write_callbacks_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+ base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
+ /*value=*/std::vector<uint8_t>()));
return;
}
@@ -118,9 +118,9 @@
<< "GattDescriptor::ReadValueWithCacheModeAsync failed: "
<< logging::SystemErrorCodeToString(hr);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_FAILED));
+ FROM_HERE, base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
@@ -133,21 +133,20 @@
BLUETOOTH_LOG(ERROR) << "PostAsyncResults failed: "
<< logging::SystemErrorCodeToString(hr);
base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::BindOnce(std::move(error_callback),
- BluetoothRemoteGattService::GATT_ERROR_FAILED));
+ FROM_HERE, base::BindOnce(std::move(callback),
+ BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>()));
return;
}
- pending_read_callbacks_ = std::make_unique<PendingReadCallbacks>(
- std::move(callback), std::move(error_callback));
+ pending_read_callback_ = std::move(callback);
}
void BluetoothRemoteGattDescriptorWinrt::WriteRemoteDescriptor(
const std::vector<uint8_t>& value,
base::OnceClosure callback,
ErrorCallback error_callback) {
- if (pending_read_callbacks_ || pending_write_callbacks_) {
+ if (pending_read_callback_ || pending_write_callbacks_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(std::move(error_callback),
@@ -215,15 +214,6 @@
return descriptor_.Get();
}
-BluetoothRemoteGattDescriptorWinrt::PendingReadCallbacks::PendingReadCallbacks(
- ValueCallback callback,
- ErrorCallback error_callback)
- : callback(std::move(callback)),
- error_callback(std::move(error_callback)) {}
-
-BluetoothRemoteGattDescriptorWinrt::PendingReadCallbacks::
- ~PendingReadCallbacks() = default;
-
BluetoothRemoteGattDescriptorWinrt::PendingWriteCallbacks::
PendingWriteCallbacks(base::OnceClosure callback,
ErrorCallback error_callback)
@@ -250,14 +240,15 @@
void BluetoothRemoteGattDescriptorWinrt::OnReadValue(
ComPtr<IGattReadResult> read_result) {
- DCHECK(pending_read_callbacks_);
- auto pending_read_callbacks = std::move(pending_read_callbacks_);
+ DCHECK(pending_read_callback_);
+ auto pending_read_callback = std::move(pending_read_callback_);
if (!read_result) {
BLUETOOTH_LOG(ERROR)
<< "GattDescriptor::ReadValueWithCacheModeAsync returned no result";
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -266,8 +257,9 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(ERROR) << "Getting GATT Communication Status failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -278,14 +270,16 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(ERROR) << "As IGattReadResult2 failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
- std::move(pending_read_callbacks->error_callback)
+ std::move(pending_read_callback)
.Run(BluetoothRemoteGattServiceWinrt::GetGattErrorCode(
- read_result_2.Get()));
+ read_result_2.Get()),
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -294,8 +288,9 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(ERROR) << "Getting Descriptor Value failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
@@ -305,13 +300,14 @@
if (FAILED(hr)) {
BLUETOOTH_LOG(ERROR) << "Getting Pointer To Buffer Data failed: "
<< logging::SystemErrorCodeToString(hr);
- std::move(pending_read_callbacks->error_callback)
- .Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(pending_read_callback)
+ .Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
value_.assign(data, data + length);
- std::move(pending_read_callbacks->callback).Run(value_);
+ std::move(pending_read_callback).Run(/*error_code=*/base::nullopt, value_);
}
void BluetoothRemoteGattDescriptorWinrt::OnWriteValueWithResult(
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.h b/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.h
index c042044..049171ea 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.h
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_winrt.h
@@ -42,8 +42,7 @@
// BluetoothRemoteGattDescriptor:
const std::vector<uint8_t>& GetValue() const override;
BluetoothRemoteGattCharacteristic* GetCharacteristic() const override;
- void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteDescriptor(ValueCallback callback) override;
void WriteRemoteDescriptor(const std::vector<uint8_t>& value,
base::OnceClosure callback,
ErrorCallback error_callback) override;
@@ -52,14 +51,6 @@
GetDescriptorForTesting();
private:
- struct PendingReadCallbacks {
- PendingReadCallbacks(ValueCallback callback, ErrorCallback error_callback);
- ~PendingReadCallbacks();
-
- ValueCallback callback;
- ErrorCallback error_callback;
- };
-
struct PendingWriteCallbacks {
PendingWriteCallbacks(base::OnceClosure callback,
ErrorCallback error_callback);
@@ -94,7 +85,7 @@
BluetoothUUID uuid_;
std::string identifier_;
std::vector<uint8_t> value_;
- std::unique_ptr<PendingReadCallbacks> pending_read_callbacks_;
+ ValueCallback pending_read_callback_;
std::unique_ptr<PendingWriteCallbacks> pending_write_callbacks_;
base::WeakPtrFactory<BluetoothRemoteGattDescriptorWinrt> weak_ptr_factory_{
diff --git a/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc b/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
index 8f48317..3105bbf 100644
--- a/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
+++ b/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
@@ -262,9 +262,16 @@
void SuccessCallback() { ++success_callback_count_; }
- void ValueCallback(const std::vector<uint8_t>& value) {
- ++success_callback_count_;
- last_read_value_ = value;
+ void ValueCallback(
+ base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ ++error_callback_count_;
+ last_service_error_ = error_code.value();
+ } else {
+ ++success_callback_count_;
+ last_read_value_ = value;
+ }
}
void GattConnectionCallback(std::unique_ptr<BluetoothGattConnection> conn) {
@@ -1139,8 +1146,6 @@
EXPECT_EQ(kBodySensorLocationUUID, characteristic->GetUUID());
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(2, success_callback_count_);
EXPECT_EQ(4, error_callback_count_);
@@ -1160,8 +1165,6 @@
EXPECT_EQ(kBodySensorLocationUUID, characteristic->GetUUID());
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
// Callback counts shouldn't change, this one will be delayed until after
@@ -1173,8 +1176,6 @@
// Next read should error because IN_PROGRESS
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(5, error_callback_count_);
EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
@@ -1190,8 +1191,6 @@
fake_bluetooth_gatt_characteristic_client_->SetAuthorized(false);
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(3, success_callback_count_);
EXPECT_EQ(6, error_callback_count_);
@@ -1204,8 +1203,6 @@
fake_bluetooth_gatt_characteristic_client_->SetAuthenticated(false);
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(3, success_callback_count_);
EXPECT_EQ(7, error_callback_count_);
@@ -1360,8 +1357,6 @@
EXPECT_EQ(kBodySensorLocationUUID, characteristic->GetUUID());
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(2, success_callback_count_);
EXPECT_EQ(4, error_callback_count_);
@@ -1381,8 +1376,6 @@
EXPECT_EQ(kBodySensorLocationUUID, characteristic->GetUUID());
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
// Callback counts shouldn't change, this one will be delayed until after
@@ -1394,8 +1387,6 @@
// Next read should error because IN_PROGRESS
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(5, error_callback_count_);
EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS,
@@ -1411,8 +1402,6 @@
fake_bluetooth_gatt_characteristic_client_->SetAuthorized(false);
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(3, success_callback_count_);
EXPECT_EQ(6, error_callback_count_);
@@ -1425,8 +1414,6 @@
fake_bluetooth_gatt_characteristic_client_->SetAuthenticated(false);
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(3, success_callback_count_);
EXPECT_EQ(7, error_callback_count_);
@@ -1465,21 +1452,18 @@
->GetBodySensorLocationPath()
.value());
- characteristic->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
- ValueCallback(data);
+ characteristic->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ ValueCallback(error_code, data);
EXPECT_EQ(1, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_EQ(characteristic->GetValue(), last_read_value_);
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
- }),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
- base::Unretained(this)));
+ }));
EXPECT_EQ(2, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_EQ(characteristic->GetValue(), last_read_value_);
@@ -1615,9 +1599,10 @@
->GetBodySensorLocationPath()
.value());
- characteristic->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
- ValueCallback(data);
+ characteristic->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ ValueCallback(error_code, data);
EXPECT_EQ(1, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_EQ(characteristic->GetValue(), last_read_value_);
@@ -1634,9 +1619,7 @@
base::Unretained(this)),
base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
- }),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
- base::Unretained(this)));
+ }));
EXPECT_EQ(2, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
}
@@ -1671,9 +1654,10 @@
->GetBodySensorLocationPath()
.value());
- characteristic->ReadRemoteCharacteristic(
- base::BindLambdaForTesting([&](const std::vector<uint8_t>& data) {
- ValueCallback(data);
+ characteristic->ReadRemoteCharacteristic(base::BindLambdaForTesting(
+ [&](base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& data) {
+ ValueCallback(error_code, data);
EXPECT_EQ(1, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_EQ(characteristic->GetValue(), last_read_value_);
@@ -1690,9 +1674,7 @@
base::Unretained(this)),
base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
- }),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
- base::Unretained(this)));
+ }));
EXPECT_EQ(2, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
}
@@ -1741,8 +1723,6 @@
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
}),
base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
@@ -1796,8 +1776,6 @@
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
}),
base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
@@ -1908,8 +1886,6 @@
// successful read.
descriptor->ReadRemoteDescriptor(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(1, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
@@ -1938,8 +1914,6 @@
// Read value. The value should remain unchanged.
descriptor->ReadRemoteDescriptor(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(2, success_callback_count_);
EXPECT_EQ(1, error_callback_count_);
@@ -1964,8 +1938,6 @@
// Read the new descriptor value. We should receive a value updated event.
descriptor->ReadRemoteDescriptor(
base::BindOnce(&BluetoothGattBlueZTest::ValueCallback,
- base::Unretained(this)),
- base::BindOnce(&BluetoothGattBlueZTest::ServiceErrorCallback,
base::Unretained(this)));
EXPECT_EQ(4, success_callback_count_);
EXPECT_EQ(1, error_callback_count_);
diff --git a/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.cc b/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.cc
index 4795e7c7..0557992 100644
--- a/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.cc
+++ b/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.cc
@@ -155,8 +155,7 @@
}
void BluetoothRemoteGattCharacteristicBlueZ::ReadRemoteCharacteristic(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
DVLOG(1) << "Sending GATT characteristic read request to characteristic: "
<< GetIdentifier() << ", UUID: " << GetUUID().canonical_value()
<< ".";
@@ -164,13 +163,14 @@
DCHECK_GE(num_of_characteristic_value_read_in_progress_, 0);
++num_of_characteristic_value_read_in_progress_;
+ auto split_callback = base::SplitOnceCallback(std::move(callback));
bluez::BluezDBusManager::Get()
->GetBluetoothGattCharacteristicClient()
->ReadValue(
- object_path(), std::move(callback),
+ object_path(), std::move(split_callback.first),
base::BindOnce(&BluetoothRemoteGattCharacteristicBlueZ::OnReadError,
weak_ptr_factory_.GetWeakPtr(),
- std::move(error_callback)));
+ std::move(split_callback.second)));
}
void BluetoothRemoteGattCharacteristicBlueZ::WriteRemoteCharacteristic(
@@ -403,15 +403,17 @@
}
void BluetoothRemoteGattCharacteristicBlueZ::OnReadError(
- ErrorCallback error_callback,
+ ValueCallback callback,
const std::string& error_name,
const std::string& error_message) {
DVLOG(1) << "Operation failed: " << error_name
<< ", message: " << error_message;
--num_of_characteristic_value_read_in_progress_;
DCHECK_GE(num_of_characteristic_value_read_in_progress_, 0);
- std::move(error_callback)
- .Run(BluetoothGattServiceBlueZ::DBusErrorToServiceError(error_name));
+ std::move(callback).Run(
+ base::make_optional(
+ BluetoothGattServiceBlueZ::DBusErrorToServiceError(error_name)),
+ /*value=*/std::vector<uint8_t>());
}
void BluetoothRemoteGattCharacteristicBlueZ::OnWriteError(
diff --git a/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h b/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h
index 4e29fae2..7fc989f 100644
--- a/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h
+++ b/device/bluetooth/bluez/bluetooth_remote_gatt_characteristic_bluez.h
@@ -55,8 +55,7 @@
const std::vector<uint8_t>& GetValue() const override;
device::BluetoothRemoteGattService* GetService() const override;
bool IsNotifying() const override;
- void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteCharacteristic(ValueCallback callback) override;
void WriteRemoteCharacteristic(const std::vector<uint8_t>& value,
WriteType write_type,
base::OnceClosure callback,
@@ -124,7 +123,7 @@
// Called by dbus:: on unsuccessful completion of a request to read
// the characteristic value.
- void OnReadError(ErrorCallback error_callback,
+ void OnReadError(ValueCallback callback,
const std::string& error_name,
const std::string& error_message);
diff --git a/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.cc b/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.cc
index 4c761750..f59af4f 100644
--- a/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.cc
+++ b/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.cc
@@ -7,11 +7,13 @@
#include <iterator>
#include <ostream>
+#include <base/callback_helpers.h>
#include "base/bind.h"
#include "base/callback.h"
#include "base/logging.h"
#include "base/strings/stringprintf.h"
#include "dbus/property.h"
+#include "device/bluetooth/bluetooth_gatt_service.h"
#include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
#include "device/bluetooth/bluez/bluetooth_gatt_characteristic_bluez.h"
#include "device/bluetooth/bluez/bluetooth_gatt_descriptor_bluez.h"
@@ -80,17 +82,17 @@
}
void BluetoothRemoteGattDescriptorBlueZ::ReadRemoteDescriptor(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
DVLOG(1) << "Sending GATT characteristic descriptor read request to "
<< "descriptor: " << GetIdentifier()
<< ", UUID: " << GetUUID().canonical_value();
+ auto split_callback = base::SplitOnceCallback(std::move(callback));
bluez::BluezDBusManager::Get()->GetBluetoothGattDescriptorClient()->ReadValue(
- object_path(), std::move(callback),
- base::BindOnce(&BluetoothRemoteGattDescriptorBlueZ::OnError,
+ object_path(), std::move(split_callback.first),
+ base::BindOnce(&BluetoothRemoteGattDescriptorBlueZ::OnReadError,
weak_ptr_factory_.GetWeakPtr(),
- std::move(error_callback)));
+ std::move(split_callback.second)));
}
void BluetoothRemoteGattDescriptorBlueZ::WriteRemoteDescriptor(
@@ -110,6 +112,18 @@
std::move(error_callback)));
}
+void BluetoothRemoteGattDescriptorBlueZ::OnReadError(
+ ValueCallback callback,
+ const std::string& error_name,
+ const std::string& error_message) {
+ DVLOG(1) << "Operation failed: " << error_name
+ << ", message: " << error_message;
+
+ std::move(callback).Run(
+ BluetoothGattServiceBlueZ::DBusErrorToServiceError(error_name),
+ /*value=*/std::vector<uint8_t>());
+}
+
void BluetoothRemoteGattDescriptorBlueZ::OnError(
ErrorCallback error_callback,
const std::string& error_name,
diff --git a/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.h b/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.h
index 513185b..743c008 100644
--- a/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.h
+++ b/device/bluetooth/bluez/bluetooth_remote_gatt_descriptor_bluez.h
@@ -35,8 +35,7 @@
device::BluetoothRemoteGattCharacteristic* GetCharacteristic() const override;
device::BluetoothRemoteGattCharacteristic::Permissions GetPermissions()
const override;
- void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteDescriptor(ValueCallback callback) override;
void WriteRemoteDescriptor(const std::vector<uint8_t>& new_value,
base::OnceClosure callback,
ErrorCallback error_callback) override;
@@ -48,7 +47,13 @@
BluetoothRemoteGattCharacteristicBlueZ* characteristic,
const dbus::ObjectPath& object_path);
- // Called by dbus:: on unsuccessful completion of a request to read or write
+ // Called by dbus:: on unsuccessful completion of a request to read
+ // the descriptor value.
+ void OnReadError(ValueCallback callback,
+ const std::string& error_name,
+ const std::string& error_message);
+
+ // Called by dbus:: on unsuccessful completion of a request to write
// the descriptor value.
void OnError(ErrorCallback error_callback,
const std::string& error_name,
diff --git a/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.cc b/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.cc
index 8b88155..45f26847 100644
--- a/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.cc
+++ b/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.cc
@@ -134,12 +134,10 @@
}
void BluetoothRemoteGattCharacteristicCast::ReadRemoteCharacteristic(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
remote_characteristic_->Read(base::BindOnce(
&BluetoothRemoteGattCharacteristicCast::OnReadRemoteCharacteristic,
- weak_factory_.GetWeakPtr(), std::move(callback),
- std::move(error_callback)));
+ weak_factory_.GetWeakPtr(), std::move(callback)));
}
void BluetoothRemoteGattCharacteristicCast::WriteRemoteCharacteristic(
@@ -214,15 +212,15 @@
void BluetoothRemoteGattCharacteristicCast::OnReadRemoteCharacteristic(
ValueCallback callback,
- ErrorCallback error_callback,
bool success,
const std::vector<uint8_t>& result) {
if (success) {
value_ = result;
- std::move(callback).Run(result);
+ std::move(callback).Run(/*error_code=*/base::nullopt, result);
return;
}
- std::move(error_callback).Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(callback).Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
void BluetoothRemoteGattCharacteristicCast::OnWriteRemoteCharacteristic(
diff --git a/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h b/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h
index af4dd71..dd297da 100644
--- a/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h
+++ b/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.h
@@ -45,8 +45,7 @@
// BluetoothRemoteGattCharacteristic implementation:
const std::vector<uint8_t>& GetValue() const override;
BluetoothRemoteGattService* GetService() const override;
- void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteCharacteristic(ValueCallback callback) override;
void WriteRemoteCharacteristic(const std::vector<uint8_t>& value,
WriteType write_type,
base::OnceClosure callback,
@@ -70,13 +69,13 @@
base::OnceClosure callback,
ErrorCallback error_callback) override;
- // Called when the remote characteristic has been read or the operation has
- // failed. If the former, |success| will be true, and |result| will be
- // valid. In this case, |value_| is updated and |callback| is run with
- // |result|. If |success| is false, |result| is ignored and |error_callback|
- // is run.
+ // Called upon completation of a read (success or failure) of a remote
+ // characteristic. When successful |success| will be true and |result|
+ // will contain the characteristic value. In this case |value_| is updated
+ // and |callback| is run with |result| and no error code. Upon failure
+ // |success| is false, |result| is undefined, and |callback| is run with an
+ // appropriate error code and the value is invalid.
void OnReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback,
bool success,
const std::vector<uint8_t>& result);
diff --git a/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.cc b/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.cc
index ac7010d..da890ed 100644
--- a/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.cc
+++ b/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.cc
@@ -50,12 +50,10 @@
}
void BluetoothRemoteGattDescriptorCast::ReadRemoteDescriptor(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
remote_descriptor_->Read(
base::BindOnce(&BluetoothRemoteGattDescriptorCast::OnReadRemoteDescriptor,
- weak_factory_.GetWeakPtr(), std::move(callback),
- std::move(error_callback)));
+ weak_factory_.GetWeakPtr(), std::move(callback)));
}
void BluetoothRemoteGattDescriptorCast::WriteRemoteDescriptor(
@@ -72,15 +70,15 @@
void BluetoothRemoteGattDescriptorCast::OnReadRemoteDescriptor(
ValueCallback callback,
- ErrorCallback error_callback,
bool success,
const std::vector<uint8_t>& result) {
if (success) {
value_ = result;
- std::move(callback).Run(result);
+ std::move(callback).Run(/*error_code=*/base::nullopt, result);
return;
}
- std::move(error_callback).Run(BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(callback).Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
}
void BluetoothRemoteGattDescriptorCast::OnWriteRemoteDescriptor(
diff --git a/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.h b/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.h
index d74bcaf..f799cb0 100644
--- a/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.h
+++ b/device/bluetooth/cast/bluetooth_remote_gatt_descriptor_cast.h
@@ -40,8 +40,7 @@
// BluetoothRemoteGattDescriptor implementation:
const std::vector<uint8_t>& GetValue() const override;
BluetoothRemoteGattCharacteristic* GetCharacteristic() const override;
- void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteDescriptor(ValueCallback callback) override;
void WriteRemoteDescriptor(const std::vector<uint8_t>& new_value,
base::OnceClosure callback,
ErrorCallback error_callback) override;
@@ -50,10 +49,9 @@
// Called when the remote descriptor has been read or the operation has
// failed. If the former, |success| will be true, and |result| will be
// valid. In this case, |value_| is updated and |callback| is run with
- // |result|. If |success| is false, |result| is ignored and |error_callback|
- // is run.
+ // |result|. If |success| is false, |callback| will be called with
+ // an appropriate error_code and the value should be ignored.
void OnReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback,
bool success,
const std::vector<uint8_t>& result);
diff --git a/device/bluetooth/dbus/bluetooth_gatt_attribute_value_delegate.h b/device/bluetooth/dbus/bluetooth_gatt_attribute_value_delegate.h
index 0e3fd2c..13c0efc7e 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_attribute_value_delegate.h
+++ b/device/bluetooth/dbus/bluetooth_gatt_attribute_value_delegate.h
@@ -41,9 +41,7 @@
// out if left pending for too long causing a disconnection.
virtual void GetValue(
const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback
- error_callback) = 0;
+ device::BluetoothLocalGattService::Delegate::ValueCallback callback) = 0;
// This method will be called, when a remote device requests to write the
// value of the exported GATT attribute. Invoke |callback| to report
diff --git a/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.cc b/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.cc
index 6311997..062efc9 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.cc
@@ -312,7 +312,7 @@
if (bytes)
value.assign(bytes, bytes + length);
- std::move(callback).Run(value);
+ std::move(callback).Run(/*error_code=*/base::nullopt, value);
}
// Called when a response for a failed method call is received.
diff --git a/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.h b/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.h
index 9e03e8f5..4eef31d7 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.h
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_client.h
@@ -12,6 +12,7 @@
#include "base/callback.h"
#include "base/macros.h"
+#include "base/optional.h"
#include "build/chromeos_buildflags.h"
#include "dbus/object_path.h"
#include "dbus/property.h"
@@ -80,8 +81,9 @@
using ErrorCallback =
base::OnceCallback<void(const std::string& error_name,
const std::string& error_message)>;
- using ValueCallback =
- base::OnceCallback<void(const std::vector<uint8_t>& value)>;
+ using ValueCallback = base::OnceCallback<void(
+ base::Optional<device::BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value)>;
~BluetoothGattCharacteristicClient() override;
diff --git a/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.cc b/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.cc
index 3b7f335..5538a14 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.cc
@@ -18,16 +18,14 @@
void BluetoothGattCharacteristicDelegateWrapper::GetValue(
const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback error_callback) {
+ device::BluetoothLocalGattService::Delegate::ValueCallback callback) {
device::BluetoothDevice* device = GetDeviceWithPath(device_path);
if (!device) {
LOG(WARNING) << "Bluetooth device not found: " << device_path.value();
return;
}
- service()->GetDelegate()->OnCharacteristicReadRequest(
- device, characteristic_, 0, std::move(callback),
- std::move(error_callback));
+ service()->GetDelegate()->OnCharacteristicReadRequest(device, characteristic_,
+ 0, std::move(callback));
}
void BluetoothGattCharacteristicDelegateWrapper::SetValue(
diff --git a/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.h b/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.h
index def89cc7..4b175f4 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.h
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.h
@@ -29,11 +29,9 @@
BluetoothLocalGattCharacteristicBlueZ* characteristic);
// BluetoothGattAttributeValueDelegate overrides:
- void GetValue(
- const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback error_callback)
- override;
+ void GetValue(const dbus::ObjectPath& device_path,
+ device::BluetoothLocalGattService::Delegate::ValueCallback
+ callback) override;
void SetValue(const dbus::ObjectPath& device_path,
const std::vector<uint8_t>& value,
base::OnceClosure callback,
diff --git a/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc b/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc
index 1a3ec9d..735381e 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc
@@ -304,20 +304,13 @@
// the delegate, which should know how to handle it.
}
- // GetValue() promises to only call either the success or error callback.
- auto split_response_sender =
- base::SplitOnceCallback(std::move(response_sender));
-
DCHECK(delegate_);
delegate_->GetValue(
device_path,
base::BindOnce(
&BluetoothGattCharacteristicServiceProviderImpl::OnReadValue,
weak_ptr_factory_.GetWeakPtr(), method_call,
- std::move(split_response_sender.first)),
- base::BindOnce(&BluetoothGattCharacteristicServiceProviderImpl::OnFailure,
- weak_ptr_factory_.GetWeakPtr(), method_call,
- std::move(split_response_sender.second)));
+ std::move(response_sender)));
}
void BluetoothGattCharacteristicServiceProviderImpl::WriteValue(
@@ -365,9 +358,10 @@
&BluetoothGattCharacteristicServiceProviderImpl::OnWriteValue,
weak_ptr_factory_.GetWeakPtr(), method_call,
std::move(split_response_sender.first)),
- base::BindOnce(&BluetoothGattCharacteristicServiceProviderImpl::OnFailure,
- weak_ptr_factory_.GetWeakPtr(), method_call,
- std::move(split_response_sender.second)));
+ base::BindOnce(
+ &BluetoothGattCharacteristicServiceProviderImpl::OnWriteValueFailure,
+ weak_ptr_factory_.GetWeakPtr(), method_call,
+ std::move(split_response_sender.second)));
}
void BluetoothGattCharacteristicServiceProviderImpl::PrepareWriteValue(
@@ -423,9 +417,10 @@
&BluetoothGattCharacteristicServiceProviderImpl::OnWriteValue,
weak_ptr_factory_.GetWeakPtr(), method_call,
std::move(split_response_sender.first)),
- base::BindOnce(&BluetoothGattCharacteristicServiceProviderImpl::OnFailure,
- weak_ptr_factory_.GetWeakPtr(), method_call,
- std::move(split_response_sender.second)));
+ base::BindOnce(
+ &BluetoothGattCharacteristicServiceProviderImpl::OnWriteValueFailure,
+ weak_ptr_factory_.GetWeakPtr(), method_call,
+ std::move(split_response_sender.second)));
}
void BluetoothGattCharacteristicServiceProviderImpl::StartNotify(
@@ -489,7 +484,16 @@
void BluetoothGattCharacteristicServiceProviderImpl::OnReadValue(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender,
+ base::Optional<device::BluetoothGattService::GattErrorCode> error_code,
const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ DVLOG(2) << "Failed to read characteristic value. Report error.";
+ std::move(response_sender)
+ .Run(dbus::ErrorResponse::FromMethodCall(
+ method_call, kErrorFailed, "Failed to read characteristic value."));
+ return;
+ }
+
DVLOG(3) << "Characteristic value obtained from delegate. Responding to "
"ReadValue.";
@@ -542,13 +546,13 @@
writer->CloseContainer(&array_writer);
}
-void BluetoothGattCharacteristicServiceProviderImpl::OnFailure(
+void BluetoothGattCharacteristicServiceProviderImpl::OnWriteValueFailure(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
- DVLOG(2) << "Failed to get/set characteristic value. Report error.";
+ DVLOG(2) << "Failed to set characteristic value. Report error.";
std::unique_ptr<dbus::ErrorResponse> error_response =
dbus::ErrorResponse::FromMethodCall(
- method_call, kErrorFailed, "Failed to get/set characteristic value.");
+ method_call, kErrorFailed, "Failed to set characteristic value.");
std::move(response_sender).Run(std::move(error_response));
}
diff --git a/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h b/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h
index 95c140e7..615d8fe 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h
@@ -42,6 +42,11 @@
void SendValueChanged(const std::vector<uint8_t>& value) override;
private:
+ FRIEND_TEST_ALL_PREFIXES(BluetoothGattCharacteristicServiceProviderTest,
+ ReadValueSuccess);
+ FRIEND_TEST_ALL_PREFIXES(BluetoothGattCharacteristicServiceProviderTest,
+ ReadValueFailure);
+
// Returns true if the current thread is on the origin thread.
bool OnOriginThread();
@@ -102,19 +107,22 @@
// Called by the Delegate in response to a method to call to read the value
// of this characteristic.
- void OnReadValue(dbus::MethodCall* method_call,
- dbus::ExportedObject::ResponseSender response_sender,
- const std::vector<uint8_t>& value);
+ void OnReadValue(
+ dbus::MethodCall* method_call,
+ dbus::ExportedObject::ResponseSender response_sender,
+ base::Optional<device::BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value);
// Called by the Delegate in response to a method to call to write the value
// of this characteristic.
void OnWriteValue(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender);
- // Called by the Delegate in response to a failed method call to get or set
+ // Called by the Delegate in response to a failed method call to set
// the characteristic value.
- void OnFailure(dbus::MethodCall* method_call,
- dbus::ExportedObject::ResponseSender response_sender);
+ void OnWriteValueFailure(
+ dbus::MethodCall* method_call,
+ dbus::ExportedObject::ResponseSender response_sender);
const dbus::ObjectPath& object_path() const override;
diff --git a/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_unittest.cc b/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_unittest.cc
new file mode 100644
index 0000000..d68de23c
--- /dev/null
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_unittest.cc
@@ -0,0 +1,90 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <cstdint>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/optional.h"
+#include "base/test/bind.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "device/bluetooth/bluetooth_gatt_service.h"
+#include "device/bluetooth/dbus/bluetooth_gatt_attribute_value_delegate.h"
+#include "device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.h"
+#include "device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace bluez {
+
+TEST(BluetoothGattCharacteristicServiceProviderTest, ReadValueSuccess) {
+ auto method_call =
+ std::make_unique<dbus::MethodCall>("com.example.Interface", "SomeMethod");
+ method_call->SetSerial(123);
+ method_call->SetReplySerial(456);
+ bool callback_called = false;
+
+ BluetoothGattCharacteristicServiceProviderImpl provider_impl(
+ /*bus=*/nullptr,
+ /*object_path=*/dbus::ObjectPath(),
+ /*delegate=*/std::unique_ptr<BluetoothGattAttributeValueDelegate>(),
+ /*uuid=*/std::string(),
+ /*flags=*/std::vector<std::string>(),
+ /*service_path=*/dbus::ObjectPath());
+
+ const std::vector<uint8_t> read_value = {1, 2, 3};
+ provider_impl.OnReadValue(
+ method_call.get(),
+ base::BindLambdaForTesting([&callback_called, read_value](
+ std::unique_ptr<dbus::Response> response) {
+ EXPECT_EQ(response->GetMessageType(), DBUS_MESSAGE_TYPE_METHOD_RETURN);
+ dbus::MessageReader reader(response.get());
+ EXPECT_EQ(reader.GetDataType(), dbus::Message::ARRAY);
+ const uint8_t* bytes = nullptr;
+ size_t length = 0;
+ EXPECT_TRUE(reader.PopArrayOfBytes(&bytes, &length));
+ EXPECT_EQ(length, read_value.size());
+ callback_called = true;
+ }),
+ /*error_code=*/base::nullopt, read_value);
+
+ EXPECT_TRUE(callback_called);
+}
+
+TEST(BluetoothGattCharacteristicServiceProviderTest, ReadValueFailure) {
+ auto method_call =
+ std::make_unique<dbus::MethodCall>("com.example.Interface", "SomeMethod");
+ method_call->SetSerial(123);
+ method_call->SetReplySerial(456);
+ bool callback_called = false;
+
+ BluetoothGattCharacteristicServiceProviderImpl provider_impl(
+ /*bus=*/nullptr,
+ /*object_path=*/dbus::ObjectPath(),
+ /*delegate=*/std::unique_ptr<BluetoothGattAttributeValueDelegate>(),
+ /*uuid=*/std::string(),
+ /*flags=*/std::vector<std::string>(),
+ /*service_path=*/dbus::ObjectPath());
+
+ const std::vector<uint8_t> read_value = {1, 2, 3};
+ provider_impl.OnReadValue(
+ method_call.get(),
+ base::BindLambdaForTesting(
+ [&callback_called](std::unique_ptr<dbus::Response> response) {
+ EXPECT_EQ(response->GetMessageType(), DBUS_MESSAGE_TYPE_ERROR);
+ dbus::MessageReader reader(response.get());
+ EXPECT_NE(reader.GetDataType(), dbus::Message::ARRAY);
+ const uint8_t* bytes = nullptr;
+ size_t length = 0;
+ EXPECT_FALSE(reader.PopArrayOfBytes(&bytes, &length));
+ callback_called = true;
+ }),
+ device::BluetoothGattService::GATT_ERROR_FAILED, read_value);
+
+ EXPECT_TRUE(callback_called);
+}
+
+} // namespace bluez
diff --git a/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.cc b/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.cc
index c2fbd87..e7e9c54 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.cc
@@ -221,7 +221,7 @@
if (bytes)
value.assign(bytes, bytes + length);
- std::move(callback).Run(value);
+ std::move(callback).Run(/*error_code=*/base::nullopt, value);
}
// Called when a response for a failed method call is received.
diff --git a/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.h b/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.h
index b9f9a96..bd5168e 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.h
+++ b/device/bluetooth/dbus/bluetooth_gatt_descriptor_client.h
@@ -12,9 +12,11 @@
#include "base/callback.h"
#include "base/macros.h"
+#include "base/optional.h"
#include "dbus/object_path.h"
#include "dbus/property.h"
#include "device/bluetooth/bluetooth_export.h"
+#include "device/bluetooth/bluetooth_gatt_service.h"
#include "device/bluetooth/dbus/bluez_dbus_client.h"
namespace bluez {
@@ -68,8 +70,9 @@
using ErrorCallback =
base::OnceCallback<void(const std::string& error_name,
const std::string& error_message)>;
- using ValueCallback =
- base::OnceCallback<void(const std::vector<uint8_t>& value)>;
+ using ValueCallback = base::OnceCallback<void(
+ base::Optional<device::BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value)>;
~BluetoothGattDescriptorClient() override;
diff --git a/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.cc b/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.cc
index a60c511..1bae1d5 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.cc
@@ -15,11 +15,9 @@
void BluetoothGattDescriptorDelegateWrapper::GetValue(
const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback error_callback) {
+ device::BluetoothLocalGattService::Delegate::ValueCallback callback) {
service()->GetDelegate()->OnDescriptorReadRequest(
- GetDeviceWithPath(device_path), descriptor_, 0, std::move(callback),
- std::move(error_callback));
+ GetDeviceWithPath(device_path), descriptor_, 0, std::move(callback));
}
void BluetoothGattDescriptorDelegateWrapper::SetValue(
diff --git a/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.h b/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.h
index 7b947bc..54e0355 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.h
+++ b/device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.h
@@ -29,11 +29,9 @@
BluetoothLocalGattDescriptorBlueZ* descriptor);
// BluetoothGattAttributeValueDelegate overrides:
- void GetValue(
- const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback error_callback)
- override;
+ void GetValue(const dbus::ObjectPath& device_path,
+ device::BluetoothLocalGattService::Delegate::ValueCallback
+ callback) override;
void SetValue(const dbus::ObjectPath& device_path,
const std::vector<uint8_t>& value,
base::OnceClosure callback,
diff --git a/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.cc b/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.cc
index 58c7046..87fc596 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.cc
@@ -10,6 +10,7 @@
#include "base/callback_helpers.h"
#include "base/logging.h"
#include "base/strings/string_util.h"
+#include "device/bluetooth/bluetooth_gatt_service.h"
#include "device/bluetooth/dbus/bluetooth_gatt_attribute_helpers.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
@@ -277,10 +278,7 @@
device_path,
base::BindOnce(&BluetoothGattDescriptorServiceProviderImpl::OnReadValue,
weak_ptr_factory_.GetWeakPtr(), method_call,
- std::move(split_response_sender.first)),
- base::BindOnce(&BluetoothGattDescriptorServiceProviderImpl::OnFailure,
- weak_ptr_factory_.GetWeakPtr(), method_call,
- std::move(split_response_sender.second)));
+ std::move(split_response_sender.first)));
}
void BluetoothGattDescriptorServiceProviderImpl::WriteValue(
@@ -327,9 +325,10 @@
base::BindOnce(&BluetoothGattDescriptorServiceProviderImpl::OnWriteValue,
weak_ptr_factory_.GetWeakPtr(), method_call,
std::move(split_response_sender.first)),
- base::BindOnce(&BluetoothGattDescriptorServiceProviderImpl::OnFailure,
- weak_ptr_factory_.GetWeakPtr(), method_call,
- std::move(split_response_sender.second)));
+ base::BindOnce(
+ &BluetoothGattDescriptorServiceProviderImpl::OnWriteFailure,
+ weak_ptr_factory_.GetWeakPtr(), method_call,
+ std::move(split_response_sender.second)));
}
void BluetoothGattDescriptorServiceProviderImpl::OnExported(
@@ -343,7 +342,17 @@
void BluetoothGattDescriptorServiceProviderImpl::OnReadValue(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender,
+ base::Optional<device::BluetoothGattService::GattErrorCode> error_code,
const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ DVLOG(2) << "Failed to get descriptor value. Report error.";
+ std::unique_ptr<dbus::ErrorResponse> error_response =
+ dbus::ErrorResponse::FromMethodCall(method_call, kErrorFailed,
+ "Failed to get descriptor value.");
+ std::move(response_sender).Run(std::move(error_response));
+ return;
+ }
+
DVLOG(3) << "Descriptor value obtained from delegate. Responding to "
"ReadValue.";
@@ -396,13 +405,13 @@
writer->CloseContainer(&array_writer);
}
-void BluetoothGattDescriptorServiceProviderImpl::OnFailure(
+void BluetoothGattDescriptorServiceProviderImpl::OnWriteFailure(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
- DVLOG(2) << "Failed to get/set descriptor value. Report error.";
+ DVLOG(2) << "Failed to set descriptor value. Report error.";
std::unique_ptr<dbus::ErrorResponse> error_response =
- dbus::ErrorResponse::FromMethodCall(
- method_call, kErrorFailed, "Failed to get/set descriptor value.");
+ dbus::ErrorResponse::FromMethodCall(method_call, kErrorFailed,
+ "Failed to set descriptor value.");
std::move(response_sender).Run(std::move(error_response));
}
diff --git a/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.h b/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.h
index 199256f7..d21f34a 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.h
+++ b/device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider_impl.h
@@ -78,19 +78,21 @@
// Called by the Delegate in response to a method to call to read the value
// of this descriptor.
- void OnReadValue(dbus::MethodCall* method_call,
- dbus::ExportedObject::ResponseSender response_sender,
- const std::vector<uint8_t>& value);
+ void OnReadValue(
+ dbus::MethodCall* method_call,
+ dbus::ExportedObject::ResponseSender response_sender,
+ base::Optional<device::BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value);
// Called by the Delegate in response to a method to call to write the value
// of this descriptor.
void OnWriteValue(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender);
- // Called by the Delegate in response to a failed method call to get or set
+ // Called by the Delegate in response to a failed method call to set
// the descriptor value.
- void OnFailure(dbus::MethodCall* method_call,
- dbus::ExportedObject::ResponseSender response_sender);
+ void OnWriteFailure(dbus::MethodCall* method_call,
+ dbus::ExportedObject::ResponseSender response_sender);
const dbus::ObjectPath& object_path() const override;
diff --git a/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_client.cc b/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_client.cc
index 0010c53..9b5d999b 100644
--- a/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_client.cc
+++ b/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_client.cc
@@ -575,7 +575,7 @@
DCHECK(properties);
properties->value.ReplaceValue(value);
- std::move(callback).Run(value);
+ std::move(callback).Run(/*error_code=*/base::nullopt, value);
}
std::vector<uint8_t>
diff --git a/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.cc b/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.cc
index a53194f..c2a337f 100644
--- a/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.cc
+++ b/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.cc
@@ -10,7 +10,9 @@
#include "base/callback.h"
#include "base/logging.h"
#include "base/strings/string_util.h"
+#include "device/bluetooth/bluetooth_adapter.h"
#include "device/bluetooth/bluetooth_gatt_characteristic.h"
+#include "device/bluetooth/bluetooth_gatt_service.h"
#include "device/bluetooth/dbus/bluetooth_gatt_attribute_value_delegate.h"
#include "device/bluetooth/dbus/bluez_dbus_manager.h"
#include "device/bluetooth/dbus/fake_bluetooth_gatt_manager_client.h"
@@ -116,8 +118,7 @@
void FakeBluetoothGattCharacteristicServiceProvider::GetValue(
const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback error_callback) {
+ device::BluetoothLocalGattService::Delegate::ValueCallback callback) {
DVLOG(1) << "GATT characteristic value Get request: " << object_path_.value()
<< " UUID: " << uuid_;
// Check if this characteristic is registered.
@@ -126,20 +127,21 @@
bluez::BluezDBusManager::Get()->GetBluetoothGattManagerClient());
if (!fake_bluetooth_gatt_manager_client->IsServiceRegistered(service_path_)) {
DVLOG(1) << "GATT characteristic not registered.";
- std::move(error_callback).Run();
+ std::move(callback).Run(device::BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
if (!CanRead(flags_)) {
DVLOG(1) << "GATT characteristic not readable.";
- std::move(error_callback).Run();
+ std::move(callback).Run(device::BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
// Pass on to the delegate.
DCHECK(delegate_);
- delegate_->GetValue(device_path, std::move(callback),
- std::move(error_callback));
+ delegate_->GetValue(device_path, std::move(callback));
}
void FakeBluetoothGattCharacteristicServiceProvider::SetValue(
diff --git a/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.h b/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.h
index 3f093f47..27ec2e3 100644
--- a/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.h
+++ b/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.h
@@ -41,9 +41,7 @@
// GATT manager.
void GetValue(
const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback
- error_callback);
+ device::BluetoothLocalGattService::Delegate::ValueCallback callback);
void SetValue(const dbus::ObjectPath& device_path,
const std::vector<uint8_t>& value,
base::OnceClosure callback,
diff --git a/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_client.cc b/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_client.cc
index b2d848e..8f40476 100644
--- a/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_client.cc
+++ b/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_client.cc
@@ -120,7 +120,8 @@
}
}
- std::move(callback).Run(iter->second->properties->value.value());
+ std::move(callback).Run(/*error_code=*/base::nullopt,
+ iter->second->properties->value.value());
}
void FakeBluetoothGattDescriptorClient::WriteValue(
diff --git a/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.cc b/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.cc
index 80d73a235..c16a6db 100644
--- a/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.cc
+++ b/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.cc
@@ -97,8 +97,7 @@
void FakeBluetoothGattDescriptorServiceProvider::GetValue(
const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback error_callback) {
+ device::BluetoothLocalGattService::Delegate::ValueCallback callback) {
DVLOG(1) << "GATT descriptor value Get request: " << object_path_.value()
<< " UUID: " << uuid_;
// Check if this descriptor is registered.
@@ -116,20 +115,22 @@
if (!fake_bluetooth_gatt_manager_client->IsServiceRegistered(
characteristic->service_path())) {
DVLOG(1) << "GATT descriptor not registered.";
- std::move(error_callback).Run();
+ std::move(callback).Run(
+ device::BluetoothGattService::GattErrorCode::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
if (!CanRead(flags_)) {
- DVLOG(1) << "GATT descriptor not readable.";
- std::move(error_callback).Run();
+ std::move(callback).Run(
+ device::BluetoothGattService::GattErrorCode::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
// Pass on to the delegate.
DCHECK(delegate_);
- delegate_->GetValue(device_path, std::move(callback),
- std::move(error_callback));
+ delegate_->GetValue(device_path, std::move(callback));
}
void FakeBluetoothGattDescriptorServiceProvider::SetValue(
diff --git a/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.h b/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.h
index 785a999..a1aa918 100644
--- a/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.h
+++ b/device/bluetooth/dbus/fake_bluetooth_gatt_descriptor_service_provider.h
@@ -41,9 +41,7 @@
// GATT manager.
void GetValue(
const dbus::ObjectPath& device_path,
- device::BluetoothLocalGattService::Delegate::ValueCallback callback,
- device::BluetoothLocalGattService::Delegate::ErrorCallback
- error_callback);
+ device::BluetoothLocalGattService::Delegate::ValueCallback callback);
void SetValue(const dbus::ObjectPath& device_path,
const std::vector<uint8_t>& value,
base::OnceClosure callback,
diff --git a/device/bluetooth/device.cc b/device/bluetooth/device.cc
index a922046..aee2806 100644
--- a/device/bluetooth/device.cc
+++ b/device/bluetooth/device.cc
@@ -152,14 +152,9 @@
return;
}
- auto split_callback = base::SplitOnceCallback(std::move(callback));
characteristic->ReadRemoteCharacteristic(
base::BindOnce(&Device::OnReadRemoteCharacteristic,
- weak_ptr_factory_.GetWeakPtr(),
- std::move(split_callback.first)),
- base::BindOnce(&Device::OnReadRemoteCharacteristicError,
- weak_ptr_factory_.GetWeakPtr(),
- std::move(split_callback.second)));
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
void Device::WriteValueForCharacteristic(
@@ -264,14 +259,9 @@
return;
}
- auto split_callback = base::SplitOnceCallback(std::move(callback));
descriptor->ReadRemoteDescriptor(
base::BindOnce(&Device::OnReadRemoteDescriptor,
- weak_ptr_factory_.GetWeakPtr(),
- std::move(split_callback.first)),
- base::BindOnce(&Device::OnReadRemoteDescriptorError,
- weak_ptr_factory_.GetWeakPtr(),
- std::move(split_callback.second)));
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
void Device::WriteValueForDescriptor(const std::string& service_id,
@@ -347,17 +337,18 @@
void Device::OnReadRemoteCharacteristic(
ReadValueForCharacteristicCallback callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ std::move(callback).Run(
+ mojo::ConvertTo<mojom::GattResult>(error_code.value()),
+ base::nullopt /* value */);
+ return;
+ }
std::move(callback).Run(mojom::GattResult::SUCCESS, std::move(value));
}
-void Device::OnReadRemoteCharacteristicError(
- ReadValueForCharacteristicCallback callback,
- device::BluetoothGattService::GattErrorCode error_code) {
- std::move(callback).Run(mojo::ConvertTo<mojom::GattResult>(error_code),
- base::nullopt /* value */);
-}
-
void Device::OnWriteRemoteCharacteristic(
WriteValueForCharacteristicCallback callback) {
std::move(callback).Run(mojom::GattResult::SUCCESS);
@@ -369,16 +360,18 @@
std::move(callback).Run(mojo::ConvertTo<mojom::GattResult>(error_code));
}
-void Device::OnReadRemoteDescriptor(ReadValueForDescriptorCallback callback,
- const std::vector<uint8_t>& value) {
- std::move(callback).Run(mojom::GattResult::SUCCESS, std::move(value));
-}
-
-void Device::OnReadRemoteDescriptorError(
+void Device::OnReadRemoteDescriptor(
ReadValueForDescriptorCallback callback,
- device::BluetoothGattService::GattErrorCode error_code) {
- std::move(callback).Run(mojo::ConvertTo<mojom::GattResult>(error_code),
- base::nullopt /* value */);
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ std::move(callback).Run(
+ mojo::ConvertTo<mojom::GattResult>(error_code.value()),
+ /*value=*/base::nullopt);
+ return;
+ }
+ std::move(callback).Run(mojom::GattResult::SUCCESS, std::move(value));
}
void Device::OnWriteRemoteDescriptor(WriteValueForDescriptorCallback callback) {
diff --git a/device/bluetooth/device.h b/device/bluetooth/device.h
index 48f407c4..b5e2f24 100644
--- a/device/bluetooth/device.h
+++ b/device/bluetooth/device.h
@@ -87,12 +87,11 @@
mojom::ServiceInfoPtr ConstructServiceInfoStruct(
const device::BluetoothRemoteGattService& service);
- void OnReadRemoteCharacteristic(ReadValueForCharacteristicCallback callback,
- const std::vector<uint8_t>& value);
-
- void OnReadRemoteCharacteristicError(
+ void OnReadRemoteCharacteristic(
ReadValueForCharacteristicCallback callback,
- device::BluetoothGattService::GattErrorCode error_code);
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value);
void OnWriteRemoteCharacteristic(
WriteValueForCharacteristicCallback callback);
@@ -101,12 +100,11 @@
WriteValueForCharacteristicCallback callback,
device::BluetoothGattService::GattErrorCode error_code);
- void OnReadRemoteDescriptor(ReadValueForDescriptorCallback callback,
- const std::vector<uint8_t>& value);
-
- void OnReadRemoteDescriptorError(
+ void OnReadRemoteDescriptor(
ReadValueForDescriptorCallback callback,
- device::BluetoothGattService::GattErrorCode error_code);
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value);
void OnWriteRemoteDescriptor(WriteValueForDescriptorCallback callback);
diff --git a/device/bluetooth/test/bluetooth_test.cc b/device/bluetooth/test/bluetooth_test.cc
index dc30a57..0d8567b 100644
--- a/device/bluetooth/test/bluetooth_test.cc
+++ b/device/bluetooth/test/bluetooth_test.cc
@@ -311,15 +311,35 @@
++actual_success_callback_calls_;
}
-void BluetoothTestBase::ReadValueCallback(Call expected,
- const std::vector<uint8_t>& value) {
- ++callback_count_;
+void BluetoothTestBase::ReadValueCallback(
+ Call expected,
+ Result expected_result,
+ base::Optional<BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value) {
+ if (expected_result == Result::FAILURE) {
+ if (error_code.has_value())
+ read_results_.failure.actual++;
+ else
+ read_results_.success.unexpected++;
+ } else {
+ if (!error_code.has_value())
+ read_results_.success.actual++;
+ else
+ read_results_.failure.unexpected++;
+ }
last_read_value_ = value;
+ if (error_code.has_value()) {
+ error_callback_count_++;
+ last_gatt_error_code_ = error_code.value();
+ } else {
+ callback_count_++;
+ }
+
if (expected == Call::EXPECTED)
- ++actual_success_callback_calls_;
+ read_callback_calls_.actual++;
else
- unexpected_success_callback_ = true;
+ read_callback_calls_.unexpected++;
}
void BluetoothTestBase::ErrorCallback(Call expected) {
@@ -469,11 +489,16 @@
}
BluetoothRemoteGattCharacteristic::ValueCallback
-BluetoothTestBase::GetReadValueCallback(Call expected) {
- if (expected == Call::EXPECTED)
- ++expected_success_callback_calls_;
+BluetoothTestBase::GetReadValueCallback(Call expected, Result expected_result) {
+ if (expected == Call::EXPECTED) {
+ read_callback_calls_.expected++;
+ if (expected_result == Result::SUCCESS)
+ read_results_.success.expected++;
+ else
+ read_results_.failure.expected++;
+ }
return base::BindOnce(&BluetoothTestBase::ReadValueCallback,
- weak_factory_.GetWeakPtr(), expected);
+ weak_factory_.GetWeakPtr(), expected, expected_result);
}
BluetoothAdapter::ErrorCallback BluetoothTestBase::GetErrorCallback(
diff --git a/device/bluetooth/test/bluetooth_test.h b/device/bluetooth/test/bluetooth_test.h
index 6308c40..871d739 100644
--- a/device/bluetooth/test/bluetooth_test.h
+++ b/device/bluetooth/test/bluetooth_test.h
@@ -43,6 +43,8 @@
public:
enum class Call { EXPECTED, NOT_EXPECTED };
+ enum class Result { SUCCESS, FAILURE };
+
// List of devices that can be simulated with
// SimulateConnectedLowEnergyDevice().
// GENERIC_DEVICE:
@@ -486,8 +488,7 @@
virtual void SimulateLocalGattCharacteristicValueReadRequest(
BluetoothDevice* from_device,
BluetoothLocalGattCharacteristic* characteristic,
- BluetoothLocalGattService::Delegate::ValueCallback value_callback,
- base::OnceClosure error_callback) {}
+ BluetoothLocalGattService::Delegate::ValueCallback value_callback) {}
// Simulates write a value to a locally hosted GATT characteristic by a
// remote central device.
@@ -515,8 +516,7 @@
virtual void SimulateLocalGattDescriptorValueReadRequest(
BluetoothDevice* from_device,
BluetoothLocalGattDescriptor* descriptor,
- BluetoothLocalGattService::Delegate::ValueCallback value_callback,
- base::OnceClosure error_callback) {}
+ BluetoothLocalGattService::Delegate::ValueCallback value_callback) {}
// Simulates write a value to a locally hosted GATT descriptor by a
// remote central device.
@@ -616,7 +616,11 @@
std::unique_ptr<BluetoothGattNotifySession>);
void StopNotifyCallback(Call expected);
void StopNotifyCheckForPrecedingCalls(int num_of_preceding_calls);
- void ReadValueCallback(Call expected, const std::vector<uint8_t>& value);
+ void ReadValueCallback(
+ Call expected,
+ Result expected_result,
+ base::Optional<BluetoothGattService::GattErrorCode> error_code,
+ const std::vector<uint8_t>& value);
void ErrorCallback(Call expected);
void AdvertisementErrorCallback(Call expected,
BluetoothAdvertisement::ErrorCode error_code);
@@ -650,7 +654,8 @@
base::OnceClosure GetStopNotifyCheckForPrecedingCalls(
int num_of_preceding_calls);
BluetoothRemoteGattCharacteristic::ValueCallback GetReadValueCallback(
- Call expected);
+ Call expected,
+ Result expected_result);
BluetoothAdapter::ErrorCallback GetErrorCallback(Call expected);
BluetoothAdapter::AdvertisementErrorCallback GetAdvertisementErrorCallback(
Call expected);
@@ -673,6 +678,19 @@
void RemoveTimedOutDevices();
protected:
+ // The expected/actual counts for tests.
+ struct EventCounts {
+ int unexpected = 0;
+ int expected = 0;
+ int actual = 0;
+ };
+
+ // Counts for an expected metric being tested.
+ struct ResultCounts {
+ EventCounts success;
+ EventCounts failure;
+ };
+
// Utility method to simplify creading a low energy device of a given
// |device_ordinal|.
LowEnergyDeviceData GetLowEnergyDeviceData(int device_ordinal) const;
@@ -692,7 +710,8 @@
std::vector<std::unique_ptr<BluetoothGattNotifySession>> notify_sessions_;
std::vector<uint8_t> last_read_value_;
std::vector<uint8_t> last_write_value_;
- BluetoothRemoteGattService::GattErrorCode last_gatt_error_code_;
+ BluetoothRemoteGattService::GattErrorCode last_gatt_error_code_ =
+ BluetoothRemoteGattService::GATT_ERROR_UNKNOWN;
int callback_count_ = 0;
int error_callback_count_ = 0;
@@ -714,6 +733,9 @@
bool unexpected_success_callback_ = false;
bool unexpected_error_callback_ = false;
+ EventCounts read_callback_calls_;
+ ResultCounts read_results_;
+
base::WeakPtrFactory<BluetoothTestBase> weak_factory_{this};
};
diff --git a/device/bluetooth/test/bluetooth_test_bluez.cc b/device/bluetooth/test/bluetooth_test_bluez.cc
index 5f2c9e17..b23d581 100644
--- a/device/bluetooth/test/bluetooth_test_bluez.cc
+++ b/device/bluetooth/test/bluetooth_test_bluez.cc
@@ -35,8 +35,9 @@
void GetValueCallback(
base::OnceClosure quit_closure,
BluetoothLocalGattService::Delegate::ValueCallback value_callback,
+ base::Optional<BluetoothGattService::GattErrorCode> error_code,
const std::vector<uint8_t>& value) {
- std::move(value_callback).Run(value);
+ std::move(value_callback).Run(error_code, value);
std::move(quit_closure).Run();
}
@@ -160,8 +161,7 @@
void BluetoothTestBlueZ::SimulateLocalGattCharacteristicValueReadRequest(
BluetoothDevice* from_device,
BluetoothLocalGattCharacteristic* characteristic,
- BluetoothLocalGattService::Delegate::ValueCallback value_callback,
- base::OnceClosure error_callback) {
+ BluetoothLocalGattService::Delegate::ValueCallback value_callback) {
bluez::BluetoothLocalGattCharacteristicBlueZ* characteristic_bluez =
static_cast<bluez::BluetoothLocalGattCharacteristicBlueZ*>(
characteristic);
@@ -184,9 +184,7 @@
characteristic_provider->GetValue(
GetDevicePath(from_device),
base::BindOnce(&GetValueCallback, run_loop.QuitClosure(),
- std::move(value_callback)),
- base::BindOnce(&ClosureCallback, run_loop.QuitClosure(),
- std::move(error_callback)));
+ std::move(value_callback)));
run_loop.Run();
}
@@ -264,8 +262,7 @@
void BluetoothTestBlueZ::SimulateLocalGattDescriptorValueReadRequest(
BluetoothDevice* from_device,
BluetoothLocalGattDescriptor* descriptor,
- BluetoothLocalGattService::Delegate::ValueCallback value_callback,
- base::OnceClosure error_callback) {
+ BluetoothLocalGattService::Delegate::ValueCallback value_callback) {
bluez::BluetoothLocalGattDescriptorBlueZ* descriptor_bluez =
static_cast<bluez::BluetoothLocalGattDescriptorBlueZ*>(descriptor);
bluez::FakeBluetoothGattManagerClient* fake_bluetooth_gatt_manager_client =
@@ -286,9 +283,7 @@
descriptor_provider->GetValue(
GetDevicePath(from_device),
base::BindOnce(&GetValueCallback, run_loop.QuitClosure(),
- std::move(value_callback)),
- base::BindOnce(&ClosureCallback, run_loop.QuitClosure(),
- std::move(error_callback)));
+ std::move(value_callback)));
run_loop.Run();
}
diff --git a/device/bluetooth/test/bluetooth_test_bluez.h b/device/bluetooth/test/bluetooth_test_bluez.h
index 8497892..f53796a 100644
--- a/device/bluetooth/test/bluetooth_test_bluez.h
+++ b/device/bluetooth/test/bluetooth_test_bluez.h
@@ -39,8 +39,8 @@
void SimulateLocalGattCharacteristicValueReadRequest(
BluetoothDevice* from_device,
BluetoothLocalGattCharacteristic* characteristic,
- BluetoothLocalGattService::Delegate::ValueCallback value_callback,
- base::OnceClosure error_callback) override;
+ BluetoothLocalGattService::Delegate::ValueCallback value_callback)
+ override;
void SimulateLocalGattCharacteristicValueWriteRequest(
BluetoothDevice* from_device,
BluetoothLocalGattCharacteristic* characteristic,
@@ -58,8 +58,8 @@
void SimulateLocalGattDescriptorValueReadRequest(
BluetoothDevice* from_device,
BluetoothLocalGattDescriptor* descriptor,
- BluetoothLocalGattService::Delegate::ValueCallback value_callback,
- base::OnceClosure error_callback) override;
+ BluetoothLocalGattService::Delegate::ValueCallback value_callback)
+ override;
void SimulateLocalGattDescriptorValueWriteRequest(
BluetoothDevice* from_device,
BluetoothLocalGattDescriptor* descriptor,
diff --git a/device/bluetooth/test/fake_remote_gatt_characteristic.cc b/device/bluetooth/test/fake_remote_gatt_characteristic.cc
index 64f025f..658b0d9 100644
--- a/device/bluetooth/test/fake_remote_gatt_characteristic.cc
+++ b/device/bluetooth/test/fake_remote_gatt_characteristic.cc
@@ -129,13 +129,11 @@
}
void FakeRemoteGattCharacteristic::ReadRemoteCharacteristic(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(&FakeRemoteGattCharacteristic::DispatchReadResponse,
- weak_ptr_factory_.GetWeakPtr(), std::move(callback),
- std::move(error_callback)));
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
void FakeRemoteGattCharacteristic::WriteRemoteCharacteristic(
@@ -231,8 +229,7 @@
}
void FakeRemoteGattCharacteristic::DispatchReadResponse(
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
DCHECK(next_read_response_);
uint16_t gatt_code = next_read_response_->gatt_code();
base::Optional<std::vector<uint8_t>> value = next_read_response_->value();
@@ -242,12 +239,12 @@
case mojom::kGATTSuccess:
DCHECK(value);
value_ = std::move(value.value());
- std::move(callback).Run(value_);
+ std::move(callback).Run(base::nullopt, value_);
break;
case mojom::kGATTInvalidHandle:
DCHECK(!value);
- std::move(error_callback)
- .Run(device::BluetoothGattService::GATT_ERROR_FAILED);
+ std::move(callback).Run(device::BluetoothGattService::GATT_ERROR_FAILED,
+ std::vector<uint8_t>());
break;
default:
NOTREACHED();
diff --git a/device/bluetooth/test/fake_remote_gatt_characteristic.h b/device/bluetooth/test/fake_remote_gatt_characteristic.h
index c6071e9e..53e1e80a 100644
--- a/device/bluetooth/test/fake_remote_gatt_characteristic.h
+++ b/device/bluetooth/test/fake_remote_gatt_characteristic.h
@@ -88,8 +88,7 @@
// device::BluetoothRemoteGattCharacteristic overrides:
const std::vector<uint8_t>& GetValue() const override;
device::BluetoothRemoteGattService* GetService() const override;
- void ReadRemoteCharacteristic(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteCharacteristic(ValueCallback callback) override;
void WriteRemoteCharacteristic(const std::vector<uint8_t>& value,
WriteType write_type,
base::OnceClosure callback,
@@ -125,8 +124,7 @@
ErrorCallback error_callback) override;
private:
- void DispatchReadResponse(ValueCallback callback,
- ErrorCallback error_callback);
+ void DispatchReadResponse(ValueCallback callback);
void DispatchWriteResponse(base::OnceClosure callback,
ErrorCallback error_callback,
const std::vector<uint8_t>& value,
diff --git a/device/bluetooth/test/fake_remote_gatt_descriptor.cc b/device/bluetooth/test/fake_remote_gatt_descriptor.cc
index 74b5801..58573e7 100644
--- a/device/bluetooth/test/fake_remote_gatt_descriptor.cc
+++ b/device/bluetooth/test/fake_remote_gatt_descriptor.cc
@@ -62,14 +62,11 @@
return characteristic_;
}
-void FakeRemoteGattDescriptor::ReadRemoteDescriptor(
- ValueCallback callback,
- ErrorCallback error_callback) {
+void FakeRemoteGattDescriptor::ReadRemoteDescriptor(ValueCallback callback) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(&FakeRemoteGattDescriptor::DispatchReadResponse,
- weak_ptr_factory_.GetWeakPtr(), std::move(callback),
- std::move(error_callback)));
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
void FakeRemoteGattDescriptor::WriteRemoteDescriptor(
@@ -83,24 +80,25 @@
std::move(error_callback), value));
}
-void FakeRemoteGattDescriptor::DispatchReadResponse(
- ValueCallback callback,
- ErrorCallback error_callback) {
+void FakeRemoteGattDescriptor::DispatchReadResponse(ValueCallback callback) {
DCHECK(next_read_response_);
uint16_t gatt_code = next_read_response_->gatt_code();
base::Optional<std::vector<uint8_t>> value = next_read_response_->value();
next_read_response_.reset();
- if (gatt_code == mojom::kGATTSuccess) {
- DCHECK(value);
- value_ = std::move(value.value());
- std::move(callback).Run(value_);
- return;
- } else if (gatt_code == mojom::kGATTInvalidHandle) {
- DCHECK(!value);
- std::move(error_callback)
- .Run(device::BluetoothGattService::GATT_ERROR_FAILED);
- return;
+ switch (gatt_code) {
+ case mojom::kGATTSuccess:
+ DCHECK(value);
+ value_ = std::move(value.value());
+ std::move(callback).Run(/*error_code=*/base::nullopt, value_);
+ break;
+ case mojom::kGATTInvalidHandle:
+ DCHECK(!value);
+ std::move(callback).Run(device::BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
+ break;
+ default:
+ NOTREACHED();
}
}
diff --git a/device/bluetooth/test/fake_remote_gatt_descriptor.h b/device/bluetooth/test/fake_remote_gatt_descriptor.h
index 0516fc1..f6e1bf2 100644
--- a/device/bluetooth/test/fake_remote_gatt_descriptor.h
+++ b/device/bluetooth/test/fake_remote_gatt_descriptor.h
@@ -57,15 +57,13 @@
// device::BluetoothRemoteGattDescriptor overrides:
const std::vector<uint8_t>& GetValue() const override;
device::BluetoothRemoteGattCharacteristic* GetCharacteristic() const override;
- void ReadRemoteDescriptor(ValueCallback callback,
- ErrorCallback error_callback) override;
+ void ReadRemoteDescriptor(ValueCallback callback) override;
void WriteRemoteDescriptor(const std::vector<uint8_t>& value,
base::OnceClosure callback,
ErrorCallback error_callback) override;
private:
- void DispatchReadResponse(ValueCallback callback,
- ErrorCallback error_callback);
+ void DispatchReadResponse(ValueCallback callback);
void DispatchWriteResponse(base::OnceClosure callback,
ErrorCallback error_callback,
diff --git a/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h b/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h
index e301b745..49e36bb5 100644
--- a/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h
+++ b/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h
@@ -68,10 +68,10 @@
}
MOCK_METHOD2(StopNotifySession_,
void(BluetoothGattNotifySession*, base::OnceClosure&));
- void ReadRemoteCharacteristic(ValueCallback c, ErrorCallback ec) override {
- ReadRemoteCharacteristic_(c, ec);
+ void ReadRemoteCharacteristic(ValueCallback c) override {
+ ReadRemoteCharacteristic_(c);
}
- MOCK_METHOD2(ReadRemoteCharacteristic_, void(ValueCallback&, ErrorCallback&));
+ MOCK_METHOD1(ReadRemoteCharacteristic_, void(ValueCallback&));
void WriteRemoteCharacteristic(const std::vector<uint8_t>& v,
WriteType t,
base::OnceClosure c,
diff --git a/device/bluetooth/test/mock_bluetooth_gatt_descriptor.h b/device/bluetooth/test/mock_bluetooth_gatt_descriptor.h
index e1de291..06af5cb 100644
--- a/device/bluetooth/test/mock_bluetooth_gatt_descriptor.h
+++ b/device/bluetooth/test/mock_bluetooth_gatt_descriptor.h
@@ -36,10 +36,10 @@
MOCK_CONST_METHOD0(GetCharacteristic, BluetoothRemoteGattCharacteristic*());
MOCK_CONST_METHOD0(GetPermissions,
BluetoothRemoteGattCharacteristic::Permissions());
- void ReadRemoteDescriptor(ValueCallback c, ErrorCallback ec) override {
- ReadRemoteDescriptor_(c, ec);
+ void ReadRemoteDescriptor(ValueCallback c) override {
+ ReadRemoteDescriptor_(c);
}
- MOCK_METHOD2(ReadRemoteDescriptor_, void(ValueCallback&, ErrorCallback&));
+ MOCK_METHOD1(ReadRemoteDescriptor_, void(ValueCallback&));
void WriteRemoteDescriptor(const std::vector<uint8_t>& v,
base::OnceClosure c,
ErrorCallback ec) override {
diff --git a/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.cc b/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.cc
index e637730..824388c6 100644
--- a/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.cc
+++ b/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.cc
@@ -24,16 +24,17 @@
const BluetoothDevice* device,
const BluetoothLocalGattCharacteristic* characteristic,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
EXPECT_EQ(expected_characteristic_->GetIdentifier(),
characteristic->GetIdentifier());
if (should_fail_) {
- std::move(error_callback).Run();
+ std::move(callback).Run(BluetoothGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
last_seen_device_ = device->GetIdentifier();
- std::move(callback).Run(BluetoothGattServerTest::GetValue(value_to_write_));
+ std::move(callback).Run(/*error_code=*/base::nullopt,
+ BluetoothGattServerTest::GetValue(value_to_write_));
}
void TestBluetoothLocalGattServiceDelegate::OnCharacteristicWriteRequest(
@@ -81,15 +82,16 @@
const BluetoothDevice* device,
const BluetoothLocalGattDescriptor* descriptor,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) {
+ ValueCallback callback) {
EXPECT_EQ(expected_descriptor_->GetIdentifier(), descriptor->GetIdentifier());
if (should_fail_) {
- std::move(error_callback).Run();
+ std::move(callback).Run(BluetoothRemoteGattService::GATT_ERROR_FAILED,
+ /*value=*/std::vector<uint8_t>());
return;
}
last_seen_device_ = device->GetIdentifier();
- std::move(callback).Run(BluetoothGattServerTest::GetValue(value_to_write_));
+ std::move(callback).Run(/*error_code=*/base::nullopt,
+ BluetoothGattServerTest::GetValue(value_to_write_));
}
void TestBluetoothLocalGattServiceDelegate::OnDescriptorWriteRequest(
diff --git a/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.h b/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.h
index 3a0189cf..4be93a9 100644
--- a/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.h
+++ b/device/bluetooth/test/test_bluetooth_local_gatt_service_delegate.h
@@ -27,8 +27,7 @@
const BluetoothDevice* device,
const BluetoothLocalGattCharacteristic* characteristic,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) override;
+ ValueCallback callback) override;
void OnCharacteristicWriteRequest(
const BluetoothDevice* device,
const BluetoothLocalGattCharacteristic* characteristic,
@@ -47,8 +46,7 @@
void OnDescriptorReadRequest(const BluetoothDevice* device,
const BluetoothLocalGattDescriptor* descriptor,
int offset,
- ValueCallback callback,
- ErrorCallback error_callback) override;
+ ValueCallback callback) override;
void OnDescriptorWriteRequest(const BluetoothDevice* device,
const BluetoothLocalGattDescriptor* descriptor,
diff --git a/device/fido/cable/fido_ble_connection.cc b/device/fido/cable/fido_ble_connection.cc
index 6b94db1..9228d17 100644
--- a/device/fido/cable/fido_ble_connection.cc
+++ b/device/fido/cable/fido_ble_connection.cc
@@ -106,8 +106,17 @@
std::move(callback).Run(false);
}
-void OnReadServiceRevisionBitfield(ServiceRevisionsCallback callback,
- const std::vector<uint8_t>& value) {
+void OnReadServiceRevisionBitfield(
+ ServiceRevisionsCallback callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ FIDO_LOG(ERROR) << "Error while reading Service Revision Bitfield: "
+ << ToString(error_code.value());
+ std::move(callback).Run({});
+ return;
+ }
if (value.empty()) {
FIDO_LOG(DEBUG) << "Service Revision Bitfield is empty.";
std::move(callback).Run({});
@@ -139,14 +148,6 @@
std::move(callback).Run(std::move(service_revisions));
}
-void OnReadServiceRevisionBitfieldError(
- ServiceRevisionsCallback callback,
- BluetoothGattService::GattErrorCode error_code) {
- FIDO_LOG(ERROR) << "Error while reading Service Revision Bitfield: "
- << ToString(error_code);
- std::move(callback).Run({});
-}
-
} // namespace
FidoBleConnection::FidoBleConnection(BluetoothAdapter* adapter,
@@ -220,12 +221,8 @@
}
FIDO_LOG(DEBUG) << "Read Control Point Length";
- // Work around legacy APIs. Only one of the callbacks to
- // ReadRemoteCharacteristic() gets invoked, but we don't know which one.
- auto copyable_callback = base::AdaptCallbackForRepeating(std::move(callback));
control_point_length->ReadRemoteCharacteristic(
- base::BindOnce(OnReadControlPointLength, copyable_callback),
- base::BindOnce(OnReadControlPointLengthError, copyable_callback));
+ base::BindOnce(OnReadControlPointLength, std::move(callback)));
}
void FidoBleConnection::WriteControlPoint(const std::vector<uint8_t>& data,
@@ -371,8 +368,7 @@
&FidoBleConnection::OnReadServiceRevisions, weak_factory_.GetWeakPtr());
fido_service->GetCharacteristic(*service_revision_bitfield_id_)
->ReadRemoteCharacteristic(
- base::BindOnce(OnReadServiceRevisionBitfield, callback),
- base::BindOnce(OnReadServiceRevisionBitfieldError, callback));
+ base::BindOnce(OnReadServiceRevisionBitfield, callback));
return;
}
@@ -518,7 +514,15 @@
// static
void FidoBleConnection::OnReadControlPointLength(
ControlPointLengthCallback callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ FIDO_LOG(ERROR) << "Error reading Control Point Length: "
+ << ToString(error_code.value());
+ std::move(callback).Run(base::nullopt);
+ return;
+ }
if (value.size() != 2) {
FIDO_LOG(ERROR) << "Wrong Control Point Length: " << value.size()
<< " bytes";
@@ -531,13 +535,4 @@
std::move(callback).Run(length);
}
-// static
-void FidoBleConnection::OnReadControlPointLengthError(
- ControlPointLengthCallback callback,
- BluetoothGattService::GattErrorCode error_code) {
- FIDO_LOG(ERROR) << "Error reading Control Point Length: "
- << ToString(error_code);
- std::move(callback).Run(base::nullopt);
-}
-
} // namespace device
diff --git a/device/fido/cable/fido_ble_connection.h b/device/fido/cable/fido_ble_connection.h
index ebc940a..b6b8fef 100644
--- a/device/fido/cable/fido_ble_connection.h
+++ b/device/fido/cable/fido_ble_connection.h
@@ -105,11 +105,11 @@
void OnStartNotifySessionError(
BluetoothGattService::GattErrorCode error_code);
- static void OnReadControlPointLength(ControlPointLengthCallback callback,
- const std::vector<uint8_t>& value);
- static void OnReadControlPointLengthError(
+ static void OnReadControlPointLength(
ControlPointLengthCallback callback,
- BluetoothGattService::GattErrorCode error_code);
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value);
std::unique_ptr<BluetoothGattConnection> connection_;
std::unique_ptr<BluetoothGattNotifySession> notify_session_;
diff --git a/device/fido/cable/fido_ble_connection_unittest.cc b/device/fido/cable/fido_ble_connection_unittest.cc
index 333d601..4ad4d13 100644
--- a/device/fido/cable/fido_ble_connection_unittest.cc
+++ b/device/fido/cable/fido_ble_connection_unittest.cc
@@ -157,10 +157,10 @@
ON_CALL(*fido_service_revision_bitfield_, ReadRemoteCharacteristic_)
.WillByDefault(Invoke(
- [=](BluetoothRemoteGattCharacteristic::ValueCallback& callback,
- BluetoothRemoteGattCharacteristic::ErrorCallback&) {
+ [=](BluetoothRemoteGattCharacteristic::ValueCallback& callback) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback),
+ /*error_code=*/base::nullopt,
std::vector<uint8_t>(
{kDefaultServiceRevision})));
}));
@@ -231,54 +231,48 @@
void SetNextReadControlPointLengthReponse(bool success,
const std::vector<uint8_t>& value) {
- EXPECT_CALL(*fido_control_point_length_, ReadRemoteCharacteristic_(_, _))
- .WillOnce(Invoke(
- [success, value](
- BluetoothRemoteGattCharacteristic::ValueCallback& callback,
- BluetoothRemoteGattCharacteristic::ErrorCallback&
- error_callback) {
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- success ? base::BindOnce(std::move(callback), value)
- : base::BindOnce(
- std::move(error_callback),
- BluetoothGattService::GATT_ERROR_FAILED));
- }));
+ EXPECT_CALL(*fido_control_point_length_, ReadRemoteCharacteristic_(_))
+ .WillOnce(Invoke([success, value](
+ BluetoothRemoteGattCharacteristic::ValueCallback&
+ callback) {
+ base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code;
+ if (!success)
+ error_code = BluetoothRemoteGattService::GATT_ERROR_FAILED;
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::BindOnce(std::move(callback), error_code, value));
+ }));
}
void SetNextReadServiceRevisionResponse(bool success,
const std::vector<uint8_t>& value) {
- EXPECT_CALL(*fido_service_revision_, ReadRemoteCharacteristic_(_, _))
- .WillOnce(Invoke(
- [success, value](
- BluetoothRemoteGattCharacteristic::ValueCallback& callback,
- BluetoothRemoteGattCharacteristic::ErrorCallback&
- error_callback) {
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- success ? base::BindOnce(std::move(callback), value)
- : base::BindOnce(
- std::move(error_callback),
- BluetoothGattService::GATT_ERROR_FAILED));
- }));
+ EXPECT_CALL(*fido_service_revision_, ReadRemoteCharacteristic_(_))
+ .WillOnce(Invoke([success, value](
+ BluetoothRemoteGattCharacteristic::ValueCallback&
+ callback) {
+ base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code;
+ if (!success)
+ error_code = BluetoothRemoteGattService::GATT_ERROR_FAILED;
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::BindOnce(std::move(callback), error_code, value));
+ }));
}
void SetNextReadServiceRevisionBitfieldResponse(
bool success,
const std::vector<uint8_t>& value) {
- EXPECT_CALL(*fido_service_revision_bitfield_,
- ReadRemoteCharacteristic_(_, _))
+ EXPECT_CALL(*fido_service_revision_bitfield_, ReadRemoteCharacteristic_(_))
.WillOnce(Invoke(
[success, value](
- BluetoothRemoteGattCharacteristic::ValueCallback& callback,
- BluetoothRemoteGattCharacteristic::ErrorCallback&
- error_callback) {
+ BluetoothRemoteGattCharacteristic::ValueCallback& callback) {
+ auto error_code =
+ success ? base::nullopt
+ : base::make_optional(
+ BluetoothRemoteGattService::GATT_ERROR_FAILED);
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- success ? base::BindOnce(std::move(callback), value)
- : base::BindOnce(
- std::move(error_callback),
- BluetoothGattService::GATT_ERROR_FAILED));
+ base::BindOnce(std::move(callback), error_code, value));
}));
}
diff --git a/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc b/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc
index ceca7b9..5cfc1b6 100644
--- a/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc
+++ b/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc
@@ -633,12 +633,9 @@
}
characteristic->ReadRemoteCharacteristic(
- base::BindOnce(
- &BluetoothLowEnergyEventRouter::OnReadRemoteCharacteristicSuccess,
- weak_ptr_factory_.GetWeakPtr(), instance_id, std::move(callback)),
- base::BindOnce(&BluetoothLowEnergyEventRouter::OnError,
- weak_ptr_factory_.GetWeakPtr(),
- std::move(error_callback)));
+ base::BindOnce(&BluetoothLowEnergyEventRouter::OnReadRemoteCharacteristic,
+ weak_ptr_factory_.GetWeakPtr(), instance_id,
+ std::move(callback), std::move(error_callback)));
}
void BluetoothLowEnergyEventRouter::WriteCharacteristicValue(
@@ -799,11 +796,8 @@
}
descriptor->ReadRemoteDescriptor(
- base::BindOnce(
- &BluetoothLowEnergyEventRouter::OnReadRemoteDescriptorSuccess,
- weak_ptr_factory_.GetWeakPtr(), std::move(callback)),
- base::BindOnce(&BluetoothLowEnergyEventRouter::OnError,
- weak_ptr_factory_.GetWeakPtr(),
+ base::BindOnce(&BluetoothLowEnergyEventRouter::OnReadRemoteDescriptor,
+ weak_ptr_factory_.GetWeakPtr(), std::move(callback),
std::move(error_callback)));
}
@@ -1089,8 +1083,7 @@
const device::BluetoothDevice* device,
const device::BluetoothLocalGattCharacteristic* characteristic,
int offset,
- Delegate::ValueCallback value_callback,
- Delegate::ErrorCallback error_callback) {
+ Delegate::ValueCallback value_callback) {
const std::string& service_id = characteristic->GetService()->GetIdentifier();
if (service_id_to_extension_id_.find(service_id) ==
service_id_to_extension_id_.end()) {
@@ -1101,9 +1094,10 @@
const std::string& extension_id = service_id_to_extension_id_.at(service_id);
apibtle::Request request;
+ // TODO(crbug.com/730593): Delete NullCallback when write callbacks combined.
request.request_id = StoreSentRequest(
extension_id, std::make_unique<AttributeValueRequest>(
- std::move(value_callback), std::move(error_callback)));
+ std::move(value_callback), base::NullCallback()));
PopulateDevice(device, &request);
DispatchEventToExtension(
extension_id, events::BLUETOOTH_LOW_ENERGY_ON_CHARACTERISTIC_READ_REQUEST,
@@ -1160,8 +1154,7 @@
const device::BluetoothDevice* device,
const device::BluetoothLocalGattDescriptor* descriptor,
int offset,
- Delegate::ValueCallback value_callback,
- Delegate::ErrorCallback error_callback) {
+ Delegate::ValueCallback value_callback) {
const std::string& service_id =
descriptor->GetCharacteristic()->GetService()->GetIdentifier();
if (service_id_to_extension_id_.find(service_id) ==
@@ -1174,9 +1167,10 @@
const std::string& extension_id = service_id_to_extension_id_.at(service_id);
apibtle::Request request;
+ // TODO(crbug.com/730593): Delete NullCallback when write callbacks combined.
request.request_id = StoreSentRequest(
extension_id, std::make_unique<AttributeValueRequest>(
- std::move(value_callback), std::move(error_callback)));
+ std::move(value_callback), base::NullCallback()));
PopulateDevice(device, &request);
DispatchEventToExtension(
extension_id,
@@ -1391,7 +1385,7 @@
}
if (request->type == AttributeValueRequest::ATTRIBUTE_READ_REQUEST) {
- std::move(request->value_callback).Run(value);
+ std::move(request->value_callback).Run(/*error_code=*/base::nullopt, value);
} else {
std::move(request->success_callback).Run();
}
@@ -1607,10 +1601,17 @@
return descriptor;
}
-void BluetoothLowEnergyEventRouter::OnReadRemoteCharacteristicSuccess(
+void BluetoothLowEnergyEventRouter::OnReadRemoteCharacteristic(
const std::string& characteristic_instance_id,
base::OnceClosure callback,
+ ErrorCallback error_callback,
+ base::Optional<BluetoothRemoteGattService::GattErrorCode> error_code,
const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ VLOG(2) << "Remote characteristic value read failed.";
+ std::move(error_callback).Run(GattErrorToRouterError(error_code.value()));
+ return;
+ }
VLOG(2) << "Remote characteristic value read successful.";
BluetoothRemoteGattCharacteristic* characteristic =
@@ -1620,9 +1621,17 @@
std::move(callback).Run();
}
-void BluetoothLowEnergyEventRouter::OnReadRemoteDescriptorSuccess(
+void BluetoothLowEnergyEventRouter::OnReadRemoteDescriptor(
base::OnceClosure callback,
+ ErrorCallback error_callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
const std::vector<uint8_t>& value) {
+ if (error_code.has_value()) {
+ VLOG(2) << "Remote characteristic/descriptor value read failed.";
+ std::move(error_callback).Run(GattErrorToRouterError(error_code.value()));
+ return;
+ }
VLOG(2) << "Remote descriptor value read successful.";
std::move(callback).Run();
}
@@ -1674,7 +1683,7 @@
void BluetoothLowEnergyEventRouter::OnError(
ErrorCallback error_callback,
BluetoothRemoteGattService::GattErrorCode error_code) {
- VLOG(2) << "Remote characteristic/descriptor value read/write failed.";
+ VLOG(2) << "Remote characteristic/descriptor operation failed.";
std::move(error_callback).Run(GattErrorToRouterError(error_code));
}
@@ -1756,7 +1765,7 @@
const std::string& extension_id,
const std::string& characteristic_id,
ErrorCallback error_callback,
- device::BluetoothRemoteGattService::GattErrorCode error_code) {
+ BluetoothRemoteGattService::GattErrorCode error_code) {
VLOG(2) << "Failed to create value update session for characteristic: "
<< characteristic_id;
diff --git a/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h b/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h
index 34bfbf77..2b2eb70 100644
--- a/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h
+++ b/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h
@@ -310,8 +310,7 @@
const device::BluetoothDevice* device,
const device::BluetoothLocalGattCharacteristic* characteristic,
int offset,
- Delegate::ValueCallback value_callback,
- Delegate::ErrorCallback error_callback) override;
+ Delegate::ValueCallback value_callback) override;
void OnCharacteristicWriteRequest(
const device::BluetoothDevice* device,
const device::BluetoothLocalGattCharacteristic* characteristic,
@@ -331,8 +330,7 @@
const device::BluetoothDevice* device,
const device::BluetoothLocalGattDescriptor* descriptor,
int offset,
- Delegate::ValueCallback value_callback,
- Delegate::ErrorCallback error_callback) override;
+ Delegate::ValueCallback value_callback) override;
void OnDescriptorWriteRequest(
const device::BluetoothDevice* device,
const device::BluetoothLocalGattDescriptor* descriptor,
@@ -435,14 +433,21 @@
// Dispatches a BLUETOOTH_LOW_ENERGY_ON_CHARACTERISTIC_VALUE_CHANGED and runs
// |callback|.
- void OnReadRemoteCharacteristicSuccess(
+ void OnReadRemoteCharacteristic(
const std::string& characteristic_instance_id,
base::OnceClosure callback,
+ ErrorCallback error_callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
const std::vector<uint8_t>& value);
// Runs |callback|.
- void OnReadRemoteDescriptorSuccess(base::OnceClosure callback,
- const std::vector<uint8_t>& value);
+ void OnReadRemoteDescriptor(
+ base::OnceClosure callback,
+ ErrorCallback error_callback,
+ base::Optional<device::BluetoothRemoteGattService::GattErrorCode>
+ error_code,
+ const std::vector<uint8_t>& value);
// Called by BluetoothDevice in response to a call to CreateGattConnection.
void OnCreateGattConnection(
@@ -464,7 +469,7 @@
// Called by BluetoothRemoteGattCharacteristic and
// BluetoothRemoteGattDescriptor in
- // case of an error during the read/write operations.
+ // case of an error during the write operations.
void OnError(ErrorCallback error_callback,
device::BluetoothRemoteGattService::GattErrorCode error_code);