DBus support for attribute properties and permissions.

device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc

Index: device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc
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 c315ea1f7aefe2ddfb6f9899bb1081f181548435..d496049f1e7709bf2e04982e9ee0fbfa49a75f45 100644
--- a/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc
+++ b/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc
@@ -4,7 +4,7 @@
 
 #include "device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h"
 
-#include <stddef.h>
+#include <cstddef>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -29,10 +29,10 @@ BluetoothGattCharacteristicServiceProviderImpl::
         std::unique_ptr<BluetoothGattAttributeValueDelegate> delegate,
         const std::string& uuid,
         const std::vector<std::string>& flags,
-        const std::vector<std::string>& permissions,
         const dbus::ObjectPath& service_path)
     : origin_thread_id_(base::PlatformThread::CurrentId()),
       uuid_(uuid),
+      flags_(flags),
       bus_(bus),
       delegate_(std::move(delegate)),
       object_path_(object_path),
@@ -40,7 +40,12 @@ BluetoothGattCharacteristicServiceProviderImpl::
       weak_ptr_factory_(this) {
   VLOG(1) << "Created Bluetooth GATT characteristic: " << object_path.value()
           << " UUID: " << uuid;
-  DCHECK(bus_);
+
+  // If we have a null bus, this means that this is being initialized for a
+  // test, hence we shouldn't do any other setup.

[hashimoto, 2016/05/07 04:06:13]

This behavior can be a bit surprising for developers.
How about passing a dbus::MockBus to this ctor in the test instead?
The same goes for BluetoothGattDescriptorServiceProviderImpl and
BluetoothGattServiceServiceProviderImpl.

[rkc, 2016/05/07 19:55:43]

Passing a MockBus means that the rest of this code will expect functional
behavior from the bus, which breaks in several places.

For example, the mock bus will return a null pointer from GetExportedObject.
This means that any calls on that object will then cause a crash.

[stevenjb, 2016/05/09 17:31:38]

I agree that passing MockBus() and fixing the code would probably be best, but
I'm not sure how messy that would actually be.

Otherwise I would prefer to see the majority of the members initialized to
nullptr or uninitialized, and an Initialize() and InitializeForTest() method
added for all of these implementations.

That said, this is an improvement over multiple constructors, but we should at
least document each of these in the header with something like "|bus| can be
null for tests".

+  if (!bus_)
+    return;
+
   DCHECK(delegate_);
   DCHECK(!uuid_.empty());
   DCHECK(object_path_.IsValid());
@@ -80,19 +85,6 @@ BluetoothGattCharacteristicServiceProviderImpl::
     bus_->UnregisterExportedObject(object_path_);
 }
 
-BluetoothGattCharacteristicServiceProviderImpl::
-    BluetoothGattCharacteristicServiceProviderImpl(
-        const dbus::ObjectPath& object_path,
-        const std::string& uuid,
-        const dbus::ObjectPath& service_path)
-    : origin_thread_id_(base::PlatformThread::CurrentId()),
-      uuid_(uuid),
-      bus_(nullptr),
-      delegate_(nullptr),
-      object_path_(object_path),
-      service_path_(service_path),
-      weak_ptr_factory_(this) {}
-
 void BluetoothGattCharacteristicServiceProviderImpl::SendValueChanged(
     const std::vector<uint8_t>& value) {
   VLOG(2) << "Emitting a PropertiesChanged signal for characteristic value.";
@@ -178,7 +170,6 @@ void BluetoothGattCharacteristicServiceProviderImpl::Get(
   dbus::MessageWriter writer(response.get());
   dbus::MessageWriter variant_writer(NULL);
 
-  // TODO(armansito): Process the "Flags" and "Permissions" properties below.
   if (property_name == bluetooth_gatt_characteristic::kUUIDProperty) {
     writer.OpenVariant("s", &variant_writer);
     variant_writer.AppendString(uuid_);
@@ -187,6 +178,10 @@ void BluetoothGattCharacteristicServiceProviderImpl::Get(
     writer.OpenVariant("o", &variant_writer);
     variant_writer.AppendObjectPath(service_path_);
     writer.CloseContainer(&variant_writer);
+  } else if (property_name == bluetooth_gatt_characteristic::kFlagsProperty) {
+    writer.OpenVariant("as", &variant_writer);
+    variant_writer.AppendArrayOfStrings(flags_);
+    writer.CloseContainer(&variant_writer);
   } else {
     response = dbus::ErrorResponse::FromMethodCall(
         method_call, kErrorInvalidArgs,
@@ -340,11 +335,13 @@ void BluetoothGattCharacteristicServiceProviderImpl::WriteProperties(
 
   writer->OpenArray("{sv}", &array_writer);
 
+  // UUID:
   array_writer.OpenDictEntry(&dict_entry_writer);
   dict_entry_writer.AppendString(bluetooth_gatt_characteristic::kUUIDProperty);
   dict_entry_writer.AppendVariantOfString(uuid_);
   array_writer.CloseContainer(&dict_entry_writer);
 
+  // Service:
   array_writer.OpenDictEntry(&dict_entry_writer);
   dict_entry_writer.AppendString(
       bluetooth_gatt_characteristic::kServiceProperty);
@@ -352,6 +349,7 @@ void BluetoothGattCharacteristicServiceProviderImpl::WriteProperties(
   array_writer.CloseContainer(&dict_entry_writer);
 
   if (value) {
+    // Value:
     array_writer.OpenDictEntry(&dict_entry_writer);
     dict_entry_writer.AppendString(
         bluetooth_gatt_characteristic::kValueProperty);
@@ -361,10 +359,16 @@ void BluetoothGattCharacteristicServiceProviderImpl::WriteProperties(
     array_writer.CloseContainer(&dict_entry_writer);
   }
 
-  // TODO(armansito): Process Flags & Permissions properties.
+  // Flags:
+  array_writer.OpenDictEntry(&dict_entry_writer);
+  dict_entry_writer.AppendString(bluetooth_gatt_characteristic::kFlagsProperty);
+  dict_entry_writer.OpenVariant("as", &variant_writer);
+  variant_writer.AppendArrayOfStrings(flags_);
+  dict_entry_writer.CloseContainer(&variant_writer);
+  array_writer.CloseContainer(&dict_entry_writer);
 
   writer->CloseContainer(&array_writer);
-};
+}
 
 void BluetoothGattCharacteristicServiceProviderImpl::OnGet(
     dbus::MethodCall* method_call,