Invoke GattDiscoveryCompleteForService by observing ServicesResolved property

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 c835899ae1f69a540bd35135420c6c6dc5f76679..46302f7dcccce708131cb4cb57dff19006fdd667 100644
--- a/device/bluetooth/bluez/bluetooth_device_bluez.cc
+++ b/device/bluetooth/bluez/bluetooth_device_bluez.cc
@@ -155,16 +155,9 @@ BluetoothDeviceBlueZ::BluetoothDeviceBlueZ(
       weak_ptr_factory_(this) {
   bluez::BluezDBusManager::Get()->GetBluetoothGattServiceClient()->AddObserver(
       this);
+  bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->AddObserver(this);
 
-  // Add all known GATT services.
-  const std::vector<dbus::ObjectPath> gatt_services =
-      bluez::BluezDBusManager::Get()
-          ->GetBluetoothGattServiceClient()
-          ->GetServices();
-  for (std::vector<dbus::ObjectPath>::const_iterator it = gatt_services.begin();
-       it != gatt_services.end(); ++it) {
-    GattServiceAdded(*it);
-  }
+  InitializeGattServiceMap();
 }
 
 BluetoothDeviceBlueZ::~BluetoothDeviceBlueZ() {
@@ -178,7 +171,7 @@ BluetoothDeviceBlueZ::~BluetoothDeviceBlueZ() {
   gatt_services_swapped.swap(gatt_services_);
   for (const auto& iter : gatt_services_swapped) {
     DCHECK(adapter_);
-    adapter()->NotifyGattServiceRemoved(
+    adapter_->NotifyGattServiceRemoved(
         static_cast<BluetoothRemoteGattServiceBlueZ*>(iter.second));
   }
 }
@@ -553,6 +546,30 @@ 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())) {
@@ -575,12 +592,15 @@ 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);
   DCHECK(service->GetUUID().IsValid());
 
-  DCHECK(adapter_);
+  DCHECK(adapter());

[rkc, 2016/05/16 16:25:22]

Nit: Use either adapter_ or adapter() at all places?

[Miao, 2016/05/17 00:22:46]

Done.

   adapter()->NotifyGattServiceAdded(service);
 }
 
@@ -608,6 +628,37 @@ void BluetoothDeviceBlueZ::GattServiceRemoved(
   adapter()->NotifyGattServiceRemoved(service);
 }
 
+void BluetoothDeviceBlueZ::InitializeGattServiceMap() {
+  if (!gatt_services_.empty()) {

[scheib, 2016/05/16 20:22:26]

If InitializeGattServiceMap is only called from constructor:
This should be a DCHECK, as it is an assertion.

If InitializeGattServiceMap is to be called also from DevicePropertyChanged then
the code here must change to accommodate that: remove this block.

It seems there are 2 things here really, add all services at construction only.

Then, at construction or at DevicePropertiesChanged(services_resolved) call
NotifyGattDiscoveryComplete as needed.

[Miao, 2016/05/17 18:08:27]

Done. Exactly. This function addresses the case where services have been
discovered and cached by BlueZ. All other incoming GATT services will go through
GattServiceAdded and will be handled later by DevicePropertiesChanged.

+    VLOG(2) << "List of Gatt services has already been initialized for device"
+            << object_path_.value();
+    return;
+  }
+
+  if (!IsGattServicesDiscoveryComplete()) {
+    VLOG(2) << "Gatt services have not been fully resolved for device "
+            << object_path_.value();
+    return;
+  }
+
+  VLOG(1) << "Initialize the list of GATT services associated with device "

[rkc, 2016/05/16 16:25:22]

Normal operation, use VLOG(3).
Also,
nit: s/Initialize/Initializing

[Miao, 2016/05/17 00:22:46]

Done.

+          << object_path_.value();
+
+  // Add all known GATT services associated with the device.
+  const std::vector<dbus::ObjectPath> gatt_services =
+      bluez::BluezDBusManager::Get()
+          ->GetBluetoothGattServiceClient()
+          ->GetServices();
+  for (const auto& it : gatt_services)
+    GattServiceAdded(it);
+
+  // 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);
+}
+
 void BluetoothDeviceBlueZ::OnGetConnInfo(const ConnectionInfoCallback& callback,
                                          int16_t rssi,
                                          int16_t transmit_power,