Bluetooth: Mac: implementation for start notification

device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc

 // Copyright 2015 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 <stdint.h>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
@@ skipping 68 matching lines @@
               .canonical_value());
       expected_descriptors_count++;
     }
     if (error == StartNotifySetupError::CONFIG_DESCRIPTOR_DUPLICATE) {
       SimulateGattDescriptor(
           characteristic1_,
           BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
               .canonical_value());
       expected_descriptors_count++;
     }
+#if !defined(OS_MACOSX)

[ortuno, 2016/06/28 16:24:50]

Move this right before expected_descriptors_count = 0;

[jlebel, 2016/06/28 18:09:52]

Done.

+    // TODO(jlebel) Need to implement descriptors.

[ortuno, 2016/06/28 16:24:50]

I don't think we need the TODO as we don't need to mess with descriptors for
enabling notifications on macOS. Something like the following should suffice:

macOS: Not applicable. CoreBluetooth exposes setNotifyValue:forCharacteristic:
which handles all interactions with the CCC descriptor.

[jlebel, 2016/06/28 18:09:53]

Done.

     ASSERT_EQ(expected_descriptors_count,
               characteristic1_->GetDescriptors().size());
+#endif  // !defined(OS_MACOSX)
 
     if (error == StartNotifySetupError::SET_NOTIFY) {
       SimulateGattCharacteristicSetNotifyWillFailSynchronouslyOnce(
           characteristic1_);
     }
 
     if (error == StartNotifySetupError::WRITE_DESCRIPTOR) {
       SimulateGattDescriptorWriteWillFailSynchronouslyOnce(
           characteristic1_->GetDescriptors()[0]);
     }
@@ skipping 14 matching lines @@
     EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
     EXPECT_EQ(1, callback_count_);
     EXPECT_EQ(0, error_callback_count_);
     ASSERT_EQ(1u, notify_sessions_.size());
     ASSERT_TRUE(notify_sessions_[0]);
     EXPECT_EQ(characteristic1_->GetIdentifier(),
               notify_sessions_[0]->GetCharacteristicIdentifier());
     EXPECT_TRUE(notify_sessions_[0]->IsActive());
 
     // Verify the Client Characteristic Configuration descriptor was written to.
+#if !defined(OS_MACOSX)
+    // TODO(jlebel) Need to implement descriptors.

[ortuno, 2016/06/28 16:24:50]

No need for the TODO. Change to match the same comment as above.

[jlebel, 2016/06/28 18:09:53]

Done.

     EXPECT_EQ(1, gatt_write_descriptor_attempts_);
     EXPECT_EQ(2u, last_write_value_.size());
     uint8_t expected_byte0 = expected_config_descriptor_value & 0xFF;
     uint8_t expected_byte1 = (expected_config_descriptor_value >> 8) & 0xFF;
     EXPECT_EQ(expected_byte0, last_write_value_[0]);
     EXPECT_EQ(expected_byte1, last_write_value_[1]);
+#endif  // !defined(OS_MACOSX)
   }
 
   BluetoothDevice* device_ = nullptr;
   BluetoothRemoteGattService* service_ = nullptr;
   BluetoothRemoteGattCharacteristic* characteristic1_ = nullptr;
   BluetoothRemoteGattCharacteristic* characteristic2_ = nullptr;
 };
 #endif
 
 #if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
@@ skipping 195 matching lines @@
 
   // Duplicate write reported from OS shouldn't cause a problem:
   SimulateGattCharacteristicWrite(characteristic1_);
 
   EXPECT_EQ(empty_vector, last_write_value_);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests ReadRemoteCharacteristic completing after Chrome objects are deleted.
-// This test is not relevant for macOS. This can never happen if CBPeripheral
-// delegate is set to nil.
+// macOS: Not applicable: CoreBluetooth doesn't support synchronous API.

[ortuno, 2016/06/28 16:24:50]

I think you meant to use the comment on the following test :)

[jlebel, 2016/06/28 18:09:53]

Done.

 TEST_F(BluetoothRemoteGattCharacteristicTest,
        ReadRemoteCharacteristic_AfterDeleted) {
   if (!PlatformSupportsLowEnergy()) {
     LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
     return;
   }
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothRemoteGattCharacteristic::PROPERTY_READ));
 
   characteristic1_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::NOT_EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
 
   RememberCharacteristicForSubsequentAction(characteristic1_);
   DeleteDevice(device_);  // TODO(576906) delete only the characteristic.
 
   std::vector<uint8_t> empty_vector;
   SimulateGattCharacteristicRead(/* use remembered characteristic */ nullptr,
                                  empty_vector);
   EXPECT_TRUE("Did not crash!");
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests WriteRemoteCharacteristic completing after Chrome objects are deleted.
-// This test is not relevant for macOS. This can never happen if CBPeripheral
-// delegate is set to nil.
+// macOS: Not applicable: This can never happen if CBPeripheral delegate is set
+// to nil.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        WriteRemoteCharacteristic_AfterDeleted) {
   if (!PlatformSupportsLowEnergy()) {
     LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
     return;
   }
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothRemoteGattCharacteristic::PROPERTY_WRITE));
 
   std::vector<uint8_t> empty_vector;
@@ skipping 503 matching lines @@
             last_gatt_error_code_);
 
   // Initial read should still succeed:
   ResetEventCounts();
   SimulateGattCharacteristicRead(characteristic1_, empty_vector);
   EXPECT_EQ(1, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
-#if defined(OS_ANDROID) || defined(OS_WIN)
+#if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // StartNotifySession fails if characteristic doesn't have Notify or Indicate
 // property.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_NoNotifyOrIndicate) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1,
       StartNotifySetupError::CHARACTERISTIC_PROPERTIES));
 
   EXPECT_EQ(0, gatt_notify_characteristic_attempts_);
 
   // The expected error callback is asynchronous:
   EXPECT_EQ(0, error_callback_count_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, error_callback_count_);
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED,
             last_gatt_error_code_);
 }
-#endif  // defined(OS_ANDROID) || defined(OS_WIN)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // StartNotifySession fails if the characteristic is missing the Client
 // Characteristic Configuration descriptor.

[ortuno, 2016/06/28 16:24:50]

Add a TODO to enable this test on macOS if we get access to the CCC descriptor.
If you don't know if we get access to the CCC descriptor then open an issue and
add a TODO saying that we need to investigate more before enabling this test on
macOS.

[jlebel, 2016/06/28 18:09:53]

Done.

 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_NoConfigDescriptor) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1,
       StartNotifySetupError::CONFIG_DESCRIPTOR_MISSING));
 
   EXPECT_EQ(0, gatt_notify_characteristic_attempts_);
 
   // The expected error callback is asynchronous:
   EXPECT_EQ(0, error_callback_count_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, error_callback_count_);
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_NOT_SUPPORTED,
             last_gatt_error_code_);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // StartNotifySession fails if the characteristic has multiple Client

[ortuno, 2016/06/28 16:24:50]

Same as above. Add TODO.

[jlebel, 2016/06/28 18:09:53]

Done.

 // Characteristic Configuration descriptors.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_MultipleConfigDescriptor) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1,
       StartNotifySetupError::CONFIG_DESCRIPTOR_DUPLICATE));
 
   EXPECT_EQ(0, gatt_notify_characteristic_attempts_);
 
   // The expected error callback is asynchronous:
   EXPECT_EQ(0, error_callback_count_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, error_callback_count_);
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_FAILED,
             last_gatt_error_code_);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID)
 // StartNotifySession fails synchronously when failing to set a characteristic
 // to enable notifications.
 // Android: This is mBluetoothGatt.setCharacteristicNotification failing.
+// macOS: Not applicable: CoreBluetooth doesn't support synchronous API.
 // Windows: Synchronous Test Not Applicable: OS calls are all made
 // asynchronously from BluetoothTaskManagerWin.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_FailToSetCharacteristicNotification) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1,
       StartNotifySetupError::SET_NOTIFY));
 
   // The expected error callback is asynchronous:
   EXPECT_EQ(0, error_callback_count_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, error_callback_count_);
 
   EXPECT_EQ(0, gatt_notify_characteristic_attempts_);
   ASSERT_EQ(0u, notify_sessions_.size());
 }
 #endif  // defined(OS_ANDROID)
 
 #if defined(OS_ANDROID)
 // Tests StartNotifySession descriptor write synchronous failure.
+// macOS: Not applicable: CoreBluetooth doesn't support synchronous API.

[ortuno, 2016/06/28 16:24:50]

I think this should be: macOS. Not applicable: No need to write to the
descriptor manually. setNotifyValue:forCharacteristic: takes care of it.

[jlebel, 2016/06/28 18:09:52]

Done.

 // Windows: Synchronous Test Not Applicable: OS calls are all made
 // asynchronously from BluetoothTaskManagerWin.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_WriteDescriptorSynchronousError) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1,
       StartNotifySetupError::WRITE_DESCRIPTOR));
 
   // The expected error callback is asynchronous:
   EXPECT_EQ(0, error_callback_count_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, error_callback_count_);
 
   EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
   ASSERT_EQ(0u, notify_sessions_.size());
 }
 #endif  // defined(OS_ANDROID)
 
-#if defined(OS_ANDROID) || defined(OS_WIN)
+#if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // Tests StartNotifySession success on a characteristic enabling Notify.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StartNotifySession) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 }
-#endif  // defined(OS_ANDROID) || defined(OS_WIN)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
-#if defined(OS_ANDROID) || defined(OS_WIN)
+#if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // Tests StartNotifySession success on a characteristic enabling Indicate.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StartNotifySession_OnIndicate) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: INDICATE */ 0x20,
       /* expected_config_descriptor_value: INDICATE */ 2));
 }
-#endif  // defined(OS_ANDROID) || defined(OS_WIN)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
-#if defined(OS_ANDROID) || defined(OS_WIN)
+#if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // Tests StartNotifySession success on a characteristic enabling Notify &
 // Indicate.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_OnNotifyAndIndicate) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY and INDICATE bits set */ 0x30,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 }
-#endif  // defined(OS_ANDROID) || defined(OS_WIN)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
-#if defined(OS_ANDROID) || defined(OS_WIN)
+#if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // Tests multiple StartNotifySession success.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StartNotifySession_Multiple) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
+#if !defined(OS_MACOSX)
+  // TODO(jlebel) Need to implement descriptors.

[ortuno, 2016/06/28 16:24:50]

nit: open/point to an issue.

[jlebel, 2016/06/28 18:09:53]

Done.

   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+#endif  // !defined(OS_MACOSX)
 
   characteristic1_->StartNotifySession(
       GetNotifyCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
   characteristic1_->StartNotifySession(
       GetNotifyCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
   EXPECT_EQ(0, callback_count_);
   SimulateGattNotifySessionStarted(characteristic1_);
   EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
   EXPECT_EQ(2, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   ASSERT_EQ(2u, notify_sessions_.size());
   ASSERT_TRUE(notify_sessions_[0]);
   ASSERT_TRUE(notify_sessions_[1]);
   EXPECT_EQ(characteristic1_->GetIdentifier(),
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_->GetIdentifier(),
             notify_sessions_[1]->GetCharacteristicIdentifier());
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
   EXPECT_TRUE(notify_sessions_[1]->IsActive());
 }
-#endif  // defined(OS_ANDROID) || defined(OS_WIN)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests multiple StartNotifySessions pending and then an error.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySessionError_Multiple) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
+#if !defined(OS_MACOSX)
+  // TODO(jlebel) Need to implement descriptors.

[ortuno, 2016/06/28 16:24:50]

nit: same here, point to an issue.

[jlebel, 2016/06/28 18:09:53]

Done.

   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+#endif  // !defined(OS_MACOSX)
 
   characteristic1_->StartNotifySession(GetNotifyCallback(Call::NOT_EXPECTED),
                                        GetGattErrorCallback(Call::EXPECTED));
   characteristic1_->StartNotifySession(GetNotifyCallback(Call::NOT_EXPECTED),
                                        GetGattErrorCallback(Call::EXPECTED));
   EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
   EXPECT_EQ(0, callback_count_);
   SimulateGattNotifySessionStartError(
       characteristic1_, BluetoothRemoteGattService::GATT_ERROR_FAILED);
   EXPECT_EQ(0, callback_count_);
   EXPECT_EQ(2, error_callback_count_);
   ASSERT_EQ(0u, notify_sessions_.size());
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_FAILED,
             last_gatt_error_code_);
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
 #if defined(OS_ANDROID)
 // Tests StartNotifySession completing after chrome objects are deleted.
+// macOS: Not applicable: This can never happen if CBPeripheral delegate is set
+// to nil.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StartNotifySession_AfterDeleted) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
+#if !defined(OS_MACOSX)

[ortuno, 2016/06/28 16:24:50]

You don't need this one since this test doesn't apply to macOS.

[jlebel, 2016/06/28 18:09:52]

Done.

+  // TODO(jlebel) Need to implement descriptors.
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+#endif  // !defined(OS_MACOSX)
 
   characteristic1_->StartNotifySession(GetNotifyCallback(Call::NOT_EXPECTED),
                                        GetGattErrorCallback(Call::EXPECTED));
   EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
   EXPECT_EQ(0, callback_count_);
 
   RememberCharacteristicForSubsequentAction(characteristic1_);
   RememberCCCDescriptorForSubsequentAction(characteristic1_);
   DeleteDevice(device_);  // TODO(576906) delete only the characteristic.
 
   SimulateGattNotifySessionStarted(/* use remembered characteristic */ nullptr);
   EXPECT_EQ(0, callback_count_);
   EXPECT_EQ(1, error_callback_count_);
   ASSERT_EQ(0u, notify_sessions_.size());
   EXPECT_EQ(BluetoothRemoteGattService::GATT_ERROR_FAILED,
             last_gatt_error_code_);
 }
 #endif  // defined(OS_ANDROID)
 
-#if defined(OS_WIN)
+#if defined(OS_MACOSX) || defined(OS_WIN)
 // Tests StartNotifySession reentrant in start notify session success callback
 // and the reentrant start notify session success.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_Reentrant_Success_Success) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid().value());
+#if !defined(OS_MACOSX)
+  // TODO(jlebel) Need to implement descriptors.
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+#endif  // !defined(OS_MACOSX)
 
   characteristic1_->StartNotifySession(
       GetReentrantStartNotifySessionSuccessCallback(Call::EXPECTED,
                                                     characteristic1_),
       GetReentrantStartNotifySessionErrorCallback(
           Call::NOT_EXPECTED, characteristic1_,
           false /* error_in_reentrant */));
   EXPECT_EQ(0, callback_count_);
   SimulateGattNotifySessionStarted(characteristic1_);
   EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
 
   // Simulate reentrant StartNotifySession request from
   // BluetoothTestBase::ReentrantStartNotifySessionSuccessCallback.
-  SimulateGattNotifySessionStarted(characteristic1_);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
   EXPECT_EQ(2, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   ASSERT_EQ(2u, notify_sessions_.size());
   for (unsigned int i = 0; i < notify_sessions_.size(); i++) {
     ASSERT_TRUE(notify_sessions_[i]);
     EXPECT_EQ(characteristic1_->GetIdentifier(),
               notify_sessions_[i]->GetCharacteristicIdentifier());
     EXPECT_TRUE(notify_sessions_[i]->IsActive());
   }
 }
-#endif  // defined(OS_WIN)
+#endif  // defined(OS_MACOSX) || defined(OS_WIN)
 
 #if defined(OS_WIN)
 // Tests StartNotifySession reentrant in start notify session error callback
 // and the reentrant start notify session success.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_Reentrant_Error_Success) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid().value());
+#if !defined(OS_MACOSX)
+  // TODO(jlebel) Need to implement descriptors.

[ortuno, 2016/06/28 16:24:50]

Remove since this is not enabled on macOS yet.

[jlebel, 2016/06/28 18:09:52]

Done.

   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+#endif  // !defined(OS_MACOSX)
 
   SimulateGattNotifySessionStartError(
       characteristic1_, BluetoothRemoteGattService::GATT_ERROR_UNKNOWN);
 
   characteristic1_->StartNotifySession(
       GetReentrantStartNotifySessionSuccessCallback(Call::NOT_EXPECTED,
                                                     characteristic1_),
       GetReentrantStartNotifySessionErrorCallback(
           Call::EXPECTED, characteristic1_, false /* error_in_reentrant */));
   EXPECT_EQ(0, callback_count_);
@@ skipping 18 matching lines @@
 #if defined(OS_WIN)
 // Tests StartNotifySession reentrant in start notify session error callback
 // and the reentrant start notify session error.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_Reentrant_Error_Error) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid().value());
+#if !defined(OS_MACOSX)

[ortuno, 2016/06/28 16:24:50]

Remove since this is not enabled on macOS yet.

[jlebel, 2016/06/28 18:09:53]

Done.

+  // TODO(jlebel) Need to implement descriptors.
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+#endif  // !defined(OS_MACOSX)
 
   SimulateGattNotifySessionStartError(
       characteristic1_, BluetoothRemoteGattService::GATT_ERROR_UNKNOWN);
 
   characteristic1_->StartNotifySession(
       GetReentrantStartNotifySessionSuccessCallback(Call::NOT_EXPECTED,
                                                     characteristic1_),
       GetReentrantStartNotifySessionErrorCallback(
           Call::EXPECTED, characteristic1_, true /* error_in_reentrant */));
   EXPECT_EQ(0, callback_count_);
   SimulateGattNotifySessionStarted(characteristic1_);
   EXPECT_EQ(0, gatt_notify_characteristic_attempts_);
 
   // Simulate reentrant StartNotifySession request from
   // BluetoothTestBase::ReentrantStartNotifySessionErrorCallback.
   SimulateGattNotifySessionStarted(characteristic1_);
   EXPECT_EQ(0, gatt_notify_characteristic_attempts_);
   EXPECT_EQ(0, callback_count_);
   EXPECT_EQ(2, error_callback_count_);
   ASSERT_EQ(0u, notify_sessions_.size());
 }
 #endif  // defined(OS_WIN)
 
-#if defined(OS_ANDROID) || defined(OS_WIN)
+#if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // Tests Characteristic Value changes during a Notify Session.
 TEST_F(BluetoothRemoteGattCharacteristicTest, GattCharacteristicValueChanged) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   TestBluetoothAdapterObserver observer(adapter_);
 
   std::vector<uint8_t> test_vector1, test_vector2;
   test_vector1.push_back(111);
   test_vector2.push_back(222);
 
   SimulateGattCharacteristicChanged(characteristic1_, test_vector1);
   EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count());
   EXPECT_EQ(test_vector1, characteristic1_->GetValue());
 
   SimulateGattCharacteristicChanged(characteristic1_, test_vector2);
   EXPECT_EQ(2, observer.gatt_characteristic_value_changed_count());
   EXPECT_EQ(test_vector2, characteristic1_->GetValue());
 }
-#endif  // defined(OS_ANDROID) || defined(OS_WIN)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests Characteristic Value changing after a Notify Session and objects being
 // destroyed.
+// macOS: Not applicable: This can never happen if CBPeripheral delegate is set
+// to nil.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        GattCharacteristicValueChanged_AfterDeleted) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
   TestBluetoothAdapterObserver observer(adapter_);
 
   RememberCharacteristicForSubsequentAction(characteristic1_);
   DeleteDevice(device_);  // TODO(576906) delete only the characteristic.
 
@@ skipping 83 matching lines @@
   EXPECT_EQ(1u, characteristic1_->GetDescriptorsByUUID(id2).size());
   EXPECT_EQ(2u, characteristic2_->GetDescriptorsByUUID(id3).size());
 
   EXPECT_EQ(0u, characteristic2_->GetDescriptorsByUUID(id1).size());
   EXPECT_EQ(0u, characteristic2_->GetDescriptorsByUUID(id2).size());
   EXPECT_EQ(0u, characteristic1_->GetDescriptorsByUUID(id3).size());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 }  // namespace device