Migrate Chrome OS Bluetooth UI to the new discovery API.

This CL migrates BluetoothOptionsHandler and ash::SystemTrayDelegate to the
new discovery session API, removing calls to the now deprecated
device::BluetoothAdapter::Start|StopDiscovering.

BUG=346982
TEST=Tested that discovery logic works on Uber tray and chrome://settings. Opened
multiple tabs with chrome://settings/search#Bluetooth. Also tested that all
BluetoothDiscoverySession objects are marked as inactive when the adapter gets
powered down via bt_console.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254727 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 40 insertions, 17 deletions

diff --git a/device/bluetooth/bluetooth_adapter_chromeos.cc b/device/bluetooth/bluetooth_adapter_chromeos.cc
index 50232f7..942f9ae 100644
--- a/device/bluetooth/bluetooth_adapter_chromeos.cc
+++ b/device/bluetooth/bluetooth_adapter_chromeos.cc
@@ -625,8 +625,10 @@ void BluetoothAdapterChromeOS::DiscoveringChanged(
     bool discovering) {
   // If the adapter stopped discovery due to a reason other than a request by
   // us, reset the count to 0.
+  VLOG(1) << "Discovering changed: " << discovering;
   if (!discovering && !discovery_request_pending_
       && num_discovery_sessions_ > 0) {
+    VLOG(1) << "Marking sessions as inactive.";
     num_discovery_sessions_ = 0;
     MarkDiscoverySessionsAsInactive();
   }
@@ -680,8 +682,8 @@ void BluetoothAdapterChromeOS::AddDiscoverySession(
     // The pending request is either to stop a previous session or to start a
     // new one. Either way, queue this one.
     DCHECK(num_discovery_sessions_ == 1 || num_discovery_sessions_ == 0);
-    VLOG(1) << "Pending request to initiate device discovery. Queueing request "
-            << "to start a new discovery session.";
+    VLOG(1) << "Pending request to start/stop device discovery. Queueing "
+            << "request to start a new discovery session.";
     discovery_request_queue_.push(std::make_pair(callback, error_callback));
     return;
   }
@@ -708,6 +710,7 @@ void BluetoothAdapterChromeOS::AddDiscoverySession(
                      callback),
           base::Bind(&BluetoothAdapterChromeOS::OnStartDiscoveryError,
                      weak_ptr_factory_.GetWeakPtr(),
+                     callback,
                      error_callback));
 }
 
@@ -726,8 +729,8 @@ void BluetoothAdapterChromeOS::RemoveDiscoverySession(
 
   // If there is a pending request to BlueZ, then queue this request.
   if (discovery_request_pending_) {
-    VLOG(1) << "Pending request to initiate device discovery. Queueing request "
-            << "to stop discovery session.";
+    VLOG(1) << "Pending request to start/stop device discovery. Queueing "
+            << "request to stop discovery session.";
     error_callback.Run();
     return;
   }
@@ -759,6 +762,7 @@ void BluetoothAdapterChromeOS::RemoveDiscoverySession(
 
 void BluetoothAdapterChromeOS::OnStartDiscovery(const base::Closure& callback) {
   // Report success on the original request and increment the count.
+  VLOG(1) << __func__;
   DCHECK(discovery_request_pending_);
   DCHECK(num_discovery_sessions_ == 0);
   discovery_request_pending_ = false;
@@ -770,6 +774,7 @@ void BluetoothAdapterChromeOS::OnStartDiscovery(const base::Closure& callback) {
 }
 
 void BluetoothAdapterChromeOS::OnStartDiscoveryError(
+    const base::Closure& callback,
     const ErrorCallback& error_callback,
     const std::string& error_name,
     const std::string& error_message) {
@@ -780,7 +785,17 @@ void BluetoothAdapterChromeOS::OnStartDiscoveryError(
   DCHECK(num_discovery_sessions_ == 0);
   DCHECK(discovery_request_pending_);
   discovery_request_pending_ = false;
-  error_callback.Run();
+
+  // Discovery request may fail if discovery was previously initiated by Chrome,
+  // but the session were invalidated due to the discovery state unexpectedly
+  // changing to false and then back to true. In this case, report success.
+  if (error_name == bluetooth_device::kErrorInProgress && IsDiscovering()) {
+    VLOG(1) << "Discovery previously initiated. Reporting success.";
+    num_discovery_sessions_++;
+    callback.Run();
+  } else {
+    error_callback.Run();
+  }
 
   // Try to add a new discovery session for each queued request.
   ProcessQueuedDiscoveryRequests();
@@ -788,6 +803,7 @@ void BluetoothAdapterChromeOS::OnStartDiscoveryError(
 
 void BluetoothAdapterChromeOS::OnStopDiscovery(const base::Closure& callback) {
   // Report success on the original request and decrement the count.
+  VLOG(1) << __func__;
   DCHECK(discovery_request_pending_);
   DCHECK(num_discovery_sessions_ == 1);
   discovery_request_pending_ = false;
@@ -817,6 +833,7 @@ void BluetoothAdapterChromeOS::OnStopDiscoveryError(
 
 void BluetoothAdapterChromeOS::ProcessQueuedDiscoveryRequests() {
   while (!discovery_request_queue_.empty()) {
+    VLOG(1) << "Process queued discovery request.";
     DiscoveryCallbackPair callbacks = discovery_request_queue_.front();
     discovery_request_queue_.pop();
     AddDiscoverySession(callbacks.first, callbacks.second);
diff --git a/device/bluetooth/bluetooth_adapter_chromeos.h b/device/bluetooth/bluetooth_adapter_chromeos.h
index fccf740..a7afdc8 100644
--- a/device/bluetooth/bluetooth_adapter_chromeos.h
+++ b/device/bluetooth/bluetooth_adapter_chromeos.h
@@ -194,7 +194,8 @@ class BluetoothAdapterChromeOS
 
   // Called by dbus:: on completion of the D-Bus method call to start discovery.
   void OnStartDiscovery(const base::Closure& callback);
-  void OnStartDiscoveryError(const ErrorCallback& error_callback,
+  void OnStartDiscoveryError(const base::Closure& callback,
+                             const ErrorCallback& error_callback,
                              const std::string& error_name,
                              const std::string& error_message);
 
diff --git a/device/bluetooth/bluetooth_chromeos_unittest.cc b/device/bluetooth/bluetooth_chromeos_unittest.cc
index 4b51bbe..316d629 100644
--- a/device/bluetooth/bluetooth_chromeos_unittest.cc
+++ b/device/bluetooth/bluetooth_chromeos_unittest.cc
@@ -246,7 +246,7 @@ class BluetoothChromeOSTest : public testing::Test {
   }
 
   virtual void TearDown() {
-    if (last_discovery_session_.get() && last_discovery_session_->active()) {
+    if (last_discovery_session_.get() && last_discovery_session_->IsActive()) {
       callback_count_ = 0;
       last_discovery_session_->Stop(
           base::Bind(&BluetoothChromeOSTest::Callback,
@@ -1323,7 +1323,7 @@ TEST_F(BluetoothChromeOSTest, StartDiscoverySession) {
   EXPECT_TRUE(observer.last_discovering_);
   EXPECT_TRUE(adapter_->IsDiscovering());
   EXPECT_TRUE(last_discovery_session_.get());
-  EXPECT_TRUE(last_discovery_session_->active());
+  EXPECT_TRUE(last_discovery_session_->IsActive());
 
   // Start another session. A new one should be returned in the callback, which
   // in turn will destroy the previous session. Adapter should still be
@@ -1340,7 +1340,7 @@ TEST_F(BluetoothChromeOSTest, StartDiscoverySession) {
   EXPECT_TRUE(observer.last_discovering_);
   EXPECT_TRUE(adapter_->IsDiscovering());
   EXPECT_TRUE(last_discovery_session_.get());
-  EXPECT_TRUE(last_discovery_session_->active());
+  EXPECT_TRUE(last_discovery_session_->IsActive());
 
   // Hold on to the current discovery session to prevent it from getting
   // destroyed during the next call to StartDiscoverySession.
@@ -1360,7 +1360,7 @@ TEST_F(BluetoothChromeOSTest, StartDiscoverySession) {
   EXPECT_TRUE(observer.last_discovering_);
   EXPECT_TRUE(adapter_->IsDiscovering());
   EXPECT_TRUE(last_discovery_session_.get());
-  EXPECT_TRUE(last_discovery_session_->active());
+  EXPECT_TRUE(last_discovery_session_->IsActive());
   EXPECT_NE(last_discovery_session_.get(), discovery_session);
 
   // Stop the previous discovery session. The session should end but discovery
@@ -1377,8 +1377,8 @@ TEST_F(BluetoothChromeOSTest, StartDiscoverySession) {
   EXPECT_TRUE(observer.last_discovering_);
   EXPECT_TRUE(adapter_->IsDiscovering());
   EXPECT_TRUE(last_discovery_session_.get());
-  EXPECT_TRUE(last_discovery_session_->active());
-  EXPECT_FALSE(discovery_session->active());
+  EXPECT_TRUE(last_discovery_session_->IsActive());
+  EXPECT_FALSE(discovery_session->IsActive());
 
   // Delete the current active session. Discovery should eventually stop.
   last_discovery_session_.reset();
diff --git a/device/bluetooth/bluetooth_discovery_session.cc b/device/bluetooth/bluetooth_discovery_session.cc
index afeacff..e07df34 100644
--- a/device/bluetooth/bluetooth_discovery_session.cc
+++ b/device/bluetooth/bluetooth_discovery_session.cc
@@ -20,6 +20,10 @@ BluetoothDiscoverySession::~BluetoothDiscoverySession() {
   adapter_->DiscoverySessionDestroyed(this);
 }
 
+bool BluetoothDiscoverySession::IsActive() const {
+  return active_;
+}
+
 void BluetoothDiscoverySession::Stop(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
diff --git a/device/bluetooth/bluetooth_discovery_session.h b/device/bluetooth/bluetooth_discovery_session.h
index 72bdb30..bc2f893 100644
--- a/device/bluetooth/bluetooth_discovery_session.h
+++ b/device/bluetooth/bluetooth_discovery_session.h
@@ -36,7 +36,7 @@ class BluetoothDiscoverySession {
   // the call may not always succeed. To be notified of such failures,
   // users are highly encouraged to call BluetoothDiscoverySession::Stop,
   // instead of relying on the destructor.
-  ~BluetoothDiscoverySession();
+  virtual ~BluetoothDiscoverySession();
 
   // Returns true if the session is active, false otherwise. If false, the
   // adapter might still be discovering as there might still be other active
@@ -44,7 +44,7 @@ class BluetoothDiscoverySession {
   // whether or not discovery should continue. In this case, the application
   // should request a new BluetoothDiscoverySession to make sure that device
   // discovery continues.
-  bool active() const { return active_; }
+  virtual bool IsActive() const;
 
   // Requests this discovery session instance to stop. If this instance is
   // active, the session will stop. On success, |callback| is called and
@@ -54,13 +54,14 @@ class BluetoothDiscoverySession {
   // to call this method to end a discovery session, instead of relying on the
   // destructor, so that they can be notified of the result via the callback
   // arguments.
-  void Stop(const base::Closure& callback,
-            const ErrorCallback& error_callback);
+  virtual void Stop(const base::Closure& callback,
+                    const ErrorCallback& error_callback);
 
- private:
+ protected:
   friend class BluetoothAdapter;
   explicit BluetoothDiscoverySession(BluetoothAdapter* adapter);
 
+ private:
   // Internal callback invoked when a call to Stop has succeeded.
   void OnStop(const base::Closure& callback);