Implement BluetoothGattNotifySession::Stop on Android, 2nd attempt

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 1151 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)

[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.
 TEST_F(BluetoothRemoteGattCharacteristicTest,
        StartNotifySession_Reentrant_Success_Success) {
   if (!PlatformSupportsLowEnergy()) {
     LOG(WARNING) << "Low Energy Bluetooth unavailable, skipping unit test.";
     return;
   }
   ASSERT_NO_FATAL_FAILURE(
@@ skipping 98 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)
+// 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) {
   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));
@@ skipping 115 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