Invoke GattDiscoveryCompleteForService by observing ServicesResolved property

device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc

 // Copyright 2016 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 <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <utility>
 
@@ skipping 307 matching lines @@
                 bluez::FakeBluetoothGattServiceClient::kHeartRateServiceUUID),
             observer.last_gatt_service_uuid());
 
   EXPECT_EQ(NULL, device->GetGattService(observer.last_gatt_service_id()));
 
   // Expose the service again.
   observer.last_gatt_service_uuid() = BluetoothUUID();
   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());
   EXPECT_EQ(1U, device->GetGattServices().size());
   EXPECT_EQ(BluetoothUUID(
                 bluez::FakeBluetoothGattServiceClient::kHeartRateServiceUUID),
             observer.last_gatt_service_uuid());
 
   // The object |service| points to should have been deallocated. |device|
   // should contain a brand new instance.
@@ skipping 14 matching lines @@
   EXPECT_EQ(2, observer.gatt_service_added_count());
   EXPECT_EQ(2, observer.gatt_service_removed_count());
   EXPECT_FALSE(observer.last_gatt_service_id().empty());
   EXPECT_EQ(BluetoothUUID(
                 bluez::FakeBluetoothGattServiceClient::kHeartRateServiceUUID),
             observer.last_gatt_service_uuid());
   EXPECT_EQ(NULL, adapter_->GetDevice(
                       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.
   fake_bluetooth_device_client_->CreateDevice(
       dbus::ObjectPath(bluez::FakeBluetoothAdapterClient::kAdapterPath),
       dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
   BluetoothDevice* device =
       adapter_->GetDevice(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress);
   bluez::FakeBluetoothDeviceClient::Properties* properties =
       fake_bluetooth_device_client_->GetProperties(
           dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
 
   ASSERT_TRUE(device);
 
   TestBluetoothAdapterObserver observer(adapter_);
 
   // 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());
   EXPECT_EQ(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress,
             observer.last_device_address());
 
   // Disconnect from the device:
   device->Disconnect(base::Bind(&BluetoothGattBlueZTest::SuccessCallback,
                                 base::Unretained(this)),
                      base::Bind(&BluetoothGattBlueZTest::ErrorCallback,
@@ skipping 11 matching lines @@
       base::Bind(&BluetoothGattBlueZTest::GattConnectionCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattBlueZTest::ConnectErrorCallback,
                  base::Unretained(this)));
   properties->connected.ReplaceValue(true);
   EXPECT_TRUE(device->IsConnected());
 
   // 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());
 }
 
 TEST_F(BluetoothGattBlueZTest, GattCharacteristicAddedAndRemoved) {
   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_);
 
   // Expose the fake Heart Rate service. This will asynchronously expose
   // characteristics.
   fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
       dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
   ASSERT_EQ(1, observer.gatt_service_added_count());
 
   BluetoothRemoteGattService* service =
       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());
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
   // Run the message loop so that the characteristics appear.
   base::MessageLoop::current()->Run();
 
   // 3 characteristics should appear. Only 1 of the characteristics sends
   // value changed signals. Service changed should be fired once for
@@ skipping 838 matching lines @@
 
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());
   EXPECT_TRUE(characteristic->IsNotifying());
   EXPECT_EQ(1U, update_sessions_.size());
   EXPECT_TRUE(update_sessions_[0]->IsActive());
 }
 
 }  // namespace bluez