bluetooth: Update the map of GATT services when services resolve

device/bluetooth/bluez/bluetooth_device_bluez.cc

Index: device/bluetooth/bluez/bluetooth_device_bluez.cc
diff --git a/device/bluetooth/bluez/bluetooth_device_bluez.cc b/device/bluetooth/bluez/bluetooth_device_bluez.cc
index affe9ab967bf339664fd7ce3604ff657f7d0268b..3cb677ce08d1335789cadb5c16558408537c4448 100644
--- a/device/bluetooth/bluez/bluetooth_device_bluez.cc
+++ b/device/bluetooth/bluez/bluetooth_device_bluez.cc
@@ -156,9 +156,15 @@ BluetoothDeviceBlueZ::BluetoothDeviceBlueZ(
       weak_ptr_factory_(this) {
   bluez::BluezDBusManager::Get()->GetBluetoothGattServiceClient()->AddObserver(
       this);
-  bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->AddObserver(this);
 
-  InitializeGattServiceMap();
+  // If GATT Services have already been discovered update the list of Gatt
+  // Services.
+  if (IsGattServicesDiscoveryComplete()) {

[ortuno, 2016/07/06 21:31:46]

luiz: You mentioned that this will always be false during construction. The case
I was thinking about was the following:

1. Chrome is closed.
2. Use bluetoothctl to connect to a device and wait for all its services to be
discovered.
3. Open chrome.

At this point the device is connected all its services are discovered so
IsGattServicesDiscoveryComplete should be true, right?

[vudentz, 2016/07/07 09:45:08]

Yep, I was assuming this object to be responsible of tracking
IsGattServicesDiscoveryComplete but it looks like it is reading from D-Bus
directly.

+    UpdateGattServices(object_path_);
+  } else {
+    VLOG(2) << "Gatt services have not been fully resolved for device "
+            << object_path_.value();
+  }
 }
 
 BluetoothDeviceBlueZ::~BluetoothDeviceBlueZ() {
@@ -166,8 +172,6 @@ BluetoothDeviceBlueZ::~BluetoothDeviceBlueZ() {
       ->GetBluetoothGattServiceClient()
       ->RemoveObserver(this);
 
-  bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->RemoveObserver(
-      this);
   // Copy the GATT services list here and clear the original so that when we
   // send GattServiceRemoved(), GetGattServices() returns no services.
   GattServiceMap gatt_services_swapped;
@@ -581,30 +585,6 @@ BluetoothAdapterBlueZ* BluetoothDeviceBlueZ::adapter() const {
   return static_cast<BluetoothAdapterBlueZ*>(adapter_);
 }
 
-void BluetoothDeviceBlueZ::DevicePropertyChanged(
-    const dbus::ObjectPath& object_path,
-    const std::string& property_name) {
-  if (object_path != object_path_)
-    return;
-
-  bluez::BluetoothDeviceClient::Properties* properties =
-      bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->GetProperties(
-          object_path);
-  DCHECK(properties);
-
-  if (property_name == properties->services_resolved.name() &&
-      properties->services_resolved.value()) {
-    VLOG(3) << "All services were discovered for device: "
-            << object_path.value();
-
-    for (const auto iter : newly_discovered_gatt_services_) {
-      adapter()->NotifyGattDiscoveryComplete(
-          static_cast<BluetoothRemoteGattService*>(iter));
-    }
-    newly_discovered_gatt_services_.clear();
-  }
-}
-
 void BluetoothDeviceBlueZ::GattServiceAdded(
     const dbus::ObjectPath& object_path) {
   if (GetGattService(object_path.value())) {
@@ -627,9 +607,6 @@ void BluetoothDeviceBlueZ::GattServiceAdded(
   BluetoothRemoteGattServiceBlueZ* service =
       new BluetoothRemoteGattServiceBlueZ(adapter(), this, object_path);
 
-  newly_discovered_gatt_services_.push_back(
-      static_cast<BluetoothRemoteGattServiceBlueZ*>(service));
-
   gatt_services_.set(service->GetIdentifier(),
                      std::unique_ptr<BluetoothRemoteGattService>(service));
   DCHECK(service->object_path() == object_path);
@@ -660,34 +637,43 @@ void BluetoothDeviceBlueZ::GattServiceRemoved(
       gatt_services_.take_and_erase(iter->first);
 
   DCHECK(adapter());
+  discovery_complete_notified_.erase(service);
   adapter()->NotifyGattServiceRemoved(service);
 }
 
-void BluetoothDeviceBlueZ::InitializeGattServiceMap() {
-  DCHECK(gatt_services_.empty());
-
-  if (!IsGattServicesDiscoveryComplete()) {
-    VLOG(2) << "Gatt services have not been fully resolved for device "
-            << object_path_.value();
+void BluetoothDeviceBlueZ::UpdateGattServices(
+    const dbus::ObjectPath& object_path) {
+  if (object_path != object_path_) {
+    // No need to update map if update is for a different device.
     return;
   }
 
-  VLOG(3) << "Initializing the list of GATT services associated with device "
+  DCHECK(IsGattServicesDiscoveryComplete());
+
+  VLOG(3) << "Updating the list of GATT services associated with device "
           << object_path_.value();
 
-  // Add all known GATT services associated with the device.
-  const std::vector<dbus::ObjectPath> gatt_services =
+  const std::vector<dbus::ObjectPath> service_paths =
       bluez::BluezDBusManager::Get()
           ->GetBluetoothGattServiceClient()
           ->GetServices();
-  for (const auto& it : gatt_services)
-    GattServiceAdded(it);
+  for (const auto& service_path : service_paths) {
+    // Add all previously unknown GATT services associated with the device.
+    GattServiceAdded(service_path);
+
+    // If the service does not belong in this device, there is nothing left to
+    // do.
+    BluetoothRemoteGattService* service = GetGattService(service_path.value());
+    if (service == nullptr) {
+      return;
+    }
 
-  // Notify on the discovery complete for each service which is found in the
-  // first discovery.
-  DCHECK(adapter());
-  for (const auto& iter : gatt_services_)
-    adapter()->NotifyGattDiscoveryComplete(iter.second);
+    // Notify of GATT discovery complete if we haven't before.
+    auto notified_pair = discovery_complete_notified_.insert(service);
+    if (notified_pair.second) {
+      adapter()->NotifyGattDiscoveryComplete(service);
+    }
+  }
 }
 
 void BluetoothDeviceBlueZ::OnGetConnInfo(const ConnectionInfoCallback& callback,