Merge "Use RepeatableExecutor to poll playback position" into rvc-dev am: ed33661150 am: b9dd87981c

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11663599 Change-Id: Idb915e8114fb158c853c9b390430316426e316b3
2020-06-05 03:17:03 +00:00
parent 7088a92dd6 b9dd87981c
commit 63567f5928
6 changed files with 92 additions and 41 deletions
--- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java
@@ -49,15 +49,17 @@ import com.android.settingslib.Utils;
 import com.android.settingslib.media.MediaOutputSliceConstants;
 import com.android.settingslib.widget.AdaptiveIcon;
 import com.android.systemui.R;
+import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.plugins.ActivityStarter;
 import com.android.systemui.util.animation.TransitionLayout;
-import com.android.systemui.util.concurrency.DelayableExecutor;

 import org.jetbrains.annotations.NotNull;

 import java.util.List;
 import java.util.concurrent.Executor;

+import javax.inject.Inject;
+
 /**
 * A view controller used for Media Playback.
 */
@@ -93,12 +95,14 @@ public class MediaControlPanel {
     * @param backgroundExecutor background executor, used for processing artwork
     * @param activityStarter activity starter
     */
-    public MediaControlPanel(Context context, DelayableExecutor backgroundExecutor,
-            ActivityStarter activityStarter, MediaHostStatesManager mediaHostStatesManager) {
+    @Inject
+    public MediaControlPanel(Context context, @Background Executor backgroundExecutor,
+            ActivityStarter activityStarter, MediaHostStatesManager mediaHostStatesManager,
+            SeekBarViewModel seekBarViewModel) {
        mContext = context;
        mBackgroundExecutor = backgroundExecutor;
        mActivityStarter = activityStarter;
-        mSeekBarViewModel = new SeekBarViewModel(backgroundExecutor);
+        mSeekBarViewModel = seekBarViewModel;
        mMediaViewController = new MediaViewController(context, mediaHostStatesManager);
        loadDimens();
    }
--- a/packages/SystemUI/src/com/android/systemui/media/MediaViewManager.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaViewManager.kt
@@ -11,14 +11,12 @@ import android.widget.HorizontalScrollView
 import android.widget.LinearLayout
 import androidx.core.view.GestureDetectorCompat
 import com.android.systemui.R
-import com.android.systemui.dagger.qualifiers.Background
-import com.android.systemui.plugins.ActivityStarter
 import com.android.systemui.qs.PageIndicator
 import com.android.systemui.statusbar.notification.VisualStabilityManager
 import com.android.systemui.util.animation.UniqueObjectHostView
 import com.android.systemui.util.animation.requiresRemeasuring
-import com.android.systemui.util.concurrency.DelayableExecutor
 import javax.inject.Inject
+import javax.inject.Provider
 import javax.inject.Singleton

 private const val FLING_SLOP = 1000000
@@ -30,9 +28,8 @@ private const val FLING_SLOP = 1000000
@Singleton
 class MediaViewManager @Inject constructor(
    private val context: Context,
-    @Background private val backgroundExecutor: DelayableExecutor,
+    private val mediaControlPanelFactory: Provider<MediaControlPanel>,
    private val visualStabilityManager: VisualStabilityManager,
-    private val activityStarter: ActivityStarter,
    private val mediaHostStatesManager: MediaHostStatesManager,
    mediaManager: MediaDataCombineLatest
 ) {
@@ -265,8 +262,7 @@ class MediaViewManager @Inject constructor(
        }
        var existingPlayer = mediaPlayers[key]
        if (existingPlayer == null) {
-            existingPlayer = MediaControlPanel(context, backgroundExecutor, activityStarter,
-                    mediaHostStatesManager)
+            existingPlayer = mediaControlPanelFactory.get()
            existingPlayer.attach(PlayerViewHolder.create(LayoutInflater.from(context),
                    mediaContent))
            mediaPlayers[key] = existingPlayer
@@ -289,7 +285,7 @@ class MediaViewManager @Inject constructor(
                needsReordering = true
            }
        }
-        existingPlayer.bind(data)
+        existingPlayer?.bind(data)
        updateMediaPaddings()
        updatePageIndicator()
    }
--- a/packages/SystemUI/src/com/android/systemui/media/SeekBarViewModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/SeekBarViewModel.kt
@@ -27,8 +27,10 @@ import androidx.annotation.AnyThread
 import androidx.annotation.WorkerThread
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.LiveData
-
-import com.android.systemui.util.concurrency.DelayableExecutor
+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

@@ -65,7 +67,7 @@ private fun PlaybackState.computePosition(duration: Long): Long {
 }

 /** ViewModel for seek bar in QS media player. */
-class SeekBarViewModel(val bgExecutor: DelayableExecutor) {
+class SeekBarViewModel @Inject constructor(@Background private val bgExecutor: RepeatableExecutor) {

    private var _data = Progress(false, false, null, null)
        set(value) {
@@ -89,10 +91,10 @@ class SeekBarViewModel(val bgExecutor: DelayableExecutor) {
    private var callback = object : MediaController.Callback() {
        override fun onPlaybackStateChanged(state: PlaybackState) {
            playbackState = state
-            if (shouldPollPlaybackPosition()) {
-                checkPlaybackPosition()
-            } else if (PlaybackState.STATE_NONE.equals(playbackState)) {
+            if (PlaybackState.STATE_NONE.equals(playbackState)) {
                clearController()
+            } else {
+                checkIfPollingNeeded()
            }
        }

@@ -100,12 +102,14 @@ class SeekBarViewModel(val bgExecutor: DelayableExecutor) {
            clearController()
        }
    }
+    private var cancel: Runnable? = null

    /** Listening state (QS open or closed) is used to control polling of progress. */
    var listening = true
-        set(value) {
-            if (value) {
-                checkPlaybackPosition()
+        set(value) = bgExecutor.execute {
+            if (field != value) {
+                field = value
+                checkIfPollingNeeded()
            }
        }

@@ -137,9 +141,7 @@ class SeekBarViewModel(val bgExecutor: DelayableExecutor) {
                playbackState?.getState() == PlaybackState.STATE_NONE ||
                (duration != null && duration <= 0)) false else true
        _data = Progress(enabled, seekAvailable, position, duration)
-        if (shouldPollPlaybackPosition()) {
-            checkPlaybackPosition()
-        }
+        checkIfPollingNeeded()
    }

    /**
@@ -151,6 +153,8 @@ class SeekBarViewModel(val bgExecutor: DelayableExecutor) {
    fun clearController() = bgExecutor.execute {
        controller = null
        playbackState = null
+        cancel?.run()
+        cancel = null
        _data = _data.copy(enabled = false)
    }

@@ -158,26 +162,34 @@ class SeekBarViewModel(val bgExecutor: DelayableExecutor) {
     * Call to clean up any resources.
     */
    @AnyThread
-    fun onDestroy() {
+    fun onDestroy() = bgExecutor.execute {
        controller = null
        playbackState = null
+        cancel?.run()
+        cancel = null
    }

-    @AnyThread
-    private fun checkPlaybackPosition(): Runnable = bgExecutor.executeDelayed({
+    @WorkerThread
+    private fun checkPlaybackPosition() {
        val duration = _data.duration ?: -1
        val currentPosition = playbackState?.computePosition(duration.toLong())?.toInt()
        if (currentPosition != null && _data.elapsedTime != currentPosition) {
            _data = _data.copy(elapsedTime = currentPosition)
        }
-        if (shouldPollPlaybackPosition()) {
-            checkPlaybackPosition()
-        }
-    }, POSITION_UPDATE_INTERVAL_MILLIS)
+    }

    @WorkerThread
-    private fun shouldPollPlaybackPosition(): Boolean {
-        return listening && playbackState?.isInMotion() ?: false
+    private fun checkIfPollingNeeded() {
+        val needed = listening && playbackState?.isInMotion() ?: false
+        if (needed) {
+            if (cancel == null) {
+                cancel = bgExecutor.executeRepeatedly(this::checkPlaybackPosition, 0L,
+                        POSITION_UPDATE_INTERVAL_MILLIS)
+            }
+        } else {
+            cancel?.run()
+            cancel = null
+        }
    }

    /** Gets a listener to attach to the seek bar to handle seeking. */
@@ -194,7 +206,7 @@ class SeekBarViewModel(val bgExecutor: DelayableExecutor) {

    private class SeekBarChangeListener(
        val viewModel: SeekBarViewModel,
-        val bgExecutor: DelayableExecutor
+        val bgExecutor: Executor
    ) : SeekBar.OnSeekBarChangeListener {
        override fun onProgressChanged(bar: SeekBar, progress: Int, fromUser: Boolean) {
            if (fromUser) {
--- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt
@@ -31,6 +31,7 @@ import android.widget.ImageButton
 import android.widget.ImageView
 import android.widget.SeekBar
 import android.widget.TextView
+import androidx.lifecycle.LiveData
 import androidx.test.filters.SmallTest
 import com.android.systemui.R
 import com.android.systemui.SysuiTestCase
@@ -41,6 +42,7 @@ import com.android.systemui.util.time.FakeSystemClock
 import com.google.common.truth.Truth.assertThat
 import org.junit.After
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentCaptor
@@ -48,6 +50,7 @@ import org.mockito.Mock
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.`when` as whenever
+import org.mockito.junit.MockitoJUnit

 private const val KEY = "TEST_KEY"
 private const val APP = "APP"
@@ -73,6 +76,8 @@ public class MediaControlPanelTest : SysuiTestCase() {
    @Mock private lateinit var holder: PlayerViewHolder
    @Mock private lateinit var view: TransitionLayout
    @Mock private lateinit var mediaHostStatesManager: MediaHostStatesManager
+    @Mock private lateinit var seekBarViewModel: SeekBarViewModel
+    @Mock private lateinit var seekBarData: LiveData<SeekBarViewModel.Progress>
    private lateinit var appIcon: ImageView
    private lateinit var appName: TextView
    private lateinit var albumView: ImageView
@@ -94,18 +99,17 @@ public class MediaControlPanelTest : SysuiTestCase() {
    private val device = MediaDeviceData(true, null, DEVICE_NAME)
    private val disabledDevice = MediaDeviceData(false, null, null)

+    @JvmField @Rule val mockito = MockitoJUnit.rule()
+
    @Before
    fun setUp() {
        bgExecutor = FakeExecutor(FakeSystemClock())

-        activityStarter = mock(ActivityStarter::class.java)
-        mediaHostStatesManager = mock(MediaHostStatesManager::class.java)
-
-        player = MediaControlPanel(context, bgExecutor, activityStarter, mediaHostStatesManager)
+        player = MediaControlPanel(context, bgExecutor, activityStarter, mediaHostStatesManager,
+                seekBarViewModel)
+        whenever(seekBarViewModel.progress).thenReturn(seekBarData)

        // Mock out a view holder for the player to attach to.
-        holder = mock(PlayerViewHolder::class.java)
-        view = mock(TransitionLayout::class.java)
        whenever(holder.player).thenReturn(view)
        appIcon = ImageView(context)
        whenever(holder.appIcon).thenReturn(appIcon)
--- a/packages/SystemUI/tests/src/com/android/systemui/media/SeekBarViewModelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/SeekBarViewModelTest.kt
@@ -29,6 +29,7 @@ import androidx.test.filters.SmallTest

 import com.android.systemui.SysuiTestCase
 import com.android.systemui.util.concurrency.FakeExecutor
+import com.android.systemui.util.concurrency.FakeRepeatableExecutor
 import com.android.systemui.util.time.FakeSystemClock
 import com.google.common.truth.Truth.assertThat

@@ -71,7 +72,7 @@ public class SeekBarViewModelTest : SysuiTestCase() {
    @Before
    fun setUp() {
        fakeExecutor = FakeExecutor(FakeSystemClock())
-        viewModel = SeekBarViewModel(fakeExecutor)
+        viewModel = SeekBarViewModel(FakeRepeatableExecutor(fakeExecutor))
        mockController = mock(MediaController::class.java)
        whenever(mockController.sessionToken).thenReturn(token1)
        mockTransport = mock(MediaController.TransportControls::class.java)
--- a/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/FakeRepeatableExecutor.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/FakeRepeatableExecutor.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.util.concurrency;
+
+/**
+ * A fake to use in tests.
+ */
+public class FakeRepeatableExecutor extends RepeatableExecutorImpl {
+
+    /**
+     * Initializes a fake RepeatableExecutor from a fake executor.
+     *
+     * Use the fake executor to actually process tasks.
+     *
+     * @param executor fake executor.
+     */
+    public FakeRepeatableExecutor(FakeExecutor executor) {
+        super(executor);
+    }
+}