Bluetooth: macOS: DidModifyServices can happens while scanning

device/bluetooth/bluetooth_remote_gatt_service_unittest.cc

Index: device/bluetooth/bluetooth_remote_gatt_service_unittest.cc
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_unittest.cc b/device/bluetooth/bluetooth_remote_gatt_service_unittest.cc
index 4daf9fe0cc08f14a0c091580402fa289c8eafcd7..d4357576cbba4bf453b2dc7513515e9c38b4439d 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_unittest.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_service_unittest.cc
@@ -281,6 +281,173 @@ TEST_F(BluetoothRemoteGattServiceTest, SimulateGattServiceRemove) {
   EXPECT_FALSE(device->GetGattService(removed_service));
   EXPECT_EQ(device->GetGattServices()[0], service2);
 }
-#endif  // defined(OS_WIN) || defined(OS_MACOSX)
+#endif  // defined(OS_MACOSX) || defined(OS_WIN)
+
+#if defined(OS_MACOSX)
+// Tests to receive a services changed notification from macOS, while
+// discovering characteristics. The gatt device should scan again for services
+// and characteristics, before scanning for descriptors. Only after the gatt
+// service changed notification should be sent.
+// Android: This test doesn't apply to Android because there is no services
+// changed event that could arrive during a discovery procedure.
+TEST_F(BluetoothRemoteGattServiceTest,
+       SimulateDeviceModificationWhileDiscoveringCharacteristics) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+  InitWithFakeAdapter();
+  StartLowEnergyDiscoverySession();
+  BluetoothDevice* device = SimulateLowEnergyDevice(3);
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+
+  TestBluetoothAdapterObserver observer(adapter_);
+
+  // Starts first discovery process.
+  SimulateGattConnection(device);
+  AddServicesToDevice(device, {kTestUUIDHeartRate});
+  SimulateDidDiscoverServices(device);
+  EXPECT_EQ(1u, device->GetGattServices().size());
+  BluetoothRemoteGattService* service = device->GetGattServices()[0];
+  std::string characteristic_uuid1 = "11111111-0000-1000-8000-00805f9b34fb";
+  AddCharacteristicToService(service, characteristic_uuid1, /* properties */ 0);
+  // Now waiting for characteristic discovery.
+
+  // Starts second discovery process.
+  SimulateGattServicesChanged(device);
+  SimulateDidDiscoverServices(device);
+  // Now waiting for the second characteristic discovery.
+
+  // First system call to -[id<CBPeripheralDelegate>
+  // peripheral:didDiscoverCharacteristicsForService:error:]
+  SimulateDidDiscoverCharacteristics(service);
+  EXPECT_EQ(0, observer.gatt_service_changed_count());
+  EXPECT_EQ(1u, service->GetCharacteristics().size());
+
+  // Finish discovery process.
+  std::string characteristic_uuid2 = "22222222-0000-1000-8000-00805f9b34fb";
+  AddCharacteristicToService(service, characteristic_uuid2, /* properties */ 0);
+  // Second system call to -[id<CBPeripheralDelegate>
+  // peripheral:didDiscoverCharacteristicsForService:error:]
+  SimulateDidDiscoverCharacteristics(service);
+  EXPECT_EQ(2u, service->GetCharacteristics().size());
+  EXPECT_EQ(0, observer.gatt_service_changed_count());
+  BluetoothRemoteGattCharacteristic* characteristic1 =
+      service->GetCharacteristics()[0];
+  BluetoothRemoteGattCharacteristic* characteristic2 =
+      service->GetCharacteristics()[1];
+  if (characteristic1->GetUUID().canonical_value() == characteristic_uuid2) {
+    BluetoothRemoteGattCharacteristic* tmp = characteristic1;
+    characteristic1 = characteristic2;
+    characteristic2 = tmp;
+  }
+  EXPECT_EQ(characteristic_uuid1, characteristic1->GetUUID().canonical_value());
+  EXPECT_EQ(characteristic_uuid2, characteristic2->GetUUID().canonical_value());
+  SimulateDidDiscoverDescriptors(characteristic1);
+  SimulateDidDiscoverDescriptors(characteristic2);
+  EXPECT_EQ(1, observer.gatt_service_changed_count());
+}
+#endif  // defined(OS_MACOSX)
+
+#if defined(OS_MACOSX)
+// Simulates to receive an extra discovery characteristic notifications from
+// macOS. Those notifications should be ignored.
+// Android: This test doesn't apply to Android because there is no services
+// changed event that could arrive during a discovery procedure.
+TEST_F(BluetoothRemoteGattServiceTest, ExtraDidDiscoverServicesCall) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+  InitWithFakeAdapter();
+  StartLowEnergyDiscoverySession();
+  BluetoothDevice* device = SimulateLowEnergyDevice(3);
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+
+  TestBluetoothAdapterObserver observer(adapter_);
+
+  // Starts first discovery process.
+  SimulateGattConnection(device);
+  AddServicesToDevice(device, {kTestUUIDHeartRate});
+  SimulateDidDiscoverServices(device);
+  EXPECT_EQ(1u, device->GetGattServices().size());
+  BluetoothRemoteGattService* service = device->GetGattServices()[0];
+  std::string characteristic_uuid = "11111111-0000-1000-8000-00805f9b34fb";
+  AddCharacteristicToService(service, characteristic_uuid, /* properties */ 0);
+  SimulateDidDiscoverCharacteristics(service);
+  EXPECT_EQ(1u, service->GetCharacteristics().size());
+  BluetoothRemoteGattCharacteristic* characteristic =
+      service->GetCharacteristics()[0];
+  std::string descriptor_uuid = "22222222-0000-1000-8000-00805f9b34fb";
+  AddDescriptorToCharacteristic(characteristic, descriptor_uuid);
+  SimulateDidDiscoverDescriptors(characteristic);
+  EXPECT_EQ(1, observer.gatt_service_changed_count());
+  EXPECT_EQ(1u, service->GetCharacteristics().size());
+  EXPECT_EQ(1u, characteristic->GetDescriptors().size());
+
+  observer.Reset();
+  SimulateDidDiscoverServices(device);  // Extra system call.
+  SimulateGattServicesChanged(device);
+  SimulateDidDiscoverServices(device);
+  SimulateDidDiscoverServices(device);  // Extra system call.
+  SimulateDidDiscoverCharacteristics(service);
+  SimulateDidDiscoverServices(device);  // Extra system call.
+  SimulateDidDiscoverDescriptors(characteristic);
+  SimulateDidDiscoverServices(device);  // Extra system call.
+  EXPECT_EQ(2, observer.device_changed_count());
+}
+#endif  // defined(OS_MACOSX)
+
+#if defined(OS_MACOSX)
+// Simulates to receive an extra discovery characteristic notifications from
+// macOS. Those notifications should be ignored.
+// Android: This test doesn't apply to Android because there is no services
+// changed event that could arrive during a discovery procedure.
+TEST_F(BluetoothRemoteGattServiceTest, ExtraDidDiscoverCharacteristicsCall) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+  InitWithFakeAdapter();
+  StartLowEnergyDiscoverySession();
+  BluetoothDevice* device = SimulateLowEnergyDevice(3);
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+
+  TestBluetoothAdapterObserver observer(adapter_);
+
+  // Starts first discovery process.
+  SimulateGattConnection(device);
+  AddServicesToDevice(device, {kTestUUIDHeartRate});
+  SimulateDidDiscoverServices(device);
+  EXPECT_EQ(1u, device->GetGattServices().size());
+  BluetoothRemoteGattService* service = device->GetGattServices()[0];
+  std::string characteristic_uuid = "11111111-0000-1000-8000-00805f9b34fb";
+  AddCharacteristicToService(service, characteristic_uuid, /* properties */ 0);
+  SimulateDidDiscoverCharacteristics(service);
+  EXPECT_EQ(1u, service->GetCharacteristics().size());
+  BluetoothRemoteGattCharacteristic* characteristic =
+      service->GetCharacteristics()[0];
+  std::string descriptor_uuid = "22222222-0000-1000-8000-00805f9b34fb";
+  AddDescriptorToCharacteristic(characteristic, descriptor_uuid);
+  SimulateDidDiscoverDescriptors(characteristic);
+  EXPECT_EQ(1, observer.gatt_service_changed_count());
+  EXPECT_EQ(1u, service->GetCharacteristics().size());
+  EXPECT_EQ(1u, characteristic->GetDescriptors().size());
+
+  observer.Reset();
+  SimulateDidDiscoverCharacteristics(service);  // Extra system call.
+  SimulateGattServicesChanged(device);
+  SimulateDidDiscoverCharacteristics(service);  // Extra system call.
+  SimulateDidDiscoverServices(device);
+  SimulateDidDiscoverCharacteristics(service);
+  SimulateDidDiscoverCharacteristics(service);  // Extra system call.
+  SimulateDidDiscoverDescriptors(characteristic);
+  SimulateDidDiscoverCharacteristics(service);  // Extra system call.
+  EXPECT_EQ(2, observer.device_changed_count());
+}
+#endif  // defined(OS_MACOSX)
 
 }  // namespace device