bluetooth: Update the map of GATT services when services resolve

device/bluetooth/bluez/bluetooth_device_bluez.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "device/bluetooth/bluez/bluetooth_device_bluez.h"
 
 #include <stdio.h>
 
 #include <memory>
 #include <utility>
@@ skipping 138 matching lines @@
     scoped_refptr<device::BluetoothSocketThread> socket_thread)
     : BluetoothDevice(adapter),
       object_path_(object_path),
       num_connecting_calls_(0),
       connection_monitor_started_(false),
       ui_task_runner_(ui_task_runner),
       socket_thread_(socket_thread),
       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() {
   bluez::BluezDBusManager::Get()
       ->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;
   gatt_services_swapped.swap(gatt_services_);
   for (const auto& iter : gatt_services_swapped) {
     DCHECK(adapter());
     adapter()->NotifyGattServiceRemoved(
         static_cast<BluetoothRemoteGattServiceBlueZ*>(iter.second));
   }
 }
@@ skipping 393 matching lines @@
 }
 
 BluetoothPairingBlueZ* BluetoothDeviceBlueZ::GetPairing() const {
   return pairing_.get();
 }
 
 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())) {
     VLOG(1) << "Remote GATT service already exists: " << object_path.value();
     return;
   }
 
   bluez::BluetoothGattServiceClient::Properties* properties =
       bluez::BluezDBusManager::Get()
           ->GetBluetoothGattServiceClient()
           ->GetProperties(object_path);
   DCHECK(properties);
   if (properties->device.value() != object_path_) {
     VLOG(2) << "Remote GATT service does not belong to this device.";
     return;
   }
 
   VLOG(1) << "Adding new remote GATT service for device: " << GetAddress();
 
   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);
   DCHECK(service->GetUUID().IsValid());
 
   DCHECK(adapter());
   adapter()->NotifyGattServiceAdded(service);
 }
 
 void BluetoothDeviceBlueZ::GattServiceRemoved(
@@ skipping 10 matching lines @@
 
   VLOG(1) << "Removing remote GATT service with UUID: '"
           << service->GetUUID().canonical_value()
           << "' from device: " << GetAddress();
 
   DCHECK(service->object_path() == object_path);
   std::unique_ptr<BluetoothRemoteGattService> scoped_service =
       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);
 
-  // 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);
+    // 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 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,
                                          int16_t rssi,
                                          int16_t transmit_power,
                                          int16_t max_transmit_power) {
   callback.Run(ConnectionInfo(rssi, transmit_power, max_transmit_power));
 }
 
 void BluetoothDeviceBlueZ::OnGetConnInfoError(
@@ skipping 186 matching lines @@
 void BluetoothDeviceBlueZ::OnForgetError(const ErrorCallback& error_callback,
                                          const std::string& error_name,
                                          const std::string& error_message) {
   LOG(WARNING) << object_path_.value()
                << ": Failed to remove device: " << error_name << ": "
                << error_message;
   error_callback.Run();
 }
 
 }  // namespace bluez