NotificationMediaManager is bad signal for resumption
Entering resumption on STATE_NONE from NotificationMediaManager
causes flicker because it sends STATE_NONE to often. Instead, we can
listen for PlaybackState changes from the MediaController directly.
For cast, we need to enter the resumption state on STATE_CONNECTING,
because that is the resulting state after ending a cast session.
Fixes: 153937833
Test: manual - play music and toggle play / pause. Look for flicker
caused by action buttons rapidly switching in and out of the
resumption state.
Test: manual - cast from YT to TV. End cast session. Verify that player
is in the resumption state.
Change-Id: Ie7f577d1b06719a1ce2ae5927b2ee20745076303
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
index 62efd8c..e5c0bf7 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
@@ -55,8 +55,6 @@
import com.android.systemui.Dependency;
import com.android.systemui.R;
import com.android.systemui.plugins.ActivityStarter;
-import com.android.systemui.statusbar.NotificationMediaManager;
-import com.android.systemui.statusbar.NotificationMediaManager.MediaListener;
import com.android.systemui.util.Assert;
import java.util.List;
@@ -67,7 +65,6 @@
*/
public class MediaControlPanel {
private static final String TAG = "MediaControlPanel";
- private final NotificationMediaManager mMediaManager;
@Nullable private final LocalMediaManager mLocalMediaManager;
private final Executor mForegroundExecutor;
private final Executor mBackgroundExecutor;
@@ -102,12 +99,15 @@
clearControls();
makeInactive();
}
- };
-
- private final MediaListener mMediaListener = new MediaListener() {
@Override
- public void onMetadataOrStateChanged(MediaMetadata metadata, int state) {
- if (state == PlaybackState.STATE_NONE) {
+ public void onPlaybackStateChanged(PlaybackState state) {
+ final int s = state != null ? state.getState() : PlaybackState.STATE_NONE;
+ // When the playback state is NONE or CONNECTING, transition the player to the
+ // resumption state. State CONNECTING needs to be considered for Cast sessions. Ending
+ // a cast session in YT results in the CONNECTING state, which makes sense if you
+ // thinking of the session as waiting to connect to another cast device.
+ if (s == PlaybackState.STATE_NONE || s == PlaybackState.STATE_CONNECTING) {
+ Log.d(TAG, "playback state change will trigger resumption, state=" + state);
clearControls();
makeInactive();
}
@@ -160,7 +160,7 @@
* @param foregroundExecutor foreground executor
* @param backgroundExecutor background executor, used for processing artwork
*/
- public MediaControlPanel(Context context, ViewGroup parent, NotificationMediaManager manager,
+ public MediaControlPanel(Context context, ViewGroup parent,
@Nullable LocalMediaManager routeManager, @LayoutRes int layoutId, int[] actionIds,
Executor foregroundExecutor, Executor backgroundExecutor) {
mContext = context;
@@ -172,7 +172,6 @@
// attach/detach of views instead of inflating them in the constructor, which would allow
// mStateListener to be unregistered in detach.
mMediaNotifView.addOnAttachStateChangeListener(mStateListener);
- mMediaManager = manager;
mLocalMediaManager = routeManager;
mActionIds = actionIds;
mForegroundExecutor = foregroundExecutor;
@@ -443,6 +442,7 @@
* Put controls into a resumption state
*/
public void clearControls() {
+ Log.d(TAG, "clearControls to resumption state package=" + getMediaPlayerPackage());
// Hide all the old buttons
for (int i = 0; i < mActionIds.length; i++) {
ImageButton thisBtn = mMediaNotifView.findViewById(mActionIds[i]);
@@ -485,7 +485,6 @@
private void makeActive() {
Assert.isMainThread();
if (!mIsRegistered) {
- mMediaManager.addCallback(mMediaListener);
if (mLocalMediaManager != null) {
mLocalMediaManager.registerCallback(mDeviceCallback);
mLocalMediaManager.startScan();
@@ -501,7 +500,6 @@
mLocalMediaManager.stopScan();
mLocalMediaManager.unregisterCallback(mDeviceCallback);
}
- mMediaManager.removeCallback(mMediaListener);
mIsRegistered = false;
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSMediaPlayer.java b/packages/SystemUI/src/com/android/systemui/qs/QSMediaPlayer.java
index e636707..6da24d2 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/QSMediaPlayer.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/QSMediaPlayer.java
@@ -39,7 +39,6 @@
import com.android.systemui.media.MediaControlPanel;
import com.android.systemui.media.SeekBarObserver;
import com.android.systemui.media.SeekBarViewModel;
-import com.android.systemui.statusbar.NotificationMediaManager;
import com.android.systemui.util.concurrency.DelayableExecutor;
import java.util.concurrent.Executor;
@@ -69,14 +68,13 @@
* Initialize quick shade version of player
* @param context
* @param parent
- * @param manager
+ * @param routeManager Provides information about device
* @param foregroundExecutor
* @param backgroundExecutor
*/
- public QSMediaPlayer(Context context, ViewGroup parent, NotificationMediaManager manager,
- LocalMediaManager routeManager, Executor foregroundExecutor,
- DelayableExecutor backgroundExecutor) {
- super(context, parent, manager, routeManager, R.layout.qs_media_panel, QS_ACTION_IDS,
+ public QSMediaPlayer(Context context, ViewGroup parent, LocalMediaManager routeManager,
+ Executor foregroundExecutor, DelayableExecutor backgroundExecutor) {
+ super(context, parent, routeManager, R.layout.qs_media_panel, QS_ACTION_IDS,
foregroundExecutor, backgroundExecutor);
mParent = (QSPanel) parent;
mBackgroundExecutor = backgroundExecutor;
diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java b/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java
index 0566b2e..80c6ccd 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java
@@ -63,7 +63,6 @@
import com.android.systemui.qs.logging.QSLogger;
import com.android.systemui.settings.BrightnessController;
import com.android.systemui.settings.ToggleSliderView;
-import com.android.systemui.statusbar.NotificationMediaManager;
import com.android.systemui.statusbar.policy.BrightnessMirrorController;
import com.android.systemui.statusbar.policy.BrightnessMirrorController.BrightnessMirrorListener;
import com.android.systemui.tuner.TunerService;
@@ -99,7 +98,6 @@
private final LinearLayout mMediaCarousel;
private final ArrayList<QSMediaPlayer> mMediaPlayers = new ArrayList<>();
- private final NotificationMediaManager mNotificationMediaManager;
private final LocalBluetoothManager mLocalBluetoothManager;
private final Executor mForegroundExecutor;
private final DelayableExecutor mBackgroundExecutor;
@@ -133,7 +131,6 @@
DumpManager dumpManager,
BroadcastDispatcher broadcastDispatcher,
QSLogger qsLogger,
- NotificationMediaManager notificationMediaManager,
@Main Executor foregroundExecutor,
@Background DelayableExecutor backgroundExecutor,
@Nullable LocalBluetoothManager localBluetoothManager
@@ -142,7 +139,6 @@
mContext = context;
mQSLogger = qsLogger;
mDumpManager = dumpManager;
- mNotificationMediaManager = notificationMediaManager;
mForegroundExecutor = foregroundExecutor;
mBackgroundExecutor = backgroundExecutor;
mLocalBluetoothManager = localBluetoothManager;
@@ -252,8 +248,8 @@
LocalMediaManager routeManager = new LocalMediaManager(mContext, mLocalBluetoothManager,
imm, notif.getPackageName());
- player = new QSMediaPlayer(mContext, this, mNotificationMediaManager, routeManager,
- mForegroundExecutor, mBackgroundExecutor);
+ player = new QSMediaPlayer(mContext, this, routeManager, mForegroundExecutor,
+ mBackgroundExecutor);
player.setListening(mListening);
if (player.isPlaying()) {
mMediaCarousel.addView(player.getView(), 0, lp); // add in front
diff --git a/packages/SystemUI/src/com/android/systemui/qs/QuickQSMediaPlayer.java b/packages/SystemUI/src/com/android/systemui/qs/QuickQSMediaPlayer.java
index 0ba4cb1..ccfddea 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/QuickQSMediaPlayer.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/QuickQSMediaPlayer.java
@@ -29,7 +29,6 @@
import com.android.systemui.R;
import com.android.systemui.media.MediaControlPanel;
-import com.android.systemui.statusbar.NotificationMediaManager;
import java.util.concurrent.Executor;
@@ -47,13 +46,12 @@
* Initialize mini media player for QQS
* @param context
* @param parent
- * @param manager
* @param foregroundExecutor
* @param backgroundExecutor
*/
- public QuickQSMediaPlayer(Context context, ViewGroup parent, NotificationMediaManager manager,
- Executor foregroundExecutor, Executor backgroundExecutor) {
- super(context, parent, manager, null, R.layout.qqs_media_panel, QQS_ACTION_IDS,
+ public QuickQSMediaPlayer(Context context, ViewGroup parent, Executor foregroundExecutor,
+ Executor backgroundExecutor) {
+ super(context, parent, null, R.layout.qqs_media_panel, QQS_ACTION_IDS,
foregroundExecutor, backgroundExecutor);
}
diff --git a/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanel.java b/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanel.java
index e6876bd..2169677 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanel.java
@@ -39,7 +39,6 @@
import com.android.systemui.plugins.qs.QSTile.State;
import com.android.systemui.qs.customize.QSCustomizer;
import com.android.systemui.qs.logging.QSLogger;
-import com.android.systemui.statusbar.NotificationMediaManager;
import com.android.systemui.tuner.TunerService;
import com.android.systemui.tuner.TunerService.Tunable;
import com.android.systemui.util.Utils;
@@ -83,12 +82,11 @@
DumpManager dumpManager,
BroadcastDispatcher broadcastDispatcher,
QSLogger qsLogger,
- NotificationMediaManager notificationMediaManager,
@Main Executor foregroundExecutor,
@Background DelayableExecutor backgroundExecutor,
@Nullable LocalBluetoothManager localBluetoothManager
) {
- super(context, attrs, dumpManager, broadcastDispatcher, qsLogger, notificationMediaManager,
+ super(context, attrs, dumpManager, broadcastDispatcher, qsLogger,
foregroundExecutor, backgroundExecutor, localBluetoothManager);
if (mFooter != null) {
removeView(mFooter.getView());
@@ -109,7 +107,7 @@
int marginSize = (int) mContext.getResources().getDimension(R.dimen.qqs_media_spacing);
mMediaPlayer = new QuickQSMediaPlayer(mContext, mHorizontalLinearLayout,
- notificationMediaManager, foregroundExecutor, backgroundExecutor);
+ foregroundExecutor, backgroundExecutor);
LayoutParams lp2 = new LayoutParams(0, LayoutParams.MATCH_PARENT, 1);
lp2.setMarginEnd(marginSize);
lp2.setMarginStart(0);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java
index 862ebe13..9112b65 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java
@@ -43,7 +43,6 @@
import com.android.systemui.qs.customize.QSCustomizer;
import com.android.systemui.qs.logging.QSLogger;
import com.android.systemui.qs.tileimpl.QSTileImpl;
-import com.android.systemui.statusbar.NotificationMediaManager;
import com.android.systemui.util.concurrency.DelayableExecutor;
import org.junit.Before;
@@ -84,8 +83,6 @@
@Mock
private QSTileView mQSTileView;
@Mock
- private NotificationMediaManager mNotificationMediaManager;
- @Mock
private Executor mForegroundExecutor;
@Mock
private DelayableExecutor mBackgroundExecutor;
@@ -100,7 +97,7 @@
mTestableLooper.runWithLooper(() -> {
mMetricsLogger = mDependency.injectMockDependency(MetricsLogger.class);
mQsPanel = new QSPanel(mContext, null, mDumpManager, mBroadcastDispatcher,
- mQSLogger, mNotificationMediaManager, mForegroundExecutor, mBackgroundExecutor,
+ mQSLogger, mForegroundExecutor, mBackgroundExecutor,
mLocalBluetoothManager);
// Provides a parent with non-zero size for QSPanel
mParentView = new FrameLayout(mContext);