[bluetooth] fix BlueZ GATT service enumeration

This fixes BlueZ GATT service enumeration when there is more than one
device. In the BluetoothDeviceBlueZ::UpdateGattServices() method, it
is possible that the services for the requested device are not the
first services in |service_paths|. If this is the case returning from
inside the loop makes it appear as if the device has no services.
By continuing the loop instead, all services associated with this device
are made available (by calling NotifyGattDiscoveryComplete() on them).

Bug: 1087648
Change-Id: I52dad45674330a52f7420b28a51fdd83439b4f9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2214098
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772992}

device/bluetooth/bluez/bluetooth_device_bluez.cc

@@ -853,11 +853,13 @@ void BluetoothDeviceBlueZ::UpdateGattServices(
     // Add all previously unknown GATT services associated with the device.
     GattServiceAdded(service_path);
 
-    // If the service does not belong in this device, there is nothing left to
-    // do.
+    // |service_paths| includes all services for all devices, not just this
+    // device. GetGattService() will return nullptr for services belonging
+    // to other devices, so we skip those and keep looking for services that
+    // belong to this device.
     BluetoothRemoteGattService* service = GetGattService(service_path.value());
     if (service == nullptr) {
-      return;
+      continue;
     }
 
     // Notify of GATT discovery complete if we haven't before.

device/bluetooth/bluez/bluetooth_gatt_bluez_unittest.cc

@@ -2403,4 +2403,50 @@ TEST_F(BluetoothGattBlueZTest, NotificationType) {
 }
 #endif
 
+TEST_F(BluetoothGattBlueZTest, MultipleDevices) {
+  fake_bluetooth_device_client_->CreateDevice(
+      dbus::ObjectPath(bluez::FakeBluetoothAdapterClient::kAdapterPath),
+      dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+  bluez::FakeBluetoothDeviceClient::Properties* properties1 =
+      fake_bluetooth_device_client_->GetProperties(
+          dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+  properties1->services_resolved.ReplaceValue(false);
+
+  fake_bluetooth_gatt_service_client_->ExposeHeartRateService(
+      dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kLowEnergyPath));
+  while (!fake_bluetooth_gatt_characteristic_client_->IsHeartRateVisible())
+    base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(fake_bluetooth_gatt_service_client_->IsHeartRateVisible());
+  ASSERT_TRUE(fake_bluetooth_gatt_characteristic_client_->IsHeartRateVisible());
+
+  fake_bluetooth_device_client_->CreateDevice(
+      dbus::ObjectPath(bluez::FakeBluetoothAdapterClient::kAdapterPath),
+      dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kDualPath));
+  bluez::FakeBluetoothDeviceClient::Properties* properties2 =
+      fake_bluetooth_device_client_->GetProperties(
+          dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kDualPath));
+  properties2->services_resolved.ReplaceValue(false);
+
+  fake_bluetooth_gatt_service_client_->ExposeBatteryService(
+      dbus::ObjectPath(bluez::FakeBluetoothDeviceClient::kDualPath));
+  ASSERT_TRUE(fake_bluetooth_gatt_service_client_->IsBatteryServiceVisible());
+
+  BluetoothDeviceBlueZ* device1 = static_cast<BluetoothDeviceBlueZ*>(
+      adapter_->GetDevice(bluez::FakeBluetoothDeviceClient::kLowEnergyAddress));
+  ASSERT_TRUE(device1);
+  BluetoothDeviceBlueZ* device2 = static_cast<BluetoothDeviceBlueZ*>(
+      adapter_->GetDevice(bluez::FakeBluetoothDeviceClient::kDualAddress));
+  ASSERT_TRUE(device2);
+
+  TestBluetoothAdapterObserver observer(adapter_);
+
+  EXPECT_EQ(0, observer.gatt_discovery_complete_count());
+
+  properties1->services_resolved.ReplaceValue(true);
+  properties2->services_resolved.ReplaceValue(true);
+
+  // Since BlueZ iterates all services for all devices for each device, this
+  // can catch errors like https://crbug.com/1087648
+  EXPECT_EQ(2, observer.gatt_discovery_complete_count());
+}
 }  // namespace bluez