[Media TTT] Remove displayedState and always use the map instead.
Two bugs around `displayedState`:
1) We weren't correctly re-setting it on timeout.
2) We weren't taking IDs into account when seeing if we could ignore a
FAR_FROM_RECEIVER.
This CL just removes `displayedState` entirely and uses `stateMap` as
the one source of truth.
Note: All the test cases added actually pass without any changes to
`MediaTttSenderCoordinator` because we had safeguards in other parts of
the code. When I wrote the tests and included assertions about
`displayedState`, then they failed. But, now that `displayedState` is
gone, we can't have those sorts of assertions anymore.
Fixes: 266218672
Test: TRIGGERED -> SUCCEEDED -> {wait for timeout} -> FAR ==> nothing in
the logs about "removal was ignored because transferStatus=SUCCEEDED"
Test: ALMOST for id=1 -> ALMOST for id=2 -> FAR for id=1 ==> chip stays,
state map just has id=2
Test: atest MediaTttSenderCoordinatorTest
Change-Id: I58955fd8c4a60842476102a6c504ca987d93739d
This commit is contained in:
@@ -56,7 +56,6 @@ constructor(
|
||||
private val uiEventLogger: MediaTttSenderUiEventLogger,
|
||||
) : CoreStartable, Dumpable {
|
||||
|
||||
private var displayedState: ChipStateSender? = null
|
||||
// A map to store current chip state per id.
|
||||
private var stateMap: MutableMap<String, ChipStateSender> = mutableMapOf()
|
||||
|
||||
@@ -96,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
|
||||
@@ -108,31 +107,29 @@ constructor(
|
||||
uiEventLogger.logSenderStateChange(chipState)
|
||||
|
||||
if (chipState == ChipStateSender.FAR_FROM_RECEIVER) {
|
||||
// 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
|
||||
}
|
||||
|
||||
// No need to store the state since it is the default state
|
||||
removeIdFromStore(routeInfo.id, reason = removalReason)
|
||||
displayedState = null
|
||||
chipbarCoordinator.removeView(routeInfo.id, removalReason)
|
||||
} else {
|
||||
stateMap[routeInfo.id] = chipState
|
||||
logger.logStateMap(stateMap)
|
||||
displayedState = chipState
|
||||
chipbarCoordinator.registerListener(displayListener)
|
||||
chipbarCoordinator.displayView(
|
||||
createChipbarInfo(
|
||||
|
||||
@@ -1193,6 +1193,168 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() {
|
||||
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 {
|
||||
val viewCaptor = ArgumentCaptor.forClass(View::class.java)
|
||||
verify(windowManager).addView(viewCaptor.capture(), any())
|
||||
|
||||
Reference in New Issue
Block a user