bluetooth: Refactor BluetoothDiscoverySession::Stop to always call its callbacks

This patch fixes an bug where Stop's callbacks wouldn't get called if the
BluetoothDiscoverySession got destroyed before Stop finished.

BUG=484502

Review URL: https://codereview.chromium.org/1125483004

Cr-Commit-Position: refs/heads/master@{#328274}

3 files changed, 60 insertions, 12 deletions

diff --git a/device/bluetooth/bluetooth_chromeos_unittest.cc b/device/bluetooth/bluetooth_chromeos_unittest.cc
index a4bcb8a..18728ef 100644
--- a/device/bluetooth/bluetooth_chromeos_unittest.cc
+++ b/device/bluetooth/bluetooth_chromeos_unittest.cc
@@ -655,6 +655,31 @@ TEST_F(BluetoothChromeOSTest, StopDiscovery) {
   EXPECT_FALSE(observer.last_discovering());
 
   EXPECT_FALSE(adapter_->IsDiscovering());
+  discovery_sessions_.clear();
+  callback_count_ = 0;
+
+  // Test that the Stop callbacks get called even if the
+  // BluetoothDiscoverySession objects gets deleted
+  adapter_->SetPowered(true, GetCallback(), GetErrorCallback());
+  adapter_->StartDiscoverySession(
+      base::Bind(&BluetoothChromeOSTest::DiscoverySessionCallback,
+                 base::Unretained(this)),
+      GetErrorCallback());
+  message_loop_.Run();
+  EXPECT_EQ(2, callback_count_);
+  EXPECT_EQ(0, error_callback_count_);
+  callback_count_ = 0;
+  ASSERT_TRUE(adapter_->IsPowered());
+  ASSERT_TRUE(adapter_->IsDiscovering());
+  ASSERT_EQ((size_t)1, discovery_sessions_.size());
+  ASSERT_TRUE(discovery_sessions_[0]->IsActive());
+
+  discovery_sessions_[0]->Stop(GetCallback(), GetErrorCallback());
+  discovery_sessions_.clear();
+
+  message_loop_.Run();
+  EXPECT_EQ(1, callback_count_);
+  EXPECT_EQ(0, error_callback_count_);
 }
 
 TEST_F(BluetoothChromeOSTest, Discovery) {
diff --git a/device/bluetooth/bluetooth_discovery_session.cc b/device/bluetooth/bluetooth_discovery_session.cc
index e30f86c..c80012e 100644
--- a/device/bluetooth/bluetooth_discovery_session.cc
+++ b/device/bluetooth/bluetooth_discovery_session.cc
@@ -197,26 +197,40 @@ bool BluetoothDiscoverySession::IsActive() const {
   return active_;
 }
 
-void BluetoothDiscoverySession::Stop(
-    const base::Closure& callback,
-    const ErrorCallback& error_callback) {
+void BluetoothDiscoverySession::Stop(const base::Closure& success_callback,
+                                     const ErrorCallback& error_callback) {
   if (!active_) {
     LOG(WARNING) << "Discovery session not active. Cannot stop.";
     error_callback.Run();
     return;
   }
   VLOG(1) << "Stopping device discovery session.";
-  adapter_->RemoveDiscoverySession(
-      discovery_filter_.get(),
-      base::Bind(&BluetoothDiscoverySession::OnStop,
-                 weak_ptr_factory_.GetWeakPtr(), callback),
-      error_callback);
+  base::Closure deactive_discovery_session =
+      base::Bind(&BluetoothDiscoverySession::DeactivateDiscoverySession,
+                 weak_ptr_factory_.GetWeakPtr());
+
+  // Create a callback that runs
+  // BluetoothDiscoverySession::DeactivateDiscoverySession if the session still
+  // exists, but always runs success_callback.
+  base::Closure discovery_session_removed_callback =
+      base::Bind(&BluetoothDiscoverySession::OnDiscoverySessionRemoved,
+                 deactive_discovery_session, success_callback);
+  adapter_->RemoveDiscoverySession(discovery_filter_.get(),
+                                   discovery_session_removed_callback,
+                                   error_callback);
+}
+
+// static
+void BluetoothDiscoverySession::OnDiscoverySessionRemoved(
+    const base::Closure& deactivate_discovery_session,
+    const base::Closure& success_callback) {
+  deactivate_discovery_session.Run();
+  success_callback.Run();
 }
 
-void BluetoothDiscoverySession::OnStop(const base::Closure& callback) {
+void BluetoothDiscoverySession::DeactivateDiscoverySession() {
   MarkAsInactive();
   discovery_filter_.reset();
-  callback.Run();
 }
 
 void BluetoothDiscoverySession::MarkAsInactive() {
diff --git a/device/bluetooth/bluetooth_discovery_session.h b/device/bluetooth/bluetooth_discovery_session.h
index d50bf9c..fe1c8f0 100644
--- a/device/bluetooth/bluetooth_discovery_session.h
+++ b/device/bluetooth/bluetooth_discovery_session.h
@@ -129,8 +129,17 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothDiscoverySession {
  private:
   friend class BluetoothAdapter;
 
-  // Internal callback invoked when a call to Stop has succeeded.
-  void OnStop(const base::Closure& callback);
+  // Internal callback invoked when a call to
+  // BluetoothAdapter::RemoveDiscoverySession has succeeded. Invokes
+  // |deactivate_discovery_session| if the session object still
+  // exists when this callback executes. Always invokes |success_callback|.
+  static void OnDiscoverySessionRemoved(
+      const base::Closure& deactivate_discovery_session,
+      const base::Closure& success_callback);
+
+  // Deactivate discovery session object after
+  // BluetoothAdapter::RemoveDiscoverySession completes.
+  void DeactivateDiscoverySession();
 
   // Marks this instance as inactive. Called by BluetoothAdapter to mark a
   // session as inactive in the case of an unexpected change to the adapter