[chromecast][BLE] Treat ReadRemoteRssi as failed when it times out

Bug: 112903714
Test: cast_bluetooth_unittests.
Change-Id: Iff20eeb9f2bb2c4b3aec4428eecc3a84980ed190
Reviewed-on: https://chromium-review.googlesource.com/1185525
Commit-Queue: Tiansong Cui <tiansong@google.com>
Reviewed-by: Bailey Forrest <bcf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585246}

chromecast/device/bluetooth/le/gatt_client_manager_impl.cc

@@ -47,6 +47,7 @@ namespace {
 
 // static
 constexpr base::TimeDelta GattClientManagerImpl::kConnectTimeout;
+constexpr base::TimeDelta GattClientManagerImpl::kReadRemoteRssiTimeout;
 
 GattClientManagerImpl::GattClientManagerImpl(
     bluetooth_v2_shlib::GattClient* gatt_client)
@@ -183,6 +184,7 @@ void GattClientManagerImpl::OnConnectChanged(
     }
 
     base::Erase(pending_read_remote_rssi_requests_, addr);
+    read_remote_rssi_timeout_timer_.Stop();
   }
 
   // We won't declare the device connected until service discovery completes.
@@ -259,17 +261,21 @@ void GattClientManagerImpl::OnReadRemoteRssi(
     bool status,
     int rssi) {
   MAKE_SURE_IO_THREAD(OnReadRemoteRssi, addr, status, rssi);
+
   auto it = addr_to_device_.find(addr);
   CHECK_DEVICE_EXISTS_IT(it);
   it->second->OnReadRemoteRssiComplete(status, rssi);
 
   if (pending_read_remote_rssi_requests_.empty() ||
       addr != pending_read_remote_rssi_requests_.front()) {
-    NOTREACHED() << "Unexpected call to " << __func__;
+    // This can happen when the regular OnReadRemoteRssi is received after
+    // ReadRemoteRssi timed out.
+    LOG(ERROR) << "Unexpected call to " << __func__;
     return;
   }
 
   pending_read_remote_rssi_requests_.pop_front();
+  read_remote_rssi_timeout_timer_.Stop();
   // Try to run the next ReadRemoteRssi request
   RunQueuedReadRemoteRssiRequest();
 }
@@ -391,14 +397,18 @@ void GattClientManagerImpl::RunQueuedReadRemoteRssiRequest() {
 
     addr = pending_read_remote_rssi_requests_.front();
   }
+
+  read_remote_rssi_timeout_timer_.Start(
+      FROM_HERE, kReadRemoteRssiTimeout,
+      base::BindRepeating(&GattClientManagerImpl::OnReadRemoteRssiTimeout,
+                          weak_this_, addr));
 }
 
 void GattClientManagerImpl::OnConnectTimeout(
     const bluetooth_v2_shlib::Addr& addr) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
   // Get the last byte because whole address is PII.
-  std::string addr_str = util::AddrToString(addr);
-  addr_str = addr_str.substr(addr_str.size() - 2);
+  std::string addr_str = util::AddrLastByteString(addr);
 
   LOG(ERROR) << "Connect (" << addr_str << ")"
              << " timed out. Disconnecting";
@@ -413,6 +423,19 @@ void GattClientManagerImpl::OnConnectTimeout(
   }
 }
 
+void GattClientManagerImpl::OnReadRemoteRssiTimeout(
+    const bluetooth_v2_shlib::Addr& addr) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  // Get the last byte because whole address is PII.
+  std::string addr_str = util::AddrLastByteString(addr);
+
+  LOG(ERROR) << "ReadRemoteRssi (" << addr_str << ")"
+             << " timed out.";
+
+  // ReadRemoteRssi times out before OnReadRemoteRssi is received.
+  RUN_ON_IO_THREAD(OnReadRemoteRssi, addr, false /* status */, 0 /* rssi */);
+}
+
 // static
 void GattClientManagerImpl::FinalizeOnIoThread(
     std::unique_ptr<base::WeakPtrFactory<GattClientManagerImpl>> weak_factory) {

chromecast/device/bluetooth/le/gatt_client_manager_impl.h

@@ -26,9 +26,13 @@ class GattClientManagerImpl
       public bluetooth_v2_shlib::Gatt::Client::Delegate {
  public:
   // If a Connect request takes longer than this amount of time, we will treat
-  // it as a Connect failure.
+  // it as a failure.
   static constexpr base::TimeDelta kConnectTimeout =
       base::TimeDelta::FromSeconds(40);
+  // If a ReadRemoteRssi request takes longer than this amount of time, we will
+  // treat it as a failure.
+  static constexpr base::TimeDelta kReadRemoteRssiTimeout =
+      base::TimeDelta::FromSeconds(10);
 
   explicit GattClientManagerImpl(bluetooth_v2_shlib::GattClient* gatt_client);
   ~GattClientManagerImpl() override;
@@ -102,6 +106,7 @@ class GattClientManagerImpl
   void RunQueuedReadRemoteRssiRequest();
 
   void OnConnectTimeout(const bluetooth_v2_shlib::Addr& addr);
+  void OnReadRemoteRssiTimeout(const bluetooth_v2_shlib::Addr& addr);
 
   static void FinalizeOnIoThread(
       std::unique_ptr<base::WeakPtrFactory<GattClientManagerImpl>>
@@ -121,9 +126,13 @@ class GattClientManagerImpl
   std::set<bluetooth_v2_shlib::Addr> connected_devices_;
 
   // Timer for pending Connect requests. If any Connect request times out, we
-  // will treat it as a Connect failure.
+  // will treat it as a failure.
   base::OneShotTimer connect_timeout_timer_;
 
+  // Timer for pending ReadRemoteRssi requests. If any ReadRemoteRssi request
+  // times out, we will treat it as a failure.
+  base::OneShotTimer read_remote_rssi_timeout_timer_;
+
   // Queue for concurrent Connect requests.
   std::deque<bluetooth_v2_shlib::Addr> pending_connect_requests_;
 

chromecast/device/bluetooth/le/gatt_client_manager_impl_test.cc

@@ -366,6 +366,34 @@ TEST_F(GattClientManagerTest, RemoteDeviceReadRssi) {
   delegate->OnReadRemoteRssi(kTestAddr1, true /* status */, kRssi);
 }
 
+TEST_F(GattClientManagerTest, ReadRemoteRssiTimeout) {
+  static const int kRssi = -34;
+
+  bluetooth_v2_shlib::Gatt::Client::Delegate* delegate =
+      gatt_client_->delegate();
+  scoped_refptr<RemoteDevice> device = GetDevice(kTestAddr1);
+
+  Connect(kTestAddr1);
+
+  // Issue a ReadRemoteRssi request.
+  base::MockCallback<RemoteDevice::RssiCallback> rssi_cb;
+  EXPECT_CALL(*gatt_client_, ReadRemoteRssi(kTestAddr1)).WillOnce(Return(true));
+  device->ReadRemoteRssi(rssi_cb.Get());
+
+  // Let ReadRemoteRssi request timeout.
+  base::TestMockTimeTaskRunner::ScopedContext context(fake_task_runner_);
+  // We should expect to receive ReadRemoteRssi failure message.
+  EXPECT_CALL(rssi_cb, Run(false, 0));
+  fake_task_runner_->FastForwardBy(
+      GattClientManagerImpl::kReadRemoteRssiTimeout);
+
+  // The following callback should be ignored.
+  delegate->OnReadRemoteRssi(kTestAddr1, true /* status */, kRssi);
+
+  // Device should remain connected.
+  EXPECT_TRUE(device->IsConnected());
+}
+
 TEST_F(GattClientManagerTest, RemoteDeviceReadRssiConcurrent) {
   static const int kRssi1 = -34;
   static const int kRssi3 = -68;