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 b2a5e356b9360..89ca5d33645c2 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 @@ -23,9 +23,11 @@ import android.view.View import com.android.internal.logging.UiEventLogger import com.android.internal.statusbar.IUndoMediaTransferCallback import com.android.systemui.CoreStartable +import com.android.systemui.Dumpable import com.android.systemui.R import com.android.systemui.common.shared.model.Text import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dump.DumpManager import com.android.systemui.media.taptotransfer.MediaTttFlags import com.android.systemui.media.taptotransfer.common.MediaTttUtils import com.android.systemui.statusbar.CommandQueue @@ -34,6 +36,7 @@ import com.android.systemui.temporarydisplay.ViewPriority import com.android.systemui.temporarydisplay.chipbar.ChipbarCoordinator import com.android.systemui.temporarydisplay.chipbar.ChipbarEndItem import com.android.systemui.temporarydisplay.chipbar.ChipbarInfo +import java.io.PrintWriter import javax.inject.Inject /** @@ -47,14 +50,13 @@ constructor( private val chipbarCoordinator: ChipbarCoordinator, private val commandQueue: CommandQueue, private val context: Context, + private val dumpManager: DumpManager, private val logger: MediaTttSenderLogger, private val mediaTttFlags: MediaTttFlags, private val uiEventLogger: MediaTttSenderUiEventLogger, -) : CoreStartable { +) : CoreStartable, Dumpable { - private var displayedState: ChipStateSender? = null // A map to store current chip state per id. - // TODO(b/265455911): Log whenever we add or remove from the store. private var stateMap: MutableMap = mutableMapOf() private val commandQueueCallbacks = @@ -75,6 +77,7 @@ constructor( override fun start() { if (mediaTttFlags.isMediaTttEnabled()) { commandQueue.addCallback(commandQueueCallbacks) + dumpManager.registerNormalDumpable(this) } } @@ -92,11 +95,11 @@ constructor( return } - val currentState = stateMap[routeInfo.id] - if (!ChipStateSender.isValidStateTransition(currentState, chipState)) { + val currentStateForId: ChipStateSender? = stateMap[routeInfo.id] + if (!ChipStateSender.isValidStateTransition(currentStateForId, chipState)) { // ChipStateSender.FAR_FROM_RECEIVER is the default state when there is no state. logger.logInvalidStateTransitionError( - currentState = currentState?.name ?: ChipStateSender.FAR_FROM_RECEIVER.name, + currentState = currentStateForId?.name ?: ChipStateSender.FAR_FROM_RECEIVER.name, chipState.name ) return @@ -104,30 +107,29 @@ constructor( uiEventLogger.logSenderStateChange(chipState) if (chipState == ChipStateSender.FAR_FROM_RECEIVER) { - // No need to store the state since it is the default state - removeIdFromStore(routeInfo.id) - // Return early if we're not displaying a chip anyway - val currentDisplayedState = displayedState ?: return + // Return early if we're not displaying a chip for this ID anyway + if (currentStateForId == null) return val removalReason = ChipStateSender.FAR_FROM_RECEIVER.name if ( - currentDisplayedState.transferStatus == TransferStatus.IN_PROGRESS || - currentDisplayedState.transferStatus == TransferStatus.SUCCEEDED + currentStateForId.transferStatus == TransferStatus.IN_PROGRESS || + currentStateForId.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}" + bypassReason = "transferStatus=${currentStateForId.transferStatus.name}" ) return } - displayedState = null + // No need to store the state since it is the default state + removeIdFromStore(routeInfo.id, reason = removalReason) chipbarCoordinator.removeView(routeInfo.id, removalReason) } else { stateMap[routeInfo.id] = chipState - displayedState = chipState + logger.logStateMap(stateMap) chipbarCoordinator.registerListener(displayListener) chipbarCoordinator.displayView( createChipbarInfo( @@ -232,12 +234,19 @@ constructor( } private val displayListener = - TemporaryViewDisplayController.Listener { id -> removeIdFromStore(id) } + TemporaryViewDisplayController.Listener { id, reason -> removeIdFromStore(id, reason) } - private fun removeIdFromStore(id: String) { + private fun removeIdFromStore(id: String, reason: String) { + logger.logStateMapRemoval(id, reason) stateMap.remove(id) + logger.logStateMap(stateMap) if (stateMap.isEmpty()) { chipbarCoordinator.unregisterListener(displayListener) } } + + override fun dump(pw: PrintWriter, args: Array) { + pw.println("Current sender states:") + pw.println(stateMap.toString()) + } } diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLogger.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLogger.kt index 6363425d6db98..964a95b9be9a8 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLogger.kt @@ -85,6 +85,29 @@ constructor( ) } + /** Logs the current contents of the state map. */ + fun logStateMap(map: Map) { + buffer.log( + TAG, + LogLevel.DEBUG, + { str1 = map.toString() }, + { "Current sender states: $str1" } + ) + } + + /** Logs that [id] has been removed from the state map due to [reason]. */ + fun logStateMapRemoval(id: String, reason: String) { + buffer.log( + TAG, + LogLevel.DEBUG, + { + str1 = id + str2 = reason + }, + { "State removal: id=$str1 reason=$str2" } + ) + } + companion object { private const val TAG = "MediaTttSender" } diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index df8d16142b8bd..1065d3349930d 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -327,7 +327,7 @@ abstract class TemporaryViewDisplayController - listener.onInfoPermanentlyRemoved(it.info.id) + listener.onInfoPermanentlyRemoved(it.info.id, REMOVAL_REASON_TIME_EXPIRED) } } } @@ -457,7 +457,7 @@ abstract class TemporaryViewDisplayController SUCCEEDED.) + assertThat(chipbarView.getChipText()) + .isEqualTo( + ChipStateSender.TRANSFER_TO_RECEIVER_SUCCEEDED.getExpectedStateText( + "Tablet Succeeded" + ) + ) + assertThat(chipbarView.getLoadingIcon().visibility).isEqualTo(View.GONE) + assertThat(chipbarView.getUndoButton().visibility).isEqualTo(View.VISIBLE) + } + + /** Regression test for b/266217596. */ + @Test + fun toThisDevice_triggeredThenFar_thenSucceeded_updatesToSucceeded() { + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_TRIGGERED, + routeInfo, + null, + ) + + // WHEN a FAR command comes in + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_FAR_FROM_RECEIVER, + routeInfo, + null, + ) + + // THEN it is ignored and the chipbar is stilled displayed + val chipbarView = getChipbarView() + assertThat(chipbarView.getChipText()) + .isEqualTo(ChipStateSender.TRANSFER_TO_THIS_DEVICE_TRIGGERED.getExpectedStateText()) + assertThat(chipbarView.getLoadingIcon().visibility).isEqualTo(View.VISIBLE) + verify(windowManager, never()).removeView(any()) + + // WHEN a SUCCEEDED command comes in + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_SUCCEEDED, + routeInfo, + /* undoCallback= */ object : IUndoMediaTransferCallback.Stub() { + override fun onUndoTriggered() {} + }, + ) + + // THEN it is *not* marked as an invalid transition and the chipbar updates to the succeeded + // state. (The "invalid transition" would be FAR => SUCCEEDED.) + assertThat(chipbarView.getChipText()) + .isEqualTo( + ChipStateSender.TRANSFER_TO_THIS_DEVICE_SUCCEEDED.getExpectedStateText( + "Tablet Succeeded" + ) + ) + assertThat(chipbarView.getLoadingIcon().visibility).isEqualTo(View.GONE) + assertThat(chipbarView.getUndoButton().visibility).isEqualTo(View.VISIBLE) + } + @Test fun receivesNewStateFromCommandQueue_isLogged() { commandQueueCallback.updateMediaTapToTransferSenderDisplay( @@ -932,6 +1028,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { mockChipbarCoordinator, commandQueue, context, + dumpManager, logger, mediaTttFlags, uiEventLogger, @@ -958,6 +1055,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { mockChipbarCoordinator, commandQueue, context, + dumpManager, logger, mediaTttFlags, uiEventLogger, @@ -975,9 +1073,10 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { verify(mockChipbarCoordinator).registerListener(capture(listenerCaptor)) // WHEN the listener is notified that the view has been removed - listenerCaptor.value.onInfoPermanentlyRemoved(DEFAULT_ID) + listenerCaptor.value.onInfoPermanentlyRemoved(DEFAULT_ID, "reason") // THEN the media coordinator unregisters the listener + verify(logger).logStateMapRemoval(DEFAULT_ID, "reason") verify(mockChipbarCoordinator).unregisterListener(listenerCaptor.value) } @@ -989,6 +1088,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { mockChipbarCoordinator, commandQueue, context, + dumpManager, logger, mediaTttFlags, uiEventLogger, @@ -1006,7 +1106,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { verify(mockChipbarCoordinator).registerListener(capture(listenerCaptor)) // WHEN the listener is notified that a different view has been removed - listenerCaptor.value.onInfoPermanentlyRemoved("differentViewId") + listenerCaptor.value.onInfoPermanentlyRemoved("differentViewId", "reason") // THEN the media coordinator doesn't unregister the listener verify(mockChipbarCoordinator, never()).unregisterListener(listenerCaptor.value) @@ -1020,6 +1120,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { mockChipbarCoordinator, commandQueue, context, + dumpManager, logger, mediaTttFlags, uiEventLogger, @@ -1055,6 +1156,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { mockChipbarCoordinator, commandQueue, context, + dumpManager, logger, mediaTttFlags, uiEventLogger, @@ -1084,10 +1186,173 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { verify(mockChipbarCoordinator, atLeast(1)).registerListener(capture(listenerCaptor)) // THEN one of them is removed - listenerCaptor.value.onInfoPermanentlyRemoved("route1") + listenerCaptor.value.onInfoPermanentlyRemoved("route1", "reason") // THEN the media coordinator doesn't unregister the listener (since route2 is still active) verify(mockChipbarCoordinator, never()).unregisterListener(listenerCaptor.value) + verify(logger).logStateMapRemoval("route1", "reason") + } + + /** Regression test for b/266218672. */ + @Test + fun twoIdsDisplayed_oldIdIsFar_viewStillDisplayed() { + // WHEN there are two different media transfers with different IDs + val route1 = + MediaRoute2Info.Builder("route1", OTHER_DEVICE_NAME) + .addFeature("feature") + .setClientPackageName(PACKAGE_NAME) + .build() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + route1, + null, + ) + verify(windowManager).addView(any(), any()) + reset(windowManager) + + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_START_CAST, + MediaRoute2Info.Builder("route2", "Route 2 name") + .addFeature("feature") + .setClientPackageName(PACKAGE_NAME) + .build(), + null, + ) + val newView = getChipbarView() + + // WHEN there's a FAR event for the earlier one + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_FAR_FROM_RECEIVER, + route1, + null, + ) + + // THEN it's ignored and the more recent one is still displayed + assertThat(newView.getChipText()) + .isEqualTo( + ChipStateSender.ALMOST_CLOSE_TO_START_CAST.getExpectedStateText("Route 2 name") + ) + } + + /** Regression test for b/266218672. */ + @Test + fun receiverSucceededThenTimedOut_internalStateResetAndCanDisplayAlmostCloseToEnd() { + displayReceiverTriggered() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_RECEIVER_SUCCEEDED, + routeInfo, + null, + ) + + fakeClock.advanceTime(TIMEOUT + 1L) + verify(windowManager).removeView(any()) + + reset(windowManager) + + // WHEN we try to show ALMOST_CLOSE_TO_END + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + routeInfo, + null, + ) + + // THEN it succeeds + val chipbarView = getChipbarView() + assertThat(chipbarView.getChipText()) + .isEqualTo(ChipStateSender.ALMOST_CLOSE_TO_END_CAST.getExpectedStateText()) + } + + /** Regression test for b/266218672. */ + @Test + fun receiverSucceededThenTimedOut_internalStateResetAndCanDisplayReceiverTriggered() { + displayReceiverTriggered() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_RECEIVER_SUCCEEDED, + routeInfo, + null, + ) + + fakeClock.advanceTime(TIMEOUT + 1L) + verify(windowManager).removeView(any()) + + reset(windowManager) + + // WHEN we try to show RECEIVER_TRIGGERED + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_RECEIVER_TRIGGERED, + routeInfo, + null, + ) + + // THEN it succeeds + val chipbarView = getChipbarView() + assertThat(chipbarView.getChipText()) + .isEqualTo(ChipStateSender.TRANSFER_TO_RECEIVER_TRIGGERED.getExpectedStateText()) + assertThat(chipbarView.getLoadingIcon().visibility).isEqualTo(View.VISIBLE) + } + + /** Regression test for b/266218672. */ + @Test + fun toThisDeviceSucceededThenTimedOut_internalStateResetAndCanDisplayAlmostCloseToStart() { + displayThisDeviceTriggered() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_SUCCEEDED, + routeInfo, + null, + ) + + fakeClock.advanceTime(TIMEOUT + 1L) + verify(windowManager).removeView(any()) + + reset(windowManager) + + // WHEN we try to show ALMOST_CLOSE_TO_START + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_START_CAST, + routeInfo, + null, + ) + + // THEN it succeeds + val chipbarView = getChipbarView() + assertThat(chipbarView.getChipText()) + .isEqualTo(ChipStateSender.ALMOST_CLOSE_TO_START_CAST.getExpectedStateText()) + } + + /** Regression test for b/266218672. */ + @Test + fun toThisDeviceSucceededThenTimedOut_internalStateResetAndCanDisplayThisDeviceTriggered() { + displayThisDeviceTriggered() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_SUCCEEDED, + routeInfo, + null, + ) + + fakeClock.advanceTime(TIMEOUT + 1L) + verify(windowManager).removeView(any()) + + reset(windowManager) + + // WHEN we try to show THIS_DEVICE_TRIGGERED + val newRouteInfo = + MediaRoute2Info.Builder(DEFAULT_ID, "New Name") + .addFeature("feature") + .setClientPackageName(PACKAGE_NAME) + .build() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_TRIGGERED, + newRouteInfo, + null, + ) + + // THEN it succeeds + val chipbarView = getChipbarView() + assertThat(chipbarView.getChipText()) + .isEqualTo( + ChipStateSender.TRANSFER_TO_THIS_DEVICE_TRIGGERED.getExpectedStateText("New Name") + ) + assertThat(chipbarView.getLoadingIcon().visibility).isEqualTo(View.VISIBLE) } private fun getChipbarView(): ViewGroup { @@ -1107,8 +1372,10 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { private fun ViewGroup.getUndoButton(): View = this.requireViewById(R.id.end_button) - private fun ChipStateSender.getExpectedStateText(): String? { - return this.getChipTextString(context, OTHER_DEVICE_NAME).loadText(context) + private fun ChipStateSender.getExpectedStateText( + otherDeviceName: String = OTHER_DEVICE_NAME, + ): String? { + return this.getChipTextString(context, otherDeviceName).loadText(context) } // display receiver triggered state helper method to make sure we start from a valid state diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLoggerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLoggerTest.kt index 59cc43f59bfc0..003375709a163 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLoggerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderLoggerTest.kt @@ -87,6 +87,32 @@ class MediaTttSenderLoggerTest : SysuiTestCase() { assertThat(actualString).contains(bypassReason) } + @Test + fun logStateMap_bufferHasInfo() { + val map = + mapOf( + "123" to ChipStateSender.ALMOST_CLOSE_TO_START_CAST, + "456" to ChipStateSender.TRANSFER_TO_THIS_DEVICE_TRIGGERED, + ) + + logger.logStateMap(map) + + val actualString = getStringFromBuffer() + assertThat(actualString).contains("123") + assertThat(actualString).contains(ChipStateSender.ALMOST_CLOSE_TO_START_CAST.name) + assertThat(actualString).contains("456") + assertThat(actualString).contains(ChipStateSender.TRANSFER_TO_THIS_DEVICE_TRIGGERED.name) + } + + @Test + fun logStateMapRemoval_bufferHasInfo() { + logger.logStateMapRemoval("456", "testReason") + + val actualString = getStringFromBuffer() + assertThat(actualString).contains("456") + assertThat(actualString).contains("testReason") + } + private fun getStringFromBuffer(): String { val stringWriter = StringWriter() buffer.dump(PrintWriter(stringWriter), tailLength = 0) 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 45f7df3c3f5ba..c7c6b945f09a3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt @@ -1172,7 +1172,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { inner class Listener : TemporaryViewDisplayController.Listener { val permanentlyRemovedIds = mutableListOf() - override fun onInfoPermanentlyRemoved(id: String) { + override fun onInfoPermanentlyRemoved(id: String, reason: String) { permanentlyRemovedIds.add(id) } }