Implement BluetoothGattNotifySession::Stop on Android, 2nd attempt

device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc

Index: device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
index ec0310b5f65f3fb7fcb3b19e43c7419929d09ece..339329b33dba2733d292c5c256a75d462bde9038 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
@@ -1169,6 +1169,40 @@ TEST_F(BluetoothRemoteGattCharacteristicTest, StartNotifySession_AfterDeleted) {
 }
 #endif  // defined(OS_ANDROID)
 
+#if defined(OS_ANDROID)

[ortuno, 2016/07/28 21:59:30]

How do you feel about splitting the testing in two parts,
Start/StopNotifySession tests and Subscribe/Unsubscribe tests?

For the Start/StopNotifySession tests you can just construct a mock
characteristic with fake subscribe and unsubscribe, that way you don't need to
worry about all the descriptor stuff.

For Subscribe/Unsubscribe you would need to worry about those but then you don't
need to worry about so many tests.

WDYT?

[tommyt, 2016/08/01 12:48:27]

I don't really have a strong opinion on this. There are advantages to testing
the whole system at once, too, since it means we test something that is more
similar to the code that will actually be run in the various browsers. Do you
think the advantages to splitting up the code outweigh the advantages of keeping
it together?

If we do go the route of splitting this up, maybe we can leave this change as it
is, and do the split in a separate change?

[ortuno, 2016/08/02 01:56:31]

I'm a fan of unit tests but scheib prefers integration tests. So it's up to you.
This needs a lot of tests so separating might be better.

+// Tests StartNotifySession completing before chrome objects are deleted.
+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());
+
+  characteristic1_->StartNotifySession(
+      GetNotifyCallback(Call::EXPECTED),
+      GetGattErrorCallback(Call::NOT_EXPECTED));
+  EXPECT_EQ(1, gatt_notify_characteristic_attempts_);
+  EXPECT_EQ(0, callback_count_);
+
+  RememberCharacteristicForSubsequentAction(characteristic1_);
+  RememberCCCDescriptorForSubsequentAction(characteristic1_);
+
+  SimulateGattNotifySessionStarted(/* use remembered characteristic */ nullptr);
+  ASSERT_EQ(1u, notify_sessions_.size());
+
+  DeleteDevice(device_);  // TODO(576906) delete only the characteristic.
+
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  SimulateGattNotifySessionStopped(/* use remembered characteristic */ nullptr);
+  EXPECT_TRUE("Did not crash!");
+  ASSERT_EQ(0u, notify_sessions_.size());
+}
+#endif  // defined(OS_ANDROID)
+
 #if defined(OS_MACOSX) || defined(OS_WIN)
 // Tests StartNotifySession reentrant in start notify session success callback
 // and the reentrant start notify session success.
@@ -1287,6 +1321,234 @@ TEST_F(BluetoothRemoteGattCharacteristicTest,
 }
 #endif  // defined(OS_WIN)
 
+#if defined(OS_ANDROID)
+// Tests StopNotifySession success on a characteristic enabling Notify.
+TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession) {
+  ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
+      /* properties: NOTIFY */ 0x10,
+      /* expected_config_descriptor_value: NOTIFY */ 1));
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  SimulateGattNotifySessionStopped(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests StopNotifySession success on a characteristic enabling Indicate.
+TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_OnIndicate) {
+  ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
+      /* properties: INDICATE */ 0x20,
+      /* expected_config_descriptor_value: INDICATE */ 2));
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  SimulateGattNotifySessionStopped(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests StopNotifySession success on a characteristic enabling Notify &
+// Indicate.
+TEST_F(BluetoothRemoteGattCharacteristicTest,
+       StopNotifySession_OnNotifyAndIndicate) {
+  ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
+      /* properties: INDICATE */ 0x30,
+      /* expected_config_descriptor_value: INDICATE */ 1));
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  SimulateGattNotifySessionStopped(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests StopNotifySession error
+TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_Error) {
+  ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
+      /* properties: NOTIFY */ 0x10,
+      /* expected_config_descriptor_value: NOTIFY */ 1));
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  SimulateGattNotifySessionStopError(
+      characteristic1_, BluetoothRemoteGattService::GATT_ERROR_UNKNOWN);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests multiple StopNotifySession calls for a single session.
+TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_Multiple1) {
+  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(
+      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());
+
+  // Stop the notify session twice
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  SimulateGattNotifySessionStopped(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests multiple StartNotifySession calls and multiple StopNotifySession calls.
+TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_Multiple2) {
+  ASSERT_NO_FATAL_FAILURE(
+      FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
+  SimulateGattDescriptor(
+      characteristic1_,
+      BluetoothRemoteGattDescriptor::ClientCharacteristicConfigurationUuid()
+          .canonical_value());
+  ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+
+  // Start notify sessions
+  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();
+  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_, notify_sessions_[0]->GetCharacteristic());
+  EXPECT_EQ(characteristic1_, notify_sessions_[1]->GetCharacteristic());
+  EXPECT_TRUE(notify_sessions_[0]->IsActive());
+  EXPECT_TRUE(notify_sessions_[1]->IsActive());
+
+  // Stop the notify sessions
+  notify_sessions_[1]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[1]));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, notify_sessions_.size());
+
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  SimulateGattNotifySessionStopped(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests starting a new notify session before the previous stop request resolves
+TEST_F(BluetoothRemoteGattCharacteristicTest, StopNotifySession_StopStart) {
+  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(
+      GetNotifyCallback(Call::EXPECTED),
+      GetGattErrorCallback(Call::NOT_EXPECTED));
+  SimulateGattNotifySessionStarted(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, notify_sessions_.size());
+  ASSERT_TRUE(notify_sessions_[0]);
+  EXPECT_TRUE(notify_sessions_[0]->IsActive());
+
+  // Stop the notify session
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+
+  // Start another notify session
+  characteristic1_->StartNotifySession(
+      GetNotifyCallback(Call::EXPECTED),
+      GetGattErrorCallback(Call::NOT_EXPECTED));
+
+  SimulateGattNotifySessionStopped(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+
+  SimulateGattNotifySessionStarted(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, notify_sessions_.size());
+  ASSERT_TRUE(notify_sessions_[0]);
+  EXPECT_TRUE(notify_sessions_[0]->IsActive());
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// Tests starting a new notify session before the previous stop request resolves
+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());
+
+  // Start notify session
+  characteristic1_->StartNotifySession(
+      GetNotifyCallback(Call::EXPECTED),
+      GetGattErrorCallback(Call::NOT_EXPECTED));
+  SimulateGattNotifySessionStarted(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, notify_sessions_.size());
+  ASSERT_TRUE(notify_sessions_[0]);
+  EXPECT_TRUE(notify_sessions_[0]->IsActive());
+
+  // Stop the notify session twice
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+  notify_sessions_[0]->Stop(
+      GetStopNotifyCallback(Call::EXPECTED, notify_sessions_[0]));
+
+  // Start another notify session
+  characteristic1_->StartNotifySession(
+      GetNotifyCallback(Call::EXPECTED),
+      GetGattErrorCallback(Call::NOT_EXPECTED));
+
+  SimulateGattNotifySessionStopped(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(0u, notify_sessions_.size());
+
+  SimulateGattNotifySessionStarted(characteristic1_);
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, notify_sessions_.size());
+  ASSERT_TRUE(notify_sessions_[0]);
+  EXPECT_TRUE(notify_sessions_[0]->IsActive());
+}
+#endif  // defined(OS_ANDROID)
+
 #if defined(OS_ANDROID) || defined(OS_MACOSX) || defined(OS_WIN)
 // Tests Characteristic Value changes during a Notify Session.
 TEST_F(BluetoothRemoteGattCharacteristicTest, GattCharacteristicValueChanged) {