From 4cd6a2ad2db15ff6d5cc4dd030d30c30460df6a0 Mon Sep 17 00:00:00 2001 From: Jordan Demeulenaere Date: Thu, 7 Oct 2021 15:22:19 +0200 Subject: [PATCH] Fix SysUI crash when locking while showing the Media Output Group dialog This CL fixes a bug introduced by http://ag/15663086: When locking the phone while showing the Media Output Group dialog, SystemUI would crash because of a mismatch between MediaOutputAdapter.currentActivePosition and MediaOutputController.mMediaDevices. That mismatch came from the sharing of the controller between the 2 dialogs, that are now shown at the same time (even though one is hidden). Starting from this CL, both dialogs now use a separate instance of MediaOutputController to avoid this kind of race condition. This CL also adds a safeguard in MediaOutputBaseDialog to ensure that we notify an item change within bounds, which would otherwise throw and crash. Bug: 200781817 Test: Show media output group dialog then lock phone. Change-Id: I7749cc963a5e145467b32aa3a9efb1e80009f324 --- .../media/dialog/MediaOutputBaseDialog.java | 2 +- .../media/dialog/MediaOutputController.java | 30 +++++-------------- 2 files changed, 8 insertions(+), 24 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseDialog.java b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseDialog.java index 85d0802012acc..6895ef10fd07e 100644 --- a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseDialog.java +++ b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputBaseDialog.java @@ -175,7 +175,7 @@ public abstract class MediaOutputBaseDialog extends SystemUIDialog implements } if (!mAdapter.isDragging() && !mAdapter.isAnimating()) { int currentActivePosition = mAdapter.getCurrentActivePosition(); - if (currentActivePosition >= 0) { + if (currentActivePosition >= 0 && currentActivePosition < mAdapter.getItemCount()) { mAdapter.notifyItemChanged(currentActivePosition); } else { mAdapter.notifyDataSetChanged(); diff --git a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java index 437a0c85a6e01..42dd8862576b7 100644 --- a/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java +++ b/packages/SystemUI/src/com/android/systemui/media/dialog/MediaOutputController.java @@ -73,6 +73,7 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback { private final String mPackageName; private final Context mContext; private final MediaSessionManager mMediaSessionManager; + private final LocalBluetoothManager mLocalBluetoothManager; private final ShadeController mShadeController; private final ActivityStarter mActivityStarter; private final DialogLaunchAnimator mDialogLaunchAnimator; @@ -85,7 +86,6 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback { private MediaController mMediaController; @VisibleForTesting Callback mCallback; - Callback mPreviousCallback; @VisibleForTesting LocalMediaManager mLocalMediaManager; @@ -101,6 +101,7 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback { mContext = context; mPackageName = packageName; mMediaSessionManager = mediaSessionManager; + mLocalBluetoothManager = lbm; mShadeController = shadeController; mActivityStarter = starter; mAboveStatusbar = aboveStatusbar; @@ -135,19 +136,7 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback { } return; } - - if (mPreviousCallback != null) { - Log.w(TAG, - "Callback started when mPreviousCallback is not null, which is unexpected"); - mPreviousCallback.dismissDialog(); - } - - // If we start the output group dialog when the output dialog is shown, we need to keep a - // reference to the output dialog to set it back as the callback once we dismiss the output - // group dialog. - mPreviousCallback = mCallback; mCallback = cb; - mLocalMediaManager.unregisterCallback(this); mLocalMediaManager.stopScan(); mLocalMediaManager.registerCallback(this); @@ -163,15 +152,6 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback { mLocalMediaManager.stopScan(); } mMediaDevices.clear(); - - // If there was a previous callback, i.e. we just dismissed the output group dialog and are - // now back on the output dialog, then we reset the callback to its previous value. - mCallback = null; - Callback previous = mPreviousCallback; - mPreviousCallback = null; - if (previous != null) { - start(previous); - } } @Override @@ -480,7 +460,11 @@ public class MediaOutputController implements LocalMediaManager.DeviceCallback { void launchMediaOutputGroupDialog(View mediaOutputDialog) { // We show the output group dialog from the output dialog. - MediaOutputGroupDialog dialog = new MediaOutputGroupDialog(mContext, mAboveStatusbar, this); + MediaOutputController controller = new MediaOutputController(mContext, mPackageName, + mAboveStatusbar, mMediaSessionManager, mLocalBluetoothManager, mShadeController, + mActivityStarter, mNotificationEntryManager, mUiEventLogger, mDialogLaunchAnimator); + MediaOutputGroupDialog dialog = new MediaOutputGroupDialog(mContext, mAboveStatusbar, + controller); mDialogLaunchAnimator.showFromView(dialog, mediaOutputDialog); }