Bluetooth: macOS: DidModifyServices can happens while scanning

device/bluetooth/bluetooth_device_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 "device/bluetooth/bluetooth_device.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "base/run_loop.h"
@@ skipping 416 matching lines @@
   base::RunLoop().RunUntilIdle();
   ASSERT_FALSE(device->IsGattConnected());
 
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_TRUE(device->GetUUIDs().empty());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
 #if defined(OS_MACOSX)
 // Tests that receiving 2 notifications in a row from macOS that services has
-// changed is handled correctly. Each notification should generate a notfication
-// that the gatt device has changed, and each notification should ask to macOS
-// to scan for services. Only after the second service scan is received, the
-// device changed notification should be sent and the characteristic discovery
-// procedure should be started.
+// changed is handled correctly. Each notification should generate a
+// notification that the gatt device has changed, and each notification should
+// ask to macOS to scan for services. Only after the second service scan is
+// received, the device changed notification should be sent and the
+// characteristic discovery procedure should be started.
 // Android: This test doesn't apply to Android because there is no services
 // changed event that could arrive during a discovery procedure.
 TEST_F(BluetoothTest, TwoPendingServiceDiscoveryRequests) {
   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);
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_FALSE(device->IsGattServicesDiscoveryComplete());
 
   observer.Reset();
   SimulateGattServicesChanged(device);
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_FALSE(device->IsGattServicesDiscoveryComplete());
 
   // Fist system call to
   // -[id<CBPeripheralDelegate> peripheral:didDiscoverServices:]
   observer.Reset();
-  SimulateDidDiscoverServices(device, {kTestUUIDHeartRate});
+  AddServicesToDevice(device, {kTestUUIDHeartRate});
+  SimulateDidDiscoverServices(device);
   EXPECT_EQ(0, observer.device_changed_count());
   EXPECT_FALSE(device->IsGattServicesDiscoveryComplete());
   EXPECT_EQ(gatt_characteristic_discovery_attempts_, 0);
 
   // Second system call to
   // -[id<CBPeripheralDelegate> peripheral:didDiscoverServices:]
   SimulateGattServicesDiscovered(

[scheib, 2017/04/18 04:43:21]

Why does the above use SimulateDidDiscoverServices, and down here
SimulateGattServicesDiscovered? Explain in test code/comments.

[jlebel, 2017/04/29 22:08:07]

Done.

       device, std::vector<std::string>({kTestUUIDImmediateAlert}));
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
   // Characteristics are discovered once for each service.
   EXPECT_EQ(gatt_characteristic_discovery_attempts_, 2);
 
   EXPECT_EQ(2u, device->GetGattServices().size());
 }
 
 // Simulate an unexpected call to -[id<CBPeripheralDelegate>
@@ skipping 18 matching lines @@
   // Legitimate system call to
   // -[id<CBPeripheralDelegate> peripheral:didDiscoverServices:]
   observer.Reset();
   SimulateGattServicesDiscovered(
       device, std::vector<std::string>({kTestUUIDHeartRate}));
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
   EXPECT_EQ(gatt_characteristic_discovery_attempts_, 1);
   EXPECT_EQ(1u, device->GetGattServices().size());
 
   // Unexpected system call to

[scheib, 2017/04/18 04:43:21]

explain here why another didDiscoverServices is unexpected to happen, because we
only expect it to be called in response to discoverservices. Note earlier where
we expect that our code has made the call to discoverservices. Make it very
obvious how this test works and what the expected result is.

[jlebel, 2017/04/29 22:08:07]

Done.

   // -[id<CBPeripheralDelegate> peripheral:didDiscoverServices:]
-  SimulateDidDiscoverServices(device, {kTestUUIDImmediateAlert});
+  AddServicesToDevice(device, {kTestUUIDImmediateAlert});
+  SimulateDidDiscoverServices(device);
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_TRUE(device->IsGattServicesDiscoveryComplete());
 
   EXPECT_EQ(1u, device->GetGattServices().size());
 }
 #endif  // defined(OS_MACOSX)
 
 #if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests Advertisement Data is updated correctly when we start discovery
 // during a connection.
@@ skipping 1086 matching lines @@
                   .empty());
   EXPECT_TRUE(device_
                   ->GetCharacteristicsByUUID(
                       service_instance_id2,
                       BluetoothUUID(characteristic_uuid_not_exist_in_setup))
                   .empty());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
 }  // namespace device