Merge "Reject flings on seek bar" into rvc-dev

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

@@ -23,6 +23,7 @@ import android.os.SystemClock
 import android.view.GestureDetector
 import android.view.MotionEvent
 import android.view.View
+import android.view.ViewConfiguration
 import android.widget.SeekBar
 import androidx.annotation.AnyThread
 import androidx.annotation.WorkerThread
@@ -31,10 +32,10 @@ import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.LiveData
 import com.android.systemui.dagger.qualifiers.Background
 import com.android.systemui.util.concurrency.RepeatableExecutor
-import java.util.concurrent.Executor
 import javax.inject.Inject
 
 private const val POSITION_UPDATE_INTERVAL_MILLIS = 100L
+private const val MIN_FLING_VELOCITY_SCALE_FACTOR = 10
 
 private fun PlaybackState.isInMotion(): Boolean {
     return this.state == PlaybackState.STATE_PLAYING ||
@@ -105,6 +106,9 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
     }
     private var cancel: Runnable? = null
 
+    /** Indicates if the seek interaction is considered a false guesture. */
+    private var isFalseSeek = false
+
     /** Listening state (QS open or closed) is used to control polling of progress. */
     var listening = true
         set(value) = bgExecutor.execute {
@@ -114,16 +118,61 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
             }
         }
 
+    /** Set to true when the user is touching the seek bar to change the position. */
+    private var scrubbing = false
+        set(value) {
+            if (field != value) {
+                field = value
+                checkIfPollingNeeded()
+            }
+        }
+
+    /**
+     * Event indicating that the user has started interacting with the seek bar.
+     */
+    @AnyThread
+    fun onSeekStarting() = bgExecutor.execute {
+        scrubbing = true
+        isFalseSeek = false
+    }
+
+    /**
+     * Event indicating that the user has moved the seek bar but hasn't yet finished the gesture.
+     * @param position Current location in the track.
+     */
+    @AnyThread
+    fun onSeekProgress(position: Long) = bgExecutor.execute {
+        if (scrubbing) {
+            _data = _data.copy(elapsedTime = position.toInt())
+        }
+    }
+
+    /**
+     * Event indicating that the seek interaction is a false gesture and it should be ignored.
+     */
+    @AnyThread
+    fun onSeekFalse() = bgExecutor.execute {
+        if (scrubbing) {
+            isFalseSeek = true
+        }
+    }
+
     /**
      * Handle request to change the current position in the media track.
      * @param position Place to seek to in the track.
      */
-    @WorkerThread
+    @AnyThread
-    fun onSeek(position: Long) {
+    fun onSeek(position: Long) = bgExecutor.execute {
+        if (isFalseSeek) {
+            scrubbing = false
+            checkPlaybackPosition()
+        } else {
             controller?.transportControls?.seekTo(position)
             // Invalidate the cached playbackState to avoid the thumb jumping back to the previous
             // position.
             playbackState = null
+            scrubbing = false
+        }
     }
 
     /**
@@ -181,7 +230,7 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
 
     @WorkerThread
     private fun checkIfPollingNeeded() {
-        val needed = listening && playbackState?.isInMotion() ?: false
+        val needed = listening && !scrubbing && playbackState?.isInMotion() ?: false
         if (needed) {
             if (cancel == null) {
                 cancel = bgExecutor.executeRepeatedly(this::checkPlaybackPosition, 0L,
@@ -196,33 +245,30 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
     /** Gets a listener to attach to the seek bar to handle seeking. */
     val seekBarListener: SeekBar.OnSeekBarChangeListener
         get() {
-            return SeekBarChangeListener(this, bgExecutor)
+            return SeekBarChangeListener(this)
         }
 
     /** Attach touch handlers to the seek bar view. */
     fun attachTouchHandlers(bar: SeekBar) {
         bar.setOnSeekBarChangeListener(seekBarListener)
-        bar.setOnTouchListener(SeekBarTouchListener(bar))
+        bar.setOnTouchListener(SeekBarTouchListener(this, bar))
     }
 
     private class SeekBarChangeListener(
-        val viewModel: SeekBarViewModel,
+        val viewModel: SeekBarViewModel
-        val bgExecutor: Executor
     ) : SeekBar.OnSeekBarChangeListener {
         override fun onProgressChanged(bar: SeekBar, progress: Int, fromUser: Boolean) {
             if (fromUser) {
-                bgExecutor.execute {
+                viewModel.onSeekProgress(progress.toLong())
-                    viewModel.onSeek(progress.toLong())
-                }
             }
         }
+
         override fun onStartTrackingTouch(bar: SeekBar) {
+            viewModel.onSeekStarting()
         }
+
         override fun onStopTrackingTouch(bar: SeekBar) {
-            val pos = bar.progress.toLong()
+            viewModel.onSeek(bar.progress.toLong())
-            bgExecutor.execute {
-                viewModel.onSeek(pos)
-            }
         }
     }
 
@@ -233,14 +279,18 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
      * they intend to scroll the carousel.
      */
     private class SeekBarTouchListener(
+        private val viewModel: SeekBarViewModel,
         private val bar: SeekBar
     ) : View.OnTouchListener, GestureDetector.OnGestureListener {
 
         // Gesture detector helps decide which touch events to intercept.
         private val detector = GestureDetectorCompat(bar.context, this)
-        // Defines a tap target around the thumb at the beginning of a gesture.
+        // Velocity threshold used to decide when a fling is considered a false gesture.
-        private var onDownTargetBoxMinX: Int = -1
+        private val flingVelocity: Int = ViewConfiguration.get(bar.context).run {
-        private var onDownTargetBoxMaxX: Int = -1
+            getScaledMinimumFlingVelocity() * MIN_FLING_VELOCITY_SCALE_FACTOR
+        }
+        // Indicates if the gesture should go to the seek bar or if it should be intercepted.
+        private var shouldGoToSeekBar = false
 
         /**
          * Decide which touch events to intercept before they reach the seek bar.
@@ -261,7 +311,7 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
             if (view != bar) {
                 return false
             }
-            val shouldGoToSeekBar = detector.onTouchEvent(event)
+            detector.onTouchEvent(event)
             return !shouldGoToSeekBar
         }
 
@@ -295,16 +345,16 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
             // Set the min, max boundaries of the thumb box.
             // I'm cheating by using the height of the seek bar as the width of the box.
             val halfHeight: Int = bar.height / 2
-            onDownTargetBoxMinX = (Math.round(thumbX) - halfHeight).toInt()
+            val targetBoxMinX = (Math.round(thumbX) - halfHeight).toInt()
-            onDownTargetBoxMaxX = (Math.round(thumbX) + halfHeight).toInt()
+            val targetBoxMaxX = (Math.round(thumbX) + halfHeight).toInt()
             // If the x position of the down event is within the box, then request that the parent
             // not intercept the event.
             val x = Math.round(event.x)
-            val accept = x >= onDownTargetBoxMinX && x <= onDownTargetBoxMaxX
+            shouldGoToSeekBar = x >= targetBoxMinX && x <= targetBoxMaxX
-            if (accept) {
+            if (shouldGoToSeekBar) {
                 bar.parent?.requestDisallowInterceptTouchEvent(true)
             }
-            return accept
+            return shouldGoToSeekBar
         }
 
         /**
@@ -312,7 +362,10 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
          *
          * This enables the user to single tap anywhere on the seek bar to seek to that position.
          */
-        override fun onSingleTapUp(event: MotionEvent) = true
+        override fun onSingleTapUp(event: MotionEvent): Boolean {
+            shouldGoToSeekBar = true
+            return true
+        }
 
         /**
          * Handle scroll events when the down event is on the thumb.
@@ -325,17 +378,13 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
             distanceX: Float,
             distanceY: Float
         ): Boolean {
-            val x = Math.round(eventStart.x)
+            return shouldGoToSeekBar
-            return x >= onDownTargetBoxMinX && x <= onDownTargetBoxMaxX
         }
 
         /**
          * Handle fling events when the down event is on the thumb.
          *
-         * TODO: Ignore entire gesture when it includes a fling.
+         * Gestures that include a fling are considered a false gesture on the seek bar.
-         * If a user is flinging, then they are probably trying to page the carousel. It would be
-         * better to ignore the entire gesture when it includes a fling. This could be achieved by
-         * reseting the seek bar position to where it was when the gesture started.
          */
         override fun onFling(
             eventStart: MotionEvent,
@@ -343,8 +392,10 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
             velocityX: Float,
             velocityY: Float
         ): Boolean {
-            val x = Math.round(eventStart.x)
+            if (Math.abs(velocityX) > flingVelocity || Math.abs(velocityY) > flingVelocity) {
-            return x >= onDownTargetBoxMinX && x <= onDownTargetBoxMaxX
+                viewModel.onSeekFalse()
+            }
+            return shouldGoToSeekBar
         }
 
         override fun onShowPress(event: MotionEvent) {}

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

@@ -40,6 +40,7 @@ import org.junit.runner.RunWith
 import org.mockito.ArgumentCaptor
 import org.mockito.Mock
 import org.mockito.Mockito.any
+import org.mockito.Mockito.eq
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.never
 import org.mockito.Mockito.times
@@ -204,7 +205,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
 
     @Test
     fun updateElapsedTime() {
-        // GIVEN that the PlaybackState contins the current position
+        // GIVEN that the PlaybackState contains the current position
         val position = 200L
         val state = PlaybackState.Builder().run {
             setState(PlaybackState.STATE_PLAYING, position, 1f)
@@ -246,7 +247,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
     }
 
     @Test
-    fun handleSeek() {
+    fun onSeek() {
         whenever(mockController.getTransportControls()).thenReturn(mockTransport)
         viewModel.updateController(mockController)
         // WHEN user input is dispatched
@@ -258,19 +259,73 @@ public class SeekBarViewModelTest : SysuiTestCase() {
     }
 
     @Test
-    fun handleProgressChangedUser() {
+    fun onSeekWithFalse() {
         whenever(mockController.getTransportControls()).thenReturn(mockTransport)
         viewModel.updateController(mockController)
-        // WHEN user starts dragging the seek bar
+        // WHEN a false is received during the seek gesture
-        val pos = 42
+        val pos = 42L
-        viewModel.seekBarListener.onProgressChanged(SeekBar(context), pos, true)
+        with(viewModel) {
+            onSeekStarting()
+            onSeekFalse()
+            onSeek(pos)
+        }
         fakeExecutor.runAllReady()
-        // THEN transport controls should be used
+        // THEN the seek is rejected and the transport never receives seekTo
-        verify(mockTransport).seekTo(pos.toLong())
+        verify(mockTransport, never()).seekTo(pos)
     }
 
     @Test
-    fun handleProgressChangedOther() {
+    fun onSeekProgress() {
+        val pos = 42L
+        with(viewModel) {
+            onSeekStarting()
+            onSeekProgress(pos)
+        }
+        fakeExecutor.runAllReady()
+        // THEN then elapsed time should be updated
+        assertThat(viewModel.progress.value!!.elapsedTime).isEqualTo(pos)
+    }
+
+    @Test
+    fun onSeekProgressWithSeekStarting() {
+        val pos = 42L
+        with(viewModel) {
+            onSeekProgress(pos)
+        }
+        fakeExecutor.runAllReady()
+        // THEN then elapsed time should not be updated
+        assertThat(viewModel.progress.value!!.elapsedTime).isNull()
+    }
+
+    @Test
+    fun onProgressChangedFromUser() {
+        // WHEN user starts dragging the seek bar
+        val pos = 42
+        val bar = SeekBar(context)
+        with(viewModel.seekBarListener) {
+            onStartTrackingTouch(bar)
+            onProgressChanged(bar, pos, true)
+        }
+        fakeExecutor.runAllReady()
+        // THEN then elapsed time should be updated
+        assertThat(viewModel.progress.value!!.elapsedTime).isEqualTo(pos)
+    }
+
+    @Test
+    fun onProgressChangedFromUserWithoutStartTrackingTouch() {
+        // WHEN user starts dragging the seek bar
+        val pos = 42
+        val bar = SeekBar(context)
+        with(viewModel.seekBarListener) {
+            onProgressChanged(bar, pos, true)
+        }
+        fakeExecutor.runAllReady()
+        // THEN then elapsed time should not be updated
+        assertThat(viewModel.progress.value!!.elapsedTime).isNull()
+    }
+
+    @Test
+    fun onProgressChangedNotFromUser() {
         whenever(mockController.getTransportControls()).thenReturn(mockTransport)
         viewModel.updateController(mockController)
         // WHEN user starts dragging the seek bar
@@ -282,7 +337,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
     }
 
     @Test
-    fun handleStartTrackingTouch() {
+    fun onStartTrackingTouch() {
         whenever(mockController.getTransportControls()).thenReturn(mockTransport)
         viewModel.updateController(mockController)
         // WHEN user starts dragging the seek bar
@@ -297,7 +352,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
     }
 
     @Test
-    fun handleStopTrackingTouch() {
+    fun onStopTrackingTouch() {
         whenever(mockController.getTransportControls()).thenReturn(mockTransport)
         viewModel.updateController(mockController)
         // WHEN user ends drag
@@ -311,6 +366,26 @@ public class SeekBarViewModelTest : SysuiTestCase() {
         verify(mockTransport).seekTo(pos.toLong())
     }
 
+    @Test
+    fun onStopTrackingTouchAfterProgress() {
+        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
+        viewModel.updateController(mockController)
+        // WHEN user starts dragging the seek bar
+        val pos = 42
+        val progPos = 84
+        val bar = SeekBar(context).apply {
+            progress = pos
+        }
+        with(viewModel.seekBarListener) {
+            onStartTrackingTouch(bar)
+            onProgressChanged(bar, progPos, true)
+            onStopTrackingTouch(bar)
+        }
+        fakeExecutor.runAllReady()
+        // THEN then elapsed time should be updated
+        verify(mockTransport).seekTo(eq(pos.toLong()))
+    }
+
     @Test
     fun queuePollTaskWhenPlaying() {
         // GIVEN that the track is playing
@@ -369,7 +444,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
         }
         // AND the playback state is playing
         val state = PlaybackState.Builder().run {
-            setState(PlaybackState.STATE_STOPPED, 200L, 1f)
+            setState(PlaybackState.STATE_PLAYING, 200L, 1f)
             build()
         }
         whenever(mockController.getPlaybackState()).thenReturn(state)
@@ -397,6 +472,90 @@ public class SeekBarViewModelTest : SysuiTestCase() {
         assertThat(fakeExecutor.numPending()).isEqualTo(1)
     }
 
+    @Test
+    fun noQueuePollTaskWhenSeeking() {
+        // GIVEN listening
+        viewModel.listening = true
+        // AND the playback state is playing
+        val state = PlaybackState.Builder().run {
+            setState(PlaybackState.STATE_PLAYING, 200L, 1f)
+            build()
+        }
+        whenever(mockController.getPlaybackState()).thenReturn(state)
+        viewModel.updateController(mockController)
+        with(fakeExecutor) {
+            advanceClockToNext()
+            runAllReady()
+        }
+        // WHEN seek starts
+        viewModel.onSeekStarting()
+        with(fakeExecutor) {
+            advanceClockToNext()
+            runAllReady()
+        }
+        // THEN an update task is not queued because we don't want it fighting with the user when
+        // they are trying to move the thumb.
+        assertThat(fakeExecutor.numPending()).isEqualTo(0)
+    }
+
+    @Test
+    fun queuePollTaskWhenDoneSeekingWithFalse() {
+        // GIVEN listening
+        viewModel.listening = true
+        // AND the playback state is playing
+        val state = PlaybackState.Builder().run {
+            setState(PlaybackState.STATE_PLAYING, 200L, 1f)
+            build()
+        }
+        whenever(mockController.getPlaybackState()).thenReturn(state)
+        viewModel.updateController(mockController)
+        with(fakeExecutor) {
+            advanceClockToNext()
+            runAllReady()
+        }
+        // WHEN seek finishes after a false
+        with(viewModel) {
+            onSeekStarting()
+            onSeekFalse()
+            onSeek(42L)
+        }
+        with(fakeExecutor) {
+            advanceClockToNext()
+            runAllReady()
+        }
+        // THEN an update task is queued because the gesture was ignored and progress was restored.
+        assertThat(fakeExecutor.numPending()).isEqualTo(1)
+    }
+
+    @Test
+    fun noQueuePollTaskWhenDoneSeeking() {
+        // GIVEN listening
+        viewModel.listening = true
+        // AND the playback state is playing
+        val state = PlaybackState.Builder().run {
+            setState(PlaybackState.STATE_PLAYING, 200L, 1f)
+            build()
+        }
+        whenever(mockController.getPlaybackState()).thenReturn(state)
+        viewModel.updateController(mockController)
+        with(fakeExecutor) {
+            advanceClockToNext()
+            runAllReady()
+        }
+        // WHEN seek finishes after a false
+        with(viewModel) {
+            onSeekStarting()
+            onSeek(42L)
+        }
+        with(fakeExecutor) {
+            advanceClockToNext()
+            runAllReady()
+        }
+        // THEN no update task is queued because we are waiting for an updated playback state to be
+        // returned in response to the seek.
+        assertThat(fakeExecutor.numPending()).isEqualTo(0)
+    }
+
     @Test
     fun startListeningQueuesPollTask() {
         // GIVEN not listening