Update tests to remove |BluetoothDiscoverySession::Stop|

The BluetoothDiscoverySession::Stop method callback params will be
removed and |Stop| will be equivalent to the session destructor, update
BluetoothBlueZTest tests to reflect the ongoing refactoring.

Bug: 991682
Change-Id: If5e743407fb08c42c50f0e5ace4da3c7771d8567
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090768Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Phillis Tang <phillis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748282}

device/bluetooth/bluetooth_adapter.cc

@@ -283,6 +283,11 @@ void BluetoothAdapter::NotifyDeviceChanged(BluetoothDevice* device) {
     observer.DeviceChanged(this, device);
 }
 
+void BluetoothAdapter::NotifyAdapterDiscoveryChangeCompletedForTesting() {
+  for (auto& observer : observers_)
+    observer.DiscoveryChangeCompletedForTesting();
+}
+
 #if defined(OS_CHROMEOS) || defined(OS_LINUX)
 void BluetoothAdapter::NotifyDevicePairedChanged(BluetoothDevice* device,
                                                  bool new_paired_status) {
@@ -468,7 +473,9 @@ void BluetoothAdapter::OnDiscoveryChangeComplete(
       return;
 
     discovery_request_pending_ = false;
+    NotifyAdapterDiscoveryChangeCompletedForTesting();
     ProcessDiscoveryQueue();
+
     return;
   }
 
@@ -494,6 +501,7 @@ void BluetoothAdapter::OnDiscoveryChangeComplete(
     return;
 
   discovery_request_pending_ = false;
+  NotifyAdapterDiscoveryChangeCompletedForTesting();
   ProcessDiscoveryQueue();
 }
 

device/bluetooth/bluetooth_adapter.h

@@ -87,6 +87,9 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapter
     virtual void DeviceAdded(BluetoothAdapter* adapter,
                              BluetoothDevice* device) {}
 
+    // Called when the adapter |DiscoveryChangeComplete| is finished
+    virtual void DiscoveryChangeCompletedForTesting() {}
+
     // Called when the result of one of the following methods of the device
     // |device| changes:
     //  * GetAddress()
@@ -592,6 +595,7 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapter
   void NotifyAdapterPresentChanged(bool present);
   void NotifyAdapterPoweredChanged(bool powered);
   void NotifyDeviceChanged(BluetoothDevice* device);
+  void NotifyAdapterDiscoveryChangeCompletedForTesting();
 
 #if defined(OS_CHROMEOS) || defined(OS_LINUX)
   void NotifyDevicePairedChanged(BluetoothDevice* device,

device/bluetooth/bluez/bluetooth_bluez_unittest.cc

@@ -1137,16 +1137,11 @@ TEST_F(BluetoothBlueZTest, UnexpectedChangesDuringMultipleDiscoverySessions) {
   ASSERT_EQ((size_t)1, discovery_sessions_.size());
   EXPECT_TRUE(discovery_sessions_[0]->IsActive());
 
-  discovery_sessions_[0]->Stop(GetCallback(), GetErrorCallback());
-  // Run the loop, as there should have been a D-Bus call.
-  base::RunLoop().Run();
+  discovery_sessions_.clear();
   EXPECT_EQ(5, observer.discovering_changed_count());
-  EXPECT_EQ(8, callback_count_);
-  EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(observer.last_discovering());
   EXPECT_TRUE(IsAdapterDiscovering());
-  EXPECT_FALSE(discovery_sessions_[0]->IsActive());
-  discovery_sessions_.clear();
+  EXPECT_TRUE(discovery_sessions_.empty());
 
   // Start discovery again.
   adapter_->StartDiscoverySession(
@@ -1156,7 +1151,7 @@ TEST_F(BluetoothBlueZTest, UnexpectedChangesDuringMultipleDiscoverySessions) {
   // Run the loop, as there should have been a D-Bus call.
   base::RunLoop().Run();
   EXPECT_EQ(5, observer.discovering_changed_count());
-  EXPECT_EQ(9, callback_count_);
+  EXPECT_EQ(8, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(observer.last_discovering());
   EXPECT_TRUE(IsAdapterDiscovering());
@@ -1173,21 +1168,21 @@ TEST_F(BluetoothBlueZTest, UnexpectedChangesDuringMultipleDiscoverySessions) {
                  base::Unretained(this)));
   base::RunLoop().Run();
   EXPECT_EQ(5, observer.discovering_changed_count());
-  EXPECT_EQ(10, callback_count_);
+  EXPECT_EQ(9, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(observer.last_discovering());
   EXPECT_TRUE(IsAdapterDiscovering());
 
   // Now end the discovery session. This should change the adapter's discovery
   // state.
-  discovery_sessions_[0]->Stop(GetCallback(), GetErrorCallback());
-  base::RunLoop().Run();
+  base::RunLoop stop_loop;
+  observer.RegisterDiscoveringChangedWatcher(stop_loop.QuitClosure());
+  discovery_sessions_.clear();
+  stop_loop.Run();
   EXPECT_EQ(6, observer.discovering_changed_count());
-  EXPECT_EQ(11, callback_count_);
-  EXPECT_EQ(0, error_callback_count_);
   EXPECT_FALSE(observer.last_discovering());
   EXPECT_FALSE(IsAdapterDiscovering());
-  EXPECT_FALSE(discovery_sessions_[0]->IsActive());
+  EXPECT_TRUE(discovery_sessions_.empty());
 }
 
 TEST_F(BluetoothBlueZTest, InvalidatedDiscoverySessions) {
@@ -1316,30 +1311,27 @@ TEST_F(BluetoothBlueZTest, StartDiscoverySession) {
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(observer.last_discovering());
   EXPECT_TRUE(IsAdapterDiscovering());
-  ASSERT_EQ((size_t)3, discovery_sessions_.size());
+  ASSERT_EQ(3u, discovery_sessions_.size());
   EXPECT_TRUE(discovery_sessions_[1]->IsActive());
   EXPECT_NE(discovery_sessions_[0], discovery_sessions_[1]);
 
   // Stop the previous discovery session. The session should end but discovery
   // should continue.
-  discovery_sessions_[0]->Stop(GetCallback(), GetErrorCallback());
-  base::RunLoop().Run();
+  discovery_sessions_.erase(discovery_sessions_.begin());
   EXPECT_EQ(1, observer.discovering_changed_count());
-  EXPECT_EQ(4, callback_count_);
-  EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(observer.last_discovering());
   EXPECT_TRUE(IsAdapterDiscovering());
-  ASSERT_EQ((size_t)3, discovery_sessions_.size());
-  EXPECT_FALSE(discovery_sessions_[0]->IsActive());
-  EXPECT_TRUE(discovery_sessions_[1]->IsActive());
+  ASSERT_EQ(2u, discovery_sessions_.size());
+  EXPECT_TRUE(discovery_sessions_[0]->IsActive());
 
-  // Delete the current active session. Discovery should eventually stop.
+  // Delete the current active session.
+  base::RunLoop stop_loop;
+  observer.RegisterDiscoveringChangedWatcher(stop_loop.QuitClosure());
   discovery_sessions_.clear();
-  while (observer.last_discovering())
-    base::RunLoop().RunUntilIdle();
+  stop_loop.Run();
 
   EXPECT_EQ(2, observer.discovering_changed_count());
-  EXPECT_EQ(4, callback_count_);
+  EXPECT_EQ(3, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_FALSE(observer.last_discovering());
   EXPECT_FALSE(IsAdapterDiscovering());
@@ -1389,21 +1381,14 @@ TEST_F(BluetoothBlueZTest, SetDiscoveryFilterBeforeStartDiscovery) {
   std::vector<std::string> uuids = *filter->uuids;
   EXPECT_TRUE(base::Contains(uuids, "1000"));
 
-  discovery_sessions_[0]->Stop(
-      base::Bind(&BluetoothBlueZTest::Callback, base::Unretained(this)),
-      base::Bind(&BluetoothBlueZTest::ErrorCallback, base::Unretained(this)));
-
-  base::RunLoop().Run();
-
-  EXPECT_EQ(1, callback_count_);
-  EXPECT_EQ(0, error_callback_count_);
+  base::RunLoop stop_loop;
+  observer.RegisterDiscoveringChangedWatcher(stop_loop.QuitClosure());
+  discovery_sessions_.clear();
+  stop_loop.Run();
 
   ASSERT_TRUE(adapter_->IsPowered());
   ASSERT_FALSE(IsAdapterDiscovering());
-  ASSERT_EQ((size_t)1, discovery_sessions_.size());
-  ASSERT_FALSE(discovery_sessions_[0]->IsActive());
-  ASSERT_EQ(discovery_sessions_[0]->GetDiscoveryFilter(),
-            (BluetoothDiscoveryFilter*)nullptr);
+  ASSERT_TRUE(discovery_sessions_.empty());
 
   filter = fake_bluetooth_adapter_client_->GetDiscoveryFilter();
   EXPECT_EQ(nullptr, filter);
@@ -1544,11 +1529,10 @@ TEST_F(BluetoothBlueZTest, SetDiscoveryFilterBeforeStartDiscoveryMultiple) {
   callback_count_ = 0;
   // Request to stop discovery twice.
   for (int i = 0; i < 2; i++) {
-    discovery_sessions_[i]->Stop(
-        base::Bind(&BluetoothBlueZTest::Callback, base::Unretained(this)),
-        base::Bind(&BluetoothBlueZTest::ErrorCallback, base::Unretained(this)));
-    base::RunLoop().Run();
-
+    base::RunLoop change_loop;
+    observer.RegisterDiscoveryChangeCompletedWatcher(change_loop.QuitClosure());
+    discovery_sessions_.erase(discovery_sessions_.begin());
+    change_loop.Run();
     if (i == 0) {
       auto* filter = fake_bluetooth_adapter_client_->GetDiscoveryFilter();
       EXPECT_EQ("le", *filter->transport);
@@ -1583,12 +1567,9 @@ TEST_F(BluetoothBlueZTest, SetDiscoveryFilterBeforeStartDiscoveryMultiple) {
 
   // The success callback should have been called 2 times and the adapter should
   // still be discovering.
-  EXPECT_EQ(2, callback_count_);
-  EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(IsAdapterDiscovering());
-  EXPECT_FALSE(discovery_sessions_[0]->IsActive());
-  EXPECT_FALSE(discovery_sessions_[1]->IsActive());
-  EXPECT_TRUE(discovery_sessions_[2]->IsActive());
+  EXPECT_TRUE(discovery_sessions_[0]->IsActive());
+  ASSERT_EQ(1u, discovery_sessions_.size());
 
   callback_count_ = 0;
 
@@ -1658,34 +1639,26 @@ TEST_F(BluetoothBlueZTest, SetDiscoveryFilterBeforeStartDiscoveryMultiple) {
   EXPECT_EQ(3, callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(IsAdapterDiscovering());
-  ASSERT_EQ((size_t)6, discovery_sessions_.size());
+  ASSERT_EQ(4u, discovery_sessions_.size());
 
   callback_count_ = 0;
   // Request to stop discovery 4 times.
+  base::RunLoop adapter_stop_loop;
+  observer.RegisterDiscoveringChangedWatcher(adapter_stop_loop.QuitClosure());
   for (int i = 2; i < 6; i++) {
-    discovery_sessions_[i]->Stop(
-        base::Bind(&BluetoothBlueZTest::Callback, base::Unretained(this)),
-        base::Bind(&BluetoothBlueZTest::ErrorCallback, base::Unretained(this)));
-
-    // filter no  2 is same as filter no 5, so removing it shouldn't cause any
-    // filter update
-    if (i != 2 && i != 5)
-      base::RunLoop().Run();
+    base::RunLoop change_loop;
+    observer.RegisterDiscoveryChangeCompletedWatcher(change_loop.QuitClosure());
+    discovery_sessions_.erase(discovery_sessions_.begin());
+    change_loop.Run();
   }
-  // Run only once, as there should have been one D-Bus call.
-  base::RunLoop().Run();
+  adapter_stop_loop.Run();
 
-  // The success callback should have been called 4 times and the adapter should
-  // no longer be discovering.
-  EXPECT_EQ(4, callback_count_);
-  EXPECT_EQ(0, error_callback_count_);
+  // The adapter should no longer be discovering.
   EXPECT_FALSE(IsAdapterDiscovering());
   EXPECT_EQ(1, observer.discovering_changed_count());
 
   // All discovery sessions should be inactive.
-  for (int i = 0; i < 6; i++)
-    EXPECT_FALSE(discovery_sessions_[i]->IsActive());
-
+  ASSERT_TRUE(discovery_sessions_.empty());
   auto* filter = fake_bluetooth_adapter_client_->GetDiscoveryFilter();
   EXPECT_EQ(nullptr, filter);
 }
@@ -1788,16 +1761,7 @@ TEST_F(BluetoothBlueZTest, SetDiscoveryFilterMergingTest) {
   EXPECT_EQ(nullptr, filter->rssi.get());
   EXPECT_EQ(nullptr, filter->pathloss.get());
   EXPECT_EQ(nullptr, filter->uuids.get());
-
-  // Request to stop discovery 4 times.
-  for (int i = 3; i >= 0; i--) {
-    discovery_sessions_[i]->Stop(
-        base::Bind(&BluetoothBlueZTest::Callback, base::Unretained(this)),
-        base::Bind(&BluetoothBlueZTest::ErrorCallback, base::Unretained(this)));
-
-    // Every session stopping would trigger filter update
-    base::RunLoop().Run();
-  }
+  discovery_sessions_.clear();
 }
 
 TEST_F(BluetoothBlueZTest, DeviceProperties) {
@@ -4252,8 +4216,6 @@ TEST_F(BluetoothBlueZTest, Shutdown) {
 
   // OnStartDiscovery tested in Shutdown_OnStartDiscovery
   // OnStartDiscoveryError tested in Shutdown_OnStartDiscoveryError
-  // OnStopDiscovery tested in Shutdown_OnStopDiscovery
-  // OnStopDiscoveryError tested in Shutdown_OnStopDiscoveryError
 
   adapter_profile_ = nullptr;
 
@@ -4374,73 +4336,6 @@ TEST_F(BluetoothBlueZTest, Shutdown_OnStartDiscoveryError) {
   EXPECT_EQ(kNumberOfDiscoverySessions, error_callback_count_);
 }
 
-// Verifies post-Shutdown of discovery sessions and OnStartDiscovery.
-TEST_F(BluetoothBlueZTest, Shutdown_OnStopDiscovery) {
-  const int kNumberOfDiscoverySessions = 10;
-  GetAdapter();
-  BluetoothAdapterBlueZ* adapter_bluez =
-      static_cast<BluetoothAdapterBlueZ*>(adapter_.get());
-
-  // In order to queue up discovery sessions before an OnStopDiscovery call
-  // RemoveDiscoverySession must be called, so Add, Start, and Remove:
-  adapter_bluez->StartDiscoverySession(
-      base::BindRepeating(&BluetoothBlueZTest::DiscoverySessionCallback,
-                          base::Unretained(this)),
-      GetErrorCallback());
-  base::RunLoop().Run();
-  discovery_sessions_[0]->Stop(GetCallback(), GetErrorCallback());
-  callback_count_ = 0;
-  error_callback_count_ = 0;
-  // Can now queue discovery sessions while waiting for OnStopDiscovery.
-  for (int i = 0; i < kNumberOfDiscoverySessions; i++) {
-    adapter_bluez->StartDiscoverySession(
-        base::BindRepeating(&BluetoothBlueZTest::DiscoverySessionCallback,
-                            base::Unretained(this)),
-        GetErrorCallback());
-  }
-  adapter_->Shutdown();
-  base::RunLoop().Run();
-
-  // 1 successful stopped discovery from RemoveDiscoverySession, and
-  // kNumberOfDiscoverySessions errors from AddDiscoverySession/OnStopDiscovery.
-  EXPECT_EQ(1, callback_count_);
-  EXPECT_EQ(kNumberOfDiscoverySessions, error_callback_count_);
-}
-
-// Verifies post-Shutdown of discovery sessions and OnStopDiscoveryError.
-TEST_F(BluetoothBlueZTest, Shutdown_OnStopDiscoveryError) {
-  const int kNumberOfDiscoverySessions = 10;
-  GetAdapter();
-  BluetoothAdapterBlueZ* adapter_bluez =
-      static_cast<BluetoothAdapterBlueZ*>(adapter_.get());
-
-  // In order to queue up discovery sessions before an OnStopDiscoveryError call
-  // RemoveDiscoverySession must be called, so Add, Start, and Remove:
-  adapter_bluez->StartDiscoverySession(
-      base::BindRepeating(&BluetoothBlueZTest::DiscoverySessionCallback,
-                          base::Unretained(this)),
-      GetErrorCallback());
-  base::RunLoop().Run();
-  discovery_sessions_[0]->Stop(GetCallback(), GetErrorCallback());
-  callback_count_ = 0;
-  error_callback_count_ = 0;
-  // Can now queue discovery sessions while waiting for OnStopDiscoveryError.
-  for (int i = 0; i < kNumberOfDiscoverySessions; i++) {
-    adapter_bluez->StartDiscoverySession(
-        base::BindRepeating(&BluetoothBlueZTest::DiscoverySessionCallback,
-                            base::Unretained(this)),
-        GetErrorCallback());
-  }
-  adapter_->Shutdown();
-  base::RunLoop().Run();
-
-  // kNumberOfDiscoverySessions reported errors because of the shutdown while
-  // starting.  However, the stop call succeeded because the adapter shutdown
-  // and is therefore not discovering(also stop always returns success).
-  EXPECT_EQ(1, callback_count_);
-  EXPECT_EQ(kNumberOfDiscoverySessions, error_callback_count_);
-}
-
 TEST_F(BluetoothBlueZTest, StartDiscoveryError_ThenStartAgain) {
   GetAdapter();
   fake_bluetooth_adapter_client_->MakeStartDiscoveryFail();

device/bluetooth/test/test_bluetooth_adapter_observer.cc

@@ -119,6 +119,11 @@ void TestBluetoothAdapterObserver::RegisterDiscoveringChangedWatcher(
   discovering_changed_callback_ = callback;
 }
 
+void TestBluetoothAdapterObserver::RegisterDiscoveryChangeCompletedWatcher(
+    base::RepeatingClosure callback) {
+  discovery_change_completed_callback_ = callback;
+}
+
 void TestBluetoothAdapterObserver::DeviceAdded(BluetoothAdapter* adapter,
                                                BluetoothDevice* device) {
   EXPECT_EQ(adapter_.get(), adapter);
@@ -130,6 +135,11 @@ void TestBluetoothAdapterObserver::DeviceAdded(BluetoothAdapter* adapter,
   QuitMessageLoop();
 }
 
+void TestBluetoothAdapterObserver::DiscoveryChangeCompletedForTesting() {
+  if (discovery_change_completed_callback_)
+    discovery_change_completed_callback_.Run();
+}
+
 void TestBluetoothAdapterObserver::DeviceChanged(BluetoothAdapter* adapter,
                                                  BluetoothDevice* device) {
   EXPECT_EQ(adapter_.get(), adapter);

device/bluetooth/test/test_bluetooth_adapter_observer.h

@@ -33,7 +33,9 @@ class TestBluetoothAdapterObserver : public BluetoothAdapter::Observer {
   void AdapterDiscoveringChanged(BluetoothAdapter* adapter,
                                  bool discovering) override;
   void RegisterDiscoveringChangedWatcher(base::RepeatingClosure callback);
+  void RegisterDiscoveryChangeCompletedWatcher(base::RepeatingClosure callback);
   void DeviceAdded(BluetoothAdapter* adapter, BluetoothDevice* device) override;
+  void DiscoveryChangeCompletedForTesting() override;
   void DeviceChanged(BluetoothAdapter* adapter,
                      BluetoothDevice* device) override;
   void DeviceAddressChanged(device::BluetoothAdapter* adapter,
@@ -235,6 +237,7 @@ class TestBluetoothAdapterObserver : public BluetoothAdapter::Observer {
   base::Optional<uint16_t> last_appearance_;
 
   base::Closure discovering_changed_callback_;
+  base::Closure discovery_change_completed_callback_;
 
 #if defined(OS_CHROMEOS) || defined(OS_LINUX)
   int device_paired_changed_count_;