Avoid modifying legacy custom layout in MediaControllerImplBase

When a session sets a custom layout, we currently convert it to the
media button preferences in MediaControllerImplBase and convert it back
to a custom layout for consumers of MediaController.getCustomLayout.

To avoid unnecessary conversions and potential changes, we can
directly use the provided custom layout to get the same logic as
before. This also means we avoid notifying a change to the custom layout
if only the implicit slots changed that weren't relevant before
introducing them (=we can remove some tests for this behavior).

PiperOrigin-RevId: 701903475
This commit is contained in:
tonihei 2024-12-02 03:16:45 -08:00 committed by Copybara-Service
parent 77d33645cc
commit da4376d48d
4 changed files with 92 additions and 220 deletions

View File

@ -2670,7 +2670,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
result.sessionExtras);
resolvedCustomLayout =
resolveCustomLayout(
resolvedMediaButtonPreferences, result.sessionExtras, intersectedPlayerCommands);
resolvedMediaButtonPreferences,
customLayoutOriginal,
result.sessionExtras,
sessionCommands,
intersectedPlayerCommands);
ImmutableMap.Builder<String, CommandButton> commandButtonsForMediaItems =
new ImmutableMap.Builder<>();
for (int i = 0; i < result.commandButtonsForMediaItems.size(); i++) {
@ -2864,7 +2868,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
sessionExtras);
resolvedCustomLayout =
resolveCustomLayout(
resolvedMediaButtonPreferences, sessionExtras, intersectedPlayerCommands);
resolvedMediaButtonPreferences,
customLayoutOriginal,
sessionExtras,
sessionCommands,
intersectedPlayerCommands);
mediaButtonPreferencesChanged =
!resolvedMediaButtonPreferences.equals(oldMediaButtonPreferences);
customLayoutChanged = !resolvedCustomLayout.equals(oldCustomLayout);
@ -2922,7 +2930,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
sessionExtras);
resolvedCustomLayout =
resolveCustomLayout(
resolvedMediaButtonPreferences, sessionExtras, intersectedPlayerCommands);
resolvedMediaButtonPreferences,
customLayoutOriginal,
sessionExtras,
sessionCommands,
intersectedPlayerCommands);
mediaButtonPreferencesChanged =
!resolvedMediaButtonPreferences.equals(oldMediaButtonPreferences);
customLayoutChanged = !resolvedCustomLayout.equals(oldCustomLayout);
@ -2960,7 +2972,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
sessionExtras);
resolvedCustomLayout =
resolveCustomLayout(
resolvedMediaButtonPreferences, sessionExtras, intersectedPlayerCommands);
resolvedMediaButtonPreferences,
layout,
sessionExtras,
sessionCommands,
intersectedPlayerCommands);
boolean mediaButtonPreferencesChanged =
!resolvedMediaButtonPreferences.equals(oldMediaButtonPreferences);
boolean customLayoutChanged = !resolvedCustomLayout.equals(oldCustomLayout);
@ -2998,7 +3014,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
sessionExtras);
resolvedCustomLayout =
resolveCustomLayout(
resolvedMediaButtonPreferences, sessionExtras, intersectedPlayerCommands);
resolvedMediaButtonPreferences,
customLayoutOriginal,
sessionExtras,
sessionCommands,
intersectedPlayerCommands);
boolean mediaButtonPreferencesChanged =
!resolvedMediaButtonPreferences.equals(oldMediaButtonPreferences);
boolean customLayoutChanged = !resolvedCustomLayout.equals(oldCustomLayout);
@ -3036,7 +3056,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
sessionExtras);
resolvedCustomLayout =
resolveCustomLayout(
resolvedMediaButtonPreferences, sessionExtras, intersectedPlayerCommands);
resolvedMediaButtonPreferences,
customLayoutOriginal,
sessionExtras,
sessionCommands,
intersectedPlayerCommands);
boolean mediaButtonPreferencesChanged =
!resolvedMediaButtonPreferences.equals(oldMediaButtonPreferences);
boolean customLayoutChanged = !resolvedCustomLayout.equals(oldCustomLayout);
@ -3415,15 +3439,21 @@ import org.checkerframework.checker.nullness.qual.NonNull;
private static ImmutableList<CommandButton> resolveCustomLayout(
List<CommandButton> mediaButtonPreferences,
List<CommandButton> customLayoutOriginal,
Bundle sessionExtras,
Player.Commands availableCommands) {
SessionCommands sessionCommands,
Player.Commands playerCommands) {
if (!customLayoutOriginal.isEmpty()) {
return CommandButton.copyWithUnavailableButtonsDisabled(
customLayoutOriginal, sessionCommands, playerCommands);
}
boolean backSlotAllowed =
!sessionExtras.getBoolean(MediaConstants.EXTRAS_KEY_SLOT_RESERVATION_SEEK_TO_PREV)
&& !availableCommands.containsAny(
&& !playerCommands.containsAny(
Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM, Player.COMMAND_SEEK_TO_PREVIOUS);
boolean forwardSlotAllowed =
!sessionExtras.getBoolean(MediaConstants.EXTRAS_KEY_SLOT_RESERVATION_SEEK_TO_NEXT)
&& !availableCommands.containsAny(
&& !playerCommands.containsAny(
Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, Player.COMMAND_SEEK_TO_NEXT);
return CommandButton.getCustomLayoutFromMediaButtonPreferences(
mediaButtonPreferences, backSlotAllowed, forwardSlotAllowed);

View File

@ -216,11 +216,11 @@ public class MediaControllerTest {
assertThat(threadTestRule.getHandler().postAndSync(controller::getCustomLayout))
.containsExactly(
withBackSlot(button1.copyWithIsEnabled(true)),
withForwardSlot(button2.copyWithIsEnabled(false)),
withOverflowSlot(button3.copyWithIsEnabled(false)),
withOverflowSlot(button4.copyWithIsEnabled(true)),
withOverflowSlot(button5.copyWithIsEnabled(false)))
button1.copyWithIsEnabled(true),
button2.copyWithIsEnabled(false),
button3.copyWithIsEnabled(false),
button4.copyWithIsEnabled(true),
button5.copyWithIsEnabled(false))
.inOrder();
session.cleanUp();
@ -300,17 +300,15 @@ public class MediaControllerTest {
threadTestRule.getHandler().postAndSync(controller::getCustomLayout);
assertThat(initialCustomLayoutFromGetter)
.containsExactly(
withBackSlot(button1.copyWithIsEnabled(true)),
withForwardSlot(button3.copyWithIsEnabled(false)))
.containsExactly(button1.copyWithIsEnabled(true), button3.copyWithIsEnabled(false))
.inOrder();
ImmutableList<CommandButton> expectedNewButtons =
ImmutableList.of(
withBackSlot(button1.copyWithIsEnabled(true)),
withForwardSlot(button2.copyWithIsEnabled(false)),
withOverflowSlot(button4.copyWithIsEnabled(false)),
withOverflowSlot(button5.copyWithIsEnabled(true)),
withOverflowSlot(button6.copyWithIsEnabled(false)));
button1.copyWithIsEnabled(true),
button2.copyWithIsEnabled(false),
button4.copyWithIsEnabled(false),
button5.copyWithIsEnabled(true),
button6.copyWithIsEnabled(false));
assertThat(newCustomLayoutFromGetter).containsExactlyElementsIn(expectedNewButtons).inOrder();
assertThat(reportedCustomLayout.get()).containsExactlyElementsIn(expectedNewButtons).inOrder();
assertThat(reportedCustomLayoutChanged.get())
@ -378,39 +376,39 @@ public class MediaControllerTest {
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
assertThat(initialCustomLayout)
.containsExactly(
withBackSlot(button1.copyWithIsEnabled(true)),
withForwardSlot(button2.copyWithIsEnabled(false)),
withOverflowSlot(button3.copyWithIsEnabled(true)),
withOverflowSlot(button4.copyWithIsEnabled(false)));
button1.copyWithIsEnabled(true),
button2.copyWithIsEnabled(false),
button3.copyWithIsEnabled(true),
button4.copyWithIsEnabled(false));
assertThat(reportedCustomLayoutChanged).hasSize(2);
assertThat(reportedCustomLayoutChanged.get(0))
.containsExactly(
withBackSlot(button1.copyWithIsEnabled(false)),
withForwardSlot(button2.copyWithIsEnabled(false)),
withOverflowSlot(button3.copyWithIsEnabled(false)),
withOverflowSlot(button4.copyWithIsEnabled(false)))
button1.copyWithIsEnabled(false),
button2.copyWithIsEnabled(false),
button3.copyWithIsEnabled(false),
button4.copyWithIsEnabled(false))
.inOrder();
assertThat(reportedCustomLayoutChanged.get(1))
.containsExactly(
withBackSlot(button1.copyWithIsEnabled(false)),
withForwardSlot(button2.copyWithIsEnabled(false)),
withOverflowSlot(button3.copyWithIsEnabled(false)),
withOverflowSlot(button4.copyWithIsEnabled(true)))
button1.copyWithIsEnabled(false),
button2.copyWithIsEnabled(false),
button3.copyWithIsEnabled(false),
button4.copyWithIsEnabled(true))
.inOrder();
assertThat(getterCustomLayoutChanged).hasSize(2);
assertThat(getterCustomLayoutChanged.get(0))
.containsExactly(
withBackSlot(button1.copyWithIsEnabled(false)),
withForwardSlot(button2.copyWithIsEnabled(false)),
withOverflowSlot(button3.copyWithIsEnabled(false)),
withOverflowSlot(button4.copyWithIsEnabled(false)))
button1.copyWithIsEnabled(false),
button2.copyWithIsEnabled(false),
button3.copyWithIsEnabled(false),
button4.copyWithIsEnabled(false))
.inOrder();
assertThat(getterCustomLayoutChanged.get(1))
.containsExactly(
withBackSlot(button1.copyWithIsEnabled(false)),
withForwardSlot(button2.copyWithIsEnabled(false)),
withOverflowSlot(button3.copyWithIsEnabled(false)),
withOverflowSlot(button4.copyWithIsEnabled(true)))
button1.copyWithIsEnabled(false),
button2.copyWithIsEnabled(false),
button3.copyWithIsEnabled(false),
button4.copyWithIsEnabled(true))
.inOrder();
session.cleanUp();
}
@ -453,17 +451,13 @@ public class MediaControllerTest {
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build());
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
assertThat(initialCustomLayout).containsExactly(withBackSlot(button.copyWithIsEnabled(true)));
assertThat(initialCustomLayout).containsExactly(button.copyWithIsEnabled(true));
assertThat(reportedCustomLayouts).hasSize(2);
assertThat(reportedCustomLayouts.get(0))
.containsExactly(withBackSlot(button.copyWithIsEnabled(false)));
assertThat(reportedCustomLayouts.get(1))
.containsExactly(withBackSlot(button.copyWithIsEnabled(true)));
assertThat(reportedCustomLayouts.get(0)).containsExactly(button.copyWithIsEnabled(false));
assertThat(reportedCustomLayouts.get(1)).containsExactly(button.copyWithIsEnabled(true));
assertThat(getterCustomLayouts).hasSize(2);
assertThat(getterCustomLayouts.get(0))
.containsExactly(withBackSlot(button.copyWithIsEnabled(false)));
assertThat(getterCustomLayouts.get(1))
.containsExactly(withBackSlot(button.copyWithIsEnabled(true)));
assertThat(getterCustomLayouts.get(0)).containsExactly(button.copyWithIsEnabled(false));
assertThat(getterCustomLayouts.get(1)).containsExactly(button.copyWithIsEnabled(true));
session.cleanUp();
}
@ -533,174 +527,36 @@ public class MediaControllerTest {
session.setCustomLayout(ImmutableList.of(button1, button2));
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
CommandButton button1EnabledBackSlot = withBackSlot(button1.copyWithIsEnabled(true));
CommandButton button2DisabledForwardSlot = withForwardSlot(button2.copyWithIsEnabled(false));
CommandButton button3DisabledBackSlot = withBackSlot(button3.copyWithIsEnabled(false));
CommandButton button4DisabledForwardSlot = withForwardSlot(button4.copyWithIsEnabled(false));
assertThat(initialCustomLayout)
.containsExactly(button1EnabledBackSlot, button2DisabledForwardSlot)
.inOrder();
CommandButton button1Enabled = button1.copyWithIsEnabled(true);
CommandButton button2Disabled = button2.copyWithIsEnabled(false);
CommandButton button3Disabled = button3.copyWithIsEnabled(false);
CommandButton button4Disabled = button4.copyWithIsEnabled(false);
assertThat(initialCustomLayout).containsExactly(button1Enabled, button2Disabled).inOrder();
assertThat(reportedCustomLayout)
.containsExactly(
ImmutableList.of(button1EnabledBackSlot, button2DisabledForwardSlot),
ImmutableList.of(button3DisabledBackSlot, button4DisabledForwardSlot),
ImmutableList.of(button1EnabledBackSlot, button2DisabledForwardSlot))
ImmutableList.of(button1Enabled, button2Disabled),
ImmutableList.of(button3Disabled, button4Disabled),
ImmutableList.of(button1Enabled, button2Disabled))
.inOrder();
assertThat(getterCustomLayout)
.containsExactly(
ImmutableList.of(button1EnabledBackSlot, button2DisabledForwardSlot),
ImmutableList.of(button3DisabledBackSlot, button4DisabledForwardSlot),
ImmutableList.of(button1EnabledBackSlot, button2DisabledForwardSlot))
ImmutableList.of(button1Enabled, button2Disabled),
ImmutableList.of(button3Disabled, button4Disabled),
ImmutableList.of(button1Enabled, button2Disabled))
.inOrder();
assertThat(reportedCustomLayoutChanged)
.containsExactly(
ImmutableList.of(button3DisabledBackSlot, button4DisabledForwardSlot),
ImmutableList.of(button1EnabledBackSlot, button2DisabledForwardSlot))
ImmutableList.of(button3Disabled, button4Disabled),
ImmutableList.of(button1Enabled, button2Disabled))
.inOrder();
assertThat(getterCustomLayoutChanged)
.containsExactly(
ImmutableList.of(button3DisabledBackSlot, button4DisabledForwardSlot),
ImmutableList.of(button1EnabledBackSlot, button2DisabledForwardSlot))
ImmutableList.of(button3Disabled, button4Disabled),
ImmutableList.of(button1Enabled, button2Disabled))
.inOrder();
session.cleanUp();
}
@Test
public void getCustomLayout_setAvailablePrevNextCommand_reportsCustomLayoutChanged()
throws Exception {
RemoteMediaSession session = createRemoteMediaSession(TEST_GET_CUSTOM_LAYOUT, null);
CommandButton button1 =
new CommandButton.Builder(CommandButton.ICON_ALBUM)
.setDisplayName("button1")
.setSessionCommand(new SessionCommand("command1", Bundle.EMPTY))
.build();
CommandButton button2 =
new CommandButton.Builder(CommandButton.ICON_REWIND)
.setDisplayName("button2")
.setSessionCommand(new SessionCommand("command2", Bundle.EMPTY))
.build();
CommandButton button3 =
new CommandButton.Builder(CommandButton.ICON_SHUFFLE_ON)
.setDisplayName("button3")
.setSessionCommand(new SessionCommand("command3", Bundle.EMPTY))
.build();
SessionCommands allSessionCommands =
new SessionCommands.Builder()
.add(button1.sessionCommand)
.add(button2.sessionCommand)
.add(button3.sessionCommand)
.build();
setupCustomLayout(session, ImmutableList.of(button1, button2, button3));
CountDownLatch latch = new CountDownLatch(4);
List<List<CommandButton>> reportedCustomLayouts = new ArrayList<>();
MediaController.Listener listener =
new MediaController.Listener() {
@Override
public void onCustomLayoutChanged(
MediaController controller, List<CommandButton> layout) {
reportedCustomLayouts.add(layout);
latch.countDown();
}
};
controllerTestRule.createController(
session.getToken(), /* connectionHints= */ Bundle.EMPTY, listener);
session.setAvailableCommands(allSessionCommands, Player.Commands.EMPTY);
session.setAvailableCommands(
allSessionCommands, new Player.Commands.Builder().add(Player.COMMAND_SEEK_TO_NEXT).build());
session.setAvailableCommands(
allSessionCommands,
new Player.Commands.Builder().add(Player.COMMAND_SEEK_TO_PREVIOUS).build());
session.setAvailableCommands(
allSessionCommands,
new Player.Commands.Builder()
.addAll(Player.COMMAND_SEEK_TO_NEXT, Player.COMMAND_SEEK_TO_PREVIOUS)
.build());
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
assertThat(reportedCustomLayouts)
.containsExactly(
ImmutableList.of(
withBackSlot(button1), withForwardSlot(button2), withOverflowSlot(button3)),
ImmutableList.of(
withBackSlot(button1), withOverflowSlot(button2), withOverflowSlot(button3)),
ImmutableList.of(
withForwardSlot(button1), withOverflowSlot(button2), withOverflowSlot(button3)),
ImmutableList.of(
withOverflowSlot(button1), withOverflowSlot(button2), withOverflowSlot(button3)));
session.cleanUp();
}
@Test
public void getCustomLayout_setSessionExtrasForPrevNextReservations_reportsCustomLayoutChanged()
throws Exception {
RemoteMediaSession session = createRemoteMediaSession(TEST_GET_CUSTOM_LAYOUT, null);
CommandButton button1 =
new CommandButton.Builder(CommandButton.ICON_ALBUM)
.setDisplayName("button1")
.setSessionCommand(new SessionCommand("command1", Bundle.EMPTY))
.build();
CommandButton button2 =
new CommandButton.Builder(CommandButton.ICON_REWIND)
.setDisplayName("button2")
.setSessionCommand(new SessionCommand("command2", Bundle.EMPTY))
.build();
CommandButton button3 =
new CommandButton.Builder(CommandButton.ICON_SHUFFLE_ON)
.setDisplayName("button3")
.setSessionCommand(new SessionCommand("command3", Bundle.EMPTY))
.build();
SessionCommands allSessionCommands =
new SessionCommands.Builder()
.add(button1.sessionCommand)
.add(button2.sessionCommand)
.add(button3.sessionCommand)
.build();
setupCustomLayout(session, ImmutableList.of(button1, button2, button3));
CountDownLatch latch = new CountDownLatch(4);
List<List<CommandButton>> reportedCustomLayouts = new ArrayList<>();
MediaController.Listener listener =
new MediaController.Listener() {
@Override
public void onCustomLayoutChanged(
MediaController controller, List<CommandButton> layout) {
reportedCustomLayouts.add(layout);
latch.countDown();
}
};
Bundle extrasNextSlotReservation = new Bundle();
extrasNextSlotReservation.putBoolean(
MediaConstants.EXTRAS_KEY_SLOT_RESERVATION_SEEK_TO_NEXT, true);
Bundle extrasPrevSlotReservation = new Bundle();
extrasPrevSlotReservation.putBoolean(
MediaConstants.EXTRAS_KEY_SLOT_RESERVATION_SEEK_TO_PREV, true);
Bundle extrasPrevNextSlotReservation = new Bundle();
extrasPrevNextSlotReservation.putBoolean(
MediaConstants.EXTRAS_KEY_SLOT_RESERVATION_SEEK_TO_PREV, true);
extrasPrevNextSlotReservation.putBoolean(
MediaConstants.EXTRAS_KEY_SLOT_RESERVATION_SEEK_TO_NEXT, true);
controllerTestRule.createController(
session.getToken(), /* connectionHints= */ Bundle.EMPTY, listener);
session.setAvailableCommands(allSessionCommands, Player.Commands.EMPTY);
session.setSessionExtras(extrasNextSlotReservation);
session.setSessionExtras(extrasPrevSlotReservation);
session.setSessionExtras(extrasPrevNextSlotReservation);
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
assertThat(reportedCustomLayouts)
.containsExactly(
ImmutableList.of(
withBackSlot(button1), withForwardSlot(button2), withOverflowSlot(button3)),
ImmutableList.of(
withBackSlot(button1), withOverflowSlot(button2), withOverflowSlot(button3)),
ImmutableList.of(
withForwardSlot(button1), withOverflowSlot(button2), withOverflowSlot(button3)),
ImmutableList.of(
withOverflowSlot(button1), withOverflowSlot(button2), withOverflowSlot(button3)));
session.cleanUp();
}
@Test
public void getMediaButtonPreferences_customLayoutBuiltWithSession_includedOnConnect()
throws Exception {

View File

@ -56,7 +56,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.LargeTest;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.primitives.ImmutableIntArray;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
@ -199,13 +198,7 @@ public class MediaSessionCallbackTest {
ImmutableList<CommandButton> layout = remoteController.getCustomLayout();
assertThat(layout)
.containsExactly(
button1
.copyWithIsEnabled(false)
.copyWithSlots(ImmutableIntArray.of(CommandButton.SLOT_OVERFLOW)),
button2.copyWithSlots(ImmutableIntArray.of(CommandButton.SLOT_OVERFLOW)))
.inOrder();
assertThat(layout).containsExactly(button1.copyWithIsEnabled(false), button2).inOrder();
assertThat(remoteController.sendCustomCommand(button1.sessionCommand, Bundle.EMPTY).resultCode)
.isEqualTo(ERROR_PERMISSION_DENIED);
assertThat(remoteController.sendCustomCommand(button2.sessionCommand, Bundle.EMPTY).resultCode)

View File

@ -47,7 +47,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.MediumTest;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.primitives.ImmutableIntArray;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
import java.util.ArrayList;
@ -332,13 +331,7 @@ public class MediaSessionServiceTest {
assertThat(mediaControllerCompat.getPlaybackState().getActions())
.isEqualTo(PlaybackStateCompat.ACTION_SET_RATING);
assertThat(remoteController.getCustomLayout())
.containsExactly(
button1
.copyWithIsEnabled(false)
.copyWithSlots(ImmutableIntArray.of(CommandButton.SLOT_FORWARD)),
button2
.copyWithIsEnabled(false)
.copyWithSlots(ImmutableIntArray.of(CommandButton.SLOT_OVERFLOW)))
.containsExactly(button1.copyWithIsEnabled(false), button2.copyWithIsEnabled(false))
.inOrder();
assertThat(initialCustomActionsInControllerCompat).isEmpty();
assertThat(mediaControllerCompat.getPlaybackState().getCustomActions()).hasSize(2);