Merge "[Media TTT] When a transfer has been initiated, listen to a future so that we can update the chip to succeeded/failed state once the future completes."

2021-12-15 23:00:58 +00:00
parent 84c6d95084 de0c86109f
commit dfc02b161a
6 changed files with 161 additions and 14 deletions
--- a/packages/SystemUI/src/com/android/systemui/media/dagger/MediaModule.java
+++ b/packages/SystemUI/src/com/android/systemui/media/dagger/MediaModule.java
@@ -20,6 +20,8 @@ import android.content.Context;
 import android.view.WindowManager;

 import com.android.systemui.dagger.SysUISingleton;
+import com.android.systemui.dagger.qualifiers.Background;
+import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.media.MediaDataManager;
 import com.android.systemui.media.MediaHierarchyManager;
 import com.android.systemui.media.MediaHost;
@@ -28,8 +30,10 @@ import com.android.systemui.media.taptotransfer.MediaTttChipController;
 import com.android.systemui.media.taptotransfer.MediaTttCommandLineHelper;
 import com.android.systemui.media.taptotransfer.MediaTttFlags;
 import com.android.systemui.statusbar.commandline.CommandRegistry;
+import com.android.systemui.util.concurrency.DelayableExecutor;

 import java.util.Optional;
+import java.util.concurrent.Executor;

 import javax.inject.Named;

@@ -79,11 +83,14 @@ public interface MediaModule {
    static Optional<MediaTttChipController> providesMediaTttChipController(
            MediaTttFlags mediaTttFlags,
            Context context,
-            WindowManager windowManager) {
+            WindowManager windowManager,
+            @Main Executor mainExecutor,
+            @Background Executor backgroundExecutor) {
        if (!mediaTttFlags.isMediaTttEnabled()) {
            return Optional.empty();
        }
-        return Optional.of(new MediaTttChipController(context, windowManager));
+        return Optional.of(new MediaTttChipController(
+                context, windowManager, mainExecutor, backgroundExecutor));
    }

    /** */
@@ -92,10 +99,12 @@ public interface MediaModule {
    static Optional<MediaTttCommandLineHelper> providesMediaTttCommandLineHelper(
            MediaTttFlags mediaTttFlags,
            CommandRegistry commandRegistry,
-            MediaTttChipController mediaTttChipController) {
+            MediaTttChipController mediaTttChipController,
+            @Main DelayableExecutor mainExecutor) {
        if (!mediaTttFlags.isMediaTttEnabled()) {
            return Optional.empty();
        }
-        return Optional.of(new MediaTttCommandLineHelper(commandRegistry, mediaTttChipController));
+        return Optional.of(new MediaTttCommandLineHelper(
+                commandRegistry, mediaTttChipController, mainExecutor));
    }
 }
--- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttChipController.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttChipController.kt
@@ -27,6 +27,10 @@ import android.widget.LinearLayout
 import android.widget.TextView
 import com.android.systemui.R
 import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.dagger.qualifiers.Background
+import com.android.systemui.dagger.qualifiers.Main
+import java.util.concurrent.Executor
+import java.util.concurrent.TimeUnit
 import javax.inject.Inject

 const val TAG = "MediaTapToTransfer"
@@ -41,6 +45,8 @@ const val TAG = "MediaTapToTransfer"
 class MediaTttChipController @Inject constructor(
    private val context: Context,
    private val windowManager: WindowManager,
+    @Main private val mainExecutor: Executor,
+    @Background private val backgroundExecutor: Executor,
 ) {

    @SuppressLint("WrongConstant") // We're allowed to use TYPE_VOLUME_OVERLAY
@@ -92,6 +98,11 @@ class MediaTttChipController @Inject constructor(
        }
        undoView.setOnClickListener(undoClickListener)

+        // Future handling
+        if (chipState is TransferInitiated) {
+            addFutureCallback(chipState)
+        }
+
        // Add view if necessary
        if (oldChipView == null) {
            windowManager.addView(chipView, windowLayoutParams)
@@ -104,4 +115,29 @@ class MediaTttChipController @Inject constructor(
        windowManager.removeView(chipView)
        chipView = null
    }
+
+    /**
+     * Adds the appropriate callbacks to [chipState.future] so that we update the chip correctly
+     * when the future resolves.
+     */
+    private fun addFutureCallback(chipState: TransferInitiated) {
+        // Listen to the future on a background thread so we don't occupy the main thread while we
+        // wait for it to complete.
+        backgroundExecutor.execute {
+            try {
+                val undoRunnable = chipState.future.get(TRANSFER_TIMEOUT_SECONDS, TimeUnit.SECONDS)
+                // Make UI changes on the main thread
+                mainExecutor.execute {
+                    displayChip(TransferSucceeded(chipState.otherDeviceName, undoRunnable))
+                }
+            } catch (ex: Exception) {
+                // TODO(b/203800327): Maybe show a failure chip here if UX decides we need one.
+                mainExecutor.execute {
+                    removeChip()
+                }
+            }
+        }
+    }
 }
+
+private const val TRANSFER_TIMEOUT_SECONDS = 10L
--- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttChipState.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttChipState.kt
@@ -18,6 +18,7 @@ package com.android.systemui.media.taptotransfer

 import androidx.annotation.StringRes
 import com.android.systemui.R
+import java.util.concurrent.Future

 /**
 * A class that stores all the information necessary to display the media tap-to-transfer chip in
@@ -43,9 +44,15 @@ class MoveCloserToTransfer(

 /**
 * A state representing that a transfer has been initiated (but not completed).
+ *
+ * @property future a future that will be resolved when the transfer has either succeeded or failed.
+ *   If the transfer succeeded, the future can optionally return an undo runnable (see
+ *   [TransferSucceeded.undoRunnable]). [MediaTttChipController] is responsible for transitioning
+ *   the chip to the [TransferSucceeded] state if the future resolves successfully.
 */
 class TransferInitiated(
-    otherDeviceName: String
+    otherDeviceName: String,
+    val future: Future<Runnable?>
 ) : MediaTttChipState(R.string.media_transfer_playing, otherDeviceName)

 /**
--- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttCommandLineHelper.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttCommandLineHelper.kt
@@ -19,9 +19,12 @@ package com.android.systemui.media.taptotransfer
 import android.util.Log
 import androidx.annotation.VisibleForTesting
 import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.dagger.qualifiers.Main
 import com.android.systemui.statusbar.commandline.Command
 import com.android.systemui.statusbar.commandline.CommandRegistry
+import com.android.systemui.util.concurrency.DelayableExecutor
 import java.io.PrintWriter
+import java.util.concurrent.FutureTask
 import javax.inject.Inject

 /**
@@ -31,7 +34,8 @@ import javax.inject.Inject
@SysUISingleton
 class MediaTttCommandLineHelper @Inject constructor(
    commandRegistry: CommandRegistry,
-    private val mediaTttChipController: MediaTttChipController
+    private val mediaTttChipController: MediaTttChipController,
+    @Main private val mainExecutor: DelayableExecutor,
 ) {
    init {
        commandRegistry.registerCommand(ADD_CHIP_COMMAND_TAG) { AddChipCommand() }
@@ -46,7 +50,12 @@ class MediaTttCommandLineHelper @Inject constructor(
                    mediaTttChipController.displayChip(MoveCloserToTransfer(otherDeviceName))
                }
                TRANSFER_INITIATED_COMMAND_NAME -> {
-                    mediaTttChipController.displayChip(TransferInitiated(otherDeviceName))
+                    val futureTask = FutureTask { fakeUndoRunnable }
+                    mediaTttChipController.displayChip(
+                        TransferInitiated(otherDeviceName, futureTask)
+                    )
+                    mainExecutor.executeDelayed({ futureTask.run() }, FUTURE_WAIT_TIME)
+
                }
                TRANSFER_SUCCEEDED_COMMAND_NAME -> {
                    mediaTttChipController.displayChip(
@@ -94,3 +103,5 @@ val MOVE_CLOSER_TO_TRANSFER_COMMAND_NAME = MoveCloserToTransfer::class.simpleNam
 val TRANSFER_INITIATED_COMMAND_NAME = TransferInitiated::class.simpleName!!
@VisibleForTesting
 val TRANSFER_SUCCEEDED_COMMAND_NAME = TransferSucceeded::class.simpleName!!
+
+private const val FUTURE_WAIT_TIME = 2000L
--- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/MediaTttChipControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/MediaTttChipControllerTest.kt
@@ -23,8 +23,11 @@ import android.widget.TextView
 import androidx.test.filters.SmallTest
 import com.android.systemui.R
 import com.android.systemui.SysuiTestCase
+import com.android.systemui.util.concurrency.FakeExecutor
 import com.android.systemui.util.mockito.any
+import com.android.systemui.util.time.FakeSystemClock
 import com.google.common.truth.Truth.assertThat
+import com.google.common.util.concurrent.SettableFuture
 import org.junit.Before
 import org.junit.Test
 import org.mockito.ArgumentCaptor
@@ -37,6 +40,11 @@ import org.mockito.MockitoAnnotations
@SmallTest
 class MediaTttChipControllerTest : SysuiTestCase() {

+    private lateinit var fakeMainClock: FakeSystemClock
+    private lateinit var fakeMainExecutor: FakeExecutor
+    private lateinit var fakeBackgroundClock: FakeSystemClock
+    private lateinit var fakeBackgroundExecutor: FakeExecutor
+
    private lateinit var mediaTttChipController: MediaTttChipController

    @Mock
@@ -45,7 +53,13 @@ class MediaTttChipControllerTest : SysuiTestCase() {
    @Before
    fun setUp() {
        MockitoAnnotations.initMocks(this)
-        mediaTttChipController = MediaTttChipController(context, windowManager)
+        fakeMainClock = FakeSystemClock()
+        fakeMainExecutor = FakeExecutor(fakeMainClock)
+        fakeBackgroundClock = FakeSystemClock()
+        fakeBackgroundExecutor = FakeExecutor(fakeBackgroundClock)
+        mediaTttChipController = MediaTttChipController(
+            context, windowManager, fakeMainExecutor, fakeBackgroundExecutor
+        )
    }

    @Test
@@ -93,15 +107,78 @@ class MediaTttChipControllerTest : SysuiTestCase() {
    }

    @Test
-    fun transferInitiated_chipTextContainsDeviceName_loadingIcon_noUndo() {
-        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME))
+    fun transferInitiated_futureNotResolvedYet_loadingIcon_noUndo() {
+        val future: SettableFuture<Runnable?> = SettableFuture.create()
+        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME, future))

+        // Don't resolve the future in any way and don't run our executors
+
+        // Assert we're still in the loading state
        val chipView = getChipView()
        assertThat(chipView.getChipText()).contains(DEVICE_NAME)
        assertThat(chipView.getLoadingIconVisibility()).isEqualTo(View.VISIBLE)
        assertThat(chipView.getUndoButton().visibility).isEqualTo(View.GONE)
    }

+    @Test
+    fun transferInitiated_futureResolvedSuccessfully_switchesToTransferSucceeded() {
+        val future: SettableFuture<Runnable?> = SettableFuture.create()
+        val undoRunnable = Runnable { }
+
+        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME, future))
+
+        future.set(undoRunnable)
+        fakeBackgroundExecutor.advanceClockToLast()
+        fakeBackgroundExecutor.runAllReady()
+        fakeMainExecutor.advanceClockToLast()
+        val numRun = fakeMainExecutor.runAllReady()
+
+        // Assert we ran the future callback
+        assertThat(numRun).isEqualTo(1)
+        // Assert that we've moved to the successful state
+        val chipView = getChipView()
+        assertThat(chipView.getChipText()).contains(DEVICE_NAME)
+        assertThat(chipView.getLoadingIconVisibility()).isEqualTo(View.GONE)
+        assertThat(chipView.getUndoButton().visibility).isEqualTo(View.VISIBLE)
+    }
+
+    @Test
+    fun transferInitiated_futureCancelled_chipRemoved() {
+        val future: SettableFuture<Runnable?> = SettableFuture.create()
+
+        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME, future))
+
+        future.cancel(true)
+        fakeBackgroundExecutor.advanceClockToLast()
+        fakeBackgroundExecutor.runAllReady()
+        fakeMainExecutor.advanceClockToLast()
+        val numRun = fakeMainExecutor.runAllReady()
+
+        // Assert we ran the future callback
+        assertThat(numRun).isEqualTo(1)
+        // Assert that we've hidden the chip
+        verify(windowManager).removeView(any())
+    }
+
+    @Test
+    fun transferInitiated_futureNotResolvedAfterTimeout_chipRemoved() {
+        val future: SettableFuture<Runnable?> = SettableFuture.create()
+        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME, future))
+
+        // We won't set anything on the future, but we will still run the executors so that we're
+        // waiting on the future resolving. If we have a bug in our code, then this test will time
+        // out because we're waiting on the future indefinitely.
+        fakeBackgroundExecutor.advanceClockToLast()
+        fakeBackgroundExecutor.runAllReady()
+        fakeMainExecutor.advanceClockToLast()
+        val numRun = fakeMainExecutor.runAllReady()
+
+        // Assert we eventually decide to not wait for the future anymore
+        assertThat(numRun).isEqualTo(1)
+        // Assert we've hidden the chip
+        verify(windowManager).removeView(any())
+    }
+
    @Test
    fun transferSucceededNullUndoRunnable_chipTextContainsDeviceName_noLoadingIcon_noUndo() {
        mediaTttChipController.displayChip(TransferSucceeded(DEVICE_NAME, undoRunnable = null))
@@ -137,14 +214,14 @@ class MediaTttChipControllerTest : SysuiTestCase() {
    @Test
    fun changeFromCloserToTransferToTransferInitiated_loadingIconAppears() {
        mediaTttChipController.displayChip(MoveCloserToTransfer(DEVICE_NAME))
-        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME))
+        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME, TEST_FUTURE))

        assertThat(getChipView().getLoadingIconVisibility()).isEqualTo(View.VISIBLE)
    }

    @Test
    fun changeFromTransferInitiatedToTransferSucceeded_loadingIconDisappears() {
-        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME))
+        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME, TEST_FUTURE))
        mediaTttChipController.displayChip(TransferSucceeded(DEVICE_NAME))

        assertThat(getChipView().getLoadingIconVisibility()).isEqualTo(View.GONE)
@@ -152,7 +229,7 @@ class MediaTttChipControllerTest : SysuiTestCase() {

    @Test
    fun changeFromTransferInitiatedToTransferSucceeded_undoButtonAppears() {
-        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME))
+        mediaTttChipController.displayChip(TransferInitiated(DEVICE_NAME, TEST_FUTURE))
        mediaTttChipController.displayChip(TransferSucceeded(DEVICE_NAME) { })

        assertThat(getChipView().getUndoButton().visibility).isEqualTo(View.VISIBLE)
@@ -182,3 +259,6 @@ class MediaTttChipControllerTest : SysuiTestCase() {
 }

 private const val DEVICE_NAME = "My Tablet"
+// Use a settable future that hasn't yet been set so that we don't immediately switch to the success
+// state.
+private val TEST_FUTURE: SettableFuture<Runnable?> = SettableFuture.create()
--- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/MediaTttCommandLineHelperTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/MediaTttCommandLineHelperTest.kt
@@ -20,7 +20,9 @@ import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.statusbar.commandline.Command
 import com.android.systemui.statusbar.commandline.CommandRegistry
+import com.android.systemui.util.concurrency.FakeExecutor
 import com.android.systemui.util.mockito.any
+import com.android.systemui.util.time.FakeSystemClock
 import org.junit.Before
 import org.junit.Test
 import org.mockito.Mock
@@ -46,7 +48,9 @@ class MediaTttCommandLineHelperTest : SysuiTestCase() {
    fun setUp() {
        MockitoAnnotations.initMocks(this)
        mediaTttCommandLineHelper =
-            MediaTttCommandLineHelper(commandRegistry, mediaTttChipController)
+            MediaTttCommandLineHelper(
+                commandRegistry, mediaTttChipController, FakeExecutor(FakeSystemClock())
+            )
    }

    @Test(expected = IllegalStateException::class)