From 8811b454bbb6887a4a22da48554e1f9f79ea1f7e Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 25 Oct 2024 05:08:59 -0700 Subject: [PATCH] Use slots to fill in default compact view indices This is equivalent to the current logic, but also allows to easily pick up other buttons that use the BACK, CENTRAL or FORWARD slots for the compact view by default. Also fix the test to actually assert the output order. PiperOrigin-RevId: 689747844 --- .../DefaultMediaNotificationProvider.java | 30 +--- .../DefaultMediaNotificationProviderTest.java | 128 ++++++++++++++---- 2 files changed, 105 insertions(+), 53 deletions(-) diff --git a/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java b/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java index 010440a67f..c5a72d9669 100644 --- a/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java +++ b/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java @@ -448,43 +448,33 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi // Skip to previous action. ImmutableList.Builder commandButtons = new ImmutableList.Builder<>(); if (playerCommands.containsAny(COMMAND_SEEK_TO_PREVIOUS, COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM)) { - Bundle commandButtonExtras = new Bundle(); - commandButtonExtras.putInt(COMMAND_KEY_COMPACT_VIEW_INDEX, INDEX_UNSET); commandButtons.add( new CommandButton.Builder(CommandButton.ICON_PREVIOUS) .setPlayerCommand(COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM) .setDisplayName( context.getString(R.string.media3_controls_seek_to_previous_description)) - .setExtras(commandButtonExtras) .build()); } if (playerCommands.contains(COMMAND_PLAY_PAUSE)) { - Bundle commandButtonExtras = new Bundle(); - commandButtonExtras.putInt(COMMAND_KEY_COMPACT_VIEW_INDEX, INDEX_UNSET); if (showPauseButton) { commandButtons.add( new CommandButton.Builder(CommandButton.ICON_PAUSE) .setPlayerCommand(COMMAND_PLAY_PAUSE) - .setExtras(commandButtonExtras) .setDisplayName(context.getString(R.string.media3_controls_pause_description)) .build()); } else { commandButtons.add( new CommandButton.Builder(CommandButton.ICON_PLAY) .setPlayerCommand(COMMAND_PLAY_PAUSE) - .setExtras(commandButtonExtras) .setDisplayName(context.getString(R.string.media3_controls_play_description)) .build()); } } // Skip to next action. if (playerCommands.containsAny(COMMAND_SEEK_TO_NEXT, COMMAND_SEEK_TO_NEXT_MEDIA_ITEM)) { - Bundle commandButtonExtras = new Bundle(); - commandButtonExtras.putInt(COMMAND_KEY_COMPACT_VIEW_INDEX, INDEX_UNSET); commandButtons.add( new CommandButton.Builder(CommandButton.ICON_NEXT) .setPlayerCommand(COMMAND_SEEK_TO_NEXT_MEDIA_ITEM) - .setExtras(commandButtonExtras) .setDisplayName(context.getString(R.string.media3_controls_seek_to_next_description)) .build()); } @@ -532,7 +522,7 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi int[] defaultCompactViewIndices = new int[3]; Arrays.fill(compactViewIndices, INDEX_UNSET); Arrays.fill(defaultCompactViewIndices, INDEX_UNSET); - int compactViewCommandCount = 0; + boolean hasCustomCompactViewIndices = false; for (int i = 0; i < mediaButtons.size(); i++) { CommandButton commandButton = mediaButtons.get(i); if (commandButton.sessionCommand != null) { @@ -547,28 +537,22 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi commandButton.displayName, commandButton.playerCommand)); } - if (compactViewCommandCount == 3) { - continue; - } int compactViewIndex = commandButton.extras.getInt( COMMAND_KEY_COMPACT_VIEW_INDEX, /* defaultValue= */ INDEX_UNSET); if (compactViewIndex >= 0 && compactViewIndex < compactViewIndices.length) { - compactViewCommandCount++; + hasCustomCompactViewIndices = true; compactViewIndices[compactViewIndex] = i; - } else if (commandButton.playerCommand == COMMAND_SEEK_TO_PREVIOUS - || commandButton.playerCommand == COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM) { + } else if (commandButton.slots.get(0) == CommandButton.SLOT_BACK) { defaultCompactViewIndices[0] = i; - } else if (commandButton.playerCommand == COMMAND_PLAY_PAUSE) { + } else if (commandButton.slots.get(0) == CommandButton.SLOT_CENTRAL) { defaultCompactViewIndices[1] = i; - } else if (commandButton.playerCommand == COMMAND_SEEK_TO_NEXT - || commandButton.playerCommand == COMMAND_SEEK_TO_NEXT_MEDIA_ITEM) { + } else if (commandButton.slots.get(0) == CommandButton.SLOT_FORWARD) { defaultCompactViewIndices[2] = i; } } - if (compactViewCommandCount == 0) { - // If there is no custom configuration we use the seekPrev (if any), play/pause (if any), - // seekNext (if any) action in compact view. + if (!hasCustomCompactViewIndices) { + // If there is no custom configuration we use the first slot preference as a proxy. int indexInCompactViewIndices = 0; for (int i = 0; i < defaultCompactViewIndices.length; i++) { if (defaultCompactViewIndices[i] == INDEX_UNSET) { diff --git a/libraries/session/src/test/java/androidx/media3/session/DefaultMediaNotificationProviderTest.java b/libraries/session/src/test/java/androidx/media3/session/DefaultMediaNotificationProviderTest.java index 7a43752f18..7828f02c0d 100644 --- a/libraries/session/src/test/java/androidx/media3/session/DefaultMediaNotificationProviderTest.java +++ b/libraries/session/src/test/java/androidx/media3/session/DefaultMediaNotificationProviderTest.java @@ -114,13 +114,13 @@ public class DefaultMediaNotificationProviderTest { defaultMediaNotificationProvider.getMediaButtons( mediaSession, commands, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), /* showPauseButton= */ true); List mediaButtonWhenPaused = defaultMediaNotificationProvider.getMediaButtons( mediaSession, commands, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), /* showPauseButton= */ false); mediaSession.release(); player.release(); @@ -169,7 +169,7 @@ public class DefaultMediaNotificationProviderTest { } @Test - public void getMediaButtons_noPlayerCommandsAvailable_onlyCustomLayoutButtons() { + public void getMediaButtons_noPlayerCommandsAvailable_onlyCustomButtons() { DefaultMediaNotificationProvider defaultMediaNotificationProvider = new DefaultMediaNotificationProvider.Builder(ApplicationProvider.getApplicationContext()) .build(); @@ -270,7 +270,7 @@ public class DefaultMediaNotificationProviderTest { @Test public void - addNotificationActions_playPauseSeekPrevSeekNextCommands_noCustomDeclaration_seekPrevPlayPauseSeekNextInCompactView() { + addNotificationActions_backCentralAndForwardButtonsAndNoCustomDeclaration_backCentralAndForwardButtonsInCompactView() { DefaultMediaNotificationProvider defaultMediaNotificationProvider = new DefaultMediaNotificationProvider.Builder(ApplicationProvider.getApplicationContext()) .build(); @@ -331,12 +331,12 @@ public class DefaultMediaNotificationProviderTest { .createMediaAction( eq(mediaSession), any(), eq("displayName"), eq(commandButton4.playerCommand)); verifyNoMoreInteractions(mockActionFactory); - assertThat(compactViewIndices).asList().containsExactly(2, 1, 3); + assertThat(compactViewIndices).asList().containsExactly(2, 1, 3).inOrder(); } @Test public void - addNotificationActions_playPauseSeekPrevCommands_noCustomDeclaration_seekPrevPlayPauseInCompactView() { + addNotificationActions_backCentralButtonsAndNoCustomDeclaration_backCentralButtonsInCompactView() { DefaultMediaNotificationProvider defaultMediaNotificationProvider = new DefaultMediaNotificationProvider.Builder(ApplicationProvider.getApplicationContext()) .build(); @@ -387,12 +387,12 @@ public class DefaultMediaNotificationProviderTest { .createMediaAction( eq(mediaSession), any(), eq("displayName"), eq(commandButton3.playerCommand)); verifyNoMoreInteractions(mockActionFactory); - assertThat(compactViewIndices).asList().containsExactly(2, 1); + assertThat(compactViewIndices).asList().containsExactly(2, 1).inOrder(); } @Test public void - addNotificationActions_noPlayPauseSeekPrevSeekNextCommands_noCustomDeclaration_emptyCompactViewIndices() { + addNotificationActions_noBackCentralForwardButtonsAndNoCustomDeclaration_emptyCompactViewIndices() { DefaultMediaNotificationProvider defaultMediaNotificationProvider = new DefaultMediaNotificationProvider.Builder(ApplicationProvider.getApplicationContext()) .build(); @@ -422,6 +422,73 @@ public class DefaultMediaNotificationProviderTest { assertThat(compactViewIndices).asList().isEmpty(); } + @Test + public void + addNotificationActions_customBackCentralForwardButtonsAndNoCustomDeclaration_customBackCentralForwardButtonsInCompactViewIndices() { + DefaultMediaNotificationProvider defaultMediaNotificationProvider = + new DefaultMediaNotificationProvider.Builder(ApplicationProvider.getApplicationContext()) + .build(); + NotificationCompat.Builder notificationBuilder = + new NotificationCompat.Builder( + ApplicationProvider.getApplicationContext(), TEST_CHANNEL_ID); + CommandButton commandButton1 = + new CommandButton.Builder(CommandButton.ICON_UNDEFINED) + .setDisplayName("displayName") + .setIconResId(R.drawable.media3_icon_circular_play) + .setSlots(CommandButton.SLOT_BACK_SECONDARY) + .setSessionCommand(new SessionCommand("action1", Bundle.EMPTY)) + .build(); + CommandButton commandButton2 = + new CommandButton.Builder(CommandButton.ICON_UNDEFINED) + .setDisplayName("displayName") + .setIconResId(R.drawable.media3_icon_circular_play) + .setSlots(CommandButton.SLOT_CENTRAL) + .setSessionCommand(new SessionCommand("action1", Bundle.EMPTY)) + .build(); + CommandButton commandButton3 = + new CommandButton.Builder(CommandButton.ICON_UNDEFINED) + .setDisplayName("displayName") + .setIconResId(R.drawable.media3_icon_circular_play) + .setSlots(CommandButton.SLOT_BACK) + .setSessionCommand(new SessionCommand("action1", Bundle.EMPTY)) + .build(); + CommandButton commandButton4 = + new CommandButton.Builder(CommandButton.ICON_UNDEFINED) + .setDisplayName("displayName") + .setIconResId(R.drawable.media3_icon_circular_play) + .setSlots(CommandButton.SLOT_FORWARD) + .setSessionCommand(new SessionCommand("action1", Bundle.EMPTY)) + .build(); + Player player = new TestExoPlayerBuilder(context).build(); + MediaSession mediaSession = new MediaSession.Builder(context, player).build(); + + int[] compactViewIndices = + defaultMediaNotificationProvider.addNotificationActions( + mediaSession, + ImmutableList.of(commandButton1, commandButton2, commandButton3, commandButton4), + notificationBuilder, + mockActionFactory); + mediaSession.release(); + player.release(); + + assertThat(notificationBuilder.build().actions).hasLength(4); + InOrder inOrder = Mockito.inOrder(mockActionFactory); + inOrder + .verify(mockActionFactory) + .createCustomActionFromCustomCommandButton(mediaSession, commandButton1); + inOrder + .verify(mockActionFactory) + .createCustomActionFromCustomCommandButton(mediaSession, commandButton2); + inOrder + .verify(mockActionFactory) + .createCustomActionFromCustomCommandButton(mediaSession, commandButton3); + inOrder + .verify(mockActionFactory) + .createCustomActionFromCustomCommandButton(mediaSession, commandButton4); + verifyNoMoreInteractions(mockActionFactory); + assertThat(compactViewIndices).asList().containsExactly(2, 1, 3).inOrder(); + } + @Test public void addNotificationActions_outOfBoundsCompactViewIndices_ignored() { DefaultMediaNotificationProvider defaultMediaNotificationProvider = @@ -579,7 +646,7 @@ public class DefaultMediaNotificationProviderTest { mock(MediaNotification.Provider.Callback.class); defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, mockOnNotificationChangedCallback1); ShadowLooper.idleMainLooper(); @@ -588,7 +655,7 @@ public class DefaultMediaNotificationProviderTest { mock(MediaNotification.Provider.Callback.class); defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, mockOnNotificationChangedCallback2); // The bitmap has arrived. @@ -605,17 +672,18 @@ public class DefaultMediaNotificationProviderTest { public void createNotification_invalidButtons_enabledSessionCommandsOnlyForGetMediaButtons() { DefaultActionFactory defaultActionFactory = new DefaultActionFactory(Robolectric.setupService(TestService.class)); - List filteredEnabledLayout = new ArrayList<>(); + List filteredMediaButtonPreferences = new ArrayList<>(); DefaultMediaNotificationProvider defaultMediaNotificationProvider = new DefaultMediaNotificationProvider(ApplicationProvider.getApplicationContext()) { @Override protected ImmutableList getMediaButtons( MediaSession session, Commands playerCommands, - ImmutableList customLayout, + ImmutableList mediaButtonPreferences, boolean showPauseButton) { - filteredEnabledLayout.addAll(customLayout); - return super.getMediaButtons(session, playerCommands, customLayout, showPauseButton); + filteredMediaButtonPreferences.addAll(mediaButtonPreferences); + return super.getMediaButtons( + session, playerCommands, mediaButtonPreferences, showPauseButton); } }; MediaSession mediaSession = @@ -646,13 +714,13 @@ public class DefaultMediaNotificationProviderTest { defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(button1, button2, button3), + /* mediaButtonPreferences= */ ImmutableList.of(button1, button2, button3), defaultActionFactory, notification -> { /* Do nothing. */ }); - assertThat(filteredEnabledLayout).containsExactly(button2); + assertThat(filteredMediaButtonPreferences).containsExactly(button2); mediaSession.getPlayer().release(); mediaSession.release(); } @@ -673,7 +741,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -700,7 +768,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -730,7 +798,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -757,7 +825,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -784,7 +852,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -814,7 +882,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -840,7 +908,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -866,7 +934,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -890,7 +958,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -914,7 +982,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -938,7 +1006,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -962,7 +1030,7 @@ public class DefaultMediaNotificationProviderTest { MediaNotification mediaNotification = defaultMediaNotificationProvider.createNotification( mediaSession, - /* customLayout= */ ImmutableList.of(), + /* mediaButtonPreferences= */ ImmutableList.of(), defaultActionFactory, notification -> {}); mediaSession.release(); @@ -1139,10 +1207,10 @@ public class DefaultMediaNotificationProviderTest { public ImmutableList getMediaButtons( MediaSession mediaSession, Player.Commands playerCommands, - ImmutableList customLayout, + ImmutableList mediaButtonPreferences, boolean showPauseButton) { return super.getMediaButtons( - mediaSession, playerCommands, customLayout, showPauseButton); + mediaSession, playerCommands, mediaButtonPreferences, showPauseButton); } @Override