arc: bluetooth: Implement Gatt server request to read/write

components/arc/bluetooth/arc_bluetooth_bridge.cc

Index: components/arc/bluetooth/arc_bluetooth_bridge.cc
diff --git a/components/arc/bluetooth/arc_bluetooth_bridge.cc b/components/arc/bluetooth/arc_bluetooth_bridge.cc
index 9489db9d4a6136b0be5a0aa3c49e92d581e76f5d..62f8dc5db0429e8aa3503a700665851d5708b22f 100644
--- a/components/arc/bluetooth/arc_bluetooth_bridge.cc
+++ b/components/arc/bluetooth/arc_bluetooth_bridge.cc
@@ -293,12 +293,74 @@ void ArcBluetoothBridge::GattDescriptorValueChanged(
   // Placeholder for GATT client functionality
 }
 
+// We will use odd transaction_id for read and even id for write to make sure
+// that it does not overlap and make it easy to determine the transaction type.
+int32_t ArcBluetoothBridge::CreateGattReadTransaction(
+    const GattServerReadCallbacks& callbacks) {
+  static int32_t transaction_id = 1;
+  transaction_id += 2;
+  gatt_server_read_callbacks_[transaction_id] = callbacks;
+  return transaction_id;
+}
+
+int32_t ArcBluetoothBridge::CreateGattWriteTransaction(
+    const GattServerWriteCallbacks& callbacks) {
+  static int32_t transaction_id = 2;
+  transaction_id += 2;
+  gatt_server_write_callbacks_[transaction_id] = callbacks;
+  return transaction_id;
+}
+
+template <class LocalGattObjectT>
+void ArcBluetoothBridge::OnGattAttributeReadRequest(
+    const BluetoothDevice* device,
+    const LocalGattObjectT* gatt_obj,
+    int offset,
+    const GattServerReadCallbacks& callbacks) {
+  if (!HasBluetoothInstance())
+    return;
+
+  if (!CheckBluetoothInstanceVersion(kMinBtleNotifyVersion))
+    return;
+
+  int32_t transaction_id = CreateGattReadTransaction(callbacks);
+
+  arc_bridge_service()->bluetooth()->instance()->RequestGattRead(
+      mojom::BluetoothAddress::From(device->GetAddress()), transaction_id,
+      gatt_handle_[gatt_obj->GetIdentifier()], offset, false /*is_long */);

[palmer, 2016/07/16 00:48:26]

Is there code somewhere (in the recipient?) that validates that |offset| is
within a reasonable range?

[puthik_chromium, 2016/07/18 21:30:32]

Look like Android API expected app to override this method and do the method
check there. I can't find any range check in the Android HAL/Framework code[1].
Android Example code:[2]
[1]
https://developer.android.com/reference/android/bluetooth/BluetoothGattServer...,
int, int, android.bluetooth.BluetoothGattCharacteristic)

[2]
https://android.googlesource.com/platform/system/bt/+/a4bd0d2/service/example...

We can't do the range check by here because we don't know the length of
characteristic/descriptor to read/write.

[puthik_chromium, 2016/07/19 01:03:15]

I also add sanity check to just call error_callback when offset < 0

+}
+
+template <class LocalGattObjectT>
+void ArcBluetoothBridge::OnGattAttributeWriteRequest(
+    const BluetoothDevice* device,
+    const LocalGattObjectT* gatt_obj,
+    const std::vector<uint8_t>& value,
+    int offset,
+    const GattServerWriteCallbacks& callbacks) {
+  if (!HasBluetoothInstance())
+    return;
+
+  if (!CheckBluetoothInstanceVersion(kMinBtleNotifyVersion))
+    return;
+
+  int32_t transaction_id = CreateGattWriteTransaction(callbacks);
+
+  arc_bridge_service()->bluetooth()->instance()->RequestGattWrite(
+      mojom::BluetoothAddress::From(device->GetAddress()), transaction_id,
+      gatt_handle_[gatt_obj->GetIdentifier()], offset,
+      mojo::Array<uint8_t>::From(value));
+}
+
 void ArcBluetoothBridge::OnCharacteristicReadRequest(
     const BluetoothDevice* device,
     const BluetoothLocalGattCharacteristic* characteristic,
     int offset,
     const ValueCallback& callback,
-    const ErrorCallback& error_callback) {}
+    const ErrorCallback& error_callback) {
+  OnGattAttributeReadRequest<BluetoothLocalGattCharacteristic>(
+      device, characteristic, offset,
+      GattServerReadCallbacks(callback, error_callback));
+}
 
 void ArcBluetoothBridge::OnCharacteristicWriteRequest(
     const BluetoothDevice* device,
@@ -306,14 +368,22 @@ void ArcBluetoothBridge::OnCharacteristicWriteRequest(
     const std::vector<uint8_t>& value,
     int offset,
     const base::Closure& callback,
-    const ErrorCallback& error_callback) {}
+    const ErrorCallback& error_callback) {
+  OnGattAttributeWriteRequest<BluetoothLocalGattCharacteristic>(
+      device, characteristic, value, offset,
+      GattServerWriteCallbacks(callback, error_callback));
+}
 
 void ArcBluetoothBridge::OnDescriptorReadRequest(
     const BluetoothDevice* device,
     const BluetoothLocalGattDescriptor* descriptor,
     int offset,
     const ValueCallback& callback,
-    const ErrorCallback& error_callback) {}
+    const ErrorCallback& error_callback) {
+  OnGattAttributeReadRequest<BluetoothLocalGattDescriptor>(
+      device, descriptor, offset,
+      GattServerReadCallbacks(callback, error_callback));
+}
 
 void ArcBluetoothBridge::OnDescriptorWriteRequest(
     const BluetoothDevice* device,
@@ -321,7 +391,11 @@ void ArcBluetoothBridge::OnDescriptorWriteRequest(
     const std::vector<uint8_t>& value,
     int offset,
     const base::Closure& callback,
-    const ErrorCallback& error_callback) {}
+    const ErrorCallback& error_callback) {
+  OnGattAttributeWriteRequest<BluetoothLocalGattDescriptor>(
+      device, descriptor, value, offset,
+      GattServerWriteCallbacks(callback, error_callback));
+}
 
 void ArcBluetoothBridge::OnNotificationsStart(
     const BluetoothDevice* device,
@@ -1064,6 +1138,7 @@ int32_t ArcBluetoothBridge::CreateGattAttributeHandle(
   gatt_server_obj_handle++;
   std::string identifier = gatt_obj->GetIdentifier();
   gatt_identifier_[gatt_server_obj_handle] = identifier;
+  gatt_handle_[identifier] = gatt_server_obj_handle;
   return gatt_server_obj_handle;
 }
 
@@ -1148,8 +1223,9 @@ void ArcBluetoothBridge::DeleteService(int32_t service_handle,
                                        const DeleteServiceCallback& callback) {
   BluetoothLocalGattService* service =
       bluetooth_adapter_->GetGattService(gatt_identifier_[service_handle]);
-  service->Delete();
   gatt_identifier_.erase(service_handle);
+  gatt_handle_.erase(service->GetIdentifier());
+  service->Delete();
   OnGattOperationDone(callback);
 }
 
@@ -1163,7 +1239,26 @@ void ArcBluetoothBridge::SendIndication(
 void ArcBluetoothBridge::SendResponse(int32_t trans_id,
                                       int32_t status,
                                       mojo::Array<uint8_t> value,
-                                      const SendResponseCallback& callback) {}
+                                      const SendResponseCallback& callback) {
+  // We used odd transaction_id for read and even id for write.
+  if (trans_id & 1) {  // read
+    DCHECK(gatt_server_read_callbacks_.find(trans_id) !=
+           gatt_server_read_callbacks_.end());
+    if (status)
+      gatt_server_read_callbacks_[trans_id].error_callback.Run();
+    else
+      gatt_server_read_callbacks_[trans_id].success_callback.Run(

[palmer, 2016/07/16 00:48:26]

When the body if an if/else block is more than 1 line, use curly braces.

[puthik_chromium, 2016/07/18 21:30:32]

Done.

+          value.To<std::vector<uint8_t>>());
+  } else {  // write
+    DCHECK(gatt_server_write_callbacks_.find(trans_id) !=
+           gatt_server_write_callbacks_.end());
+    if (status)
+      gatt_server_write_callbacks_[trans_id].error_callback.Run();
+    else
+      gatt_server_write_callbacks_[trans_id].success_callback.Run();
+  }
+  OnGattOperationDone(callback);
+}
 
 void ArcBluetoothBridge::OnDiscoveryError() {
   LOG(WARNING) << "failed to change discovery state";