From bd589528f6d6dcbbb606f2d7307f2893613b1392 Mon Sep 17 00:00:00 2001
From: Ajay Panicker <apanicke@google.com>
Date: Fri, 14 Dec 2018 14:55:02 -0800
Subject: [PATCH] DO NOT MERGE: Use a weak pointer to deliver updates to AVRCP
 devices.

If a device disconnects right before a update message gets queued, the
device becomes null and there is a crash when the callback for the
update executes on the disconnected device. This patch switches the
device reference from being Unretained to using a weak pointer so that
the callback just doesn't execute if the device is disconnected.

Bug: 120431125
Bug: 120445479
Test: Use the same test as b/120477414 as that bug causes a disconnect
at the same time as a media update.

Change-Id: I1dcc08e5c9866106e7ec0dad52505e34b42da600
(cherry picked from commit f083d1e076ea97e6feaa363f03dab3656bd03ee0)
---
 btif/avrcp/avrcp_service.cc | 19 ++++++++++---------
 profile/avrcp/device.cc     |  2 ++
 profile/avrcp/device.h      |  6 ++++++
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/btif/avrcp/avrcp_service.cc b/btif/avrcp/avrcp_service.cc
index 8c22365b2..5a058f4af 100644
--- a/btif/avrcp/avrcp_service.cc
+++ b/btif/avrcp/avrcp_service.cc
@@ -346,10 +346,11 @@ void AvrcpService::SendMediaUpdate(bool track_changed, bool play_state,
 
   // This function may be called on any thread, we need to make sure that the
   // device update happens on the main thread.
-  for (auto device : instance_->connection_handler_->GetListOfDevices()) {
-    do_in_bta_thread(FROM_HERE, base::Bind(&Device::SendMediaUpdate,
-                                           base::Unretained(device.get()),
-                                           track_changed, play_state, queue));
+  for (const auto& device :
+       instance_->connection_handler_->GetListOfDevices()) {
+    do_in_bta_thread(FROM_HERE,
+                     base::Bind(&Device::SendMediaUpdate, device.get()->Get(),
+                                track_changed, play_state, queue));
   }
 }
 
@@ -361,11 +362,11 @@ void AvrcpService::SendFolderUpdate(bool available_players,
             << " uids=" << uids;
 
   // Ensure that the update is posted to the correct thread
-  for (auto device : instance_->connection_handler_->GetListOfDevices()) {
-    do_in_bta_thread(
-        FROM_HERE,
-        base::Bind(&Device::SendFolderUpdate, base::Unretained(device.get()),
-                   available_players, addressed_players, uids));
+  for (const auto& device :
+       instance_->connection_handler_->GetListOfDevices()) {
+    do_in_bta_thread(FROM_HERE,
+                     base::Bind(&Device::SendFolderUpdate, device.get()->Get(),
+                                available_players, addressed_players, uids));
   }
 }
 
diff --git a/profile/avrcp/device.cc b/profile/avrcp/device.cc
index d2a985f98..58effc3f7 100644
--- a/profile/avrcp/device.cc
+++ b/profile/avrcp/device.cc
@@ -52,6 +52,8 @@ void Device::RegisterInterfaces(MediaInterface* media_interface,
   volume_interface_ = volume_interface;
 }
 
+base::WeakPtr<Device> Device::Get() { return weak_ptr_factory_.GetWeakPtr(); }
+
 bool Device::IsActive() const {
   return address_ == a2dp_interface_->active_peer();
 }
diff --git a/profile/avrcp/device.h b/profile/avrcp/device.h
index 27f7674d9..f939ce6b6 100644
--- a/profile/avrcp/device.h
+++ b/profile/avrcp/device.h
@@ -55,6 +55,12 @@ class Device {
       uint16_t ctrl_mtu, uint16_t browse_mtu);
   virtual ~Device() = default;
 
+  /**
+   * Gets a weak pointer to this device that is invalidated when the device is
+   * disconnected.
+   */
+  base::WeakPtr<Device> Get();
+
   const RawAddress& GetAddress() const { return address_; };
 
   /**
-- 
2.11.0