Invoke GattDiscoveryCompleteForService by observing ServicesResolved property

device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc

Index: device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
diff --git a/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc b/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
index ce7087eac54dbcbd2531be33b5e3c23f228bd4ac..d73845ce86781f8258a01b6667f70c61a50c9dcd 100644
--- a/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
+++ b/device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc
@@ -325,6 +325,7 @@ TEST_F(BluetoothGattBlueZTest, GattServiceAddedAndRemoved) {
   observer.last_gatt_service_id().clear();
   fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
       dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+
   EXPECT_EQ(2, observer.gatt_service_added_count());
   EXPECT_EQ(1, observer.gatt_service_removed_count());
   EXPECT_FALSE(observer.last_gatt_service_id().empty());
@@ -359,6 +360,60 @@ TEST_F(BluetoothGattBlueZTest, GattServiceAddedAndRemoved) {
                       bluez::FakeBluetoothDeviceClient::kLowEnergyAddress));
 }
 
+TEST_F(BluetoothGattBlueZTest, DiscoveryCompleteForCashedGattService) {

[rkc, 2016/05/17 18:38:50]

s/Cashed/Cached

[Miao, 2016/05/18 03:26:58]

Done.

+  // This tests if the notifications on service discovered complete are invoked
+  // with the cached services and added to the GATT service map of |device|.
+  TestBluetoothAdapterObserver observer(adapter_);
+
+  // Create the device and pre-expose the fake Heart Rate service. This will
+  // synchronously expose characteristics.
+  fake_bluetooth_device_client_->CreateDevice(
+      dbus::ObjectPath(bluez::FakeBluetoothAdapterClient::kAdapterPath),
+      dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kCachedLowEnergyPath));
+  BluetoothDevice* device =
+      adapter_->GetDevice(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress);
+  ASSERT_TRUE(device);
+
+  device::BluetoothRemoteGattService* service = device->GetGattServices()[0];
+  EXPECT_EQ(1u, device->GetGattServices().size());
+  EXPECT_EQ(1, observer.gatt_discovery_complete_count());
+  EXPECT_EQ(device, service->GetDevice());
+  EXPECT_EQ(service, device->GetGattService(service->GetIdentifier()));
+  EXPECT_EQ(3U, service->GetCharacteristics().size());
+}
+
+TEST_F(BluetoothGattBlueZTest, DiscoveryCompleteForNewGattService) {
+  // This tests the discovery complete notification on a newly-added GATT
+  // service.
+  fake_bluetooth_device_client_->CreateDevice(
+      dbus::ObjectPath(bluez::FakeBluetoothAdapterClient::kAdapterPath),
+      dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+  BluetoothDevice* device =
+      adapter_->GetDevice(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress);
+
+  ASSERT_TRUE(device);
+
+  TestBluetoothAdapterObserver observer(adapter_);

[scheib, 2016/05/17 19:24:47]

Move the observer up to before CreateDevice and verify here:
EXPECT_EQ(0, device->GetGattServices().size());
EXPECT_EQ(0, observer.gatt_discovery_complete_count());

[Miao, 2016/05/18 03:26:58]

Done.

+
+  // Expose the fake Heart Rate service. This will asynchronously expose
+  // characteristics.
+  fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
+      dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+
+  // Run the message loop so that the characteristics/descriptors appear.
+  base::MessageLoop::current()->Run();
+
+  // The discovery completed event of a newly-added GATT service should not be
+  // fired until ServicesResolved property becomes true. And the new service
+  // will be added immediately to the GATT service map of |device|.
+  BluetoothRemoteGattService* service = device->GetGattServices()[0];
+  EXPECT_EQ(1u, device->GetGattServices().size());
+  EXPECT_EQ(1, observer.gatt_discovery_complete_count());
+  EXPECT_EQ(device, service->GetDevice());
+  EXPECT_EQ(service, device->GetGattService(service->GetIdentifier()));
+  EXPECT_EQ(3U, service->GetCharacteristics().size());
+}
+
 TEST_F(BluetoothGattBlueZTest, ServicesDiscovered) {
   // Create a fake LE device. We store the device pointer here because this is a
   // test. It's unsafe to do this in production as the device might get deleted.
@@ -378,9 +433,8 @@ TEST_F(BluetoothGattBlueZTest, ServicesDiscovered) {
   // Expose the fake Heart Rate Service.
   fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
       dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
-  // Notify that all services have been discovered.
-  properties->services_resolved.ReplaceValue(true);
-
+  // Run the message loop so that the characteristics/descriptors appear.
+  base::MessageLoop::current()->Run();
   EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
   EXPECT_EQ(1u, device->GetGattServices().size());
   EXPECT_EQ(device, observer.last_device());
@@ -412,7 +466,8 @@ TEST_F(BluetoothGattBlueZTest, ServicesDiscovered) {
   // Verify that service discovery can be done again:
   fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
       dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
-  properties->services_resolved.ReplaceValue(true);
+  // Run the message loop so that the characteristics/descriptors appear.
+  base::MessageLoop::current()->Run();
   EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
   EXPECT_EQ(1u, device->GetGattServices().size());
 }
@@ -437,7 +492,6 @@ TEST_F(BluetoothGattBlueZTest, GattCharacteristicAddedAndRemoved) {
       device->GetGattService(observer.last_gatt_service_id());
 
   EXPECT_EQ(0, observer.gatt_service_changed_count());

[scheib, 2016/05/17 19:24:47]

Does this line need to be removed? Did behavior change?

[Miao, 2016/05/18 03:26:58]

The Behavior doesn't not change. It is unclear to me that why
observer.gatt_discovery_complete_count() is checked here but it is not checked
below.

-  EXPECT_EQ(0, observer.gatt_discovery_complete_count());
   EXPECT_EQ(0, observer.gatt_characteristic_added_count());
   EXPECT_EQ(0, observer.gatt_characteristic_removed_count());
   EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count());