From 1deb027c9db509b9c4595e807f9bbc5a0da77026 Mon Sep 17 00:00:00 2001 From: Ritwika Mitra Date: Tue, 19 Nov 2019 08:37:52 -0800 Subject: Disconnect from MAP on onDestroy This CL fixes a couple of bugs: 1. When Messenger gets onDestroy we should call closeProfileProxy as that is what is causing ServiceConnection leaks 2. onServiceDisconnected is called when the connection is lost, not necessarily when the connection is closed so instead of calling BMC.close in this situation, we should only set BMC to null and not make any BMC API calls until onServiceConnected 3. Because of 2, trying to reconnect by calling getProfileProxy won't speed up getting the BMC, so we should stop doing that. 4. Sal mentioned most BT lifecycle logic relies on closeProfileProxy, not BMC.close, so I stopped calling that API 5. For readability's sake, I renamed the cleanup() methods to specifically onDestroy so readers can understand when this method is invoked. Change-Id: I7727df11664a67b809a2361289afbc4623877631 Fix: 142077620 Test: manual, force onDestroy --- .../android/car/messenger/MessengerDelegate.java | 19 ++--------- .../android/car/messenger/MessengerService.java | 4 +-- .../car/messenger/bluetooth/BluetoothMonitor.java | 37 ++++++++-------------- 3 files changed, 18 insertions(+), 42 deletions(-) (limited to 'src') diff --git a/src/com/android/car/messenger/MessengerDelegate.java b/src/com/android/car/messenger/MessengerDelegate.java index 5a3d2e0..4662625 100644 --- a/src/com/android/car/messenger/MessengerDelegate.java +++ b/src/com/android/car/messenger/MessengerDelegate.java @@ -55,7 +55,7 @@ public class MessengerDelegate implements BluetoothMonitor.OnBluetoothEventListe private final Context mContext; @GuardedBy("mMapClientLock") private BluetoothMapClient mBluetoothMapClient; - private NotificationManager mNotificationManager; + private final NotificationManager mNotificationManager; private final SmsDatabaseHandler mSmsDatabaseHandler; private boolean mShouldLoadExistingMessages; @@ -142,10 +142,6 @@ public class MessengerDelegate implements BluetoothMonitor.OnBluetoothEventListe return; } - if (mBluetoothMapClient != null) { - mBluetoothMapClient.close(); - } - mBluetoothMapClient = client; connectedDevices = mBluetoothMapClient.getConnectedDevices(); } @@ -157,14 +153,10 @@ public class MessengerDelegate implements BluetoothMonitor.OnBluetoothEventListe } @Override - public void onMapDisconnected(int profile) { + public void onMapDisconnected() { L.d(TAG, "Disconnected from BluetoothMapClient"); cleanupMessagesAndNotifications(key -> true); synchronized (mMapClientLock) { - BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter(); - if (adapter != null) { - adapter.closeProfileProxy(BluetoothProfile.MAP_CLIENT, mBluetoothMapClient); - } mBluetoothMapClient = null; } } @@ -318,13 +310,8 @@ public class MessengerDelegate implements BluetoothMonitor.OnBluetoothEventListe return 0; } - protected void cleanup() { + protected void onDestroy() { cleanupMessagesAndNotifications(key -> true); - synchronized (mMapClientLock) { - if (mBluetoothMapClient != null) { - mBluetoothMapClient.close(); - } - } } private Notification createNotification( diff --git a/src/com/android/car/messenger/MessengerService.java b/src/com/android/car/messenger/MessengerService.java index 3ea681f..0b0e4e4 100644 --- a/src/com/android/car/messenger/MessengerService.java +++ b/src/com/android/car/messenger/MessengerService.java @@ -136,8 +136,8 @@ public class MessengerService extends Service { public void onDestroy() { super.onDestroy(); L.d(TAG, "onDestroy"); - mMessengerDelegate.cleanup(); - mBluetoothMonitor.cleanup(); + mMessengerDelegate.onDestroy(); + mBluetoothMonitor.onDestroy(); } @Override diff --git a/src/com/android/car/messenger/bluetooth/BluetoothMonitor.java b/src/com/android/car/messenger/bluetooth/BluetoothMonitor.java index 0015ebc..095474f 100644 --- a/src/com/android/car/messenger/bluetooth/BluetoothMonitor.java +++ b/src/com/android/car/messenger/bluetooth/BluetoothMonitor.java @@ -9,15 +9,10 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.content.res.Resources.NotFoundException; import android.os.Parcelable; - import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; - -import com.android.car.messenger.R; import com.android.car.messenger.log.L; - import java.util.HashSet; import java.util.Set; @@ -33,12 +28,12 @@ public class BluetoothMonitor { private final BluetoothSdpReceiver mBluetoothSdpReceiver; private final MapDeviceMonitor mMapDeviceMonitor; private final BluetoothProfile.ServiceListener mMapServiceListener; + private BluetoothMapClient mBluetoothMapClient; private final Set mListeners; public BluetoothMonitor(@NonNull Context context) { mContext = context; - mBluetoothMapReceiver = new BluetoothMapReceiver(); mBluetoothSdpReceiver = new BluetoothSdpReceiver(); mMapDeviceMonitor = new MapDeviceMonitor(); @@ -52,7 +47,7 @@ public class BluetoothMonitor { @Override public void onServiceDisconnected(int profile) { L.d(TAG, "Disconnected from MAP service!"); - onMapDisconnected(profile); + onMapDisconnected(); } }; mListeners = new HashSet<>(); @@ -119,10 +114,8 @@ public class BluetoothMonitor { /** * Callback issued when a MAP client has been disconnected. - * - * @param profile see {@link BluetoothProfile.ServiceListener#onServiceDisconnected(int)} */ - void onMapDisconnected(int profile); + void onMapDisconnected(); /** * Callback issued when a new SDP record has been detected. @@ -150,22 +143,13 @@ public class BluetoothMonitor { } private void onMapConnected(BluetoothMapClient client) { + mBluetoothMapClient = client; mListeners.forEach(listener -> listener.onMapConnected(client)); } - private void onMapDisconnected(int profile) { - mListeners.forEach(listener -> listener.onMapDisconnected(profile)); - boolean shouldReconnectToMap = false; - try { - shouldReconnectToMap = mContext.getResources().getBoolean( - R.bool.config_loadExistingMessages); - } catch (NotFoundException e) { - // Should only happen for robolectric unit tests - L.e(TAG, e, "Could not find loadExistingMessages config"); - } - if (shouldReconnectToMap) { - connectToMap(); - } + private void onMapDisconnected() { + mBluetoothMapClient = null; + mListeners.forEach(listener -> listener.onMapDisconnected()); } private void onSdpRecord(BluetoothDevice device, boolean supportsReply) { @@ -193,7 +177,12 @@ public class BluetoothMonitor { /** * Performs {@link Context} related cleanup (such as unregistering from receivers). */ - public void cleanup() { + public void onDestroy() { + BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter(); + if (adapter != null) { + adapter.closeProfileProxy(BluetoothProfile.MAP_CLIENT, mBluetoothMapClient); + } + onMapDisconnected(); mListeners.clear(); mBluetoothMapReceiver.unregisterReceivers(); mBluetoothSdpReceiver.unregisterReceivers(); -- cgit v1.2.3