//device/bluetooth support for attribute properties and permissions.

device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc

Index: device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc
diff --git a/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc b/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc
index 4b0e04775aecefbfed34fdf7c2f14e3fd40af331..5d554c7b345e4854958432cc4c5984ce97f96029 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc
@@ -6,22 +6,90 @@
 
 #include <string>
 #include <utility>
-#include <vector>
 
 #include "base/memory/ptr_util.h"
+#include "device/bluetooth/bluetooth_gatt_characteristic.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 #include "device/bluetooth/bluez/bluetooth_gatt_service_bluez.h"
 #include "device/bluetooth/dbus/bluetooth_gatt_application_service_provider_impl.h"
 #include "device/bluetooth/dbus/bluetooth_gatt_characteristic_delegate_wrapper.h"
-#include "device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider.h"
 #include "device/bluetooth/dbus/bluetooth_gatt_descriptor_delegate_wrapper.h"
-#include "device/bluetooth/dbus/bluetooth_gatt_descriptor_service_provider.h"
-#include "device/bluetooth/dbus/bluetooth_gatt_service_service_provider.h"
 #include "device/bluetooth/dbus/bluez_dbus_manager.h"
 #include "device/bluetooth/dbus/fake_bluetooth_gatt_application_service_provider.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace bluez {
 
+namespace {
+
+const std::vector<std::string> FlagsFromProperties(

[scheib, 2016/05/07 01:12:45]

Add a compile time check that all enum values are handled here, likely be adding
a NUM_PROPERTY to the enum. EG:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

[rkc, 2016/05/07 20:27:00]

Done.

[scheib, 2016/05/08 03:29:56]

Flags too please.

[rkc, 2016/05/08 22:15:30]

I presume you mean permissions too?
Done.

[scheib, 2016/05/08 23:43:51]

Yep.

+    device::BluetoothGattCharacteristic::Properties properties) {
+  std::vector<std::string> flags;
+  if (properties & device::BluetoothGattCharacteristic::PROPERTY_BROADCAST)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagBroadcast);
+  if (properties & device::BluetoothGattCharacteristic::PROPERTY_READ)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagRead);
+  if (properties &
+      device::BluetoothGattCharacteristic::PROPERTY_WRITE_WITHOUT_RESPONSE)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagWriteWithoutResponse);
+  if (properties & device::BluetoothGattCharacteristic::PROPERTY_WRITE)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagWrite);
+  if (properties & device::BluetoothGattCharacteristic::PROPERTY_NOTIFY)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagNotify);
+  if (properties & device::BluetoothGattCharacteristic::PROPERTY_INDICATE)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagIndicate);
+  if (properties &
+      device::BluetoothGattCharacteristic::PROPERTY_AUTHENTICATED_SIGNED_WRITES)
+    flags.push_back(
+        bluetooth_gatt_characteristic::kFlagAuthenticatedSignedWrites);
+  if (properties &
+      device::BluetoothGattCharacteristic::PROPERTY_EXTENDED_PROPERTIES)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagExtendedProperties);
+  if (properties & device::BluetoothGattCharacteristic::PROPERTY_RELIABLE_WRITE)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagReliableWrite);
+  if (properties &
+      device::BluetoothGattCharacteristic::PROPERTY_WRITABLE_AUXILIARIES)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagWritableAuxiliaries);
+  if (properties & device::BluetoothGattCharacteristic::PROPERTY_READ_ENCRYPTED)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagEncryptRead);
+  if (properties &
+      device::BluetoothGattCharacteristic::PROPERTY_WRITE_ENCRYPTED)
+    flags.push_back(bluetooth_gatt_characteristic::kFlagEncryptWrite);
+  if (properties & device::BluetoothGattCharacteristic::
+                       PROPERTY_READ_ENCRYPTED_AUTHENTICATED)
+    flags.push_back(
+        bluetooth_gatt_characteristic::kFlagEncryptAuthenticatedRead);
+  if (properties & device::BluetoothGattCharacteristic::
+                       PROPERTY_WRITE_ENCRYPTED_AUTHENTICATED)
+    flags.push_back(
+        bluetooth_gatt_characteristic::kFlagEncryptAuthenticatedWrite);
+  return flags;
+}
+
+const std::vector<std::string> FlagsFromPermissions(
+    device::BluetoothGattCharacteristic::Permissions permissions) {
+  std::vector<std::string> flags;
+  if (permissions & device::BluetoothGattCharacteristic::PERMISSION_READ)
+    flags.push_back(bluetooth_gatt_descriptor::kFlagRead);
+  if (permissions & device::BluetoothGattCharacteristic::PERMISSION_WRITE)
+    flags.push_back(bluetooth_gatt_descriptor::kFlagWrite);
+  if (permissions &
+      device::BluetoothGattCharacteristic::PERMISSION_READ_ENCRYPTED)
+    flags.push_back(bluetooth_gatt_descriptor::kFlagEncryptRead);
+  if (permissions &
+      device::BluetoothGattCharacteristic::PERMISSION_WRITE_ENCRYPTED)
+    flags.push_back(bluetooth_gatt_descriptor::kFlagEncryptWrite);
+  if (permissions & device::BluetoothGattCharacteristic::
+                        PERMISSION_READ_ENCRYPTED_AUTHENTICATED)
+    flags.push_back(bluetooth_gatt_descriptor::kFlagEncryptAuthenticatedRead);
+  if (permissions & device::BluetoothGattCharacteristic::
+                        PERMISSION_WRITE_ENCRYPTED_AUTHENTICATED)
+    flags.push_back(bluetooth_gatt_descriptor::kFlagEncryptAuthenticatedWrite);
+  return flags;
+}
+
+}  // namespace
+
 BluetoothGattApplicationServiceProvider::
     BluetoothGattApplicationServiceProvider() {}
 
@@ -51,14 +119,16 @@ void BluetoothGattApplicationServiceProvider::CreateAttributeServiceProviders(
               base::WrapUnique(new BluetoothGattCharacteristicDelegateWrapper(
                   service.second, characteristic.second.get())),
               characteristic.second->GetUUID().value(),
-              std::vector<std::string>(), service.second->object_path())));
+              FlagsFromProperties(characteristic.second->GetProperties()),

[scheib, 2016/05/07 01:12:45]

It would be nice if this code was covered in
bluetooth_gatt_application_service_provider_unittest.cc. However, it seems that
to do so construction code of CreateFakeAttributes would need to be called
before CreateAttributeServiceProviders, which isn't currently possible due to
being in the constructor for an object used by CreateFakeAttributes.

What sets service_providers_ in BluetoothGattApplicationServiceProviderImpl
before CreateAttributeServiceProviders is called? It would be much nicer if we
didn't have to ask that question by having it demonstrated in test.

[rkc, 2016/05/07 20:27:01]

To use this method, we'd need to have a valid set of services, to create which,
will require all the infrastructure needed for a regular BlueZ unittest
(including bringing up an adapter). That is overkill for a unit test trying to
test specifically DBus methods.

This method does get called and tested in our BluetoothLocalGatt* unit tests
though and this code does get exercised. Which is why it is a lower priority to
to try to test this in a DBus unit test too.

[scheib, 2016/05/08 03:29:56]

Acknowledged.

+              service.second->object_path())));
       for (const auto& descriptor : characteristic.second->GetDescriptors()) {
         descriptor_providers->push_back(
             base::WrapUnique(BluetoothGattDescriptorServiceProvider::Create(
                 bus, descriptor->object_path(),
                 base::WrapUnique(new BluetoothGattDescriptorDelegateWrapper(
                     service.second, descriptor.get())),
-                descriptor->GetUUID().value(), std::vector<std::string>(),
+                descriptor->GetUUID().value(),
+                FlagsFromPermissions(descriptor->GetPermissions()),
                 characteristic.second->object_path())));
       }
     }