Merge "[Media] Ensure that we update the seekbar visibility whenever its enabled status changes." into tm-dev

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

@@ -176,9 +176,12 @@ public class MediaControlPanel {
     private String mPackageName;
 
     private boolean mIsScrubbing = false;
+    private boolean mIsSeekBarEnabled = false;
 
     private final SeekBarViewModel.ScrubbingChangeListener mScrubbingChangeListener =
             this::setIsScrubbing;
+    private final SeekBarViewModel.EnabledChangeListener mEnabledChangeListener =
+            this::setIsSeekBarEnabled;
 
     /**
      * Initialize a new control panel
@@ -235,8 +238,9 @@ public class MediaControlPanel {
     public void onDestroy() {
         if (mSeekBarObserver != null) {
             mSeekBarViewModel.getProgress().removeObserver(mSeekBarObserver);
-            mSeekBarViewModel.removeScrubbingChangeListener(mScrubbingChangeListener);
         }
+        mSeekBarViewModel.removeScrubbingChangeListener(mScrubbingChangeListener);
+        mSeekBarViewModel.removeEnabledChangeListener(mEnabledChangeListener);
         mSeekBarViewModel.onDestroy();
         mMediaViewController.onDestroy();
     }
@@ -283,7 +287,7 @@ public class MediaControlPanel {
     }
 
     /** Sets whether the user is touching the seek bar to change the track position. */
-    public void setIsScrubbing(boolean isScrubbing) {
+    private void setIsScrubbing(boolean isScrubbing) {
         if (mMediaData == null || mMediaData.getSemanticActions() == null) {
             return;
         }
@@ -295,6 +299,14 @@ public class MediaControlPanel {
                 updateDisplayForScrubbingChange(mMediaData.getSemanticActions()));
     }
 
+    private void setIsSeekBarEnabled(boolean isSeekBarEnabled) {
+        if (isSeekBarEnabled == this.mIsSeekBarEnabled) {
+            return;
+        }
+        this.mIsSeekBarEnabled = isSeekBarEnabled;
+        updateSeekBarVisibility();
+    }
+
     /**
      * Get the context
      *
@@ -313,6 +325,7 @@ public class MediaControlPanel {
         mSeekBarViewModel.getProgress().observeForever(mSeekBarObserver);
         mSeekBarViewModel.attachTouchHandlers(vh.getSeekBar());
         mSeekBarViewModel.setScrubbingChangeListener(mScrubbingChangeListener);
+        mSeekBarViewModel.setEnabledChangeListener(mEnabledChangeListener);
         mMediaViewController.attach(player, MediaViewController.TYPE.PLAYER);
 
         vh.getPlayer().setOnLongClickListener(v -> {
@@ -450,8 +463,8 @@ public class MediaControlPanel {
 
         bindOutputSwitcherChip(data);
         bindLongPressMenu(data);
-        bindActionButtons(data);
         bindScrubbingTime(data);
+        bindActionButtons(data);
 
         boolean isSongUpdated = bindSongMetadata(data);
         bindArtworkAndColors(data, isSongUpdated);
@@ -735,13 +748,18 @@ public class MediaControlPanel {
                         /* showInCompact= */ false);
             }
         }
+
+        updateSeekBarVisibility();
+    }
+
+    private void updateSeekBarVisibility() {
+        ConstraintSet expandedSet = mMediaViewController.getExpandedLayout();
         expandedSet.setVisibility(R.id.media_progress_bar, getSeekBarVisibility());
-        expandedSet.setAlpha(R.id.media_progress_bar, mSeekBarViewModel.getEnabled() ? 1.0f : 0.0f);
+        expandedSet.setAlpha(R.id.media_progress_bar, mIsSeekBarEnabled ? 1.0f : 0.0f);
     }
 
     private int getSeekBarVisibility() {
-        boolean seekbarEnabled = mSeekBarViewModel.getEnabled();
+        if (mIsSeekBarEnabled) {
-        if (seekbarEnabled) {
             return ConstraintSet.VISIBLE;
         }
         // If disabled and "neighbours" are visible, set progress bar to INVISIBLE instead of GONE
@@ -751,8 +769,7 @@ public class MediaControlPanel {
 
     private boolean areAnyExpandedBottomActionsVisible() {
         ConstraintSet expandedSet = mMediaViewController.getExpandedLayout();
-        int[] referencedIds = mMediaViewHolder.getActionsTopBarrier().getReferencedIds();
+        for (int id : MediaViewHolder.Companion.getExpandedBottomActionIds()) {
-        for (int id : referencedIds) {
             if (expandedSet.getVisibility(id) == ConstraintSet.VISIBLE) {
                 return true;
             }
@@ -872,7 +889,6 @@ public class MediaControlPanel {
     }
 
     /** Updates all the views that might change due to a scrubbing state change. */
-    // TODO(b/209656742): Handle scenarios where actionPrev and/or actionNext aren't active.
     private void updateDisplayForScrubbingChange(@NonNull MediaButton semanticActions) {
         // Update visibilities of the scrubbing time views and the scrubbing-dependent buttons.
         bindScrubbingTime(mMediaData);

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

@@ -187,5 +187,17 @@ class MediaViewHolder constructor(itemView: View) {
             R.id.action3,
             R.id.action4
         )
+
+        val expandedBottomActionIds = setOf(
+            R.id.actionPrev,
+            R.id.actionNext,
+            R.id.action0,
+            R.id.action1,
+            R.id.action2,
+            R.id.action3,
+            R.id.action4,
+            R.id.media_scrubbing_elapsed_time,
+            R.id.media_scrubbing_total_time
+        )
     }
-}
+}

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

@@ -76,7 +76,11 @@ class SeekBarViewModel @Inject constructor(
 ) {
     private var _data = Progress(false, false, false, false, null, 0)
         set(value) {
+            val enabledChanged = value.enabled != field.enabled
             field = value
+            if (enabledChanged) {
+                enabledChangeListener?.onEnabledChanged(value.enabled)
+            }
             _progress.postValue(value)
         }
     private val _progress = MutableLiveData<Progress>().apply {
@@ -122,6 +126,7 @@ class SeekBarViewModel @Inject constructor(
         }
 
     private var scrubbingChangeListener: ScrubbingChangeListener? = null
+    private var enabledChangeListener: EnabledChangeListener? = null
 
     /** Set to true when the user is touching the seek bar to change the position. */
     private var scrubbing = false
@@ -136,8 +141,6 @@ class SeekBarViewModel @Inject constructor(
 
     lateinit var logSeek: () -> Unit
 
-    fun getEnabled() = _data.enabled
-
     /**
      * Event indicating that the user has started interacting with the seek bar.
      */
@@ -189,6 +192,9 @@ class SeekBarViewModel @Inject constructor(
 
     /**
      * Updates media information.
+     *
+     * This function makes a binder call, so it must happen on a worker thread.
+     *
      * @param mediaController controller for media session
      */
     @WorkerThread
@@ -232,6 +238,7 @@ class SeekBarViewModel @Inject constructor(
         cancel?.run()
         cancel = null
         scrubbingChangeListener = null
+        enabledChangeListener = null
     }
 
     @WorkerThread
@@ -279,11 +286,26 @@ class SeekBarViewModel @Inject constructor(
         }
     }
 
+    fun setEnabledChangeListener(listener: EnabledChangeListener) {
+        enabledChangeListener = listener
+    }
+
+    fun removeEnabledChangeListener(listener: EnabledChangeListener) {
+        if (listener == enabledChangeListener) {
+            enabledChangeListener = null
+        }
+    }
+
     /** Listener interface to be notified when the user starts or stops scrubbing. */
     interface ScrubbingChangeListener {
         fun onScrubbingChanged(scrubbing: Boolean)
     }
 
+    /** Listener interface to be notified when the seekbar's enabled status changes. */
+    interface EnabledChangeListener {
+        fun onEnabledChanged(enabled: Boolean)
+    }
+
     private class SeekBarChangeListener(
         val viewModel: SeekBarViewModel
     ) : SeekBar.OnSeekBarChangeListener {

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

@@ -291,7 +291,7 @@ public class MediaControlPanelTest : SysuiTestCase() {
         seamlessButton = View(context)
         seamlessIcon = ImageView(context)
         seamlessText = TextView(context)
-        seekBar = SeekBar(context)
+        seekBar = SeekBar(context).also { it.id = R.id.media_progress_bar }
         settings = ImageButton(context)
         cancel = View(context)
         cancelText = TextView(context)
@@ -539,8 +539,8 @@ public class MediaControlPanelTest : SysuiTestCase() {
     }
 
     @Test
-    fun bind_seekBarDisabled_seekBarVisibilityIsSetToInvisible() {
+    fun bind_seekBarDisabled_hasActions_seekBarVisibilityIsSetToInvisible() {
-        whenever(seekBarViewModel.getEnabled()).thenReturn(false)
+        useRealConstraintSets()
 
         val icon = context.getDrawable(android.R.drawable.ic_media_play)
         val semanticActions = MediaButton(
@@ -550,21 +550,84 @@ public class MediaControlPanelTest : SysuiTestCase() {
         val state = mediaData.copy(semanticActions = semanticActions)
 
         player.attachPlayer(viewHolder)
+        getEnabledChangeListener().onEnabledChanged(enabled = false)
+
         player.bindPlayer(state, PACKAGE)
 
-        verify(expandedSet).setVisibility(R.id.media_progress_bar, ConstraintSet.INVISIBLE)
+        assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.INVISIBLE)
     }
 
     @Test
     fun bind_seekBarDisabled_noActions_seekBarVisibilityIsSetToGone() {
-        whenever(seekBarViewModel.getEnabled()).thenReturn(false)
+        useRealConstraintSets()
+
+        val state = mediaData.copy(semanticActions = MediaButton())
+        player.attachPlayer(viewHolder)
+        getEnabledChangeListener().onEnabledChanged(enabled = false)
+
+        player.bindPlayer(state, PACKAGE)
+
+        assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.GONE)
+    }
+
+    @Test
+    fun bind_seekBarEnabled_seekBarVisible() {
+        useRealConstraintSets()
+
+        val state = mediaData.copy(semanticActions = MediaButton())
+        player.attachPlayer(viewHolder)
+        getEnabledChangeListener().onEnabledChanged(enabled = true)
+
+        player.bindPlayer(state, PACKAGE)
+
+        assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.VISIBLE)
+    }
+
+    @Test
+    fun seekBarChangesToEnabledAfterBind_seekBarChangesToVisible() {
+        useRealConstraintSets()
+
+        val state = mediaData.copy(semanticActions = MediaButton())
+        player.attachPlayer(viewHolder)
+        player.bindPlayer(state, PACKAGE)
+
+        getEnabledChangeListener().onEnabledChanged(enabled = true)
+
+        assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.VISIBLE)
+    }
+
+    @Test
+    fun seekBarChangesToDisabledAfterBind_noActions_seekBarChangesToGone() {
+        useRealConstraintSets()
 
         val state = mediaData.copy(semanticActions = MediaButton())
 
         player.attachPlayer(viewHolder)
+        getEnabledChangeListener().onEnabledChanged(enabled = true)
         player.bindPlayer(state, PACKAGE)
 
-        verify(expandedSet).setVisibility(R.id.media_progress_bar, ConstraintSet.INVISIBLE)
+        getEnabledChangeListener().onEnabledChanged(enabled = false)
+
+        assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.GONE)
+    }
+
+    @Test
+    fun seekBarChangesToDisabledAfterBind_hasActions_seekBarChangesToInvisible() {
+        useRealConstraintSets()
+
+        val icon = context.getDrawable(android.R.drawable.ic_media_play)
+        val semanticActions = MediaButton(
+            nextOrCustom = MediaAction(icon, Runnable {}, "next", null)
+        )
+        val state = mediaData.copy(semanticActions = semanticActions)
+
+        player.attachPlayer(viewHolder)
+        getEnabledChangeListener().onEnabledChanged(enabled = true)
+        player.bindPlayer(state, PACKAGE)
+
+        getEnabledChangeListener().onEnabledChanged(enabled = false)
+
+        assertThat(expandedSet.getVisibility(seekBar.id)).isEqualTo(ConstraintSet.INVISIBLE)
     }
 
     @Test
@@ -1317,4 +1380,24 @@ public class MediaControlPanelTest : SysuiTestCase() {
 
     private fun getScrubbingChangeListener(): SeekBarViewModel.ScrubbingChangeListener =
         withArgCaptor { verify(seekBarViewModel).setScrubbingChangeListener(capture()) }
+
+    private fun getEnabledChangeListener(): SeekBarViewModel.EnabledChangeListener =
+        withArgCaptor { verify(seekBarViewModel).setEnabledChangeListener(capture()) }
+
+    /**
+     *  Update our test to use real ConstraintSets instead of mocks.
+     *
+     *  Some item visibilities, such as the seekbar visibility, are dependent on other action's
+     *  visibilities. If we use mocks for the ConstraintSets, then action visibility changes are
+     *  just thrown away instead of being saved for reference later. This method sets us up to use
+     *  ConstraintSets so that we do save visibility changes.
+     *
+     *  TODO(b/229740380): Can/should we use real expanded and collapsed sets for all tests?
+     */
+    private fun useRealConstraintSets() {
+        expandedSet = ConstraintSet()
+        collapsedSet = ConstraintSet()
+        whenever(mediaViewController.expandedLayout).thenReturn(expandedSet)
+        whenever(mediaViewController.collapsedLayout).thenReturn(collapsedSet)
+    }
 }