bluetooth: Test & make StartNotifySession reentrant.

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 54 matching lines @@
       uint16_t expected_config_descriptor_value,
       StartNotifySetupError error = StartNotifySetupError::NONE) {
     if (error == StartNotifySetupError::CHARACTERISTIC_PROPERTIES) {
       properties = 0;
     }
     ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(properties));
 
     if (error != StartNotifySetupError::CONFIG_DESCRIPTOR_MISSING) {
       SimulateGattDescriptor(
           characteristic1_,
-          /* Client Characteristic Configuration descriptor's standard UUID: */
-          "00002902-0000-1000-8000-00805F9B34FB");
+          BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
+              .canonical_value());
       ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
     }
 
     if (error == StartNotifySetupError::SET_NOTIFY) {
       SimulateGattCharacteristicSetNotifyWillFailSynchronouslyOnce(
           characteristic1_);
     }
 
     if (error == StartNotifySetupError::WRITE_DESCRIPTOR) {
       SimulateGattDescriptorWriteWillFailSynchronouslyOnce(
@@ skipping 204 matching lines @@
   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_);
 
+  // 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_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));
 
   characteristic1_->ReadRemoteCharacteristic(
       GetReadValueCallback(Call::NOT_EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
 
   RememberCharacteristicForSubsequentAction(characteristic1_);
-  DeleteDevice(device_);
+  DeleteDevice(device_);  // TODO(576906) delete only the characteristic.

[ortuno, 2016/03/13 22:45:03]

nit: This should follow the same pattern as other TODOs in file:

TODO(crbug.com/576906)

[scheib, 2016/03/14 20:00:16]

Done, by fixing the other TODOs to the shorter form. Keeping more descriptive
text on a one line is better than redundant crbug.com.

 
   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.
 TEST_F(BluetoothGattCharacteristicTest,
        WriteRemoteCharacteristic_AfterDeleted) {
   ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(
       BluetoothGattCharacteristic::PROPERTY_WRITE));
 
   std::vector<uint8_t> empty_vector;
   characteristic1_->WriteRemoteCharacteristic(
       empty_vector, GetCallback(Call::NOT_EXPECTED),
       GetGattErrorCallback(Call::NOT_EXPECTED));
 
   RememberCharacteristicForSubsequentAction(characteristic1_);
-  DeleteDevice(device_);
+  DeleteDevice(device_);  // TODO(576906) delete only the characteristic.

[ortuno, 2016/03/13 22:45:03]

Same here. TODO should follow same pattern as other TODOs.

[scheib, 2016/03/14 20:00:16]

Done.

 
   SimulateGattCharacteristicWrite(/* use remembered characteristic */ nullptr);
   EXPECT_TRUE("Did not crash!");
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 #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(
@@ skipping 493 matching lines @@
 // 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)
 
 #if defined(OS_ANDROID)
+// Tests multiple StartNotifySession success.
+TEST_F(BluetoothGattCharacteristicTest, StartNotifySession_Multiple) {
+  ASSERT_NO_FATAL_FAILURE(
+      FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
+  SimulateGattDescriptor(
+      characteristic1_,
+      BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
+          .canonical_value());
+  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(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)
+
+#if defined(OS_ANDROID)
+// Tests multiple StartNotifySession success.
+TEST_F(BluetoothGattCharacteristicTest, StartNotifySessionError_Multiple) {
+  ASSERT_NO_FATAL_FAILURE(
+      FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
+  SimulateGattDescriptor(
+      characteristic1_,
+      BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
+          .canonical_value());
+  ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+
+  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_,
+                                      BluetoothGattService::GATT_ERROR_FAILED);
+  EXPECT_EQ(0, callback_count_);
+  EXPECT_EQ(2, error_callback_count_);
+  ASSERT_EQ(0u, notify_sessions_.size());
+  EXPECT_EQ(BluetoothGattService::GATT_ERROR_FAILED, last_gatt_error_code_);
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests multiple StartNotifySession success.

[ortuno, 2016/03/13 22:45:03]

Wrong test description.

[scheib, 2016/03/14 20:00:16]

Done.

+TEST_F(BluetoothGattCharacteristicTest, StartNotifySession_AfterDeleted) {
+  ASSERT_NO_FATAL_FAILURE(
+      FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
+  SimulateGattDescriptor(
+      characteristic1_,
+      BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
+          .canonical_value());
+  ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+
+  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());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
 // 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);
@@ skipping 12 matching lines @@
 #if defined(OS_ANDROID)
 // Tests Characteristic Value changing after a Notify Session and objects being
 // destroyed.
 TEST_F(BluetoothGattCharacteristicTest,
        GattCharacteristicValueChanged_AfterDeleted) {
   ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
       /* properties: NOTIFY */ 0x10,
       /* expected_config_descriptor_value: NOTIFY */ 1));
 
   RememberCharacteristicForSubsequentAction(characteristic1_);
-  DeleteDevice(device_);
+  DeleteDevice(device_);  // TODO(576906) delete only the characteristic.
 
   std::vector<uint8_t> empty_vector;
   SimulateGattCharacteristicChanged(/* use remembered characteristic */ nullptr,
                                     empty_vector);
   EXPECT_TRUE("Did not crash!");
 }
 #endif  // defined(OS_ANDROID)
 
-#if defined(OS_ANDROID)
-// Tests multiple StartNotifySession success.
-TEST_F(BluetoothGattCharacteristicTest, StartNotifySession_Multiple) {
-  ASSERT_NO_FATAL_FAILURE(
-      FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
-  SimulateGattDescriptor(
-      characteristic1_,
-      BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
-          .canonical_value());
-  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(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)
-
 #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 61 matching lines @@
   EXPECT_EQ(1u, characteristic1_->GetDescriptorsForUUID(id2).size());
   EXPECT_EQ(2u, characteristic2_->GetDescriptorsForUUID(id3).size());
 
   EXPECT_EQ(0u, characteristic2_->GetDescriptorsForUUID(id1).size());
   EXPECT_EQ(0u, characteristic2_->GetDescriptorsForUUID(id2).size());
   EXPECT_EQ(0u, characteristic1_->GetDescriptorsForUUID(id3).size());
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
 }  // namespace device