Bluetooth: macOS: Implement BluetoothRemoteGattCharacteristicMac::UnsubscribeFromNotifications

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 992 matching lines @@
 
   // 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_MACOSX) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)

[ortuno, 2017/02/09 00:34:48]

Enable this test as well.

[jlebel, 2017/02/09 01:20:18]

Done.

 // StartNotifySession fails if the characteristic is missing the Client
 // Characteristic Configuration descriptor.
 // macOS: TODO(crbug.com/624017) Need to implement CCC descriptors.
 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)

[ortuno, 2017/02/09 00:34:48]

Enable this test as well.

[jlebel, 2017/02/09 01:20:18]

Done.

 // StartNotifySession fails if the characteristic has multiple Client
 // Characteristic Configuration descriptors.
 // macOS: TODO(crbug.com/624017) Need to implement CCC descriptors.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_MultipleConfigDescriptor) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1,
       StartNotifySetupError::CONFIG_DESCRIPTOR_DUPLICATE));
 
@@ skipping 102 matching lines @@
   if (!PlatformSupportsLowEnergy()) {
     LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
     return;
   }
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
-#if !defined(OS_MACOSX)
-  // TODO(crbug.com/624017): Need implementation for descriptors.
   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_);
   base::RunLoop().RunUntilIdle();
@@ skipping 19 matching lines @@
   if (!PlatformSupportsLowEnergy()) {
     LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
     return;
   }
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
 #if !defined(OS_MACOSX)

[ortuno, 2017/02/09 00:34:48]

You can remove this #if now.

[jlebel, 2017/02/09 01:20:18]

Done.

   // TODO(crbug.com/624017): Need implementation for descriptors.
   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_);
@@ skipping 33 matching lines @@
   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_ANDROID)
 // Tests StartNotifySession completing before chrome objects are deleted.

[ortuno, 2017/02/09 00:34:48]

Also enable this test.

[jlebel, 2017/02/09 01:20:17]

Done.

 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_BeforeDeleted) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
 
@@ skipping 135 matching lines @@
   // 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)

[ortuno, 2017/02/09 00:34:48]

Also enable this one

[jlebel, 2017/02/09 01:20:18]

I can't. Write is not implemented yet.

[ortuno, 2017/02/09 03:07:03]

This is testing that:

1. We wrote the correct value to the descriptor.
2. The session is marked as inactive.
3. That the characteristic is no longer notifying.

(1) doesn't apply to macOS because CoreBluetooth handles writing to the
descriptor, so we don't really need write here.

Can you surround the descriptor check in an #if !defined(OS_MACOS) and explain
why that doesn't apply to macOS:

    // macOS: Not applicable. CoreBluetooth exposes -[CBPeripheral
    // setNotifyValue:forCharacteristic:] which handles all interactions with
    // the CCC descriptor.

[jlebel, 2017/02/09 21:17:38]

Done.

 // Tests StopNotifySession success on a characteristic that enabled Notify.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
   EXPECT_EQ(1, gatt_write_descriptor_attempts_);
 
   notify_sessions_[0]->Stop(GetStopNotifyCallback(Call::EXPECTED));
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
 
   // Check that the right values were written to the descriptor.
   EXPECT_EQ(2, gatt_write_descriptor_attempts_);
   ASSERT_EQ(2u, last_write_value_.size());
   EXPECT_EQ(0, last_write_value_[0]);
   EXPECT_EQ(0, last_write_value_[1]);
 
   // Check that the notify session is inactive.
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
 #endif  // defined(OS_ANDROID)
 
 #if defined(OS_ANDROID)

[ortuno, 2017/02/09 00:34:48]

Also enable this test.

[jlebel, 2017/02/09 01:20:18]

Same here.

[ortuno, 2017/02/09 03:07:03]

Similarly here this tests that:

1. The correct value was written to the descriptor.
2. That the notification is no longer active.

(1) doesn't apply to macOS but (2) does. So do the same as above and surround
the non-mac part in an #if and mention why we do this:

    // macOS: Not applicable. CoreBluetooth exposes -[CBPeripheral
    // setNotifyValue:forCharacteristic:] which handles all interactions with
    // the CCC descriptor.

[jlebel, 2017/02/09 21:17:38]

Done.

 // Tests that deleted sessions are stopped.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StopNotifySession_SessionDeleted) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
   EXPECT_EQ(1, gatt_write_descriptor_attempts_);
 
   notify_sessions_.clear();
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
 
   // Check that the right values were written to the descriptor.
   EXPECT_EQ(2, gatt_write_descriptor_attempts_);
   ASSERT_EQ(2u, last_write_value_.size());
   EXPECT_EQ(0, last_write_value_[0]);
   EXPECT_EQ(0, last_write_value_[1]);
 
   // Check that the notify session is inactive.
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
 #endif  // defined(OS_ANDROID)
 
 #if defined(OS_ANDROID)

[ortuno, 2017/02/09 00:34:48]

You forgot to enable this test.

[jlebel, 2017/02/09 01:20:18]

Done.

 // Tests that deleting the sessions before the stop callbacks have been
 // invoked does not cause problems.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StopNotifySession_SessionDeleted2) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
 
 #if !defined(OS_MACOSX)
   // TODO(624017) enable for macosx
   SimulateGattDescriptor(
@@ skipping 36 matching lines @@
   base::RunLoop().RunUntilIdle();
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
 
   // Check that the state is correct.
   EXPECT_TRUE("Did not crash!");
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
 #endif  // defined(OS_ANDROID)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests that cancelling StopNotifySession works.
-// TODO(crbug.com/633191): Enable on macOS when SubscribeToNotifications is
-// implemented.
 // TODO(crbug.com/636270): Enable on Windows when SubscribeToNotifications is
 // implemented.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_Cancelled) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   // Check that the session is correctly setup.
   std::string characteristic_identifier = characteristic1_->GetIdentifier();
   EXPECT_EQ(characteristic_identifier,
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
 
   // Queue a Stop request.
   notify_sessions_[0]->Stop(GetStopNotifyCallback(Call::EXPECTED));
 
   // Cancel Stop by deleting the device before Stop finishes.
   DeleteDevice(device_);  // TODO(576906) delete only the characteristic.
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests that deleted sessions are stopped.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_AfterDeleted) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   // Check that the session is correctly setup
   std::string characteristic_identifier = characteristic1_->GetIdentifier();
   EXPECT_EQ(characteristic_identifier,
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
 
   DeleteDevice(device_);  // TODO(576906) delete only the characteristic.
 
   ResetEventCounts();
   notify_sessions_[0]->Stop(GetStopNotifyCallback(Call::EXPECTED));
 
   // Check that the callback does not arrive synchronously.
   EXPECT_EQ(0, callback_count_);
 
   // Trigger the stop callback
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, callback_count_);
 
   EXPECT_TRUE("Did not crash!");
   ASSERT_TRUE(notify_sessions_[0]);
   EXPECT_EQ(characteristic_identifier,
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
 #if defined(OS_ANDROID)

[ortuno, 2017/02/09 00:34:48]

Can you mention why this is not enabled on macOS? e.g.

    // macOS: Not applicable. CoreBluetooth exposes -[CBPeripheral
    // setNotifyValue:forCharacteristic:] which handles all interactions with
    // the CCC descriptor.

[jlebel, 2017/02/09 01:20:18]

Done.

 // Tests StopNotifySession success on a characteristic that enabled Indicate.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_OnIndicate) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: INDICATE */ 0x20,
       /* expected_config_descriptor_value: INDICATE */ 2));
   EXPECT_EQ(1, gatt_write_descriptor_attempts_);
 
   notify_sessions_[0]->Stop(GetStopNotifyCallback(Call::EXPECTED));
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
 
   // Check that the right values were written to the descriptor.
   EXPECT_EQ(2, gatt_write_descriptor_attempts_);
   ASSERT_EQ(2u, last_write_value_.size());
   EXPECT_EQ(0, last_write_value_[0]);
   EXPECT_EQ(0, last_write_value_[1]);
 
   // Check that the notify session is inactive.
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
 #endif  // defined(OS_ANDROID)
 
 #if defined(OS_ANDROID)
 // Tests StopNotifySession success on a characteristic that enabled Notify &

[ortuno, 2017/02/09 00:34:48]

Can you mention why this is not enabled on macOS? e.g.

    // macOS: Not applicable. CoreBluetooth exposes -[CBPeripheral
    // setNotifyValue:forCharacteristic:] which handles all interactions with
    // the CCC descriptor.

[jlebel, 2017/02/09 01:20:18]

Done.

 // Indicate.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StopNotifySession_OnNotifyAndIndicate) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY and INDICATE bits set */ 0x30,
       /* expected_config_descriptor_value: INDICATE */ 1));
   EXPECT_EQ(1, gatt_write_descriptor_attempts_);
 
   notify_sessions_[0]->Stop(GetStopNotifyCallback(Call::EXPECTED));
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
 
   // Check that the right values were written to the descriptor.
   EXPECT_EQ(2, gatt_write_descriptor_attempts_);
   ASSERT_EQ(2u, last_write_value_.size());
   EXPECT_EQ(0, last_write_value_[0]);
   EXPECT_EQ(0, last_write_value_[1]);
 
   // Check that the notify session is inactive.
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
 #endif  // defined(OS_ANDROID)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests StopNotifySession error
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_Error) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   // Check that the notify session is active.
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
   EXPECT_EQ(characteristic1_->GetIdentifier(),
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_TRUE(characteristic1_->IsNotifying());
 
   notify_sessions_[0]->Stop(GetStopNotifyCallback(Call::EXPECTED));
   SimulateGattNotifySessionStopError(
       characteristic1_, BluetoothRemoteGattService::GATT_ERROR_UNKNOWN);
   base::RunLoop().RunUntilIdle();
 
   // Check that the notify session is inactive.
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests multiple StopNotifySession calls for a single session.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_Multiple1) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
 
-#if !defined(OS_MACOSX)
-  // TODO(624017) enable for macosx
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
-#endif
 
   // Start notify session
   characteristic1_->StartNotifySession(
       GetNotifyCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
   EXPECT_EQ(0, callback_count_);
   SimulateGattNotifySessionStarted(characteristic1_);
   base::RunLoop().RunUntilIdle();
   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_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
   EXPECT_TRUE(characteristic1_->IsNotifying());
 
   // Stop the notify session twice
   ResetEventCounts();
   notify_sessions_[0]->Stop(GetStopNotifyCheckForPrecedingCalls(0));
   notify_sessions_[0]->Stop(GetStopNotifyCheckForPrecedingCalls(1));
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
 
   // Check that the notify session is inactive.
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests multiple StartNotifySession calls and multiple StopNotifySession calls.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_Multiple2) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
 
-#if !defined(OS_MACOSX)
   // TODO(624017) enable for macosx

[ortuno, 2017/02/09 00:34:48]

nit: remove the TODO.

[jlebel, 2017/02/09 01:20:18]

Done.

   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
-#endif
 
   // Start notify sessions
   characteristic1_->StartNotifySession(
       GetNotifyCheckForPrecedingCalls(0),
       GetGattErrorCallback(Call::NOT_EXPECTED));
   characteristic1_->StartNotifySession(
       GetNotifyCheckForPrecedingCalls(1),
       GetGattErrorCallback(Call::NOT_EXPECTED));
   EXPECT_EQ(0, callback_count_);
   SimulateGattNotifySessionStarted(characteristic1_);
@@ skipping 19 matching lines @@
 
   notify_sessions_[0]->Stop(GetStopNotifyCheckForPrecedingCalls(3));
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
 
   // Check that the notify sessions is inactive.
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(notify_sessions_[1]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 // Tests starting a new notify session before the previous stop request
 // resolves.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_StopStart) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
 
   // Start notify session
   ResetEventCounts();
@@ skipping 20 matching lines @@
 
   SimulateGattNotifySessionStarted(characteristic1_);
   base::RunLoop().RunUntilIdle();
   ASSERT_EQ(2u, notify_sessions_.size());
   ASSERT_TRUE(notify_sessions_[0]);
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   ASSERT_TRUE(notify_sessions_[1]);
   EXPECT_TRUE(notify_sessions_[1]->IsActive());
   EXPECT_TRUE(characteristic1_->IsNotifying());
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StopNotifySession_StartStopStart) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   // Check that the initial notify session is active.
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
   EXPECT_EQ(characteristic1_->GetIdentifier(),
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_TRUE(characteristic1_->IsNotifying());
@@ skipping 27 matching lines @@
   EXPECT_EQ(characteristic1_, notify_sessions_[1]->GetCharacteristic());
   EXPECT_TRUE(notify_sessions_[1]->IsActive());
 
   EXPECT_EQ(characteristic1_->GetIdentifier(),
             notify_sessions_[2]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_, notify_sessions_[2]->GetCharacteristic());
   EXPECT_TRUE(notify_sessions_[2]->IsActive());
 
   EXPECT_TRUE(characteristic1_->IsNotifying());
 }
-#endif
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
 #if defined(OS_ANDROID)

[ortuno, 2017/02/09 00:34:48]

You are missing this one.

[jlebel, 2017/02/09 01:20:18]

Write is still missing

[ortuno, 2017/02/09 03:07:03]

Same here. Surround the part that checks for the descriptor being written in an
#if and explain why:

    // macOS: Not applicable. CoreBluetooth exposes -[CBPeripheral
    // setNotifyValue:forCharacteristic:] which handles all interactions with
    // the CCC descriptor.

[jlebel, 2017/02/09 21:17:38]

Done.

 // Tests starting a new notify session before the previous stop requests
 // resolve.
 TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_StopStopStart) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
@@ skipping 44 matching lines @@
   // Check the notify state
   ASSERT_EQ(2u, notify_sessions_.size());
   ASSERT_TRUE(notify_sessions_[0]);
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   ASSERT_TRUE(notify_sessions_[1]);
   EXPECT_TRUE(notify_sessions_[1]->IsActive());
   EXPECT_TRUE(characteristic1_->IsNotifying());
 }
 #endif  // defined(OS_ANDROID)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StopNotifySession_Reentrant_Success_Stop) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
           .canonical_value());
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
 
   // Start notify session
   characteristic1_->StartNotifySession(
@@ skipping 15 matching lines @@
   ASSERT_EQ(1u, notify_sessions_.size());
   ASSERT_TRUE(notify_sessions_[0]);
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_TRUE(characteristic1_->IsNotifying());
 
   SimulateGattNotifySessionStopped(characteristic1_);
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
-#endif
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StopNotifySession_Reentrant_Stop_StartSuccess) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   // Check that the notify session is active.
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
   EXPECT_EQ(characteristic1_->GetIdentifier(),
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_TRUE(characteristic1_->IsNotifying());
@@ skipping 17 matching lines @@
   base::RunLoop().RunUntilIdle();
   ASSERT_EQ(2u, notify_sessions_.size());
   ASSERT_TRUE(notify_sessions_[0]);
   ASSERT_TRUE(notify_sessions_[1]);
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_EQ(characteristic1_, notify_sessions_[1]->GetCharacteristic());
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_TRUE(notify_sessions_[1]->IsActive());
   EXPECT_TRUE(characteristic1_->IsNotifying());
 }
-#endif
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StopNotifySession_Reentrant_Stop_StartError) {
+  if (!PlatformSupportsLowEnergy()) {
+    LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
+    return;
+  }
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   // Check that the notify session is active.
   EXPECT_TRUE(notify_sessions_[0]->IsActive());
   EXPECT_EQ(characteristic1_->GetIdentifier(),
             notify_sessions_[0]->GetCharacteristicIdentifier());
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_TRUE(characteristic1_->IsNotifying());
@@ skipping 15 matching lines @@
 
   SimulateGattNotifySessionStartError(
       characteristic1_, BluetoothRemoteGattService::GATT_ERROR_FAILED);
   base::RunLoop().RunUntilIdle();
   ASSERT_EQ(1u, notify_sessions_.size());
   ASSERT_TRUE(notify_sessions_[0]);
   EXPECT_EQ(characteristic1_, notify_sessions_[0]->GetCharacteristic());
   EXPECT_FALSE(notify_sessions_[0]->IsActive());
   EXPECT_FALSE(characteristic1_->IsNotifying());
 }
-#endif
+#endif  // defined(OS_ANDROID) || defined(OS_MACOSX)
 
 #if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // Tests Characteristic Value changes during a Notify Session.
 TEST_F(BluetoothRemoteGattCharacteristicTest, GattCharacteristicValueChanged) {
   if (!PlatformSupportsLowEnergy()) {
     LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
     return;
   }
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
@@ skipping 116 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