From fd2c9fbaed2384c0f2e1cbe5afc40a57b03a582d Mon Sep 17 00:00:00 2001 From: Michael Mikhail Date: Fri, 14 Apr 2023 16:55:00 +0000 Subject: [PATCH] Cancel vertical seekbar grab This cl cancels seekbar grabs that takes more vertical distance than horizontal one. This follows the criteria made to check for falsing touches of type classifier in FalsingManager. Bug: 243977055. Test: atest SeekBarViewModelTest Change-Id: Iee2edd81180020e3654695f15c88793b6b09ae45 --- .../models/player/SeekBarViewModel.kt | 29 +++++++++- .../models/player/SeekBarViewModelTest.kt | 55 +++++++++++++++++-- 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/models/player/SeekBarViewModel.kt b/packages/SystemUI/src/com/android/systemui/media/controls/models/player/SeekBarViewModel.kt index 2509f21242cdd..6b34f267f7ef5 100644 --- a/packages/SystemUI/src/com/android/systemui/media/controls/models/player/SeekBarViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/media/controls/models/player/SeekBarViewModel.kt @@ -26,6 +26,7 @@ import android.view.View import android.view.ViewConfiguration import android.widget.SeekBar import androidx.annotation.AnyThread +import androidx.annotation.VisibleForTesting import androidx.annotation.WorkerThread import androidx.core.view.GestureDetectorCompat import androidx.lifecycle.LiveData @@ -36,6 +37,7 @@ import com.android.systemui.plugins.FalsingManager import com.android.systemui.statusbar.NotificationMediaManager import com.android.systemui.util.concurrency.RepeatableExecutor import javax.inject.Inject +import kotlin.math.abs private const val POSITION_UPDATE_INTERVAL_MILLIS = 100L private const val MIN_FLING_VELOCITY_SCALE_FACTOR = 10 @@ -316,6 +318,10 @@ constructor( return SeekBarChangeListener(this, falsingManager) } + /** first and last motion events of seekbar grab. */ + @VisibleForTesting var firstMotionEvent: MotionEvent? = null + @VisibleForTesting var lastMotionEvent: MotionEvent? = null + /** Attach touch handlers to the seek bar view. */ fun attachTouchHandlers(bar: SeekBar) { bar.setOnSeekBarChangeListener(seekBarListener) @@ -342,6 +348,23 @@ constructor( } } + /** + * This method specifies if user made a bad seekbar grab or not. If the vertical distance from + * first touch on seekbar is more than the horizontal distance, this means that the seekbar grab + * is more vertical and should be rejected. Seekbar accepts horizontal grabs only. + * + * Single tap has the same first and last motion event, it is counted as a valid grab. + * + * @return whether the touch on seekbar is valid. + */ + private fun isValidSeekbarGrab(): Boolean { + if (firstMotionEvent == null || lastMotionEvent == null) { + return true + } + return abs(firstMotionEvent!!.x - lastMotionEvent!!.x) >= + abs(firstMotionEvent!!.y - lastMotionEvent!!.y) + } + /** Listener interface to be notified when the user starts or stops scrubbing. */ interface ScrubbingChangeListener { fun onScrubbingChanged(scrubbing: Boolean) @@ -367,7 +390,7 @@ constructor( } override fun onStopTrackingTouch(bar: SeekBar) { - if (falsingManager.isFalseTouch(MEDIA_SEEKBAR)) { + if (!viewModel.isValidSeekbarGrab() || falsingManager.isFalseTouch(MEDIA_SEEKBAR)) { viewModel.onSeekFalse() } viewModel.onSeek(bar.progress.toLong()) @@ -415,6 +438,8 @@ constructor( return false } detector.onTouchEvent(event) + // Store the last motion event done on seekbar. + viewModel.lastMotionEvent = event.copy() return !shouldGoToSeekBar } @@ -459,6 +484,8 @@ constructor( if (shouldGoToSeekBar) { bar.parent?.requestDisallowInterceptTouchEvent(true) } + // Store the first motion event done on seekbar. + viewModel.firstMotionEvent = event.copy() return shouldGoToSeekBar } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/models/player/SeekBarViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/models/player/SeekBarViewModelTest.kt index 56c91bc4525d6..e3c8b052327cc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/models/player/SeekBarViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/models/player/SeekBarViewModelTest.kt @@ -22,6 +22,7 @@ import android.media.session.MediaSession import android.media.session.PlaybackState import android.testing.AndroidTestingRunner import android.testing.TestableLooper +import android.view.MotionEvent import android.widget.SeekBar import androidx.arch.core.executor.ArchTaskExecutor import androidx.arch.core.executor.TaskExecutor @@ -466,20 +467,62 @@ public class SeekBarViewModelTest : SysuiTestCase() { whenever(mockController.getTransportControls()).thenReturn(mockTransport) whenever(falsingManager.isFalseTouch(Classifier.MEDIA_SEEKBAR)).thenReturn(true) whenever(falsingManager.isFalseTap(anyInt())).thenReturn(true) - viewModel.updateController(mockController) - val pos = 169 - viewModel.attachTouchHandlers(mockBar) + viewModel.updateController(mockController) + val pos = 40 + val bar = SeekBar(context).apply { progress = pos } with(viewModel.seekBarListener) { - onStartTrackingTouch(mockBar) - onProgressChanged(mockBar, pos, true) - onStopTrackingTouch(mockBar) + onStartTrackingTouch(bar) + onStopTrackingTouch(bar) } + fakeExecutor.runAllReady() // THEN transport controls should not be used verify(mockTransport, never()).seekTo(pos.toLong()) } + @Test + fun onSeekbarGrabInvalidTouch() { + whenever(mockController.getTransportControls()).thenReturn(mockTransport) + viewModel.firstMotionEvent = + MotionEvent.obtain(12L, 13L, MotionEvent.ACTION_DOWN, 76F, 0F, 0) + viewModel.lastMotionEvent = MotionEvent.obtain(12L, 14L, MotionEvent.ACTION_UP, 78F, 4F, 0) + val pos = 78 + + viewModel.updateController(mockController) + // WHEN user ends drag + val bar = SeekBar(context).apply { progress = pos } + with(viewModel.seekBarListener) { + onStartTrackingTouch(bar) + onStopTrackingTouch(bar) + } + fakeExecutor.runAllReady() + + // THEN transport controls should not be used + verify(mockTransport, never()).seekTo(pos.toLong()) + } + + @Test + fun onSeekbarGrabValidTouch() { + whenever(mockController.transportControls).thenReturn(mockTransport) + viewModel.firstMotionEvent = + MotionEvent.obtain(12L, 13L, MotionEvent.ACTION_DOWN, 36F, 0F, 0) + viewModel.lastMotionEvent = MotionEvent.obtain(12L, 14L, MotionEvent.ACTION_UP, 40F, 1F, 0) + val pos = 40 + + viewModel.updateController(mockController) + // WHEN user ends drag + val bar = SeekBar(context).apply { progress = pos } + with(viewModel.seekBarListener) { + onStartTrackingTouch(bar) + onStopTrackingTouch(bar) + } + fakeExecutor.runAllReady() + + // THEN transport controls should be used + verify(mockTransport).seekTo(pos.toLong()) + } + @Test fun queuePollTaskWhenPlaying() { // GIVEN that the track is playing