Implement BluetoothRemoteGattCharacteristicWin::StartNotifySession and related unit tests

device/bluetooth/bluetooth_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 "device/bluetooth/bluetooth_gatt_characteristic.h"
 
 #include <stdint.h>
 
 #include "base/macros.h"
 #include "base/run_loop.h"
@@ skipping 80 matching lines @@
       characteristic1_->StartNotifySession(
           GetNotifyCallback(Call::NOT_EXPECTED),
           GetGattErrorCallback(Call::EXPECTED));
       return;
     }
 
     characteristic1_->StartNotifySession(
         GetNotifyCallback(Call::EXPECTED),
         GetGattErrorCallback(Call::NOT_EXPECTED));
 
-    EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
     EXPECT_EQ(0, callback_count_);
     SimulateGattNotifySessionStarted(characteristic1_);
+    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());
 
+#ifndef OS_WIN
     // Verify the Client Characteristic Configuration descriptor was written to.
+    // Windows: No need to write CCCD explicitly to start notification.
     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
   }
 
   BluetoothDevice* device_ = nullptr;
   BluetoothGattService* service_ = nullptr;
   BluetoothGattCharacteristic* characteristic1_ = nullptr;
   BluetoothGattCharacteristic* characteristic2_ = nullptr;
 };
 #endif
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
@@ skipping 137 matching lines @@
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests ReadRemoteCharacteristic and GetValue with empty value buffer.
 TEST_F(BluetoothGattCharacteristicTest, ReadRemoteCharacteristic_Empty) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_READ));
 
   characteristic1_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
   std::vector<uint8_t> empty_vector;
   SimulateGattCharacteristicRead(characteristic1_, empty_vector);
 
   // Duplicate read reported from OS shouldn't cause a problem:
   SimulateGattCharacteristicRead(characteristic1_, empty_vector);
 
+  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
   EXPECT_EQ(empty_vector, last_read_value_);
   EXPECT_EQ(empty_vector, characteristic1_->GetValue());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests WriteRemoteCharacteristic with empty value buffer.
 TEST_F(BluetoothGattCharacteristicTest, WriteRemoteCharacteristic_Empty) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_WRITE));
 
   std::vector<uint8_t> empty_vector;
   characteristic1_->WriteRemoteCharacteristic(
       empty_vector, GetCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_write_characteristic_attempts_);
   SimulateGattCharacteristicWrite(characteristic1_);
 
+  EXPECT_EQ(1, gatt_write_characteristic_attempts_);
   EXPECT_EQ(empty_vector, last_write_value_);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests ReadRemoteCharacteristic completing after Chrome objects are deleted.
 TEST_F(BluetoothGattCharacteristicTest, ReadRemoteCharacteristic_AfterDeleted) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_READ));
 
@@ skipping 33 matching lines @@
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests ReadRemoteCharacteristic and GetValue with non-empty value buffer.
 TEST_F(BluetoothGattCharacteristicTest, ReadRemoteCharacteristic) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_READ));
 
   characteristic1_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
 
   uint8_t values[] = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
   std::vector<uint8_t> test_vector(values, values + arraysize(values));
   SimulateGattCharacteristicRead(characteristic1_, test_vector);
 
   // Duplicate read reported from OS shouldn't cause a problem:
   std::vector<uint8_t> empty_vector;
   SimulateGattCharacteristicRead(characteristic1_, empty_vector);
 
+  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
   EXPECT_EQ(test_vector, last_read_value_);
   EXPECT_EQ(test_vector, characteristic1_->GetValue());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests WriteRemoteCharacteristic with non-empty value buffer.
 TEST_F(BluetoothGattCharacteristicTest, WriteRemoteCharacteristic) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_WRITE));
 
   uint8_t values[] = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
   std::vector<uint8_t> test_vector(values, values + arraysize(values));
   characteristic1_->WriteRemoteCharacteristic(
       test_vector, GetCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_write_characteristic_attempts_);
 
   SimulateGattCharacteristicWrite(characteristic1_);
 
+  EXPECT_EQ(1, gatt_write_characteristic_attempts_);
   EXPECT_EQ(test_vector, last_write_value_);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests ReadRemoteCharacteristic and GetValue multiple times.
 TEST_F(BluetoothGattCharacteristicTest, ReadRemoteCharacteristic_Twice) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_READ));
 
   characteristic1_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
 
   uint8_t values[] = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
   std::vector<uint8_t> test_vector(values, values + arraysize(values));
   SimulateGattCharacteristicRead(characteristic1_, test_vector);
+  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
   EXPECT_EQ(1, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(test_vector, last_read_value_);
   EXPECT_EQ(test_vector, characteristic1_->GetValue());
 
   // Read again, with different value:
   ResetEventCounts();
   characteristic1_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
   std::vector<uint8_t> empty_vector;
   SimulateGattCharacteristicRead(characteristic1_, empty_vector);
+  EXPECT_EQ(1, gatt_read_characteristic_attempts_);
   EXPECT_EQ(1, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(empty_vector, last_read_value_);
   EXPECT_EQ(empty_vector, characteristic1_->GetValue());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests WriteRemoteCharacteristic multiple times.
 TEST_F(BluetoothGattCharacteristicTest, WriteRemoteCharacteristic_Twice) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_WRITE));
 
   uint8_t values[] = {0, 1, 2, 3, 4, 0xf, 0xf0, 0xff};
   std::vector<uint8_t> test_vector(values, values + arraysize(values));
   characteristic1_->WriteRemoteCharacteristic(
       test_vector, GetCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_write_characteristic_attempts_);
 
   SimulateGattCharacteristicWrite(characteristic1_);
+  EXPECT_EQ(1, gatt_write_characteristic_attempts_);
   EXPECT_EQ(1, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(test_vector, last_write_value_);
 
   // Write again, with different value:
   ResetEventCounts();
   std::vector<uint8_t> empty_vector;
   characteristic1_->WriteRemoteCharacteristic(
       empty_vector, GetCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
+
+  SimulateGattCharacteristicWrite(characteristic1_);
   EXPECT_EQ(1, gatt_write_characteristic_attempts_);
-  SimulateGattCharacteristicWrite(characteristic1_);
   EXPECT_EQ(1, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(empty_vector, last_write_value_);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests ReadRemoteCharacteristic on two characteristics.
 TEST_F(BluetoothGattCharacteristicTest,
        ReadRemoteCharacteristic_MultipleCharacteristics) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_READ));
 
   characteristic1_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
   characteristic2_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(2, gatt_read_characteristic_attempts_);
   EXPECT_EQ(0, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
 
   std::vector<uint8_t> test_vector1;
   test_vector1.push_back(111);
   SimulateGattCharacteristicRead(characteristic1_, test_vector1);
   EXPECT_EQ(test_vector1, last_read_value_);
 
   std::vector<uint8_t> test_vector2;
   test_vector2.push_back(222);
   SimulateGattCharacteristicRead(characteristic2_, test_vector2);
   EXPECT_EQ(test_vector2, last_read_value_);
 
+  EXPECT_EQ(2, gatt_read_characteristic_attempts_);
   EXPECT_EQ(2, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_EQ(test_vector1, characteristic1_->GetValue());
   EXPECT_EQ(test_vector2, characteristic2_->GetValue());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests WriteRemoteCharacteristic on two characteristics.
 TEST_F(BluetoothGattCharacteristicTest,
@@ skipping 12 matching lines @@
 
   std::vector<uint8_t> test_vector2;
   test_vector2.push_back(222);
   characteristic2_->WriteRemoteCharacteristic(
       test_vector2, GetCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
 #ifdef OS_ANDROID
   EXPECT_EQ(test_vector2, last_write_value_);
 #endif
 
-  EXPECT_EQ(2, gatt_write_characteristic_attempts_);
   EXPECT_EQ(0, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
 
   SimulateGattCharacteristicWrite(characteristic1_);
 #ifndef OS_ANDROID
   EXPECT_EQ(test_vector1, last_write_value_);
 #endif
 
   SimulateGattCharacteristicWrite(characteristic2_);
 #ifndef OS_ANDROID
   EXPECT_EQ(test_vector2, last_write_value_);
 #endif
 
+  EXPECT_EQ(2, gatt_write_characteristic_attempts_);
   EXPECT_EQ(2, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
 
   // TODO(crbug.com/591740): Remove if define for OS_ANDROID in this test.
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests ReadRemoteCharacteristic asynchronous error.
 TEST_F(BluetoothGattCharacteristicTest, ReadError) {
@@ skipping 199 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_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_WIN)
 // StartNotifySession fails if characteristic doesn't have Notify or Indicate
 // property.
 TEST_F(BluetoothGattCharacteristicTest, 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_);
+// TODO(crbug.com/584369) Android doesn't return the correct errors.
+#ifndef OS_ANDROID

[ortuno, 2016/03/15 02:55:31]

I think the style guide prefers #if defined(OS_ANDROID)

https://www.chromium.org/developers/coding-style#TOC-Platform-specific-code

[gogerald1, 2016/03/15 18:28:22]

Done. Do not see it explicitly, especially this looks only for Android, anyway,
change it to reviewer happy.

+  EXPECT_EQ(BluetoothGattService::GATT_ERROR_NOT_SUPPORTED,
+            last_gatt_error_code_);
+#endif

[ortuno, 2016/03/15 02:55:31]

#endif // defined(OS_ANDROID)

[gogerald1, 2016/03/15 18:28:22]

Done.

 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || !defined(OS_WIN)
 // StartNotifySession fails if the characteristic is missing the Client
 // Characteristic Configuration descriptor.
+// Windows: TODO(crbug.com/594697):
+// BluetoothRemoteGattCharacteristicWin::StartNotifySession doesn't check the
+// existence of client characteristic configuration descriptor.
 TEST_F(BluetoothGattCharacteristicTest, 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_);
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || !defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || !defined(OS_WIN)
 // StartNotifySession fails synchronously when failing to set a characteristic
 // to enable notifications.
 // Android: This is mBluetoothGatt.setCharacteristicNotification failing.
+// Windows: Characteristic notification is set to OS asynchronously.
 TEST_F(BluetoothGattCharacteristicTest,
        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)
+#endif  // defined(OS_ANDROID) || !defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || !defined(OS_WIN)
 // Tests StartNotifySession descriptor write synchronous failure.
+// Windows: No need to write CCCD explicitly to start notification.
 TEST_F(BluetoothGattCharacteristicTest,
        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)
+#endif  // defined(OS_ANDROID) || !defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests StartNotifySession success on a characteristic enabling Notify.
 TEST_F(BluetoothGattCharacteristicTest, StartNotifySession) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests StartNotifySession success on a characteristic enabling Indicate.
 TEST_F(BluetoothGattCharacteristicTest, StartNotifySession_OnIndicate) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: INDICATE */ 0x20,
       /* expected_config_descriptor_value: INDICATE */ 2));
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests StartNotifySession success on a characteristic enabling Notify &
 // Indicate.
 TEST_F(BluetoothGattCharacteristicTest,
        StartNotifySession_OnNotifyAndIndicate) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY and INDICATE bits set */ 0x30,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests multiple StartNotifySession success.
 TEST_F(BluetoothGattCharacteristicTest, StartNotifySession_Multiple) {
   ASSERT_NO_FATAL_FAILURE(
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
       /* Client Characteristic Configuration descriptor's standard UUID: */
       "00002902-0000-1000-8000-00805F9B34FB");
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
 
   characteristic1_->StartNotifySession(
       GetNotifyCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
   characteristic1_->StartNotifySession(
       GetNotifyCallback(Call::EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
-  EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
   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)
+#endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || defined(OS_WIN)
 // Tests Characteristic Value changes during a Notify Session.
 TEST_F(BluetoothGattCharacteristicTest, 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)
+#endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
-#if defined(OS_ANDROID)
+#if defined(OS_ANDROID) || !defined(OS_WIN)
 // Tests Characteristic Value changing after a Notify Session and objects being
 // destroyed.
+// Windows: Characteristic changed notification is not available after deleting
+// the device because OS detects device deleting before corresponding objects
+// being destroyed.

[ortuno, 2016/03/15 02:55:31]

What happens when you enable this test on windows?

[gogerald1, 2016/03/15 18:28:22]

SimulateGattCharacteristicChanged has no target after the device has been
deleted from the OS.

[scheib, 2016/03/19 04:57:54]

ortuno mentioned in person wanting to understand this more.
RememberCharacteristicForSubsequentAction impl in BluetoothTestWin stores a
pointer, which is invalid and then can't be used by GetSimulatedCharacteristic.
That means that for Windows all tests using
RememberCharacteristicForSubsequentAction won't work.

We 3 had discussed in chat that the Windows impl expects the characteristic
object to always outlive the OS referencing that object. However, reading over
the code I see that there is a fair amount of complexity, and it's not trivial
to assert this even now, but certainly not in the future with refactoring being
done.

Please modify this code so that OnGetGattEventWin is called with cached data
from RememberCharacteristicForSubsequentAction.

To test that code works correctly it should be possible see 
 SimulateGattCharacteristicChanged(/* use remembered characteristic */ nullptr
...
succeed if a DeleteDevice(device_) was not called.

(that is, verify that RememberCharacteristicForSubsequentAction works.)

Then, after DeleteDevice is called, SimulateGattCharacteristicChanged should not
crash.

[gogerald1, 2016/03/21 14:22:22]

Done.

[ortuno, 2016/03/21 17:00:52]

Have you tried this locally? I just tried it and it's crashing.

[gogerald1, 2016/03/21 22:09:16]

Of course, and it passed trybots

[ortuno, 2016/03/22 15:02:24]

After looking into it a bit more I think we need to fix the test. The test is
incorrect because no observers are added so the code that would trigger a crash
is not being tested. If you add an observer then the test will be correct and
you'll observer a crash.

[gogerald1, 2016/03/29 15:12:00]

Done. Rebase to fixed test

 TEST_F(BluetoothGattCharacteristicTest,
        GattCharacteristicValueChanged_AfterDeleted) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   RememberCharacteristicForSubsequentAction(characteristic1_);
   DeleteDevice(device_);
 
   std::vector<uint8_t> empty_vector;
   SimulateGattCharacteristicChanged(/* use remembered characteristic */ nullptr,
                                     empty_vector);
   EXPECT_TRUE("Did not crash!");
 }
-#endif  // defined(OS_ANDROID)
+#endif  // defined(OS_ANDROID) || !defined(OS_WIN)
 

[ortuno, 2016/03/15 02:55:31]

You still need to write a reentrancy test:

// Test
// ...
characteristic1_.StartNotifySession(GetReentrantStartNotifySessionCallback(
    Call::EXPECTED, characteristic1_));
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_[1]);
EXPECT_EQ(characteristic1_->GetIdentifier(),
          notify_sessions_[0]->GetCharacteristicIdentifier());
EXPECT_TRUE(notify_sessions_[0]->IsActive());
EXPECT_EQ(characteristic1_->GetIdentifier(),
          notify_sessions_[1]->GetCharacteristicIdentifier());
EXPECT_TRUE(notify_sessions_[1]->IsActive());
// ...

// bluetooth_test.cc
BluetoothGattCharacteristic::NotifySessionCallback
BluetoothTestBase::GetReentrantStartNotifySessionCallback(
    Call expected,
    BluetoothGattCharacteristic* characteristic) {
  if (expected == Call::EXPECTED)
    ++expected_success_callback_calls_;
  return base::Bind(&BluetoothTestBase::ReentrantStartNotifySessionCallback,
                    weak_factory_.GetWeakPtr(), expected, characteristic);
}

void BluetoothTestBase::ReentrantStartNotifySessionCallback(
    Call expected,
    BluetoothGattCharacteristic* characteristic,
    scoped_ptr<BluetoothGattNotifySession> notify_session) {
  ++callback_count_;
  notify_sessions_.push_back(notify_session.release());

  if (expected == Call::EXPECTED)
    ++actual_success_callback_calls_;
  else
    unexpected_success_callback_ = true;

  characteristic->StartNotifySesssion(GetNotifyCallback(Call::EXPECTED));
}

[gogerald1, 2016/03/15 18:28:22]

Do it in the next CL

 #if defined(OS_ANDROID) || defined(OS_WIN)
 TEST_F(BluetoothGattCharacteristicTest, GetDescriptors_FindNone) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate());
 
   EXPECT_EQ(0u, characteristic1_->GetDescriptors().size());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #if defined(OS_ANDROID) || defined(OS_WIN)
 TEST_F(BluetoothGattCharacteristicTest, GetDescriptors_and_GetDescriptor) {
@@ skipping 34 matching lines @@
 
   // Characteristic 1 has descriptor uuids 1 and 2 (we don't know the order).
   EXPECT_TRUE(c1_uuid1 == uuid1 || c1_uuid2 == uuid1);
   EXPECT_TRUE(c1_uuid1 == uuid2 || c1_uuid2 == uuid2);
   // ... but not uuid 3
   EXPECT_FALSE(c1_uuid1 == uuid3 || c1_uuid2 == uuid3);
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 }  // namespace device