bluetoothLowEnergy: Send onServiceAdded after all characteristics are discovered

device/bluetooth/bluetooth_gatt_chromeos_unittest.cc

 // Copyright 2014 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 "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "chromeos/dbus/fake_bluetooth_adapter_client.h"
 #include "chromeos/dbus/fake_bluetooth_agent_manager_client.h"
 #include "chromeos/dbus/fake_bluetooth_device_client.h"
@@ skipping 116 matching lines @@
   std::string device_address_;
   scoped_refptr<BluetoothAdapter> adapter_;
 };
 
 class TestGattServiceObserver : public BluetoothGattService::Observer {
  public:
   TestGattServiceObserver(scoped_refptr<BluetoothAdapter> adapter,
                           BluetoothDevice* device,
                           BluetoothGattService* service)
       : gatt_service_changed_count_(0),
+        gatt_discovery_complete_count_(0),
         gatt_characteristic_added_count_(0),
         gatt_characteristic_removed_count_(0),
         gatt_characteristic_value_changed_count_(0),
         gatt_descriptor_added_count_(0),
         gatt_descriptor_removed_count_(0),
         gatt_descriptor_value_changed_count_(0),
         device_address_(device->GetAddress()),
         gatt_service_id_(service->GetIdentifier()),
         adapter_(adapter) {
     service->AddObserver(this);
   }
 
   virtual ~TestGattServiceObserver() {
     // See if either the device or the service even exist.
     BluetoothDevice* device = adapter_->GetDevice(device_address_);
     if (!device)
       return;
 
     BluetoothGattService* service = device->GetGattService(gatt_service_id_);
     if (!service)
       return;
 
     service->RemoveObserver(this);
   }
 
   // BluetoothGattService::Observer overrides.
+  virtual void GattDiscoveryCompleteForService(
+      BluetoothGattService* service) OVERRIDE {
+    ASSERT_EQ(gatt_service_id_, service->GetIdentifier());
+    ++gatt_discovery_complete_count_;
+
+    QuitMessageLoop();
+  }
+
   virtual void GattServiceChanged(BluetoothGattService* service) OVERRIDE {
     ASSERT_EQ(gatt_service_id_, service->GetIdentifier());
     ++gatt_service_changed_count_;
 
     QuitMessageLoop();
   }
 
   virtual void GattCharacteristicAdded(
       BluetoothGattService* service,
       BluetoothGattCharacteristic* characteristic) OVERRIDE {
@@ skipping 88 matching lines @@
     last_changed_descriptor_value_ = value;
 
     EXPECT_EQ(characteristic->GetDescriptor(last_gatt_descriptor_id_),
               descriptor);
     EXPECT_EQ(characteristic, descriptor->GetCharacteristic());
 
     QuitMessageLoop();
   }
 
   int gatt_service_changed_count_;
+  int gatt_discovery_complete_count_;
   int gatt_characteristic_added_count_;
   int gatt_characteristic_removed_count_;
   int gatt_characteristic_value_changed_count_;
   int gatt_descriptor_added_count_;
   int gatt_descriptor_removed_count_;
   int gatt_descriptor_value_changed_count_;
   std::string last_gatt_characteristic_id_;
   BluetoothUUID last_gatt_characteristic_uuid_;
   std::vector<uint8> last_changed_characteristic_value_;
   std::string last_gatt_descriptor_id_;
@@ skipping 320 matching lines @@
   // characteristics.
   fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
       dbus::ObjectPath(FakeBluetoothDeviceClient::kLowEnergyPath));
   ASSERT_EQ(1, observer.gatt_service_added_count_);
 
   BluetoothGattService* service =
       device->GetGattService(observer.last_gatt_service_id_);
 
   TestGattServiceObserver service_observer(adapter_, device, service);
   EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_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
   // descriptor added.
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
   EXPECT_EQ(3U, service->GetCharacteristics().size());
 
   // Hide the characteristics. 3 removed signals should be received.
   fake_bluetooth_gatt_characteristic_client_->HideHeartRateCharacteristics();
-  EXPECT_EQ(8, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
-  // Re-expose the heart rate characteristics.
+  // Re-expose the heart rate characteristics. We shouldn't get another
+  // GattDiscoveryCompleteForService call, since the service thinks that
+  // discovery is done. On the bluetoothd side, characteristics will be removed
+  // only if the service will also be subsequently removed.
   fake_bluetooth_gatt_characteristic_client_->ExposeHeartRateCharacteristics(
       fake_bluetooth_gatt_service_client_->GetHeartRateServicePath());
-  EXPECT_EQ(12, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(6, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
   EXPECT_EQ(3U, service->GetCharacteristics().size());
 
   // Hide the service. All characteristics should disappear.
   fake_bluetooth_gatt_service_client_->HideHeartRateService();
-  EXPECT_EQ(16, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(6, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(6, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
 }
 
 TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
   fake_bluetooth_device_client_->CreateDevice(
       dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath),
       dbus::ObjectPath(FakeBluetoothDeviceClient::kLowEnergyPath));
   BluetoothDevice* device = adapter_->GetDevice(
@@ skipping 14 matching lines @@
   TestGattServiceObserver service_observer(adapter_, device, service);
   EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_added_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
 
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
   // Run the message loop so that the characteristics appear.
   base::MessageLoop::current()->Run();
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
 
   // Only the Heart Rate Measurement characteristic has a descriptor.
   EXPECT_EQ(1, service_observer.gatt_descriptor_added_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
 
   BluetoothGattCharacteristic* characteristic = service->GetCharacteristic(
       fake_bluetooth_gatt_characteristic_client_->
           GetBodySensorLocationPath().value());
   ASSERT_TRUE(characteristic);
@@ skipping 17 matching lines @@
             descriptor->GetUUID());
   EXPECT_EQ(descriptor->GetUUID(),
             service_observer.last_gatt_descriptor_uuid_);
   EXPECT_EQ(descriptor->GetIdentifier(),
             service_observer.last_gatt_descriptor_id_);
 
   // Hide the descriptor.
   fake_bluetooth_gatt_descriptor_client_->HideDescriptor(
       dbus::ObjectPath(descriptor->GetIdentifier()));
   EXPECT_TRUE(characteristic->GetDescriptors().empty());
-  EXPECT_EQ(5, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_added_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
 
   // Expose the descriptor again.
   service_observer.last_gatt_descriptor_id_.clear();
   service_observer.last_gatt_descriptor_uuid_ = BluetoothUUID();
   fake_bluetooth_gatt_descriptor_client_->ExposeDescriptor(
       dbus::ObjectPath(characteristic->GetIdentifier()),
       FakeBluetoothGattDescriptorClient::
           kClientCharacteristicConfigurationUUID);
-  EXPECT_EQ(6, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1U, characteristic->GetDescriptors().size());
   EXPECT_EQ(2, service_observer.gatt_descriptor_added_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
 
   descriptor = characteristic->GetDescriptors()[0];
   EXPECT_FALSE(descriptor->IsLocal());
   EXPECT_EQ(BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid(),
             descriptor->GetUUID());
   EXPECT_EQ(descriptor->GetUUID(), service_observer.last_gatt_descriptor_uuid_);
@@ skipping 251 matching lines @@
   // characteristics.
   fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
       dbus::ObjectPath(FakeBluetoothDeviceClient::kLowEnergyPath));
   ASSERT_EQ(1, observer.gatt_service_added_count_);
 
   BluetoothGattService* service =
       device->GetGattService(observer.last_gatt_service_id_);
 
   TestGattServiceObserver service_observer(adapter_, device, service);
   EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
   // Run the message loop so that the characteristics appear.
   base::MessageLoop::current()->Run();
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_discovery_complete_count_);
 
   // Only the Heart Rate Measurement characteristic has a descriptor.
   BluetoothGattCharacteristic* characteristic = service->GetCharacteristic(
       fake_bluetooth_gatt_characteristic_client_->
           GetHeartRateMeasurementPath().value());
   ASSERT_TRUE(characteristic);
   EXPECT_EQ(1U, characteristic->GetDescriptors().size());
 
   BluetoothGattDescriptor* descriptor = characteristic->GetDescriptors()[0];
   EXPECT_FALSE(descriptor->IsLocal());
@@ skipping 16 matching lines @@
   // successful read.
   descriptor->ReadRemoteDescriptor(
       base::Bind(&BluetoothGattChromeOSTest::ValueCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_TRUE(ValuesEqual(desc_value, descriptor->GetValue()));
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_value_changed_count_);
 
   // Write value. Writes to this descriptor will fail.
   desc_value[0] = 0x03;
   descriptor->WriteRemoteDescriptor(
       desc_value,
       base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(1, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_value_changed_count_);
 
   // Read new value.
   descriptor->ReadRemoteDescriptor(
       base::Bind(&BluetoothGattChromeOSTest::ValueCallback,
                  base::Unretained(this)),
       base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(2, success_callback_count_);
   EXPECT_EQ(1, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(2, service_observer.gatt_descriptor_value_changed_count_);
 }
 
 TEST_F(BluetoothGattChromeOSTest, NotifySessions) {
   fake_bluetooth_device_client_->CreateDevice(
       dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath),
       dbus::ObjectPath(FakeBluetoothDeviceClient::kLowEnergyPath));
   BluetoothDevice* device =
       adapter_->GetDevice(FakeBluetoothDeviceClient::kLowEnergyAddress);
   ASSERT_TRUE(device);
@@ skipping 252 matching lines @@
 
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(1, service_observer.gatt_characteristic_value_changed_count_);
   EXPECT_TRUE(characteristic->IsNotifying());
   EXPECT_EQ(1U, update_sessions_.size());
   EXPECT_TRUE(update_sessions_[0]->IsActive());
 }
 
 }  // namespace chromeos