From 9c0e888732f7607b2097405bf6d55cf727baabe7 Mon Sep 17 00:00:00 2001
From: Caitlin Shkuratov <caitlinshk@google.com>
Date: Wed, 12 Oct 2022 17:19:39 +0000
Subject: [PATCH] [Chipbar] Have MediaTttSenderCoordinator be responsible for
 not hiding in certain cases.

Bug: 245610654
Test: manual: Send a TRANSFER_TRIGGERED event then FAR_FROM_RECEIVER
event and verify the chip is still displayed (i.e. the removal request
is ignored)
Test: manual: Send a ALMOST_CLOSE event then FAR_FROM_RECEIVER event and
verify the chip is hidden
Test: atest MediaTttSenderCoordinatorTest
Test: atest ChipbarCoordinatorTest
Change-Id: I8cd33a65ec48f37aa83179b348f60916df3990a8

Change-Id: Ic1f1d2fcb09727ebc24d649a26dcea40ca8287b2
---
 .../sender/MediaTttSenderCoordinator.kt       | 23 ++++-
 .../TemporaryViewDisplayController.kt         | 17 +---
 .../chipbar/ChipbarCoordinator.kt             | 18 ----
 .../sender/MediaTttSenderCoordinatorTest.kt   | 27 ++++--
 .../TemporaryViewDisplayControllerTest.kt     | 26 ------
 .../chipbar/ChipbarCoordinatorTest.kt         | 86 -------------------
 6 files changed, 46 insertions(+), 151 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt
index 5aaab14c1065e..224303ac098c4 100644
--- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt
@@ -47,6 +47,8 @@ constructor(
     private val uiEventLogger: MediaTttSenderUiEventLogger,
 ) : CoreStartable {
 
+    private var displayedState: ChipStateSender? = null
+
     private val commandQueueCallbacks =
         object : CommandQueue.Callbacks {
             override fun updateMediaTapToTransferSenderDisplay(
@@ -84,8 +86,27 @@ constructor(
         uiEventLogger.logSenderStateChange(chipState)
 
         if (chipState == ChipStateSender.FAR_FROM_RECEIVER) {
-            chipbarCoordinator.removeView(removalReason = ChipStateSender.FAR_FROM_RECEIVER.name)
+            // Return early if we're not displaying a chip anyway
+            val currentDisplayedState = displayedState ?: return
+
+            val removalReason = ChipStateSender.FAR_FROM_RECEIVER.name
+            if (
+                currentDisplayedState.transferStatus == TransferStatus.IN_PROGRESS ||
+                    currentDisplayedState.transferStatus == TransferStatus.SUCCEEDED
+            ) {
+                // Don't remove the chip if we're in progress or succeeded, since the user should
+                // still be able to see the status of the transfer.
+                logger.logRemovalBypass(
+                    removalReason,
+                    bypassReason = "transferStatus=${currentDisplayedState.transferStatus.name}"
+                )
+                return
+            }
+
+            displayedState = null
+            chipbarCoordinator.removeView(removalReason)
         } else {
+            displayedState = chipState
             chipbarCoordinator.displayView(ChipSenderInfo(chipState, routeInfo, undoCallback))
         }
     }
diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt
index 5cbdf7c43a123..d5d904c50f209 100644
--- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt
+++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt
@@ -131,7 +131,7 @@ abstract class TemporaryViewDisplayController<T : TemporaryViewInfo, U : Tempora
        )
         cancelViewTimeout?.run()
         cancelViewTimeout = mainExecutor.executeDelayed(
-            { removeView(TemporaryDisplayRemovalReason.REASON_TIMEOUT) },
+            { removeView(REMOVAL_REASON_TIMEOUT) },
             timeout.toLong()
         )
     }
@@ -175,9 +175,6 @@ abstract class TemporaryViewDisplayController<T : TemporaryViewInfo, U : Tempora
      */
     fun removeView(removalReason: String) {
         val currentDisplayInfo = displayInfo ?: return
-        if (shouldIgnoreViewRemoval(currentDisplayInfo.info, removalReason)) {
-            return
-        }
 
         val currentView = currentDisplayInfo.view
         animateViewOut(currentView) { windowManager.removeView(currentView) }
@@ -192,13 +189,6 @@ abstract class TemporaryViewDisplayController<T : TemporaryViewInfo, U : Tempora
         cancelViewTimeout?.run()
     }
 
-    /**
-     * Returns true if a view removal request should be ignored and false otherwise.
-     *
-     * Allows subclasses to keep the view visible for longer in certain circumstances.
-     */
-    open fun shouldIgnoreViewRemoval(info: T, removalReason: String): Boolean = false
-
     /**
      * A method implemented by subclasses to update [currentView] based on [newInfo].
      */
@@ -236,10 +226,7 @@ abstract class TemporaryViewDisplayController<T : TemporaryViewInfo, U : Tempora
     )
 }
 
-object TemporaryDisplayRemovalReason {
-    const val REASON_TIMEOUT = "TIMEOUT"
-    const val REASON_SCREEN_TAP = "SCREEN_TAP"
-}
+private const val REMOVAL_REASON_TIMEOUT = "TIMEOUT"
 
 private data class IconInfo(
     val iconName: String,
diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt
index a2cd1420a41c0..1a25e4df3a61e 100644
--- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt
+++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt
@@ -44,7 +44,6 @@ import com.android.systemui.media.taptotransfer.sender.MediaTttSenderUiEventLogg
 import com.android.systemui.media.taptotransfer.sender.TransferStatus
 import com.android.systemui.plugins.FalsingManager
 import com.android.systemui.statusbar.policy.ConfigurationController
-import com.android.systemui.temporarydisplay.TemporaryDisplayRemovalReason
 import com.android.systemui.temporarydisplay.TemporaryViewDisplayController
 import com.android.systemui.temporarydisplay.TemporaryViewInfo
 import com.android.systemui.util.concurrency.DelayableExecutor
@@ -185,23 +184,6 @@ open class ChipbarCoordinator @Inject constructor(
         )
     }
 
-    override fun shouldIgnoreViewRemoval(info: ChipSenderInfo, removalReason: String): Boolean {
-        // Don't remove the chip if we're in progress or succeeded, since the user should still be
-        // able to see the status of the transfer. (But do remove it if it's finally timed out.)
-        val transferStatus = info.state.transferStatus
-        if (
-            (transferStatus == TransferStatus.IN_PROGRESS ||
-                transferStatus == TransferStatus.SUCCEEDED) &&
-            removalReason != TemporaryDisplayRemovalReason.REASON_TIMEOUT
-        ) {
-            logger.logRemovalBypass(
-                removalReason, bypassReason = "transferStatus=${transferStatus.name}"
-            )
-            return true
-        }
-        return false
-    }
-
     override fun getTouchableRegion(view: View, outRect: Rect) {
         viewUtil.setRectToViewWindowLocation(view, outRect)
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt
index 616a349520ee9..110bbb80df3a0 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt
@@ -83,7 +83,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
     fun setUp() {
         MockitoAnnotations.initMocks(this)
         whenever(mediaTttFlags.isMediaTttEnabled()).thenReturn(true)
-        whenever(accessibilityManager.getRecommendedTimeoutMillis(any(), any())).thenReturn(1000)
+        whenever(accessibilityManager.getRecommendedTimeoutMillis(any(), any())).thenReturn(TIMEOUT)
 
         fakeClock = FakeSystemClock()
         fakeExecutor = FakeExecutor(fakeClock)
@@ -316,7 +316,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
     }
 
     @Test
-    fun transferToReceiverTriggeredThenFarFromReceiver_viewStillDisplayed() {
+    fun transferToReceiverTriggeredThenFarFromReceiver_viewStillDisplayedButStillTimesOut() {
         commandQueueCallback.updateMediaTapToTransferSenderDisplay(
             StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_RECEIVER_TRIGGERED,
             routeInfo,
@@ -332,10 +332,14 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
 
         verify(windowManager, never()).removeView(any())
         verify(logger).logRemovalBypass(any(), any())
+
+        fakeClock.advanceTime(TIMEOUT + 1L)
+
+        verify(windowManager).removeView(any())
     }
 
     @Test
-    fun transferToThisDeviceTriggeredThenFarFromReceiver_viewStillDisplayed() {
+    fun transferToThisDeviceTriggeredThenFarFromReceiver_viewStillDisplayedButDoesTimeOut() {
         commandQueueCallback.updateMediaTapToTransferSenderDisplay(
             StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_TRIGGERED,
             routeInfo,
@@ -351,10 +355,14 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
 
         verify(windowManager, never()).removeView(any())
         verify(logger).logRemovalBypass(any(), any())
+
+        fakeClock.advanceTime(TIMEOUT + 1L)
+
+        verify(windowManager).removeView(any())
     }
 
     @Test
-    fun transferToReceiverSucceededThenFarFromReceiver_viewStillDisplayed() {
+    fun transferToReceiverSucceededThenFarFromReceiver_viewStillDisplayedButDoesTimeOut() {
         commandQueueCallback.updateMediaTapToTransferSenderDisplay(
             StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_RECEIVER_SUCCEEDED,
             routeInfo,
@@ -370,10 +378,14 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
 
         verify(windowManager, never()).removeView(any())
         verify(logger).logRemovalBypass(any(), any())
+
+        fakeClock.advanceTime(TIMEOUT + 1L)
+
+        verify(windowManager).removeView(any())
     }
 
     @Test
-    fun transferToThisDeviceSucceededThenFarFromReceiver_viewStillDisplayed() {
+    fun transferToThisDeviceSucceededThenFarFromReceiver_viewStillDisplayedButDoesTimeOut() {
         commandQueueCallback.updateMediaTapToTransferSenderDisplay(
             StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_SUCCEEDED,
             routeInfo,
@@ -389,6 +401,10 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
 
         verify(windowManager, never()).removeView(any())
         verify(logger).logRemovalBypass(any(), any())
+
+        fakeClock.advanceTime(TIMEOUT + 1L)
+
+        verify(windowManager).removeView(any())
     }
 
     private fun getChipView(): ViewGroup {
@@ -434,6 +450,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
 }
 
 private const val OTHER_DEVICE_NAME = "My Tablet"
+private const val TIMEOUT = 10000
 
 private val routeInfo =
     MediaRoute2Info.Builder("id", OTHER_DEVICE_NAME)
diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt
index b10aa125c69d6..b68eb88d46db2 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt
@@ -63,8 +63,6 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() {
     @Mock
     private lateinit var powerManager: PowerManager
 
-    private var shouldIgnoreViewRemoval: Boolean = false
-
     @Before
     fun setUp() {
         MockitoAnnotations.initMocks(this)
@@ -209,26 +207,6 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() {
         verify(windowManager, never()).removeView(any())
     }
 
-    @Test
-    fun removeView_shouldIgnoreRemovalFalse_viewRemoved() {
-        shouldIgnoreViewRemoval = false
-        underTest.displayView(getState())
-
-        underTest.removeView("reason")
-
-        verify(windowManager).removeView(any())
-    }
-
-    @Test
-    fun removeView_shouldIgnoreRemovalTrue_viewNotRemoved() {
-        shouldIgnoreViewRemoval = true
-        underTest.displayView(getState())
-
-        underTest.removeView("reason")
-
-        verify(windowManager, never()).removeView(any())
-    }
-
     private fun getState(name: String = "name") = ViewInfo(name)
 
     private fun getConfigurationListener(): ConfigurationListener {
@@ -267,10 +245,6 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() {
             mostRecentViewInfo = newInfo
         }
 
-        override fun shouldIgnoreViewRemoval(info: ViewInfo, removalReason: String): Boolean {
-            return shouldIgnoreViewRemoval
-        }
-
         override fun getTouchableRegion(view: View, outRect: Rect) {
             outRect.setEmpty()
         }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt
index 2af48021d0994..6225d0c722ae8 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt
@@ -434,92 +434,6 @@ class ChipbarCoordinatorTest : SysuiTestCase() {
         assertThat(getChipView().getFailureIcon().visibility).isEqualTo(View.VISIBLE)
     }
 
-    @Test
-    fun transferToReceiverTriggeredThenRemoveView_viewStillDisplayed() {
-        underTest.displayView(transferToReceiverTriggered())
-        fakeClock.advanceTime(1000L)
-
-        underTest.removeView("fakeRemovalReason")
-        fakeExecutor.runAllReady()
-
-        verify(windowManager, never()).removeView(any())
-        verify(logger).logRemovalBypass(any(), any())
-    }
-
-    @Test
-    fun transferToReceiverTriggeredThenRemoveView_eventuallyTimesOut() {
-        underTest.displayView(transferToReceiverTriggered())
-
-        underTest.removeView("fakeRemovalReason")
-        fakeClock.advanceTime(TIMEOUT + 1L)
-
-        verify(windowManager).removeView(any())
-    }
-
-    @Test
-    fun transferToThisDeviceTriggeredThenRemoveView_viewStillDisplayed() {
-        underTest.displayView(transferToThisDeviceTriggered())
-        fakeClock.advanceTime(1000L)
-
-        underTest.removeView("fakeRemovalReason")
-        fakeExecutor.runAllReady()
-
-        verify(windowManager, never()).removeView(any())
-        verify(logger).logRemovalBypass(any(), any())
-    }
-
-    @Test
-    fun transferToThisDeviceTriggeredThenRemoveView_eventuallyTimesOut() {
-        underTest.displayView(transferToThisDeviceTriggered())
-
-        underTest.removeView("fakeRemovalReason")
-        fakeClock.advanceTime(TIMEOUT + 1L)
-
-        verify(windowManager).removeView(any())
-    }
-
-    @Test
-    fun transferToReceiverSucceededThenRemoveView_viewStillDisplayed() {
-        underTest.displayView(transferToReceiverSucceeded())
-
-        underTest.removeView("fakeRemovalReason")
-        fakeExecutor.runAllReady()
-
-        verify(windowManager, never()).removeView(any())
-        verify(logger).logRemovalBypass(any(), any())
-    }
-
-    @Test
-    fun transferToReceiverSucceededThenRemoveView_eventuallyTimesOut() {
-        underTest.displayView(transferToReceiverSucceeded())
-
-        underTest.removeView("fakeRemovalReason")
-        fakeClock.advanceTime(TIMEOUT + 1L)
-
-        verify(windowManager).removeView(any())
-    }
-
-    @Test
-    fun transferToThisDeviceSucceededThenRemoveView_viewStillDisplayed() {
-        underTest.displayView(transferToThisDeviceSucceeded())
-
-        underTest.removeView("fakeRemovalReason")
-        fakeExecutor.runAllReady()
-
-        verify(windowManager, never()).removeView(any())
-        verify(logger).logRemovalBypass(any(), any())
-    }
-
-    @Test
-    fun transferToThisDeviceSucceededThenRemoveView_eventuallyTimesOut() {
-        underTest.displayView(transferToThisDeviceSucceeded())
-
-        underTest.removeView("fakeRemovalReason")
-        fakeClock.advanceTime(TIMEOUT + 1L)
-
-        verify(windowManager).removeView(any())
-    }
-
     private fun ViewGroup.getAppIconView() = this.requireViewById<ImageView>(R.id.app_icon)
 
     private fun ViewGroup.getChipText(): String =