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

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,
+    @Inject
-            ActivityStarter activityStarter, MediaHostStatesManager mediaHostStatesManager) {
+    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();
     }

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,
+            existingPlayer = mediaControlPanelFactory.get()
-                    mediaHostStatesManager)
             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()
     }

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.dagger.qualifiers.Background
-import com.android.systemui.util.concurrency.DelayableExecutor
+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()) {
+            if (PlaybackState.STATE_NONE.equals(playbackState)) {
-                checkPlaybackPosition()
-            } else 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) {
+        set(value) = bgExecutor.execute {
-            if (value) {
+            if (field != value) {
-                checkPlaybackPosition()
+                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()) {
+        checkIfPollingNeeded()
-            checkPlaybackPosition()
-        }
     }
 
     /**
@@ -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
+    @WorkerThread
-    private fun checkPlaybackPosition(): Runnable = bgExecutor.executeDelayed({
+    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 {
+    private fun checkIfPollingNeeded() {
-        return listening && playbackState?.isInMotion() ?: false
+        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) {

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)
+        player = MediaControlPanel(context, bgExecutor, activityStarter, mediaHostStatesManager,
-        mediaHostStatesManager = mock(MediaHostStatesManager::class.java)
+                seekBarViewModel)
-
+        whenever(seekBarViewModel.progress).thenReturn(seekBarData)
-        player = MediaControlPanel(context, bgExecutor, activityStarter, mediaHostStatesManager)
 
         // 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)

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)

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);
+    }
+}