[chromecast] gatt_client: Fix Connect callback GetServices issue

Also fix unittest from last CL to test the desired code path.

BUG=internal b/110058717
TEST=cast_bluetooth_unittests, manual

Change-Id: Ib54c0627d90e7fb589e131e009298c362d25dbfd
Reviewed-on: https://chromium-review.googlesource.com/1096614
Commit-Queue: Bailey Forrest <bcf@chromium.org>
Reviewed-by: Stephen Lanham <slan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566623}

chromecast/device/bluetooth/le/gatt_client_manager_impl_test.cc

@@ -101,7 +101,7 @@ std::vector<bluetooth_v2_shlib::Gatt::Service> GenerateServices() {
 }
 
 class GattClientManagerTest : public ::testing::Test {
- protected:
+ public:
   void SetUp() override {
     message_loop_ =
         std::make_unique<base::MessageLoop>(base::MessageLoop::TYPE_DEFAULT);
@@ -606,15 +606,53 @@ TEST_F(GattClientManagerTest, ConnectMultiple) {
 }
 
 TEST_F(GattClientManagerTest, GetServicesFailOnConnect) {
-  EXPECT_CALL(cb_, Run(false));
   scoped_refptr<RemoteDevice> device = GetDevice(kTestAddr1);
+  EXPECT_CALL(*gatt_client_, Connect(kTestAddr1)).WillOnce(Return(true));
   device->Connect(cb_.Get());
   bluetooth_v2_shlib::Gatt::Client::Delegate* delegate =
       gatt_client_->delegate();
+
+  EXPECT_CALL(cb_, Run(false));
   EXPECT_CALL(*gatt_client_, GetServices(kTestAddr1)).WillOnce(Return(false));
   delegate->OnConnectChanged(kTestAddr1, true /* status */,
                              true /* connected */);
-  ASSERT_FALSE(device->IsConnected());
+  EXPECT_FALSE(device->IsConnected());
+}
+
+TEST_F(GattClientManagerTest, GetServicesSuccessAfterConnectCallback) {
+  const auto kServices = GenerateServices();
+  scoped_refptr<RemoteDevice> device = GetDevice(kTestAddr1);
+
+  // Callback that checks when Connect()'s callback returns, GetServices returns
+  // the correct services.
+  bool cb_called = false;
+  auto cb = base::BindOnce(
+      [](GattClientManagerTest* gcmt,
+         const std::vector<bluetooth_v2_shlib::Gatt::Service>*
+             expected_services,
+         bool* cb_called, bool success) {
+        EXPECT_TRUE(success);
+        *cb_called = true;
+
+        auto device = gcmt->GetDevice(kTestAddr1);
+        auto services = gcmt->GetServices(device.get());
+        EXPECT_EQ(expected_services->size(), services.size());
+      },
+      this, &kServices, &cb_called);
+  EXPECT_CALL(*gatt_client_, Connect(kTestAddr1)).WillOnce(Return(true));
+  device->Connect(std::move(cb));
+
+  bluetooth_v2_shlib::Gatt::Client::Delegate* delegate =
+      gatt_client_->delegate();
+  EXPECT_CALL(*gatt_client_, GetServices(kTestAddr1)).WillOnce(Return(true));
+  delegate->OnConnectChanged(kTestAddr1, true /* status */,
+                             true /* connected */);
+
+  // Connect's callback should not be called until service discovery is
+  // complete.
+  EXPECT_FALSE(cb_called);
+  delegate->OnGetServices(kTestAddr1, kServices);
+  EXPECT_TRUE(cb_called);
 }
 
 }  // namespace bluetooth

chromecast/device/bluetooth/le/remote_device_impl.cc

@@ -231,15 +231,11 @@ scoped_refptr<RemoteService> RemoteDeviceImpl::GetServiceByUuidSync(
 
 void RemoteDeviceImpl::SetConnected(bool connected) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
-  // We only set connected = true after services are discovered.
+  // We only set connected = true and call the callback after services are
+  // discovered.
   if (!connected) {
     connected_ = false;
-  }
-  if (connect_pending_) {
-    connect_pending_ = false;
-    if (connect_cb_) {
-      std::move(connect_cb_).Run(connected);
-    }
+    ConnectComplete(false);
   }
 
   if (disconnect_pending_) {
@@ -287,9 +283,7 @@ void RemoteDeviceImpl::SetConnected(bool connected) {
     if (!gatt_client_manager_->gatt_client()->GetServices(addr_)) {
       LOG(ERROR) << "Couldn't discover services, disconnecting";
       Disconnect({});
-      if (connect_cb_) {
-        std::move(connect_cb_).Run(false);
-      }
+      ConnectComplete(false);
     }
   } else {
     uuid_to_service_.clear();
@@ -305,9 +299,7 @@ void RemoteDeviceImpl::SetServicesDiscovered(bool discovered) {
     return;
   }
   connected_ = true;
-  if (connect_cb_) {
-    std::move(connect_cb_).Run(true);
-  }
+  ConnectComplete(true);
 }
 
 bool RemoteDeviceImpl::GetServicesDiscovered() {
@@ -400,5 +392,15 @@ void RemoteDeviceImpl::OnReadRemoteRssiComplete(bool status, int rssi) {
   }
 }
 
+void RemoteDeviceImpl::ConnectComplete(bool success) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  if (connect_pending_) {
+    connect_pending_ = false;
+    if (connect_cb_) {
+      std::move(connect_cb_).Run(success);
+    }
+  }
+}
+
 }  // namespace bluetooth
 }  // namespace chromecast

chromecast/device/bluetooth/le/remote_device_impl.h

@@ -73,6 +73,8 @@ class RemoteDeviceImpl : public RemoteDevice {
   void OnReadRemoteRssiComplete(bool status, int rssi);
   // end Friend methods for GattClientManagerImpl
 
+  void ConnectComplete(bool success);
+
   const base::WeakPtr<GattClientManagerImpl> gatt_client_manager_;
   const bluetooth_v2_shlib::Addr addr_;