Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8140ae8d authored by Robert Snoeberger's avatar Robert Snoeberger Committed by Android (Google) Code Review
Browse files

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

parents 93e4930a 21d133ed
Loading
Loading
Loading
Loading
+88 −37
Original line number Original line Diff line number Diff line
@@ -23,6 +23,7 @@ import android.os.SystemClock
import android.view.GestureDetector
import android.view.GestureDetector
import android.view.MotionEvent
import android.view.MotionEvent
import android.view.View
import android.view.View
import android.view.ViewConfiguration
import android.widget.SeekBar
import android.widget.SeekBar
import androidx.annotation.AnyThread
import androidx.annotation.AnyThread
import androidx.annotation.WorkerThread
import androidx.annotation.WorkerThread
@@ -31,10 +32,10 @@ import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.LiveData
import androidx.lifecycle.LiveData
import com.android.systemui.dagger.qualifiers.Background
import com.android.systemui.dagger.qualifiers.Background
import com.android.systemui.util.concurrency.RepeatableExecutor
import com.android.systemui.util.concurrency.RepeatableExecutor
import java.util.concurrent.Executor
import javax.inject.Inject
import javax.inject.Inject


private const val POSITION_UPDATE_INTERVAL_MILLIS = 100L
private const val POSITION_UPDATE_INTERVAL_MILLIS = 100L
private const val MIN_FLING_VELOCITY_SCALE_FACTOR = 10


private fun PlaybackState.isInMotion(): Boolean {
private fun PlaybackState.isInMotion(): Boolean {
    return this.state == PlaybackState.STATE_PLAYING ||
    return this.state == PlaybackState.STATE_PLAYING ||
@@ -105,6 +106,9 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
    }
    }
    private var cancel: Runnable? = null
    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. */
    /** Listening state (QS open or closed) is used to control polling of progress. */
    var listening = true
    var listening = true
        set(value) = bgExecutor.execute {
        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.
     * Handle request to change the current position in the media track.
     * @param position Place to seek to in the 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)
            controller?.transportControls?.seekTo(position)
            // Invalidate the cached playbackState to avoid the thumb jumping back to the previous
            // Invalidate the cached playbackState to avoid the thumb jumping back to the previous
            // position.
            // position.
            playbackState = null
            playbackState = null
            scrubbing = false
        }
    }
    }


    /**
    /**
@@ -181,7 +230,7 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R


    @WorkerThread
    @WorkerThread
    private fun checkIfPollingNeeded() {
    private fun checkIfPollingNeeded() {
        val needed = listening && playbackState?.isInMotion() ?: false
        val needed = listening && !scrubbing && playbackState?.isInMotion() ?: false
        if (needed) {
        if (needed) {
            if (cancel == null) {
            if (cancel == null) {
                cancel = bgExecutor.executeRepeatedly(this::checkPlaybackPosition, 0L,
                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. */
    /** Gets a listener to attach to the seek bar to handle seeking. */
    val seekBarListener: SeekBar.OnSeekBarChangeListener
    val seekBarListener: SeekBar.OnSeekBarChangeListener
        get() {
        get() {
            return SeekBarChangeListener(this, bgExecutor)
            return SeekBarChangeListener(this)
        }
        }


    /** Attach touch handlers to the seek bar view. */
    /** Attach touch handlers to the seek bar view. */
    fun attachTouchHandlers(bar: SeekBar) {
    fun attachTouchHandlers(bar: SeekBar) {
        bar.setOnSeekBarChangeListener(seekBarListener)
        bar.setOnSeekBarChangeListener(seekBarListener)
        bar.setOnTouchListener(SeekBarTouchListener(bar))
        bar.setOnTouchListener(SeekBarTouchListener(this, bar))
    }
    }


    private class SeekBarChangeListener(
    private class SeekBarChangeListener(
        val viewModel: SeekBarViewModel,
        val viewModel: SeekBarViewModel
        val bgExecutor: Executor
    ) : SeekBar.OnSeekBarChangeListener {
    ) : SeekBar.OnSeekBarChangeListener {
        override fun onProgressChanged(bar: SeekBar, progress: Int, fromUser: Boolean) {
        override fun onProgressChanged(bar: SeekBar, progress: Int, fromUser: Boolean) {
            if (fromUser) {
            if (fromUser) {
                bgExecutor.execute {
                viewModel.onSeekProgress(progress.toLong())
                    viewModel.onSeek(progress.toLong())
                }
            }
            }
        }
        }

        override fun onStartTrackingTouch(bar: SeekBar) {
        override fun onStartTrackingTouch(bar: SeekBar) {
            viewModel.onSeekStarting()
        }
        }

        override fun onStopTrackingTouch(bar: SeekBar) {
        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.
     * they intend to scroll the carousel.
     */
     */
    private class SeekBarTouchListener(
    private class SeekBarTouchListener(
        private val viewModel: SeekBarViewModel,
        private val bar: SeekBar
        private val bar: SeekBar
    ) : View.OnTouchListener, GestureDetector.OnGestureListener {
    ) : View.OnTouchListener, GestureDetector.OnGestureListener {


        // Gesture detector helps decide which touch events to intercept.
        // Gesture detector helps decide which touch events to intercept.
        private val detector = GestureDetectorCompat(bar.context, this)
        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.
         * 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) {
            if (view != bar) {
                return false
                return false
            }
            }
            val shouldGoToSeekBar = detector.onTouchEvent(event)
            detector.onTouchEvent(event)
            return !shouldGoToSeekBar
            return !shouldGoToSeekBar
        }
        }


@@ -295,16 +345,16 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
            // Set the min, max boundaries of the thumb box.
            // 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.
            // I'm cheating by using the height of the seek bar as the width of the box.
            val halfHeight: Int = bar.height / 2
            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
            // If the x position of the down event is within the box, then request that the parent
            // not intercept the event.
            // not intercept the event.
            val x = Math.round(event.x)
            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)
                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.
         * 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.
         * 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,
            distanceX: Float,
            distanceY: Float
            distanceY: Float
        ): Boolean {
        ): 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.
         * 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(
        override fun onFling(
            eventStart: MotionEvent,
            eventStart: MotionEvent,
@@ -343,8 +392,10 @@ class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: R
            velocityX: Float,
            velocityX: Float,
            velocityY: Float
            velocityY: Float
        ): Boolean {
        ): 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) {}
        override fun onShowPress(event: MotionEvent) {}
+169 −10
Original line number Original line Diff line number Diff line
@@ -40,6 +40,7 @@ import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.ArgumentCaptor
import org.mockito.Mock
import org.mockito.Mock
import org.mockito.Mockito.any
import org.mockito.Mockito.any
import org.mockito.Mockito.eq
import org.mockito.Mockito.mock
import org.mockito.Mockito.mock
import org.mockito.Mockito.never
import org.mockito.Mockito.never
import org.mockito.Mockito.times
import org.mockito.Mockito.times
@@ -204,7 +205,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {


    @Test
    @Test
    fun updateElapsedTime() {
    fun updateElapsedTime() {
        // GIVEN that the PlaybackState contins the current position
        // GIVEN that the PlaybackState contains the current position
        val position = 200L
        val position = 200L
        val state = PlaybackState.Builder().run {
        val state = PlaybackState.Builder().run {
            setState(PlaybackState.STATE_PLAYING, position, 1f)
            setState(PlaybackState.STATE_PLAYING, position, 1f)
@@ -246,7 +247,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
    }
    }


    @Test
    @Test
    fun handleSeek() {
    fun onSeek() {
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        viewModel.updateController(mockController)
        viewModel.updateController(mockController)
        // WHEN user input is dispatched
        // WHEN user input is dispatched
@@ -258,19 +259,73 @@ public class SeekBarViewModelTest : SysuiTestCase() {
    }
    }


    @Test
    @Test
    fun handleProgressChangedUser() {
    fun onSeekWithFalse() {
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        viewModel.updateController(mockController)
        viewModel.updateController(mockController)
        // WHEN a false is received during the seek gesture
        val pos = 42L
        with(viewModel) {
            onSeekStarting()
            onSeekFalse()
            onSeek(pos)
        }
        fakeExecutor.runAllReady()
        // THEN the seek is rejected and the transport never receives seekTo
        verify(mockTransport, never()).seekTo(pos)
    }

    @Test
    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
        // WHEN user starts dragging the seek bar
        val pos = 42
        val pos = 42
        viewModel.seekBarListener.onProgressChanged(SeekBar(context), pos, true)
        val bar = SeekBar(context)
        with(viewModel.seekBarListener) {
            onStartTrackingTouch(bar)
            onProgressChanged(bar, pos, true)
        }
        fakeExecutor.runAllReady()
        fakeExecutor.runAllReady()
        // THEN transport controls should be used
        // THEN then elapsed time should be updated
        verify(mockTransport).seekTo(pos.toLong())
        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
    @Test
    fun handleProgressChangedOther() {
    fun onProgressChangedNotFromUser() {
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        viewModel.updateController(mockController)
        viewModel.updateController(mockController)
        // WHEN user starts dragging the seek bar
        // WHEN user starts dragging the seek bar
@@ -282,7 +337,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
    }
    }


    @Test
    @Test
    fun handleStartTrackingTouch() {
    fun onStartTrackingTouch() {
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        viewModel.updateController(mockController)
        viewModel.updateController(mockController)
        // WHEN user starts dragging the seek bar
        // WHEN user starts dragging the seek bar
@@ -297,7 +352,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
    }
    }


    @Test
    @Test
    fun handleStopTrackingTouch() {
    fun onStopTrackingTouch() {
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        whenever(mockController.getTransportControls()).thenReturn(mockTransport)
        viewModel.updateController(mockController)
        viewModel.updateController(mockController)
        // WHEN user ends drag
        // WHEN user ends drag
@@ -311,6 +366,26 @@ public class SeekBarViewModelTest : SysuiTestCase() {
        verify(mockTransport).seekTo(pos.toLong())
        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
    @Test
    fun queuePollTaskWhenPlaying() {
    fun queuePollTaskWhenPlaying() {
        // GIVEN that the track is playing
        // GIVEN that the track is playing
@@ -369,7 +444,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
        }
        }
        // AND the playback state is playing
        // AND the playback state is playing
        val state = PlaybackState.Builder().run {
        val state = PlaybackState.Builder().run {
            setState(PlaybackState.STATE_STOPPED, 200L, 1f)
            setState(PlaybackState.STATE_PLAYING, 200L, 1f)
            build()
            build()
        }
        }
        whenever(mockController.getPlaybackState()).thenReturn(state)
        whenever(mockController.getPlaybackState()).thenReturn(state)
@@ -397,6 +472,90 @@ public class SeekBarViewModelTest : SysuiTestCase() {
        assertThat(fakeExecutor.numPending()).isEqualTo(1)
        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
    @Test
    fun startListeningQueuesPollTask() {
    fun startListeningQueuesPollTask() {
        // GIVEN not listening
        // GIVEN not listening