arc: bluetooth: Implement Gatt Server add/delete/start/stop service

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 4c5637a916ed6a6b7ea58c9e586cecffed959aaf..57ae9914b3815ce532d1324765a8f088a851456e 100644
--- a/components/arc/bluetooth/arc_bluetooth_bridge.cc
+++ b/components/arc/bluetooth/arc_bluetooth_bridge.cc
@@ -9,6 +9,7 @@
 
 #include <iomanip>
 #include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -49,6 +50,7 @@ using device::BluetoothUUID;
 namespace {
 constexpr int32_t kMinBtleVersion = 1;
 constexpr int32_t kMinBtleNotifyVersion = 2;
+constexpr int32_t kMinGattServerVersion = 3;
 constexpr uint32_t kGattReadPermission =
     BluetoothGattCharacteristic::Permission::PERMISSION_READ |
     BluetoothGattCharacteristic::Permission::PERMISSION_READ_ENCRYPTED |
@@ -59,6 +61,7 @@ constexpr uint32_t kGattWritePermission =
     BluetoothGattCharacteristic::Permission::PERMISSION_WRITE_ENCRYPTED |
     BluetoothGattCharacteristic::Permission::
         PERMISSION_WRITE_ENCRYPTED_AUTHENTICATED;
+constexpr int32_t kInvalidGattAttributeHandle = -1;
 }  // namespace
 
 namespace arc {
@@ -751,17 +754,17 @@ int ArcBluetoothBridge::ConvertGattIdentifierToId(
 
 // Create GattDBElement and fill in common data for
 // Gatt Service/Characteristic/Descriptor.
-template <class T>
+template <class RemoteGattAttribute>
 mojom::BluetoothGattDBElementPtr ArcBluetoothBridge::CreateGattDBElement(

[Luis Héctor Chávez, 2016/07/19 01:42:17]

anonymous namespace?

[puthik_chromium, 2016/07/19 22:21:30]

Done.

     const mojom::BluetoothGattDBAttributeType type,
-    const T* gattObject) const {
+    const RemoteGattAttribute* gatt_attr) const {
   mojom::BluetoothGattDBElementPtr element =
       mojom::BluetoothGattDBElement::New();
   element->type = type;
-  element->uuid = mojom::BluetoothUUID::From(gattObject->GetUUID());
+  element->uuid = mojom::BluetoothUUID::From(gatt_attr->GetUUID());
   element->id = element->attribute_handle = element->start_handle =
       element->end_handle =
-          ConvertGattIdentifierToId(gattObject->GetIdentifier());
+          ConvertGattIdentifierToId(gatt_attr->GetIdentifier());
   element->properties = 0;
   return element;
 }
@@ -813,13 +816,14 @@ void ArcBluetoothBridge::GetGattDB(mojom::BluetoothAddressPtr remote_addr) {
 }
 
 // Find Gatt Service/Characteristic/Descriptor from std::vector from UUID.
-template <class T>
-T* ArcBluetoothBridge::FindGattObjectFromUuid(
-    const std::vector<T*> gatt_objs,
+template <class RemoteGattAttribute>
+RemoteGattAttribute* ArcBluetoothBridge::FindGattAttributeFromUuid(

[Luis Héctor Chávez, 2016/07/19 01:42:17]

Anonymous namespace?

[puthik_chromium, 2016/07/19 22:21:30]

Done.

+    const std::vector<RemoteGattAttribute*> gatt_attrs,
     const device::BluetoothUUID uuid) const {
-  auto it = std::find_if(gatt_objs.begin(), gatt_objs.end(),
-                         [&](T* obj) { return obj->GetUUID() == uuid; });
-  if (it == gatt_objs.end())
+  auto it = std::find_if(
+      gatt_attrs.begin(), gatt_attrs.end(),
+      [&](RemoteGattAttribute* attr) { return attr->GetUUID() == uuid; });

[Luis Héctor Chávez, 2016/07/19 01:42:17]

default captures are disallowed:
https://chromium-cpp.appspot.com/#core-whitelist

use

[uuid](RemoteGattAttribute* attr) { ... }

instead

[puthik_chromium, 2016/07/19 22:21:30]

Done.

+  if (it == gatt_attrs.end())
     return nullptr;
   return *it;
 }
@@ -838,12 +842,12 @@ BluetoothRemoteGattCharacteristic* ArcBluetoothBridge::FindGattCharacteristic(
     return nullptr;
 
   BluetoothRemoteGattService* service =
-      FindGattObjectFromUuid<BluetoothRemoteGattService>(
+      FindGattAttributeFromUuid<BluetoothRemoteGattService>(
           device->GetGattServices(), service_id->id->uuid.To<BluetoothUUID>());
   if (!service)
     return nullptr;
 
-  return FindGattObjectFromUuid<BluetoothRemoteGattCharacteristic>(
+  return FindGattAttributeFromUuid<BluetoothRemoteGattCharacteristic>(
       service->GetCharacteristics(), char_id->uuid.To<BluetoothUUID>());
 }
 
@@ -857,10 +861,23 @@ BluetoothRemoteGattDescriptor* ArcBluetoothBridge::FindGattDescriptor(
   if (!characteristic)
     return nullptr;
 
-  return FindGattObjectFromUuid<BluetoothRemoteGattDescriptor>(
+  return FindGattAttributeFromUuid<BluetoothRemoteGattDescriptor>(
       characteristic->GetDescriptors(), desc_id->uuid.To<BluetoothUUID>());
 }
 
+// Common callback for GATT operations that only need to report GATT status
+// back to Android.
+void ArcBluetoothBridge::OnGattOperationDone(

[Luis Héctor Chávez, 2016/07/19 01:42:17]

Definitely anonymous namespace.

+    const GattStatusCallback& callback) const {
+  callback.Run(mojom::BluetoothGattStatus::GATT_SUCCESS);
+}
+
+void ArcBluetoothBridge::OnGattOperationError(

[Luis Héctor Chávez, 2016/07/19 01:42:17]

Anonymous namespace

[puthik_chromium, 2016/07/19 22:21:30]

Done.

+    const GattStatusCallback& callback,
+    BluetoothGattService::GattErrorCode error_code) const {
+  callback.Run(mojo::ConvertTo<mojom::BluetoothGattStatus>(error_code));
+}
+
 // Same callback for both ReadGattCharacteristic and ReadGattDescriptor
 void ArcBluetoothBridge::OnGattReadDone(
     const GattReadCallback& callback,
@@ -881,18 +898,6 @@ void ArcBluetoothBridge::OnGattReadError(
   callback.Run(std::move(gattValue));
 }
 
-// Same callback for both WriteGattCharacteristic and WriteGattDescriptor
-void ArcBluetoothBridge::OnGattWriteDone(
-    const GattWriteCallback& callback) const {
-  callback.Run(mojom::BluetoothGattStatus::GATT_SUCCESS);
-}
-
-void ArcBluetoothBridge::OnGattWriteError(
-    const GattWriteCallback& callback,
-    BluetoothGattService::GattErrorCode error_code) const {
-  callback.Run(mojo::ConvertTo<mojom::BluetoothGattStatus>(error_code));
-}
-
 void ArcBluetoothBridge::ReadGattCharacteristic(
     mojom::BluetoothAddressPtr remote_addr,
     mojom::BluetoothGattServiceIDPtr service_id,
@@ -923,9 +928,9 @@ void ArcBluetoothBridge::WriteGattCharacteristic(
 
   characteristic->WriteRemoteCharacteristic(
       value->value.To<std::vector<uint8_t>>(),
-      base::Bind(&ArcBluetoothBridge::OnGattWriteDone,
+      base::Bind(&ArcBluetoothBridge::OnGattOperationDone,
                  weak_factory_.GetWeakPtr(), callback),
-      base::Bind(&ArcBluetoothBridge::OnGattWriteError,
+      base::Bind(&ArcBluetoothBridge::OnGattOperationError,
                  weak_factory_.GetWeakPtr(), callback));
 }
 
@@ -970,15 +975,15 @@ void ArcBluetoothBridge::WriteGattDescriptor(
   // TODO(http://crbug.com/622832)
   if (descriptor->GetUUID() ==
       BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()) {
-    OnGattWriteDone(callback);
+    OnGattOperationDone(callback);
     return;
   }
 
   descriptor->WriteRemoteDescriptor(
       value->value.To<std::vector<uint8_t>>(),
-      base::Bind(&ArcBluetoothBridge::OnGattWriteDone,
+      base::Bind(&ArcBluetoothBridge::OnGattOperationDone,
                  weak_factory_.GetWeakPtr(), callback),
-      base::Bind(&ArcBluetoothBridge::OnGattWriteError,
+      base::Bind(&ArcBluetoothBridge::OnGattOperationError,
                  weak_factory_.GetWeakPtr(), callback));
 }
 
@@ -990,17 +995,6 @@ void ArcBluetoothBridge::OnGattNotifyStartDone(
   callback.Run(mojom::BluetoothGattStatus::GATT_SUCCESS);
 }
 
-void ArcBluetoothBridge::OnGattNotifyStartError(
-    const RegisterForGattNotificationCallback& callback,
-    BluetoothGattService::GattErrorCode error_code) const {
-  callback.Run(mojo::ConvertTo<mojom::BluetoothGattStatus>(error_code));
-}
-
-void ArcBluetoothBridge::OnGattNotifyStopDone(
-    const DeregisterForGattNotificationCallback& callback) const {
-  callback.Run(mojom::BluetoothGattStatus::GATT_SUCCESS);
-}
-
 void ArcBluetoothBridge::RegisterForGattNotification(
     mojom::BluetoothAddressPtr remote_addr,
     mojom::BluetoothGattServiceIDPtr service_id,
@@ -1023,7 +1017,7 @@ void ArcBluetoothBridge::RegisterForGattNotification(
       base::Bind(&ArcBluetoothBridge::OnGattNotifyStartDone,
                  weak_factory_.GetWeakPtr(), callback,
                  characteristic->GetIdentifier()),
-      base::Bind(&ArcBluetoothBridge::OnGattNotifyStartError,
+      base::Bind(&ArcBluetoothBridge::OnGattOperationError,
                  weak_factory_.GetWeakPtr(), callback));
 }
 
@@ -1049,7 +1043,7 @@ void ArcBluetoothBridge::DeregisterForGattNotification(
   std::unique_ptr<BluetoothGattNotifySession> notify =
       std::move(notification_session_[char_id_str]);
   notification_session_.erase(char_id_str);
-  notify->Stop(base::Bind(&ArcBluetoothBridge::OnGattNotifyStopDone,
+  notify->Stop(base::Bind(&ArcBluetoothBridge::OnGattOperationDone,
                           weak_factory_.GetWeakPtr(), callback));
 }
 
@@ -1062,30 +1056,104 @@ void ArcBluetoothBridge::ReadRemoteRssi(
   callback.Run(rssi);
 }
 
+template <class LocalGattAttribute>
+int32_t ArcBluetoothBridge::CreateGattAttributeHandle(
+    LocalGattAttribute* gatt_attr) {

[Luis Héctor Chávez, 2016/07/19 01:42:17]

This needs a DCHECK to validate it's running on the correct thread.

[puthik_chromium, 2016/07/19 22:21:30]

Done.
Also added thread check when dealing with observer.

+  if (!gatt_attr)
+    return kInvalidGattAttributeHandle;
+  gatt_server_attr_handle++;

[Luis Héctor Chávez, 2016/07/19 01:42:17]

int32_t handle = ++gatt_server_attribute_next_handle_;

[puthik_chromium, 2016/07/19 22:21:30]

Done.

+  std::string identifier = gatt_attr->GetIdentifier();

[Luis Héctor Chávez, 2016/07/19 01:42:17]

can this be const std::string&?

[puthik_chromium, 2016/07/19 22:21:30]

Done.

+  gatt_identifier_[gatt_server_attr_handle] = identifier;
+  return gatt_server_attr_handle;
+}
+
 void ArcBluetoothBridge::AddService(mojom::BluetoothGattServiceIDPtr service_id,
                                     int32_t num_handles,
-                                    const AddServiceCallback& callback) {}
+                                    const AddServiceCallback& callback) {
+  base::WeakPtr<BluetoothLocalGattService> service =
+      BluetoothLocalGattService::Create(
+          bluetooth_adapter_.get(), service_id->id->uuid.To<BluetoothUUID>(),
+          service_id->is_primary, nullptr /*included_service */,
+          this /* delegate*/);
+  callback.Run(
+      CreateGattAttributeHandle<BluetoothLocalGattService>(service.get()));
+}
 
 void ArcBluetoothBridge::AddCharacteristic(
     int32_t service_handle,
     mojom::BluetoothUUIDPtr uuid,
     int32_t properties,
     int32_t permissions,
-    const AddCharacteristicCallback& callback) {}
+    const AddCharacteristicCallback& callback) {
+  base::WeakPtr<BluetoothLocalGattCharacteristic> characteristic =
+      BluetoothLocalGattCharacteristic::Create(
+          uuid.To<BluetoothUUID>(), properties, permissions,
+          bluetooth_adapter_->GetGattService(gatt_identifier_[service_handle]));
+  int32_t characteristic_handle =
+      CreateGattAttributeHandle<BluetoothLocalGattCharacteristic>(
+          characteristic.get());
+  last_characteristic_[service_handle] = characteristic_handle;
+  callback.Run(characteristic_handle);
+}
 
 void ArcBluetoothBridge::AddDescriptor(int32_t service_handle,
                                        mojom::BluetoothUUIDPtr uuid,
                                        int32_t permissions,
-                                       const AddDescriptorCallback& callback) {}
+                                       const AddDescriptorCallback& callback) {
+  // Chrome automatically adds a CCC Descriptor to a characteristic when needed.
+  // We will generate a bogus handle for Android.
+  if (uuid.To<BluetoothUUID>() ==
+      BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()) {
+    gatt_server_attr_handle++;

[Luis Héctor Chávez, 2016/07/19 01:42:17]

You should ideally modify this value in a single function. Please create an
inline

int32_t next_handle() { return ++gatt_server_attribute_next_handle_; }

in the .h

[puthik_chromium, 2016/07/19 22:21:30]

Done.

+    callback.Run(gatt_server_attr_handle);
+    return;
+  }
+
+  BluetoothLocalGattService* service =
+      bluetooth_adapter_->GetGattService(gatt_identifier_[service_handle]);

[Luis Héctor Chávez, 2016/07/19 01:42:17]

What happens when |service_handle| is invalid? Same in all other places you do
|mapping[service_handle]|

[puthik_chromium, 2016/07/19 22:21:30]

It shouldn't invalid according to upper layer android code. I think just add
DCHECK is enough.

+  // Since the Android API does not give information about which characteristic
+  // is the parent of the new descriptor, we assume that it would be the last
+  // characteristic that was added to the given service. This matches the
+  // Android framework code at android/bluetooth/BluetoothGattServer.java#594.
+  // Link: https://goo.gl/cJZl1u
+  BluetoothLocalGattCharacteristic* characteristic = service->GetCharacteristic(
+      gatt_identifier_[last_characteristic_[service_handle]]);
+
+  base::WeakPtr<BluetoothLocalGattDescriptor> descriptor =
+      BluetoothLocalGattDescriptor::Create(uuid.To<BluetoothUUID>(),
+                                           permissions, characteristic);
+  callback.Run(CreateGattAttributeHandle<BluetoothLocalGattDescriptor>(
+      descriptor.get()));
+}
 
 void ArcBluetoothBridge::StartService(int32_t service_handle,
-                                      const StartServiceCallback& callback) {}
+                                      const StartServiceCallback& callback) {
+  BluetoothLocalGattService* service =
+      bluetooth_adapter_->GetGattService(gatt_identifier_[service_handle]);
+  service->Register(base::Bind(&ArcBluetoothBridge::OnGattOperationDone,
+                               weak_factory_.GetWeakPtr(), callback),
+                    base::Bind(&ArcBluetoothBridge::OnGattOperationError,
+                               weak_factory_.GetWeakPtr(), callback));
+}
 
 void ArcBluetoothBridge::StopService(int32_t service_handle,
-                                     const StopServiceCallback& callback) {}
+                                     const StopServiceCallback& callback) {
+  BluetoothLocalGattService* service =
+      bluetooth_adapter_->GetGattService(gatt_identifier_[service_handle]);
+  service->Unregister(base::Bind(&ArcBluetoothBridge::OnGattOperationDone,
+                                 weak_factory_.GetWeakPtr(), callback),
+                      base::Bind(&ArcBluetoothBridge::OnGattOperationError,
+                                 weak_factory_.GetWeakPtr(), callback));
+}
 
 void ArcBluetoothBridge::DeleteService(int32_t service_handle,
-                                       const DeleteServiceCallback& callback) {}
+                                       const DeleteServiceCallback& callback) {
+  BluetoothLocalGattService* service =
+      bluetooth_adapter_->GetGattService(gatt_identifier_[service_handle]);
+  service->Delete();
+  gatt_identifier_.erase(service_handle);
+  OnGattOperationDone(callback);
+}
 
 void ArcBluetoothBridge::SendIndication(
     int32_t attribute_handle,