Run the AVRCP Service interface functions on the BTA thread

Bug: 80416347
Test: Turn off Bluetooth while connected to a device we initiated the
connection to.

Change-Id: I8f10409c495213ef3117aedf66919de7c0b3d164
(cherry picked from commit 95e1a9e65f2d61523753f19d8855144672413506)

diff --git a/btif/avrcp/avrcp_service.cc b/btif/avrcp/avrcp_service.cc
index 82ede13..186866b 100644 (file)
--- a/btif/avrcp/avrcp_service.cc
+++ b/btif/avrcp/avrcp_service.cc
@@ -282,19 +282,11 @@ class VolumeInterfaceWrapper : public VolumeInterface {
 void AvrcpService::Init(MediaInterface* media_interface,
                         VolumeInterface* volume_interface) {
   LOG(INFO) << "AVRCP Target Service started";
-  if (instance_ == nullptr) {
-    instance_ = new AvrcpService();
-  }
-
-  {
-    std::lock_guard<std::mutex> lock(jni_mutex_);
-    jni_message_loop_ = get_jni_message_loop();
-  }
 
   // TODO (apanicke): Add a function that sets up the SDP records once we
   // remove the AVRCP SDP setup in AVDTP (bta_av_main.cc)
 
-  instance_->media_interface_ = new MediaInterfaceWrapper(media_interface);
+  media_interface_ = new MediaInterfaceWrapper(media_interface);
   media_interface->RegisterUpdateCallback(instance_);
 
   VolumeInterfaceWrapper* wrapped_volume_interface = nullptr;
@@ -302,28 +294,23 @@ void AvrcpService::Init(MediaInterface* media_interface,
     wrapped_volume_interface = new VolumeInterfaceWrapper(volume_interface);
   }
 
-  instance_->volume_interface_ = wrapped_volume_interface;
+  volume_interface_ = wrapped_volume_interface;
 
   ConnectionHandler::Initialize(
       base::Bind(&AvrcpService::DeviceCallback, base::Unretained(instance_)),
       &avrcp_interface_, &sdp_interface_, wrapped_volume_interface);
-  instance_->connection_handler_ = ConnectionHandler::Get();
+  connection_handler_ = ConnectionHandler::Get();
 }
 
 void AvrcpService::Cleanup() {
   LOG(INFO) << "AVRCP Target Service stopped";
-  {
-    std::lock_guard<std::mutex> lock(jni_mutex_);
-    task_tracker_.TryCancelAll();
-    jni_message_loop_ = nullptr;
-  }
 
-  instance_->connection_handler_->CleanUp();
-  instance_->connection_handler_ = nullptr;
-  if (instance_->volume_interface_ != nullptr) {
-    delete instance_->volume_interface_;
+  connection_handler_->CleanUp();
+  connection_handler_ = nullptr;
+  if (volume_interface_ != nullptr) {
+    delete volume_interface_;
   }
-  delete instance_->media_interface_;
+  delete media_interface_;
 }
 
 AvrcpService* AvrcpService::Get() {
@@ -339,15 +326,15 @@ ServiceInterface* AvrcpService::GetServiceInterface() {
   return service_interface_;
 }
 
-bool AvrcpService::ConnectDevice(const RawAddress& bdaddr) {
+void AvrcpService::ConnectDevice(const RawAddress& bdaddr) {
   LOG(INFO) << __PRETTY_FUNCTION__ << ": address=" << bdaddr.ToString();
 
-  return connection_handler_->ConnectDevice(bdaddr);
+  connection_handler_->ConnectDevice(bdaddr);
 }
 
-bool AvrcpService::DisconnectDevice(const RawAddress& bdaddr) {
+void AvrcpService::DisconnectDevice(const RawAddress& bdaddr) {
   LOG(INFO) << __PRETTY_FUNCTION__ << ": address=" << bdaddr.ToString();
-  return connection_handler_->DisconnectDevice(bdaddr);
+  connection_handler_->DisconnectDevice(bdaddr);
 }
 
 void AvrcpService::SendMediaUpdate(bool track_changed, bool play_state,
@@ -398,37 +385,62 @@ void AvrcpService::DeviceCallback(std::shared_ptr<Device> new_device) {
 
 // Service Interface
 void AvrcpService::ServiceInterfaceImpl::Init(
-    MediaInterface* mediaInterface, VolumeInterface* volume_interface) {
-  // TODO (apanicke): Run this in a message loop. This currently works though
-  // since the underlying AVRC_API will correctly run this in a message loop so
-  // no race conditions can occur, but if that changes it could change the
-  // behaviour here.
+    MediaInterface* media_interface, VolumeInterface* volume_interface) {
+  std::lock_guard<std::mutex> lock(service_interface_lock_);
+
+  // TODO: This function should block until the service is completely up so
+  // that its possible to call Get() on the service immediately after calling
+  // init without issues.
+
   CHECK(instance_ == nullptr);
+  instance_ = new AvrcpService();
+
+  {
+    std::lock_guard<std::mutex> jni_lock(jni_mutex_);
+    jni_message_loop_ = get_jni_message_loop();
+  }
 
-  AvrcpService::Init(mediaInterface, volume_interface);
+  do_in_bta_thread(FROM_HERE,
+                   base::Bind(&AvrcpService::Init, base::Unretained(instance_),
+                              media_interface, volume_interface));
 }
 
 bool AvrcpService::ServiceInterfaceImpl::ConnectDevice(
     const RawAddress& bdaddr) {
-  // TODO (apanicke): Same as above
+  std::lock_guard<std::mutex> lock(service_interface_lock_);
   CHECK(instance_ != nullptr);
-  return instance_->ConnectDevice(bdaddr);
+  do_in_bta_thread(FROM_HERE, base::Bind(&AvrcpService::ConnectDevice,
+                                         base::Unretained(instance_), bdaddr));
+  return true;
 }
 
 bool AvrcpService::ServiceInterfaceImpl::DisconnectDevice(
     const RawAddress& bdaddr) {
-  // TODO (apanicke): Same as above
+  std::lock_guard<std::mutex> lock(service_interface_lock_);
   CHECK(instance_ != nullptr);
-  return instance_->DisconnectDevice(bdaddr);
+  do_in_bta_thread(FROM_HERE, base::Bind(&AvrcpService::DisconnectDevice,
+                                         base::Unretained(instance_), bdaddr));
+  return true;
 }
 
 bool AvrcpService::ServiceInterfaceImpl::Cleanup() {
-  // TODO (apanicke): Same as above
+  std::lock_guard<std::mutex> lock(service_interface_lock_);
+
   if (instance_ == nullptr) return false;
 
-  instance_->Cleanup();
-  delete instance_;
+  {
+    std::lock_guard<std::mutex> jni_lock(jni_mutex_);
+    task_tracker_.TryCancelAll();
+    jni_message_loop_ = nullptr;
+  }
+
+  do_in_bta_thread(FROM_HERE,
+                   base::Bind(&AvrcpService::Cleanup, base::Owned(instance_)));
+
+  // Setting instance to nullptr here is fine since it will be deleted on the
+  // other thread.
   instance_ = nullptr;
+
   return true;
 }
 

diff --git a/btif/avrcp/avrcp_service.h b/btif/avrcp/avrcp_service.h
index 5f8fa2d..e19899d 100644 (file)
--- a/btif/avrcp/avrcp_service.h
+++ b/btif/avrcp/avrcp_service.h
@@ -37,9 +37,6 @@ namespace avrcp {
  */
 class AvrcpService : public MediaCallbacks {
  public:
-  static void Init(MediaInterface* media_interface,
-                   VolumeInterface* volume_interface);
-
   /**
    * Gets a handle to the AvrcpService
    *
@@ -56,10 +53,11 @@ class AvrcpService : public MediaCallbacks {
    */
   static ServiceInterface* GetServiceInterface();
 
+  void Init(MediaInterface* media_interface, VolumeInterface* volume_interface);
   void Cleanup();
 
-  bool ConnectDevice(const RawAddress& bdaddr);
-  bool DisconnectDevice(const RawAddress& bdaddr);
+  void ConnectDevice(const RawAddress& bdaddr);
+  void DisconnectDevice(const RawAddress& bdaddr);
 
   // Functions inherited from MediaCallbacks in order to receive updates
   void SendMediaUpdate(bool track_changed, bool play_state,
@@ -75,6 +73,9 @@ class AvrcpService : public MediaCallbacks {
     bool ConnectDevice(const RawAddress& bdaddr) override;
     bool DisconnectDevice(const RawAddress& bdaddr) override;
     bool Cleanup() override;
+
+   private:
+    std::mutex service_interface_lock_;
   };
 
   static void DebugDump(int fd);