Allow null prev/next buttons to be INVISIBLE instead of GONE

Added an overload method for setVisibleAndAlpha that allows callers to
specify the value for null as INVISIBLE instead of GONE and called that
instead for the prev/next buttons. Added booleans to MediaButton to
allow it to say whether to reserve the space for the prev/next buttons
when they are null. Made the MediaButton class immutable.

Fixes: 224749799
Test: tested manually and added 2 tests to MediaControlPanelTest
Change-Id: I5ccac017eccfb2dc2ecdd42a1e6517d4787c68b6

packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java

@@ -786,7 +786,14 @@ public class MediaControlPanel {
                 scrubbingTimeViewsEnabled(semanticActions) && hideWhenScrubbing && mIsScrubbing;
         boolean visible = mediaAction != null && !shouldBeHiddenDueToScrubbing;
 
-        setVisibleAndAlpha(expandedSet, buttonId, visible);
+        int notVisibleValue;
+        if ((buttonId == R.id.actionPrev && semanticActions.getReservePrev())
+                || (buttonId == R.id.actionNext && semanticActions.getReserveNext())) {
+            notVisibleValue = ConstraintSet.INVISIBLE;
+        } else {
+            notVisibleValue = ConstraintSet.GONE;
+        }
+        setVisibleAndAlpha(expandedSet, buttonId, visible, notVisibleValue);
         setVisibleAndAlpha(collapsedSet, buttonId, visible && showInCompact);
     }
 
@@ -1191,7 +1198,12 @@ public class MediaControlPanel {
     }
 
     private void setVisibleAndAlpha(ConstraintSet set, int actionId, boolean visible) {
-        set.setVisibility(actionId, visible ? ConstraintSet.VISIBLE : ConstraintSet.GONE);
+        setVisibleAndAlpha(set, actionId, visible, ConstraintSet.GONE);
+    }
+
+    private void setVisibleAndAlpha(ConstraintSet set, int actionId, boolean visible,
+            int notVisibleValue) {
+        set.setVisibility(actionId, visible ? ConstraintSet.VISIBLE : notVisibleValue);
         set.setAlpha(actionId, visible ? 1.0f : 0.0f);
     }
 

packages/SystemUI/src/com/android/systemui/media/MediaData.kt

@@ -149,23 +149,31 @@ data class MediaButton(
     /**
      * Play/pause button
      */
-    var playOrPause: MediaAction? = null,
+    val playOrPause: MediaAction? = null,
     /**
      * Next button, or custom action
      */
-    var nextOrCustom: MediaAction? = null,
+    val nextOrCustom: MediaAction? = null,
     /**
      * Previous button, or custom action
      */
-    var prevOrCustom: MediaAction? = null,
+    val prevOrCustom: MediaAction? = null,
     /**
      * First custom action space
      */
-    var custom0: MediaAction? = null,
+    val custom0: MediaAction? = null,
     /**
      * Second custom action space
      */
-    var custom1: MediaAction? = null
+    val custom1: MediaAction? = null,
+    /**
+     * Whether to reserve the empty space when the nextOrCustom is null
+     */
+    val reserveNext: Boolean = false,
+    /**
+     * Whether to reserve the empty space when the prevOrCustom is null
+     */
+    val reservePrev: Boolean = false
 ) {
     fun getActionById(id: Int): MediaAction? {
         return when (id) {

packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt

@@ -791,67 +791,74 @@ class MediaDataManager(
      */
     private fun createActionsFromState(packageName: String, controller: MediaController):
             MediaButton? {
-        val actions = MediaButton()
-        controller.playbackState?.let { state ->
-            // First, check for standard actions
-            actions.playOrPause = if (isConnectingState(state.state)) {
-                // Spinner needs to be animating to render anything. Start it here.
-                val drawable = context.getDrawable(
-                        com.android.internal.R.drawable.progress_small_material)
-                (drawable as Animatable).start()
-                MediaAction(
-                    drawable,
-                    null, // no action to perform when clicked
-                    context.getString(R.string.controls_media_button_connecting),
-                    context.getDrawable(R.drawable.ic_media_connecting_container),
-                    // Specify a rebind id to prevent the spinner from restarting on later binds.
-                    com.android.internal.R.drawable.progress_small_material
-                )
-            } else if (isPlayingState(state.state)) {
-                getStandardAction(controller, state.actions, PlaybackState.ACTION_PAUSE)
-            } else {
-                getStandardAction(controller, state.actions, PlaybackState.ACTION_PLAY)
-            }
-            val prevButton = getStandardAction(controller, state.actions,
-                    PlaybackState.ACTION_SKIP_TO_PREVIOUS)
-            val nextButton = getStandardAction(controller, state.actions,
-                    PlaybackState.ACTION_SKIP_TO_NEXT)
-
-            // Then, create a way to build any custom actions that will be needed
-            val customActions = state.customActions.asSequence().filterNotNull().map {
-                getCustomAction(state, packageName, controller, it)
-            }.iterator()
-            fun nextCustomAction() = if (customActions.hasNext()) customActions.next() else null
-
-            // Finally, assign the remaining button slots: play/pause A B C D
-            // A = previous, else custom action (if not reserved)
-            // B = next, else custom action (if not reserved)
-            // C and D are always custom actions
-            val reservePrev = controller.extras?.getBoolean(
-                    MediaConstants.SESSION_EXTRAS_KEY_SLOT_RESERVATION_SKIP_TO_PREV) == true
-            val reserveNext = controller.extras?.getBoolean(
-                    MediaConstants.SESSION_EXTRAS_KEY_SLOT_RESERVATION_SKIP_TO_NEXT) == true
-
-            actions.prevOrCustom = if (prevButton != null) {
-                prevButton
-            } else if (!reservePrev) {
-                nextCustomAction()
-            } else {
-                null
-            }
-
-            actions.nextOrCustom = if (nextButton != null) {
-                nextButton
-            } else if (!reserveNext) {
-                nextCustomAction()
-            } else {
-                null
-            }
-
-            actions.custom0 = nextCustomAction()
-            actions.custom1 = nextCustomAction()
+        val state = controller.playbackState
+        if (state == null) {
+            return MediaButton()
         }
-        return actions
+        // First, check for} standard actions
+        val playOrPause = if (isConnectingState(state.state)) {
+            // Spinner needs to be animating to render anything. Start it here.
+            val drawable = context.getDrawable(
+                com.android.internal.R.drawable.progress_small_material)
+            (drawable as Animatable).start()
+            MediaAction(
+                drawable,
+                null, // no action to perform when clicked
+                context.getString(R.string.controls_media_button_connecting),
+                context.getDrawable(R.drawable.ic_media_connecting_container),
+                // Specify a rebind id to prevent the spinner from restarting on later binds.
+                com.android.internal.R.drawable.progress_small_material
+            )
+        } else if (isPlayingState(state.state)) {
+            getStandardAction(controller, state.actions, PlaybackState.ACTION_PAUSE)
+        } else {
+            getStandardAction(controller, state.actions, PlaybackState.ACTION_PLAY)
+        }
+        val prevButton = getStandardAction(controller, state.actions,
+            PlaybackState.ACTION_SKIP_TO_PREVIOUS)
+        val nextButton = getStandardAction(controller, state.actions,
+            PlaybackState.ACTION_SKIP_TO_NEXT)
+
+        // Then, create a way to build any custom actions that will be needed
+        val customActions = state.customActions.asSequence().filterNotNull().map {
+            getCustomAction(state, packageName, controller, it)
+        }.iterator()
+        fun nextCustomAction() = if (customActions.hasNext()) customActions.next() else null
+
+        // Finally, assign the remaining button slots: play/pause A B C D
+        // A = previous, else custom action (if not reserved)
+        // B = next, else custom action (if not reserved)
+        // C and D are always custom actions
+        val reservePrev = controller.extras?.getBoolean(
+            MediaConstants.SESSION_EXTRAS_KEY_SLOT_RESERVATION_SKIP_TO_PREV) == true
+        val reserveNext = controller.extras?.getBoolean(
+            MediaConstants.SESSION_EXTRAS_KEY_SLOT_RESERVATION_SKIP_TO_NEXT) == true
+
+        val prevOrCustom = if (prevButton != null) {
+            prevButton
+        } else if (!reservePrev) {
+            nextCustomAction()
+        } else {
+            null
+        }
+
+        val nextOrCustom = if (nextButton != null) {
+            nextButton
+        } else if (!reserveNext) {
+            nextCustomAction()
+        } else {
+            null
+        }
+
+        return MediaButton(
+            playOrPause,
+            nextOrCustom,
+            prevOrCustom,
+            nextCustomAction(),
+            nextCustomAction(),
+            reserveNext,
+            reservePrev
+        )
     }
 
     /**

packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt

@@ -436,6 +436,64 @@ public class MediaControlPanelTest : SysuiTestCase() {
         verify(expandedSet).setVisibility(R.id.action4, ConstraintSet.GONE)
     }
 
+    @Test
+    fun bindSemanticActions_reservedPrev() {
+        val icon = context.getDrawable(android.R.drawable.ic_media_play)
+        val bg = context.getDrawable(R.drawable.qs_media_round_button_background)
+
+        // Setup button state: no prev or next button and their slots reserved
+        val semanticActions = MediaButton(
+            playOrPause = MediaAction(icon, Runnable {}, "play", bg),
+            nextOrCustom = null,
+            prevOrCustom = null,
+            custom0 = MediaAction(icon, null, "custom 0", bg),
+            custom1 = MediaAction(icon, null, "custom 1", bg),
+            false,
+            true
+        )
+        val state = mediaData.copy(semanticActions = semanticActions)
+
+        player.attachPlayer(viewHolder)
+        player.bindPlayer(state, PACKAGE)
+
+        assertThat(actionPrev.isEnabled()).isFalse()
+        assertThat(actionPrev.drawable).isNull()
+        verify(expandedSet).setVisibility(R.id.actionPrev, ConstraintSet.INVISIBLE)
+
+        assertThat(actionNext.isEnabled()).isFalse()
+        assertThat(actionNext.drawable).isNull()
+        verify(expandedSet).setVisibility(R.id.actionNext, ConstraintSet.GONE)
+    }
+
+    @Test
+    fun bindSemanticActions_reservedNext() {
+        val icon = context.getDrawable(android.R.drawable.ic_media_play)
+        val bg = context.getDrawable(R.drawable.qs_media_round_button_background)
+
+        // Setup button state: no prev or next button and their slots reserved
+        val semanticActions = MediaButton(
+            playOrPause = MediaAction(icon, Runnable {}, "play", bg),
+            nextOrCustom = null,
+            prevOrCustom = null,
+            custom0 = MediaAction(icon, null, "custom 0", bg),
+            custom1 = MediaAction(icon, null, "custom 1", bg),
+            true,
+            false
+        )
+        val state = mediaData.copy(semanticActions = semanticActions)
+
+        player.attachPlayer(viewHolder)
+        player.bindPlayer(state, PACKAGE)
+
+        assertThat(actionPrev.isEnabled()).isFalse()
+        assertThat(actionPrev.drawable).isNull()
+        verify(expandedSet).setVisibility(R.id.actionPrev, ConstraintSet.GONE)
+
+        assertThat(actionNext.isEnabled()).isFalse()
+        assertThat(actionNext.drawable).isNull()
+        verify(expandedSet).setVisibility(R.id.actionNext, ConstraintSet.INVISIBLE)
+    }
+
     @Test
     fun bind_seekBarDisabled_seekBarVisibilityIsSetToInvisible() {
         whenever(seekBarViewModel.getEnabled()).thenReturn(false)

packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt

@@ -838,6 +838,9 @@ class MediaDataManagerTest : SysuiTestCase() {
 
         assertThat(actions.custom1).isNotNull()
         assertThat(actions.custom1!!.contentDescription).isEqualTo(customDesc[1])
+
+        assertThat(actions.reserveNext).isTrue()
+        assertThat(actions.reservePrev).isTrue()
     }
 
     @Test