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..b5d8ba86e36131fbf33b9e0759644a9c7f161e00 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_unittest.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_service_unittest.cc
@@ -281,6 +281,117 @@ 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, while waiting for

[scheib, 2017/04/10 20:55:54]

The comment for TwoPendingServiceDiscoveryRequests helps explain the test more,
please improve these test descriptions and include why they're likely only macOS
relevant.

[jlebel, 2017/04/11 20:44:41]

Is my comment clear?

+// characteristics.
+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());
+}
+
+// Simulates to receive an extra discvoery characteristic notification from

[scheib, 2017/04/10 20:55:54]

Keep the #if defined mac guard around each test. Also, spelling 'discovery'.

[jlebel, 2017/04/11 20:44:41]

Done.

+// macOS.
+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);
+  SimulateDidDiscoverServices(device);
+
+  // Legitimate system call to -[id<CBPeripheralDelegate>
+  // peripheral:didDiscoverCharacteristicsForService:error:]
+  SimulateDidDiscoverCharacteristics(service);
+  EXPECT_EQ(1u, service->GetCharacteristics().size());
+  EXPECT_EQ(0, observer.gatt_service_changed_count());
+
+  // Unexpected system call to -[id<CBPeripheralDelegate>
+  // peripheral:didDiscoverCharacteristicsForService:error:]
+  SimulateDidDiscoverCharacteristics(service);
+  EXPECT_EQ(1u, service->GetCharacteristics().size());
+  EXPECT_EQ(0, observer.gatt_service_changed_count());
+  BluetoothRemoteGattCharacteristic* characteristic =
+      service->GetCharacteristics()[0];
+  EXPECT_EQ(characteristic_uuid, characteristic->GetUUID().canonical_value());
+  SimulateDidDiscoverDescriptors(characteristic);
+  EXPECT_EQ(1, observer.gatt_service_changed_count());
+
+  // Unexpected system call to -[id<CBPeripheralDelegate>
+  // peripheral:didDiscoverCharacteristicsForService:error:]
+  SimulateDidDiscoverCharacteristics(service);
+  EXPECT_EQ(1, observer.gatt_service_changed_count());
+}
+#endif  // defined(OS_MACOSX)
 
 }  // namespace device