Ensure CombinedInfo always represents a consistent state.
Prior to this change, a MediaController change would cause momentary inconsistencies in the dependencies of the CombinedInfo LiveData. This change fetches the values directly while still being updated whenever those values change.
Bug: 112702750
Test: PlaybackViewModelTest
Change-Id: I169ffef1e635a976be47fa1b6b543993bde38de2
diff --git a/car-media-common/src/com/android/car/media/common/playback/PlaybackViewModel.java b/car-media-common/src/com/android/car/media/common/playback/PlaybackViewModel.java
index ac67ee1..e20eb5e 100644
--- a/car-media-common/src/com/android/car/media/common/playback/PlaybackViewModel.java
+++ b/car-media-common/src/com/android/car/media/common/playback/PlaybackViewModel.java
@@ -19,6 +19,7 @@
import static androidx.lifecycle.Transformations.map;
import static androidx.lifecycle.Transformations.switchMap;
+import static com.android.car.arch.common.LiveDataFunctions.combine;
import static com.android.car.arch.common.LiveDataFunctions.dataOf;
import static com.android.car.arch.common.LiveDataFunctions.distinct;
import static com.android.car.arch.common.LiveDataFunctions.nullLiveData;
@@ -133,10 +134,31 @@
private final LiveData<PlaybackController> mPlaybackControls = map(mMediaController,
PlaybackController::new);
- private final LiveData<CombinedInfo> mCombinedInfo = map(
- pair(mMediaController, pair(mMetadata, mPlaybackState)),
- input -> input.first == null ? null
- : new CombinedInfo(input.first, input.second.first, input.second.second));
+ private final LiveData<CombinedInfo> mCombinedInfo = combine(mMetadata, mPlaybackState,
+ // getValue() on mMediaController is safe because mMetadata and mPlaybackState are
+ // keeping it active. Don't pass through the values since they may be stale if the
+ // MediaController is being updated.
+ (mediaMetadata, playbackState) -> getCombinedInfo(mMediaController.getValue())
+ );
+
+ @Nullable
+ private CombinedInfo getCombinedInfo(@Nullable MediaController mediaController) {
+ if (mediaController == null) return null;
+
+ MediaMetadata metadata = mediaController.getMetadata();
+ PlaybackState playbackState = mediaController.getPlaybackState();
+ CombinedInfo oldInfo = mCombinedInfo.getValue();
+ // Minimize object churn
+ if (oldInfo == null
+ || !Objects.equals(mediaController, oldInfo.mMediaController)
+ // MediaMetadata.equals() doesn't check some relevant fields
+ || !Objects.equals(playbackState, oldInfo.mPlaybackState)) {
+ return new CombinedInfo(mediaController, metadata, playbackState);
+ } else {
+ return oldInfo;
+ }
+ }
+
private final PlaybackInfo mPlaybackInfo = new PlaybackInfo();
@@ -217,6 +239,11 @@
return mPlaybackInfo;
}
+ @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+ LiveData<CombinedInfo> getCombinedInfoForTesting() {
+ return mCombinedInfo;
+ }
+
/**
* Contains LiveDatas related to the current PlaybackState. A single instance of this object is
* created for each PlaybackViewModel.
@@ -301,9 +328,8 @@
state -> state == null ? MediaSession.QueueItem.UNKNOWN_ID
: state.getActiveQueueItemId());
- private final LiveData<List<RawCustomPlaybackAction>> mCustomActions = distinct(
- map(mCombinedInfo,
- this::getCustomActions));
+ private final LiveData<List<RawCustomPlaybackAction>> mCustomActions =
+ distinct(map(mCombinedInfo, this::getCustomActions));
private PlaybackInfo() {
}
diff --git a/car-media-common/tests/robotests/src/com/android/car/media/common/playback/PlaybackViewModelTest.java b/car-media-common/tests/robotests/src/com/android/car/media/common/playback/PlaybackViewModelTest.java
index 82a7a6c..0e3e0c3 100644
--- a/car-media-common/tests/robotests/src/com/android/car/media/common/playback/PlaybackViewModelTest.java
+++ b/car-media-common/tests/robotests/src/com/android/car/media/common/playback/PlaybackViewModelTest.java
@@ -21,14 +21,16 @@
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import static org.robolectric.RuntimeEnvironment.application;
+import android.annotation.NonNull;
import android.media.MediaDescription;
import android.media.MediaMetadata;
import android.media.session.MediaController;
import android.media.session.MediaSession;
import android.media.session.PlaybackState;
-import android.support.annotation.NonNull;
import androidx.arch.core.executor.testing.InstantTaskExecutorRule;
@@ -179,6 +181,52 @@
assertThat(observer.getObservedValue()).isEqualTo(true);
}
+ @Test
+ public void testChangeMediaSource_combinedInfoConsistent() {
+ // Ensure getters are consistent with values delivered by callback
+ when(mMediaController.getMetadata()).thenReturn(mMediaMetadata);
+ when(mMediaController.getPlaybackState()).thenReturn(mPlaybackState);
+ deliverValuesToCallbacks(mCapturedCallback, mMediaMetadata, mPlaybackState);
+
+ // Create new MediaController and associated callback captor
+ MediaController newController = mock(MediaController.class);
+ ArgumentCaptor<MediaController.Callback> newCallbackCaptor =
+ ArgumentCaptor.forClass(MediaController.Callback.class);
+ doNothing().when(newController).registerCallback(newCallbackCaptor.capture());
+
+ // Wire up new data for new MediaController
+ MediaMetadata newMetadata = mock(MediaMetadata.class);
+ PlaybackState newPlaybackState = mock(PlaybackState.class);
+ when(newController.getMetadata()).thenReturn(newMetadata);
+ when(newController.getPlaybackState()).thenReturn(newPlaybackState);
+
+ // Ensure that whenever the CombinedInfo value changes, all values are coming from the
+ // same MediaController.
+ mPlaybackViewModel.getCombinedInfoForTesting().observe(mLifecycleOwner, combinedInfo -> {
+ if (combinedInfo.mMetadata == newMetadata
+ || combinedInfo.mPlaybackState == newPlaybackState) {
+ assertThat(combinedInfo.mMediaController).isSameAs(newController);
+ }
+ if (combinedInfo.mMetadata == mMediaMetadata
+ || combinedInfo.mPlaybackState == mPlaybackState) {
+ assertThat(combinedInfo.mMediaController).isSameAs(mMediaController);
+ }
+ });
+
+ mPlaybackViewModel.setMediaController(dataOf(newController));
+ deliverValuesToCallbacks(newCallbackCaptor, newMetadata, newPlaybackState);
+ }
+
+ private void deliverValuesToCallbacks(
+ ArgumentCaptor<MediaController.Callback> callbackCaptor,
+ MediaMetadata metadata,
+ PlaybackState playbackState) {
+ for (MediaController.Callback callback : callbackCaptor.getAllValues()) {
+ callback.onMetadataChanged(metadata);
+ callback.onPlaybackStateChanged(playbackState);
+ }
+ }
+
@NonNull
private MediaSession.QueueItem createQueueItem(String title, int queueId) {
MediaDescription description = new MediaDescription.Builder().setTitle(title).build();