Bluetooth: macOS: Adding counter for service discovery callbacks.

device/bluetooth/bluetooth_device_unittest.cc

Index: device/bluetooth/bluetooth_device_unittest.cc
diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc
index 85c8c9a97aa44fc480e67d253307a5067299e1da..ce936e2981c6ab92e847d53d6a6a7ef348fe546a 100644
--- a/device/bluetooth/bluetooth_device_unittest.cc
+++ b/device/bluetooth/bluetooth_device_unittest.cc
@@ -399,6 +399,39 @@ TEST_F(BluetoothTest, GetUUIDs_Connection) {
 }
 #endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
+#if defined(OS_MACOSX)
+TEST_F(BluetoothTest, DoubleDidModifyServicesEvent) {

[ortuno, 2017/01/23 06:12:38]

This is a good test for the case in which we get two DidModifyServices but if I
understood you correctly this morning we need a test for when DidConnectGatt and
DidModifyServices are called one right after the other.

So:

// Starts one discovery procedure.
SimulateGattConnection()
// Starts another discovery procedure.
SimulateGattServicesChanged(...);

SimulateGattServicesDiscovered(...);
SimulateGattServicesDiscovered(...);

[jlebel, 2017/01/23 18:49:38]

Done.

+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
+
+  InitWithFakeAdapter();
+  TestBluetoothAdapterObserver observer(adapter_);
+
+  BluetoothDevice* device = SimulateLowEnergyDevice(1);
+  device->CreateGattConnection(GetGattConnectionCallback(Call::EXPECTED),
+                               GetConnectErrorCallback(Call::NOT_EXPECTED));
+  SimulateGattConnection(device);
+
+  SimulateGattServicesDiscovered(device, {} /* services */);
+  EXPECT_EQ(2, observer.device_changed_count());
+  EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
+
+  observer.Reset();
+  SimulateGattServicesChanged(device);
+  SimulateGattServicesChanged(device);
+  EXPECT_EQ(2, observer.device_changed_count());
+  EXPECT_FALSE(device->IsGattServicesDiscoveryComplete());
+
+  observer.Reset();
+  SimulateGattServicesDiscovered(device, {} /* services */);
+  SimulateGattServicesDiscovered(device, {} /* services */);

[ortuno, 2017/01/23 06:12:38]

Is there any way we can test that DiscoveryCharacteristics was not called for
the first SimulateGattServicesDiscovered?

[jlebel, 2017/01/23 18:49:38]

I guess the simplest way to do that would be to add a counter in
BluetoothTestMac, and make the MockCBPeripheral increment the counter.
MockCBPeripheral instance already know about the BluetoothTestMac instance. So
it would not be too hard. But it doesn't go exactly like the observer pattern
that already exist.
Would it work for you with this simple way?

[ortuno, 2017/01/23 20:44:02]

SGTM

+  EXPECT_EQ(1, observer.device_changed_count());
+  EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
+}
+#endif  // defined(OS_MACOSX)
+
 #if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests Advertisement Data is updated correctly when we start discovery
 // during a connection.