From 5a7e9e634979a50ec38cf6994c9ae420ffdbb1ac Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Fri, 13 Jan 2023 20:07:31 +0000 Subject: [PATCH 1/4] [Media TTT] Use a listener pattern to notify about view removals. In order to handle the swipe-to-dismiss gesture correctly, we need to notify MediaTttSenderCoordinator about both a view timing out, and a view being removed due to swipe. So, having the `onViewTimeout` Runnable will no longer work. This CL instead uses a listener interface to notify `MediaTttSenderCoordinator` whenever the view has timed out / been removed. A future CL will add the swipe-to-dismiss gesture. That gesture will also trigger this new listener method, so that MediaTttSenderCoordinator always stays up-to-date. Bug: 262584940 Test: manual: verify media ttt chipbars with different IDs still works (id1=triggered, then id2=almostClose. When id2 times out, id1 is redisplayed, then also times out) Test: atest MediaTttSenderCoordinatorTest TemporaryViewDisplayControllerTest Change-Id: I73c6235718f3b660ea416ca459ae777d8624a7be --- .../sender/MediaTttSenderCoordinator.kt | 19 +- .../TemporaryViewDisplayController.kt | 45 ++-- .../sender/MediaTttSenderCoordinatorTest.kt | 187 ++++++++++++++++- .../TemporaryViewDisplayControllerTest.kt | 198 +++++++++++++++--- 4 files changed, 400 insertions(+), 49 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 935f38de2e4f1..be93c54b498f1 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 @@ -30,6 +30,7 @@ import com.android.systemui.media.taptotransfer.MediaTttFlags import com.android.systemui.media.taptotransfer.common.MediaTttLogger import com.android.systemui.media.taptotransfer.common.MediaTttUtils import com.android.systemui.statusbar.CommandQueue +import com.android.systemui.temporarydisplay.TemporaryViewDisplayController import com.android.systemui.temporarydisplay.ViewPriority import com.android.systemui.temporarydisplay.chipbar.ChipbarCoordinator import com.android.systemui.temporarydisplay.chipbar.ChipbarEndItem @@ -54,6 +55,7 @@ constructor( 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 = @@ -102,10 +104,9 @@ constructor( } uiEventLogger.logSenderStateChange(chipState) - stateMap.put(routeInfo.id, chipState) if (chipState == ChipStateSender.FAR_FROM_RECEIVER) { // No need to store the state since it is the default state - stateMap.remove(routeInfo.id) + removeIdFromStore(routeInfo.id) // Return early if we're not displaying a chip anyway val currentDisplayedState = displayedState ?: return @@ -126,7 +127,9 @@ constructor( displayedState = null chipbarCoordinator.removeView(routeInfo.id, removalReason) } else { + stateMap[routeInfo.id] = chipState displayedState = chipState + chipbarCoordinator.registerListener(displayListener) chipbarCoordinator.displayView( createChipbarInfo( chipState, @@ -135,7 +138,7 @@ constructor( context, logger, ) - ) { stateMap.remove(routeInfo.id) } + ) } } @@ -225,4 +228,14 @@ constructor( onClickListener, ) } + + private val displayListener = + TemporaryViewDisplayController.Listener { id -> removeIdFromStore(id) } + + private fun removeIdFromStore(id: String) { + stateMap.remove(id) + if (stateMap.isEmpty()) { + chipbarCoordinator.unregisterListener(displayListener) + } + } } diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index ad48e218687d1..cf722ce8ecfab 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -119,15 +119,26 @@ abstract class TemporaryViewDisplayController = mutableSetOf() + + /** Registers a listener. */ + fun registerListener(listener: Listener) { + listeners.add(listener) + } + + /** Unregisters a listener. */ + fun unregisterListener(listener: Listener) { + listeners.remove(listener) + } + /** * Displays the view with the provided [newInfo]. * * This method handles inflating and attaching the view, then delegates to [updateView] to * display the correct information in the view. - * @param onViewTimeout a runnable that runs after the view timeout. */ @Synchronized - fun displayView(newInfo: T, onViewTimeout: Runnable? = null) { + fun displayView(newInfo: T) { val timeout = accessibilityManager.getRecommendedTimeoutMillis( newInfo.timeoutMs, // Not all views have controls so FLAG_CONTENT_CONTROLS might be superfluous, but @@ -146,14 +157,13 @@ abstract class TemporaryViewDisplayController + listener.onInfoPermanentlyRemoved(it.info.id) + } } } @@ -436,6 +449,15 @@ abstract class TemporaryViewDisplayController() + underTest = + MediaTttSenderCoordinator( + mockChipbarCoordinator, + commandQueue, + context, + logger, + mediaTttFlags, + uiEventLogger, + ) + underTest.start() + // Re-set the command queue callback since we've created a new [MediaTttSenderCoordinator] + // with a new callback. + setCommandQueueCallback() + + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + routeInfo, + null, + ) + + verify(mockChipbarCoordinator).registerListener(any()) + } + + @Test + fun onInfoPermanentlyRemoved_viewListenerUnregistered() { + val mockChipbarCoordinator = mock() + underTest = + MediaTttSenderCoordinator( + mockChipbarCoordinator, + commandQueue, + context, + logger, + mediaTttFlags, + uiEventLogger, + ) + underTest.start() + setCommandQueueCallback() + + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + routeInfo, + null, + ) + + val listenerCaptor = argumentCaptor() + verify(mockChipbarCoordinator).registerListener(capture(listenerCaptor)) + + // WHEN the listener is notified that the view has been removed + listenerCaptor.value.onInfoPermanentlyRemoved(DEFAULT_ID) + + // THEN the media coordinator unregisters the listener + verify(mockChipbarCoordinator).unregisterListener(listenerCaptor.value) + } + + @Test + fun onInfoPermanentlyRemoved_wrongId_viewListenerNotUnregistered() { + val mockChipbarCoordinator = mock() + underTest = + MediaTttSenderCoordinator( + mockChipbarCoordinator, + commandQueue, + context, + logger, + mediaTttFlags, + uiEventLogger, + ) + underTest.start() + setCommandQueueCallback() + + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + routeInfo, + null, + ) + + val listenerCaptor = argumentCaptor() + verify(mockChipbarCoordinator).registerListener(capture(listenerCaptor)) + + // WHEN the listener is notified that a different view has been removed + listenerCaptor.value.onInfoPermanentlyRemoved("differentViewId") + + // THEN the media coordinator doesn't unregister the listener + verify(mockChipbarCoordinator, never()).unregisterListener(listenerCaptor.value) + } + + @Test + fun farFromReceiverState_viewListenerUnregistered() { + val mockChipbarCoordinator = mock() + underTest = + MediaTttSenderCoordinator( + mockChipbarCoordinator, + commandQueue, + context, + logger, + mediaTttFlags, + uiEventLogger, + ) + underTest.start() + setCommandQueueCallback() + + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + routeInfo, + null, + ) + + val listenerCaptor = argumentCaptor() + verify(mockChipbarCoordinator).registerListener(capture(listenerCaptor)) + + // WHEN we go to the FAR_FROM_RECEIVER state + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_FAR_FROM_RECEIVER, + routeInfo, + null + ) + + // THEN the media coordinator unregisters the listener + verify(mockChipbarCoordinator).unregisterListener(listenerCaptor.value) + } + + @Test + fun statesWithDifferentIds_onInfoPermanentlyRemovedForOneId_viewListenerNotUnregistered() { + val mockChipbarCoordinator = mock() + underTest = + MediaTttSenderCoordinator( + mockChipbarCoordinator, + commandQueue, + context, + logger, + mediaTttFlags, + uiEventLogger, + ) + underTest.start() + setCommandQueueCallback() + + // WHEN there are two different media transfers with different IDs + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + MediaRoute2Info.Builder("route1", OTHER_DEVICE_NAME) + .addFeature("feature") + .setClientPackageName(PACKAGE_NAME) + .build(), + null, + ) + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_END_CAST, + MediaRoute2Info.Builder("route2", OTHER_DEVICE_NAME) + .addFeature("feature") + .setClientPackageName(PACKAGE_NAME) + .build(), + null, + ) + + val listenerCaptor = argumentCaptor() + verify(mockChipbarCoordinator, atLeast(1)).registerListener(capture(listenerCaptor)) + + // THEN one of them is removed + listenerCaptor.value.onInfoPermanentlyRemoved("route1") + + // THEN the media coordinator doesn't unregister the listener (since route2 is still active) + verify(mockChipbarCoordinator, never()).unregisterListener(listenerCaptor.value) + } + private fun getChipbarView(): ViewGroup { val viewCaptor = ArgumentCaptor.forClass(View::class.java) verify(windowManager).addView(viewCaptor.capture(), any()) @@ -960,8 +1129,16 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { null ) } + + private fun setCommandQueueCallback() { + val callbackCaptor = argumentCaptor() + verify(commandQueue).addCallback(capture(callbackCaptor)) + commandQueueCallback = callbackCaptor.value + reset(commandQueue) + } } +private const val DEFAULT_ID = "defaultId" private const val APP_NAME = "Fake app name" private const val OTHER_DEVICE_NAME = "My Tablet" private const val BLANK_DEVICE_NAME = " " @@ -969,13 +1146,13 @@ private const val PACKAGE_NAME = "com.android.systemui" private const val TIMEOUT = 10000 private val routeInfo = - MediaRoute2Info.Builder("id", OTHER_DEVICE_NAME) + MediaRoute2Info.Builder(DEFAULT_ID, OTHER_DEVICE_NAME) .addFeature("feature") .setClientPackageName(PACKAGE_NAME) .build() private val routeInfoWithBlankDeviceName = - MediaRoute2Info.Builder("id", BLANK_DEVICE_NAME) + MediaRoute2Info.Builder(DEFAULT_ID, BLANK_DEVICE_NAME) .addFeature("feature") .setClientPackageName(PACKAGE_NAME) .build() 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 99e20124f5974..45f7df3c3f5ba 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt @@ -159,7 +159,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { underTest.displayView(getState()) assertThat(fakeWakeLock.isHeld).isTrue() - underTest.removeView("id", "test reason") + underTest.removeView(DEFAULT_ID, "test reason") assertThat(fakeWakeLock.isHeld).isFalse() } @@ -175,6 +175,8 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { @Test fun displayView_twiceWithDifferentIds_oldViewRemovedNewViewAdded() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "name", @@ -199,10 +201,15 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { assertThat(windowParamsCaptor.allValues[0].title).isEqualTo("First Fake Window Title") assertThat(windowParamsCaptor.allValues[1].title).isEqualTo("Second Fake Window Title") verify(windowManager).removeView(viewCaptor.allValues[0]) + // Since the controller is still storing the older view in case it'll get re-displayed + // later, the listener shouldn't be notified + assertThat(listener.permanentlyRemovedIds).isEmpty() } @Test fun displayView_viewDoesNotDisappearsBeforeTimeout() { + val listener = registerListener() + val state = getState() underTest.displayView(state) reset(windowManager) @@ -210,10 +217,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { fakeClock.advanceTime(TIMEOUT_MS - 1) verify(windowManager, never()).removeView(any()) + assertThat(listener.permanentlyRemovedIds).isEmpty() } @Test fun displayView_viewDisappearsAfterTimeout() { + val listener = registerListener() + val state = getState() underTest.displayView(state) reset(windowManager) @@ -221,10 +231,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { fakeClock.advanceTime(TIMEOUT_MS + 1) verify(windowManager).removeView(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly(DEFAULT_ID) } @Test fun displayView_calledAgainBeforeTimeout_timeoutReset() { + val listener = registerListener() + // First, display the view val state = getState() underTest.displayView(state) @@ -239,10 +252,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { // Verify we didn't hide the view verify(windowManager, never()).removeView(any()) + assertThat(listener.permanentlyRemovedIds).isEmpty() } @Test fun displayView_calledAgainBeforeTimeout_eventuallyTimesOut() { + val listener = registerListener() + // First, display the view val state = getState() underTest.displayView(state) @@ -255,6 +271,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { fakeClock.advanceTime(TIMEOUT_MS + 1) verify(windowManager).removeView(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly(DEFAULT_ID) } @Test @@ -270,26 +287,10 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { assertThat(underTest.mostRecentViewInfo?.name).isEqualTo("Second name") } - @Test - fun viewUpdatedWithNewOnViewTimeoutRunnable_newRunnableUsed() { - var runnable1Run = false - underTest.displayView(ViewInfo(name = "name", id = "id1", windowTitle = "1")) { - runnable1Run = true - } - - var runnable2Run = false - underTest.displayView(ViewInfo(name = "name", id = "id1", windowTitle = "1")) { - runnable2Run = true - } - - fakeClock.advanceTime(TIMEOUT_MS + 1) - - assertThat(runnable1Run).isFalse() - assertThat(runnable2Run).isTrue() - } - @Test fun multipleViewsWithDifferentIds_moreRecentReplacesOlder() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "name", @@ -315,10 +316,16 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { assertThat(windowParamsCaptor.allValues[1].title).isEqualTo("Second Fake Window Title") verify(windowManager).removeView(viewCaptor.allValues[0]) verify(configurationController, never()).removeCallback(any()) + + // Since the controller is still storing the older view in case it'll get re-displayed + // later, the listener shouldn't be notified + assertThat(listener.permanentlyRemovedIds).isEmpty() } @Test - fun multipleViewsWithDifferentIds_recentActiveViewIsDisplayed() { + fun multipleViewsWithDifferentIds_newViewRemoved_previousViewIsDisplayed() { + val listener = registerListener() + underTest.displayView(ViewInfo("First name", id = "id1")) verify(windowManager).addView(any(), any()) @@ -329,24 +336,35 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { verify(windowManager).removeView(any()) verify(windowManager).addView(any(), any()) reset(windowManager) + assertThat(listener.permanentlyRemovedIds).isEmpty() + // WHEN the current view is removed underTest.removeView("id2", "test reason") + // THEN it's correctly removed verify(windowManager).removeView(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly("id2") + + // And the previous view is correctly added verify(windowManager).addView(any(), any()) assertThat(underTest.mostRecentViewInfo?.id).isEqualTo("id1") assertThat(underTest.mostRecentViewInfo?.name).isEqualTo("First name") + // WHEN the previous view times out reset(windowManager) fakeClock.advanceTime(TIMEOUT_MS + 1) + // THEN it is also removed verify(windowManager).removeView(any()) assertThat(underTest.activeViews.size).isEqualTo(0) verify(configurationController).removeCallback(any()) + assertThat(listener.permanentlyRemovedIds).isEqualTo(listOf("id2", "id1")) } @Test fun multipleViewsWithDifferentIds_oldViewRemoved_recentViewIsDisplayed() { + val listener = registerListener() + underTest.displayView(ViewInfo("First name", id = "id1")) verify(windowManager).addView(any(), any()) @@ -361,7 +379,8 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { // WHEN an old view is removed underTest.removeView("id1", "test reason") - // THEN we don't update anything + // THEN we don't update anything except the listener + assertThat(listener.permanentlyRemovedIds).containsExactly("id1") verify(windowManager, never()).removeView(any()) assertThat(underTest.mostRecentViewInfo?.id).isEqualTo("id2") assertThat(underTest.mostRecentViewInfo?.name).isEqualTo("Second name") @@ -372,10 +391,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { verify(windowManager).removeView(any()) assertThat(underTest.activeViews.size).isEqualTo(0) verify(configurationController).removeCallback(any()) + assertThat(listener.permanentlyRemovedIds).isEqualTo(listOf("id1", "id2")) } @Test fun multipleViewsWithDifferentIds_threeDifferentViews_recentActiveViewIsDisplayed() { + val listener = registerListener() + underTest.displayView(ViewInfo("First name", id = "id1")) underTest.displayView(ViewInfo("Second name", id = "id2")) underTest.displayView(ViewInfo("Third name", id = "id3")) @@ -387,6 +409,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { underTest.removeView("id3", "test reason") verify(windowManager).removeView(any()) + assertThat(listener.permanentlyRemovedIds).isEqualTo(listOf("id3")) assertThat(underTest.mostRecentViewInfo?.id).isEqualTo("id2") assertThat(underTest.mostRecentViewInfo?.name).isEqualTo("Second name") verify(configurationController, never()).removeCallback(any()) @@ -395,6 +418,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { underTest.removeView("id2", "test reason") verify(windowManager).removeView(any()) + assertThat(listener.permanentlyRemovedIds).isEqualTo(listOf("id3", "id2")) assertThat(underTest.mostRecentViewInfo?.id).isEqualTo("id1") assertThat(underTest.mostRecentViewInfo?.name).isEqualTo("First name") verify(configurationController, never()).removeCallback(any()) @@ -403,6 +427,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { fakeClock.advanceTime(TIMEOUT_MS + 1) verify(windowManager).removeView(any()) + assertThat(listener.permanentlyRemovedIds).isEqualTo(listOf("id3", "id2", "id1")) assertThat(underTest.activeViews.size).isEqualTo(0) verify(configurationController).removeCallback(any()) } @@ -438,6 +463,8 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { @Test fun multipleViews_mostRecentViewRemoved_otherViewsTimedOutAndNotDisplayed() { + val listener = registerListener() + underTest.displayView(ViewInfo("First name", id = "id1", timeoutMs = 4000)) fakeClock.advanceTime(1000) underTest.displayView(ViewInfo("Second name", id = "id2", timeoutMs = 4000)) @@ -451,10 +478,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { verify(windowManager, never()).addView(any(), any()) assertThat(underTest.activeViews.size).isEqualTo(0) verify(configurationController).removeCallback(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly("id1", "id2", "id3") } @Test fun multipleViews_mostRecentViewRemoved_viewWithShortTimeLeftNotDisplayed() { + val listener = registerListener() + underTest.displayView(ViewInfo("First name", id = "id1", timeoutMs = 4000)) fakeClock.advanceTime(1000) underTest.displayView(ViewInfo("Second name", id = "id2", timeoutMs = 2500)) @@ -467,10 +497,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { verify(windowManager, never()).addView(any(), any()) assertThat(underTest.activeViews.size).isEqualTo(0) verify(configurationController).removeCallback(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly("id1", "id2") } @Test fun lowerThenHigherPriority_higherReplacesLower() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "normal", @@ -499,10 +532,15 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { verify(windowManager).addView(capture(viewCaptor), capture(windowParamsCaptor)) assertThat(windowParamsCaptor.value.title).isEqualTo("Critical Window Title") verify(configurationController, never()).removeCallback(any()) + // Since the controller is still storing the older view in case it'll get re-displayed + // later, the listener shouldn't be notified + assertThat(listener.permanentlyRemovedIds).isEmpty() } @Test fun lowerThenHigherPriority_lowerPriorityRedisplayed() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "normal", @@ -537,6 +575,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { // THEN the normal view is re-displayed verify(windowManager).removeView(viewCaptor.allValues[1]) + assertThat(listener.permanentlyRemovedIds).containsExactly("critical") verify(windowManager).addView(any(), capture(windowParamsCaptor)) assertThat(windowParamsCaptor.value.title).isEqualTo("Normal Window Title") verify(configurationController, never()).removeCallback(any()) @@ -544,6 +583,8 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { @Test fun lowerThenHigherPriority_lowerPriorityNotRedisplayedBecauseTimedOut() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "normal", @@ -573,6 +614,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { verify(windowManager, never()).addView(any(), any()) assertThat(underTest.activeViews).isEmpty() verify(configurationController).removeCallback(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly("critical", "normal") } @Test @@ -609,6 +651,8 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { @Test fun higherThenLowerPriority_lowerEventuallyDisplayed() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "critical", @@ -644,6 +688,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { // THEN the second normal view is displayed verify(windowManager).removeView(viewCaptor.value) + assertThat(listener.permanentlyRemovedIds).containsExactly("critical") verify(windowManager).addView(capture(viewCaptor), capture(windowParamsCaptor)) assertThat(windowParamsCaptor.value.title).isEqualTo("Normal Window Title") assertThat(underTest.activeViews.size).isEqualTo(1) @@ -652,6 +697,8 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { @Test fun higherThenLowerPriority_lowerNotDisplayedBecauseTimedOut() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "critical", @@ -691,10 +738,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { verify(windowManager, never()).addView(any(), any()) assertThat(underTest.activeViews).isEmpty() verify(configurationController).removeCallback(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly("critical", "normal") } @Test fun criticalThenNewCritical_newCriticalDisplayed() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "critical 1", @@ -724,10 +774,15 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { assertThat(windowParamsCaptor.value.title).isEqualTo("Critical Window Title 2") assertThat(underTest.activeViews.size).isEqualTo(2) verify(configurationController, never()).removeCallback(any()) + // Since the controller is still storing the older view in case it'll get re-displayed + // later, the listener shouldn't be notified + assertThat(listener.permanentlyRemovedIds).isEmpty() } @Test fun normalThenNewNormal_newNormalDisplayed() { + val listener = registerListener() + underTest.displayView( ViewInfo( name = "normal 1", @@ -757,6 +812,9 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { assertThat(windowParamsCaptor.value.title).isEqualTo("Normal Window Title 2") assertThat(underTest.activeViews.size).isEqualTo(2) verify(configurationController, never()).removeCallback(any()) + // Since the controller is still storing the older view in case it'll get re-displayed + // later, the listener shouldn't be notified + assertThat(listener.permanentlyRemovedIds).isEmpty() } @Test @@ -957,25 +1015,103 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { } @Test - fun removeView_viewRemovedAndRemovalLogged() { + fun removeView_viewRemovedAndRemovalLoggedAndListenerNotified() { + val listener = registerListener() + // First, add the view underTest.displayView(getState()) // Then, remove it val reason = "test reason" - val deviceId = "id" - underTest.removeView(deviceId, reason) + underTest.removeView(DEFAULT_ID, reason) verify(windowManager).removeView(any()) - verify(logger).logViewRemoval(deviceId, reason) + verify(logger).logViewRemoval(DEFAULT_ID, reason) verify(configurationController).removeCallback(any()) + assertThat(listener.permanentlyRemovedIds).containsExactly(DEFAULT_ID) } @Test - fun removeView_noAdd_viewNotRemoved() { + fun removeView_noAdd_viewNotRemovedAndListenerNotNotified() { + val listener = registerListener() + underTest.removeView("id", "reason") verify(windowManager, never()).removeView(any()) + assertThat(listener.permanentlyRemovedIds).isEmpty() + } + + @Test + fun listenerRegistered_notifiedOnRemoval() { + val listener = registerListener() + underTest.displayView(getState()) + + underTest.removeView(DEFAULT_ID, "reason") + + assertThat(listener.permanentlyRemovedIds).containsExactly(DEFAULT_ID) + } + + @Test + fun listenerRegistered_notifiedOnTimedOutEvenWhenNotDisplayed() { + val listener = registerListener() + underTest.displayView( + ViewInfo( + id = "id1", + name = "name1", + timeoutMs = 3000, + ), + ) + + // Display a second view + underTest.displayView( + ViewInfo( + id = "id2", + name = "name2", + timeoutMs = 2500, + ), + ) + + // WHEN the second view times out + fakeClock.advanceTime(2501) + + // THEN the listener is notified of both IDs, since id2 timed out and id1 doesn't have + // enough time left to be redisplayed + assertThat(listener.permanentlyRemovedIds).containsExactly("id1", "id2") + } + + @Test + fun multipleListeners_allNotified() { + val listener1 = registerListener() + val listener2 = registerListener() + val listener3 = registerListener() + + underTest.displayView(getState()) + + underTest.removeView(DEFAULT_ID, "reason") + + assertThat(listener1.permanentlyRemovedIds).containsExactly(DEFAULT_ID) + assertThat(listener2.permanentlyRemovedIds).containsExactly(DEFAULT_ID) + assertThat(listener3.permanentlyRemovedIds).containsExactly(DEFAULT_ID) + } + + @Test + fun sameListenerRegisteredMultipleTimes_onlyNotifiedOnce() { + val listener = registerListener() + underTest.registerListener(listener) + underTest.registerListener(listener) + + underTest.displayView(getState()) + + underTest.removeView(DEFAULT_ID, "reason") + + assertThat(listener.permanentlyRemovedIds).hasSize(1) + assertThat(listener.permanentlyRemovedIds).containsExactly(DEFAULT_ID) + } + + private fun registerListener(): Listener { + return Listener().also { + underTest.registerListener(it) + } } private fun getState(name: String = "name") = ViewInfo(name) @@ -1030,9 +1166,17 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { override val windowTitle: String = "Window Title", override val wakeReason: String = "WAKE_REASON", override val timeoutMs: Int = TIMEOUT_MS.toInt(), - override val id: String = "id", + override val id: String = DEFAULT_ID, override val priority: ViewPriority = ViewPriority.NORMAL, ) : TemporaryViewInfo() + + inner class Listener : TemporaryViewDisplayController.Listener { + val permanentlyRemovedIds = mutableListOf() + override fun onInfoPermanentlyRemoved(id: String) { + permanentlyRemovedIds.add(id) + } + } } private const val TIMEOUT_MS = 10000L +private const val DEFAULT_ID = "defaultId" From 82042d1876bcbb8435f67d168b69d01122df0398 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Tue, 17 Jan 2023 16:57:44 +0000 Subject: [PATCH 2/4] [Media TTT] Re-name the status bar gesture handler to be more generic. The next CL will split out the generic swipe-up detection and the status-bar-specific gesture information, and will add back the class `SwipeStatusBarAwayGestureHandler`. So, this CL leaves the variable names to be status-bar-specific. Bug: 262584940 Test: verify swiping the status bar away when there's an ongoing call still works Test: atest OngoingCallControllerTest Change-Id: Ief581a073ca3809963014405c8233dc13e7e8266 Change-Id: I3da3c306733545df44adfb9518fb4f762f2ace6c --- packages/SystemUI/ktfmt_includes.txt | 4 ++-- .../com/android/systemui/log/dagger/LogModule.java | 11 ++++------- .../{SwipeStatusBarAwayLog.java => SwipeUpLog.java} | 4 ++-- .../dagger/CentralSurfacesDependenciesModule.java | 6 +++--- ...wayGestureHandler.kt => SwipeUpGestureHandler.kt} | 12 ++++++------ ...rAwayGestureLogger.kt => SwipeUpGestureLogger.kt} | 10 +++++----- .../phone/ongoingcall/OngoingCallController.kt | 4 ++-- .../phone/ongoingcall/OngoingCallControllerTest.kt | 4 ++-- 8 files changed, 26 insertions(+), 29 deletions(-) rename packages/SystemUI/src/com/android/systemui/log/dagger/{SwipeStatusBarAwayLog.java => SwipeUpLog.java} (88%) rename packages/SystemUI/src/com/android/systemui/statusbar/gesture/{SwipeStatusBarAwayGestureHandler.kt => SwipeUpGestureHandler.kt} (90%) rename packages/SystemUI/src/com/android/systemui/statusbar/gesture/{SwipeStatusBarAwayGestureLogger.kt => SwipeUpGestureLogger.kt} (85%) diff --git a/packages/SystemUI/ktfmt_includes.txt b/packages/SystemUI/ktfmt_includes.txt index f53e3f6b2ea91..7e621cfda0064 100644 --- a/packages/SystemUI/ktfmt_includes.txt +++ b/packages/SystemUI/ktfmt_includes.txt @@ -322,8 +322,8 @@ -packages/SystemUI/src/com/android/systemui/statusbar/events/SystemEventCoordinator.kt -packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationScheduler.kt -packages/SystemUI/src/com/android/systemui/statusbar/gesture/GenericGestureDetector.kt --packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt --packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureLogger.kt +-packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt +-packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt -packages/SystemUI/src/com/android/systemui/statusbar/gesture/TapGestureDetector.kt -packages/SystemUI/src/com/android/systemui/statusbar/lockscreen/LockscreenSmartspaceController.kt -packages/SystemUI/src/com/android/systemui/statusbar/notification/ConversationNotifications.kt diff --git a/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java b/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java index d7817e1dc4f0a..15306f9cc23e4 100644 --- a/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java +++ b/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java @@ -191,15 +191,12 @@ public class LogModule { false /* systrace */); } - /** - * Provides a logging buffer for logs related to swiping away the status bar while in immersive - * mode. See {@link com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureLogger}. - */ + /** Provides a logging buffer for logs related to swipe up gestures. */ @Provides @SysUISingleton - @SwipeStatusBarAwayLog - public static LogBuffer provideSwipeAwayGestureLogBuffer(LogBufferFactory factory) { - return factory.create("SwipeStatusBarAwayLog", 30); + @SwipeUpLog + public static LogBuffer provideSwipeUpLogBuffer(LogBufferFactory factory) { + return factory.create("SwipeUpLog", 30); } /** diff --git a/packages/SystemUI/src/com/android/systemui/log/dagger/SwipeStatusBarAwayLog.java b/packages/SystemUI/src/com/android/systemui/log/dagger/SwipeUpLog.java similarity index 88% rename from packages/SystemUI/src/com/android/systemui/log/dagger/SwipeStatusBarAwayLog.java rename to packages/SystemUI/src/com/android/systemui/log/dagger/SwipeUpLog.java index 4c276e2bfaab0..d58b538f20477 100644 --- a/packages/SystemUI/src/com/android/systemui/log/dagger/SwipeStatusBarAwayLog.java +++ b/packages/SystemUI/src/com/android/systemui/log/dagger/SwipeUpLog.java @@ -27,10 +27,10 @@ import javax.inject.Qualifier; /** * A {@link LogBuffer} for - * {@link com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureLogger}. + * {@link com.android.systemui.statusbar.gesture.SwipeUpGestureLogger}. */ @Qualifier @Documented @Retention(RUNTIME) -public @interface SwipeStatusBarAwayLog { +public @interface SwipeUpLog { } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java index 9a65e342478e4..adaae442d3fb0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java @@ -48,7 +48,7 @@ import com.android.systemui.statusbar.SmartReplyController; import com.android.systemui.statusbar.StatusBarStateControllerImpl; import com.android.systemui.statusbar.SysuiStatusBarStateController; import com.android.systemui.statusbar.commandline.CommandRegistry; -import com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureHandler; +import com.android.systemui.statusbar.gesture.SwipeUpGestureHandler; import com.android.systemui.statusbar.notification.NotifPipelineFlags; import com.android.systemui.statusbar.notification.collection.NotifCollection; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -230,7 +230,7 @@ public interface CentralSurfacesDependenciesModule { OngoingCallLogger logger, DumpManager dumpManager, StatusBarWindowController statusBarWindowController, - SwipeStatusBarAwayGestureHandler swipeStatusBarAwayGestureHandler, + SwipeUpGestureHandler swipeStatusBarAwayGestureHandler, StatusBarStateController statusBarStateController, OngoingCallFlags ongoingCallFlags) { @@ -239,7 +239,7 @@ public interface CentralSurfacesDependenciesModule { ongoingCallInImmersiveEnabled ? Optional.of(statusBarWindowController) : Optional.empty(); - Optional gestureHandler = + Optional gestureHandler = ongoingCallInImmersiveEnabled ? Optional.of(swipeStatusBarAwayGestureHandler) : Optional.empty(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt similarity index 90% rename from packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt rename to packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt index 6115819b967aa..4ff1423f293c4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt @@ -32,11 +32,11 @@ import javax.inject.Inject * gesture is detected, add a callback via [addOnGestureDetectedCallback]. */ @SysUISingleton -open class SwipeStatusBarAwayGestureHandler @Inject constructor( +open class SwipeUpGestureHandler @Inject constructor( context: Context, private val statusBarWindowController: StatusBarWindowController, - private val logger: SwipeStatusBarAwayGestureLogger -) : GenericGestureDetector(SwipeStatusBarAwayGestureHandler::class.simpleName!!) { + private val logger: SwipeUpGestureLogger +) : GenericGestureDetector(SwipeUpGestureHandler::class.simpleName!!) { private var startY: Float = 0f private var startTime: Long = 0L @@ -72,11 +72,11 @@ open class SwipeStatusBarAwayGestureHandler @Inject constructor( } if ( // Gesture is up - ev.y < startY + ev.y < startY && // Gesture went far enough - && (startY - ev.y) >= swipeDistanceThreshold + (startY - ev.y) >= swipeDistanceThreshold && // Gesture completed quickly enough - && (ev.eventTime - startTime) < SWIPE_TIMEOUT_MS + (ev.eventTime - startTime) < SWIPE_TIMEOUT_MS ) { monitoringCurrentTouch = false logger.logGestureDetected(ev.y.toInt()) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt similarity index 85% rename from packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureLogger.kt rename to packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt index 9bdff928c44b0..72759c7d929bf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt @@ -16,14 +16,14 @@ package com.android.systemui.statusbar.gesture -import com.android.systemui.log.dagger.SwipeStatusBarAwayLog +import com.android.systemui.log.dagger.SwipeUpLog import com.android.systemui.plugins.log.LogBuffer import com.android.systemui.plugins.log.LogLevel import javax.inject.Inject -/** Log messages for [SwipeStatusBarAwayGestureHandler]. */ -class SwipeStatusBarAwayGestureLogger @Inject constructor( - @SwipeStatusBarAwayLog private val buffer: LogBuffer +/** Log messages for [SwipeUpGestureHandler]. */ +class SwipeUpGestureLogger @Inject constructor( + @SwipeUpLog private val buffer: LogBuffer, ) { fun logGestureDetectionStarted(y: Int) { buffer.log( @@ -61,4 +61,4 @@ class SwipeStatusBarAwayGestureLogger @Inject constructor( } } -private const val TAG = "SwipeStatusBarAwayGestureHandler" \ No newline at end of file +private const val TAG = "SwipeUpGestureHandler" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt index dfa68383bd035..9d5d2a20914f0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt @@ -35,7 +35,7 @@ import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.dump.DumpManager import com.android.systemui.plugins.ActivityStarter import com.android.systemui.plugins.statusbar.StatusBarStateController -import com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureHandler +import com.android.systemui.statusbar.gesture.SwipeUpGestureHandler import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener @@ -62,7 +62,7 @@ class OngoingCallController @Inject constructor( private val logger: OngoingCallLogger, private val dumpManager: DumpManager, private val statusBarWindowController: Optional, - private val swipeStatusBarAwayGestureHandler: Optional, + private val swipeStatusBarAwayGestureHandler: Optional, private val statusBarStateController: StatusBarStateController ) : CallbackController, Dumpable { private var isFullscreen: Boolean = false diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt index d30222f2b462f..7e2275c714262 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt @@ -35,7 +35,7 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager import com.android.systemui.plugins.ActivityStarter import com.android.systemui.plugins.statusbar.StatusBarStateController -import com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureHandler +import com.android.systemui.statusbar.gesture.SwipeUpGestureHandler import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection @@ -83,7 +83,7 @@ class OngoingCallControllerTest : SysuiTestCase() { private lateinit var notifCollectionListener: NotifCollectionListener @Mock private lateinit var mockOngoingCallFlags: OngoingCallFlags - @Mock private lateinit var mockSwipeStatusBarAwayGestureHandler: SwipeStatusBarAwayGestureHandler + @Mock private lateinit var mockSwipeStatusBarAwayGestureHandler: SwipeUpGestureHandler @Mock private lateinit var mockOngoingCallListener: OngoingCallListener @Mock private lateinit var mockActivityStarter: ActivityStarter @Mock private lateinit var mockIActivityManager: IActivityManager From b7ec39e18d9cfda63f9bc833a730c559ffc9dc9b Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Tue, 17 Jan 2023 17:05:58 +0000 Subject: [PATCH 3/4] [Media TTT] Make the SwipeUpGestureHandler generic. The media swipe gesture will re-use SwipeUpGestureHandler, so this CL removes the status-bar-specific items from the handler. Bug: 262584940 Test: verify swiping the status bar away when there's an ongoing call still works Test: Verify SwipeUpLog logs still appear Test: atest OngoingCallControllerTest Change-Id: I18707ac9bb5f6f92531ac660f29bfc6a59437314 --- .../CentralSurfacesDependenciesModule.java | 6 +-- .../SwipeStatusBarAwayGestureHandler.kt | 41 +++++++++++++++++++ .../gesture/SwipeUpGestureHandler.kt | 35 +++++++++------- .../statusbar/gesture/SwipeUpGestureLogger.kt | 24 +++++------ .../ongoingcall/OngoingCallController.kt | 4 +- .../ongoingcall/OngoingCallControllerTest.kt | 9 ++-- 6 files changed, 84 insertions(+), 35 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java index adaae442d3fb0..9a65e342478e4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java @@ -48,7 +48,7 @@ import com.android.systemui.statusbar.SmartReplyController; import com.android.systemui.statusbar.StatusBarStateControllerImpl; import com.android.systemui.statusbar.SysuiStatusBarStateController; import com.android.systemui.statusbar.commandline.CommandRegistry; -import com.android.systemui.statusbar.gesture.SwipeUpGestureHandler; +import com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureHandler; import com.android.systemui.statusbar.notification.NotifPipelineFlags; import com.android.systemui.statusbar.notification.collection.NotifCollection; import com.android.systemui.statusbar.notification.collection.NotifPipeline; @@ -230,7 +230,7 @@ public interface CentralSurfacesDependenciesModule { OngoingCallLogger logger, DumpManager dumpManager, StatusBarWindowController statusBarWindowController, - SwipeUpGestureHandler swipeStatusBarAwayGestureHandler, + SwipeStatusBarAwayGestureHandler swipeStatusBarAwayGestureHandler, StatusBarStateController statusBarStateController, OngoingCallFlags ongoingCallFlags) { @@ -239,7 +239,7 @@ public interface CentralSurfacesDependenciesModule { ongoingCallInImmersiveEnabled ? Optional.of(statusBarWindowController) : Optional.empty(); - Optional gestureHandler = + Optional gestureHandler = ongoingCallInImmersiveEnabled ? Optional.of(swipeStatusBarAwayGestureHandler) : Optional.empty(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt new file mode 100644 index 0000000000000..5ab3d7ce9becd --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeStatusBarAwayGestureHandler.kt @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2021 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.statusbar.gesture + +import android.content.Context +import android.view.MotionEvent +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.statusbar.window.StatusBarWindowController +import javax.inject.Inject + +/** A class to detect when a user swipes away the status bar. */ +@SysUISingleton +class SwipeStatusBarAwayGestureHandler +@Inject +constructor( + context: Context, + logger: SwipeUpGestureLogger, + private val statusBarWindowController: StatusBarWindowController, +) : SwipeUpGestureHandler(context, logger, loggerTag = LOGGER_TAG) { + override fun startOfGestureIsWithinBounds(ev: MotionEvent): Boolean { + // Gesture starts just below the status bar + return ev.y >= statusBarWindowController.statusBarHeight && + ev.y <= 3 * statusBarWindowController.statusBarHeight + } +} + +private const val LOGGER_TAG = "SwipeStatusBarAway" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt index 4ff1423f293c4..5ecc35ca45761 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureHandler.kt @@ -24,18 +24,16 @@ import android.view.MotionEvent.ACTION_DOWN import android.view.MotionEvent.ACTION_MOVE import android.view.MotionEvent.ACTION_UP import com.android.systemui.dagger.SysUISingleton -import com.android.systemui.statusbar.window.StatusBarWindowController -import javax.inject.Inject /** - * A class to detect when a user swipes away the status bar. To be notified when the swipe away - * gesture is detected, add a callback via [addOnGestureDetectedCallback]. + * A class to detect a generic "swipe up" gesture. To be notified when the swipe up gesture is + * detected, add a callback via [addOnGestureDetectedCallback]. */ @SysUISingleton -open class SwipeUpGestureHandler @Inject constructor( +abstract class SwipeUpGestureHandler( context: Context, - private val statusBarWindowController: StatusBarWindowController, - private val logger: SwipeUpGestureLogger + private val logger: SwipeUpGestureLogger, + private val loggerTag: String, ) : GenericGestureDetector(SwipeUpGestureHandler::class.simpleName!!) { private var startY: Float = 0f @@ -54,11 +52,9 @@ open class SwipeUpGestureHandler @Inject constructor( when (ev.actionMasked) { ACTION_DOWN -> { if ( - // Gesture starts just below the status bar - ev.y >= statusBarWindowController.statusBarHeight - && ev.y <= 3 * statusBarWindowController.statusBarHeight + startOfGestureIsWithinBounds(ev) ) { - logger.logGestureDetectionStarted(ev.y.toInt()) + logger.logGestureDetectionStarted(loggerTag, ev.y.toInt()) startY = ev.y startTime = ev.eventTime monitoringCurrentTouch = true @@ -79,27 +75,36 @@ open class SwipeUpGestureHandler @Inject constructor( (ev.eventTime - startTime) < SWIPE_TIMEOUT_MS ) { monitoringCurrentTouch = false - logger.logGestureDetected(ev.y.toInt()) + logger.logGestureDetected(loggerTag, ev.y.toInt()) onGestureDetected(ev) } } ACTION_CANCEL, ACTION_UP -> { if (monitoringCurrentTouch) { - logger.logGestureDetectionEndedWithoutTriggering(ev.y.toInt()) + logger.logGestureDetectionEndedWithoutTriggering(loggerTag, ev.y.toInt()) } monitoringCurrentTouch = false } } } + /** + * Returns true if the [ACTION_DOWN] event falls within bounds for this specific swipe-up + * gesture. + * + * Implementations must override this method to specify what part(s) of the screen are valid + * locations for the swipe up gesture to start at. + */ + abstract fun startOfGestureIsWithinBounds(ev: MotionEvent): Boolean + override fun startGestureListening() { super.startGestureListening() - logger.logInputListeningStarted() + logger.logInputListeningStarted(loggerTag) } override fun stopGestureListening() { super.stopGestureListening() - logger.logInputListeningStopped() + logger.logInputListeningStopped(loggerTag) } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt index 72759c7d929bf..9ce6b02e55d9a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/gesture/SwipeUpGestureLogger.kt @@ -16,49 +16,49 @@ package com.android.systemui.statusbar.gesture +import com.android.systemui.dagger.SysUISingleton import com.android.systemui.log.dagger.SwipeUpLog import com.android.systemui.plugins.log.LogBuffer import com.android.systemui.plugins.log.LogLevel import javax.inject.Inject /** Log messages for [SwipeUpGestureHandler]. */ +@SysUISingleton class SwipeUpGestureLogger @Inject constructor( @SwipeUpLog private val buffer: LogBuffer, ) { - fun logGestureDetectionStarted(y: Int) { + fun logGestureDetectionStarted(tag: String, y: Int) { buffer.log( - TAG, + tag, LogLevel.DEBUG, { int1 = y }, { "Beginning gesture detection. y=$int1" } ) } - fun logGestureDetectionEndedWithoutTriggering(y: Int) { + fun logGestureDetectionEndedWithoutTriggering(tag: String, y: Int) { buffer.log( - TAG, + tag, LogLevel.DEBUG, { int1 = y }, { "Gesture finished; no swipe up gesture detected. Final y=$int1" } ) } - fun logGestureDetected(y: Int) { + fun logGestureDetected(tag: String, y: Int) { buffer.log( - TAG, + tag, LogLevel.INFO, { int1 = y }, { "Gesture detected; notifying callbacks. y=$int1" } ) } - fun logInputListeningStarted() { - buffer.log(TAG, LogLevel.VERBOSE, {}, { "Input listening started "}) + fun logInputListeningStarted(tag: String) { + buffer.log(tag, LogLevel.VERBOSE, {}, { "Input listening started "}) } - fun logInputListeningStopped() { - buffer.log(TAG, LogLevel.VERBOSE, {}, { "Input listening stopped "}) + fun logInputListeningStopped(tag: String) { + buffer.log(tag, LogLevel.VERBOSE, {}, { "Input listening stopped "}) } } - -private const val TAG = "SwipeUpGestureHandler" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt index 9d5d2a20914f0..dfa68383bd035 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt @@ -35,7 +35,7 @@ import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.dump.DumpManager import com.android.systemui.plugins.ActivityStarter import com.android.systemui.plugins.statusbar.StatusBarStateController -import com.android.systemui.statusbar.gesture.SwipeUpGestureHandler +import com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureHandler import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection import com.android.systemui.statusbar.notification.collection.notifcollection.NotifCollectionListener @@ -62,7 +62,7 @@ class OngoingCallController @Inject constructor( private val logger: OngoingCallLogger, private val dumpManager: DumpManager, private val statusBarWindowController: Optional, - private val swipeStatusBarAwayGestureHandler: Optional, + private val swipeStatusBarAwayGestureHandler: Optional, private val statusBarStateController: StatusBarStateController ) : CallbackController, Dumpable { private var isFullscreen: Boolean = false diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt index 7e2275c714262..711e4ac937cce 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt @@ -35,7 +35,7 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager import com.android.systemui.plugins.ActivityStarter import com.android.systemui.plugins.statusbar.StatusBarStateController -import com.android.systemui.statusbar.gesture.SwipeUpGestureHandler +import com.android.systemui.statusbar.gesture.SwipeStatusBarAwayGestureHandler import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder import com.android.systemui.statusbar.notification.collection.notifcollection.CommonNotifCollection @@ -50,7 +50,9 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor -import org.mockito.ArgumentMatchers.* +import org.mockito.ArgumentMatchers.anyBoolean +import org.mockito.ArgumentMatchers.anyString +import org.mockito.ArgumentMatchers.nullable import org.mockito.Mock import org.mockito.Mockito.`when` import org.mockito.Mockito.eq @@ -83,7 +85,8 @@ class OngoingCallControllerTest : SysuiTestCase() { private lateinit var notifCollectionListener: NotifCollectionListener @Mock private lateinit var mockOngoingCallFlags: OngoingCallFlags - @Mock private lateinit var mockSwipeStatusBarAwayGestureHandler: SwipeUpGestureHandler + @Mock private lateinit var mockSwipeStatusBarAwayGestureHandler: + SwipeStatusBarAwayGestureHandler @Mock private lateinit var mockOngoingCallListener: OngoingCallListener @Mock private lateinit var mockActivityStarter: ActivityStarter @Mock private lateinit var mockIActivityManager: IActivityManager From b4314f1e4f3016d33d4f7a0552517d06faf1c5ea Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Thu, 22 Dec 2022 19:12:23 +0000 Subject: [PATCH 4/4] [Media TTT] Allow swiping up to dismiss the chipbar. Bug: 262584940 Test: with flag off, verify that you can't swipe up to dismiss the chipbar Test: with flag on, verify that starting a swipe around the chipbar area and swiping up will dismiss the chipbar Test: atest ChipbarCoordinatorTest SwipeChipbarAwayGestureHandlerTest Change-Id: I8440b9b2d7d715981bbf8ee9e44d9202068d4a88 Change-Id: I38aa257855eacaaae6708c796f3f91ea4584342a --- .../src/com/android/systemui/flags/Flags.kt | 4 + .../media/taptotransfer/MediaTttFlags.kt | 4 + .../sender/MediaTttSenderCoordinator.kt | 1 + .../TemporaryViewDisplayController.kt | 4 +- .../chipbar/ChipbarCoordinator.kt | 41 +++++++ .../temporarydisplay/chipbar/ChipbarInfo.kt | 2 + .../temporarydisplay/chipbar/ChipbarLogger.kt | 12 ++ .../chipbar/SwipeChipbarAwayGestureHandler.kt | 62 ++++++++++ .../dagger/TemporaryDisplayModule.kt | 20 +++- .../sender/MediaTttSenderCoordinatorTest.kt | 3 + .../chipbar/ChipbarCoordinatorTest.kt | 90 +++++++++++++++ .../chipbar/FakeChipbarCoordinator.kt | 2 + .../SwipeChipbarAwayGestureHandlerTest.kt | 109 ++++++++++++++++++ 13 files changed, 351 insertions(+), 3 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandler.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandlerTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt index f0416e595ebaf..b2d9e74d5b3f0 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt @@ -341,6 +341,10 @@ object Flags { // TODO(b/263512203): Tracking Bug val MEDIA_EXPLICIT_INDICATOR = unreleasedFlag(911, "media_explicit_indicator", teamfood = true) + // TODO(b/265813373): Tracking Bug + val MEDIA_TAP_TO_TRANSFER_DISMISS_GESTURE = + unreleasedFlag(912, "media_ttt_dismiss_gesture", teamfood = true) + // 1000 - dock val SIMULATE_DOCK_THROUGH_CHARGING = releasedFlag(1000, "simulate_dock_through_charging") diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttFlags.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttFlags.kt index 8a565fa86b353..60504e43465a3 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttFlags.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/MediaTttFlags.kt @@ -30,4 +30,8 @@ class MediaTttFlags @Inject constructor(private val featureFlags: FeatureFlags) /** Check whether the flag for the receiver success state is enabled. */ fun isMediaTttReceiverSuccessRippleEnabled(): Boolean = featureFlags.isEnabled(Flags.MEDIA_TTT_RECEIVER_SUCCESS_RIPPLE) + + /** True if the media transfer chip can be dismissed via a gesture. */ + fun isMediaTttDismissGestureEnabled(): Boolean = + featureFlags.isEnabled(Flags.MEDIA_TAP_TO_TRANSFER_DISMISS_GESTURE) } 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 be93c54b498f1..902a10a0cea92 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 @@ -185,6 +185,7 @@ constructor( } }, vibrationEffect = chipStateSender.transferStatus.vibrationEffect, + allowSwipeToDismiss = true, windowTitle = MediaTttUtils.WINDOW_TITLE_SENDER, wakeReason = MediaTttUtils.WAKE_REASON_SENDER, timeoutMs = chipStateSender.timeout, diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index cf722ce8ecfab..df8d16142b8bd 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -31,6 +31,7 @@ import android.view.accessibility.AccessibilityManager.FLAG_CONTENT_CONTROLS import android.view.accessibility.AccessibilityManager.FLAG_CONTENT_ICONS import android.view.accessibility.AccessibilityManager.FLAG_CONTENT_TEXT import androidx.annotation.CallSuper +import androidx.annotation.VisibleForTesting import com.android.systemui.CoreStartable import com.android.systemui.Dumpable import com.android.systemui.dagger.qualifiers.Main @@ -108,9 +109,10 @@ abstract class TemporaryViewDisplayController = mutableListOf() - private fun getCurrentDisplayInfo(): DisplayInfo? { + internal fun getCurrentDisplayInfo(): DisplayInfo? { return activeViews.getOrNull(0) } 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 04b1a50169899..9e0bbb7624bdd 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt @@ -80,6 +80,7 @@ constructor( powerManager: PowerManager, private val falsingManager: FalsingManager, private val falsingCollector: FalsingCollector, + private val swipeChipbarAwayGestureHandler: SwipeChipbarAwayGestureHandler?, private val viewUtil: ViewUtil, private val vibratorHelper: VibratorHelper, wakeLockBuilder: WakeLock.Builder, @@ -105,6 +106,8 @@ constructor( commonWindowLayoutParams.apply { gravity = Gravity.TOP.or(Gravity.CENTER_HORIZONTAL) } override fun updateView(newInfo: ChipbarInfo, currentView: ViewGroup) { + updateGestureListening() + logger.logViewUpdate( newInfo.windowTitle, newInfo.text.loadText(context), @@ -228,6 +231,42 @@ constructor( includeMargins = true, onAnimationEnd, ) + + updateGestureListening() + } + + private fun updateGestureListening() { + if (swipeChipbarAwayGestureHandler == null) { + return + } + + val currentDisplayInfo = getCurrentDisplayInfo() + if (currentDisplayInfo != null && currentDisplayInfo.info.allowSwipeToDismiss) { + swipeChipbarAwayGestureHandler.setViewFetcher { currentDisplayInfo.view } + swipeChipbarAwayGestureHandler.addOnGestureDetectedCallback(TAG) { + onSwipeUpGestureDetected() + } + } else { + swipeChipbarAwayGestureHandler.resetViewFetcher() + swipeChipbarAwayGestureHandler.removeOnGestureDetectedCallback(TAG) + } + } + + private fun onSwipeUpGestureDetected() { + val currentDisplayInfo = getCurrentDisplayInfo() + if (currentDisplayInfo == null) { + logger.logSwipeGestureError(id = null, errorMsg = "No info is being displayed") + return + } + if (!currentDisplayInfo.info.allowSwipeToDismiss) { + logger.logSwipeGestureError( + id = currentDisplayInfo.info.id, + errorMsg = "This view prohibits swipe-to-dismiss", + ) + return + } + removeView(currentDisplayInfo.info.id, SWIPE_UP_GESTURE_REASON) + updateGestureListening() } private fun ViewGroup.getInnerView(): ViewGroup { @@ -250,3 +289,5 @@ constructor( private const val ANIMATION_IN_DURATION = 500L private const val ANIMATION_OUT_DURATION = 250L @IdRes private val INFO_TAG = R.id.tag_chipbar_info +private const val SWIPE_UP_GESTURE_REASON = "SWIPE_UP_GESTURE_DETECTED" +private const val TAG = "ChipbarCoordinator" diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarInfo.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarInfo.kt index dd4bd26e3bcd7..fe46318daa30b 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarInfo.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarInfo.kt @@ -33,12 +33,14 @@ import com.android.systemui.temporarydisplay.ViewPriority * @property endItem an optional end item to display at the end of the chipbar (on the right in LTR * locales; on the left in RTL locales). * @property vibrationEffect an optional vibration effect when the chipbar is displayed + * @property allowSwipeToDismiss true if users are allowed to swipe up to dismiss this chipbar. */ data class ChipbarInfo( val startIcon: TintedIcon, val text: Text, val endItem: ChipbarEndItem?, val vibrationEffect: VibrationEffect? = null, + val allowSwipeToDismiss: Boolean = false, override val windowTitle: String, override val wakeReason: String, override val timeoutMs: Int, diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarLogger.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarLogger.kt index fcfbe0aeedf60..f23942847e682 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarLogger.kt @@ -46,4 +46,16 @@ constructor( { "Chipbar updated. window=$str1 text=$str2 endItem=$str3" } ) } + + fun logSwipeGestureError(id: String?, errorMsg: String) { + buffer.log( + tag, + LogLevel.WARNING, + { + str1 = id + str2 = errorMsg + }, + { "Chipbar swipe gesture detected for incorrect state. id=$str1 error=$str2" } + ) + } } diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandler.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandler.kt new file mode 100644 index 0000000000000..6e3cb4823afa6 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandler.kt @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2023 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.temporarydisplay.chipbar + +import android.content.Context +import android.view.MotionEvent +import android.view.View +import com.android.systemui.statusbar.gesture.SwipeUpGestureHandler +import com.android.systemui.statusbar.gesture.SwipeUpGestureLogger +import com.android.systemui.util.boundsOnScreen + +/** + * A class to detect when a user has swiped the chipbar away. + * + * Effectively [SysUISingleton]. But, this shouldn't be created if the gesture isn't enabled. See + * [TemporaryDisplayModule.provideSwipeChipbarAwayGestureHandler]. + */ +class SwipeChipbarAwayGestureHandler( + context: Context, + logger: SwipeUpGestureLogger, +) : SwipeUpGestureHandler(context, logger, loggerTag = LOGGER_TAG) { + + private var viewFetcher: () -> View? = { null } + + override fun startOfGestureIsWithinBounds(ev: MotionEvent): Boolean { + val view = viewFetcher.invoke() ?: return false + // Since chipbar is in its own window, we need to use [boundsOnScreen] to get an accurate + // bottom. ([view.bottom] would be relative to its window, which would be too small.) + val viewBottom = view.boundsOnScreen.bottom + // Allow the gesture to start a bit below the chipbar + return ev.y <= 1.5 * viewBottom + } + + /** + * Sets a fetcher that returns the current chipbar view. The fetcher will be invoked whenever a + * gesture starts to determine if the gesture is near the chipbar. + */ + fun setViewFetcher(fetcher: () -> View?) { + viewFetcher = fetcher + } + + /** Removes the current view fetcher. */ + fun resetViewFetcher() { + viewFetcher = { null } + } +} + +private const val LOGGER_TAG = "SwipeChipbarAway" diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/dagger/TemporaryDisplayModule.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/dagger/TemporaryDisplayModule.kt index cf0a1835c8e89..933c0604a3b92 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/dagger/TemporaryDisplayModule.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/dagger/TemporaryDisplayModule.kt @@ -16,22 +16,38 @@ package com.android.systemui.temporarydisplay.dagger +import android.content.Context import com.android.systemui.dagger.SysUISingleton import com.android.systemui.log.LogBufferFactory +import com.android.systemui.media.taptotransfer.MediaTttFlags import com.android.systemui.plugins.log.LogBuffer +import com.android.systemui.statusbar.gesture.SwipeUpGestureLogger +import com.android.systemui.temporarydisplay.chipbar.SwipeChipbarAwayGestureHandler import dagger.Module import dagger.Provides @Module interface TemporaryDisplayModule { - @Module companion object { - @JvmStatic @Provides @SysUISingleton @ChipbarLog fun provideChipbarLogBuffer(factory: LogBufferFactory): LogBuffer { return factory.create("ChipbarLog", 40) } + + @Provides + @SysUISingleton + fun provideSwipeChipbarAwayGestureHandler( + mediaTttFlags: MediaTttFlags, + context: Context, + logger: SwipeUpGestureLogger, + ): SwipeChipbarAwayGestureHandler? { + return if (mediaTttFlags.isMediaTttDismissGestureEnabled()) { + SwipeChipbarAwayGestureHandler(context, logger) + } else { + null + } + } } } 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 1cdce997b19b0..54d4460af6e4d 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 @@ -50,6 +50,7 @@ import com.android.systemui.temporarydisplay.chipbar.ChipbarCoordinator import com.android.systemui.temporarydisplay.chipbar.ChipbarInfo import com.android.systemui.temporarydisplay.chipbar.ChipbarLogger import com.android.systemui.temporarydisplay.chipbar.FakeChipbarCoordinator +import com.android.systemui.temporarydisplay.chipbar.SwipeChipbarAwayGestureHandler import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.argumentCaptor @@ -98,6 +99,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { @Mock private lateinit var viewUtil: ViewUtil @Mock private lateinit var windowManager: WindowManager @Mock private lateinit var vibratorHelper: VibratorHelper + @Mock private lateinit var swipeHandler: SwipeChipbarAwayGestureHandler private lateinit var fakeWakeLockBuilder: WakeLockFake.Builder private lateinit var fakeWakeLock: WakeLockFake private lateinit var chipbarCoordinator: ChipbarCoordinator @@ -148,6 +150,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { powerManager, falsingManager, falsingCollector, + swipeHandler, viewUtil, vibratorHelper, fakeWakeLockBuilder, 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 90178c6a00966..45eb1f9ec4310 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 @@ -20,6 +20,7 @@ import android.os.PowerManager import android.os.VibrationEffect import android.testing.AndroidTestingRunner import android.testing.TestableLooper +import android.view.MotionEvent import android.view.View import android.view.ViewGroup import android.view.WindowManager @@ -43,6 +44,8 @@ import com.android.systemui.statusbar.policy.ConfigurationController import com.android.systemui.temporarydisplay.ViewPriority import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.any +import com.android.systemui.util.mockito.argumentCaptor +import com.android.systemui.util.mockito.capture import com.android.systemui.util.mockito.eq import com.android.systemui.util.time.FakeSystemClock import com.android.systemui.util.view.ViewUtil @@ -54,6 +57,7 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.anyInt import org.mockito.Mock +import org.mockito.Mockito.never import org.mockito.Mockito.verify import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations @@ -74,6 +78,7 @@ class ChipbarCoordinatorTest : SysuiTestCase() { @Mock private lateinit var falsingCollector: FalsingCollector @Mock private lateinit var viewUtil: ViewUtil @Mock private lateinit var vibratorHelper: VibratorHelper + @Mock private lateinit var swipeGestureHandler: SwipeChipbarAwayGestureHandler private lateinit var fakeWakeLockBuilder: WakeLockFake.Builder private lateinit var fakeWakeLock: WakeLockFake private lateinit var fakeClock: FakeSystemClock @@ -106,6 +111,7 @@ class ChipbarCoordinatorTest : SysuiTestCase() { powerManager, falsingManager, falsingCollector, + swipeGestureHandler, viewUtil, vibratorHelper, fakeWakeLockBuilder, @@ -430,17 +436,101 @@ class ChipbarCoordinatorTest : SysuiTestCase() { verify(logger).logViewUpdate(eq(WINDOW_TITLE), eq("new title text"), any()) } + @Test + fun swipeToDismiss_false_neverListensForGesture() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.drawable.ic_cake, contentDescription = null), + Text.Loaded("title text"), + endItem = ChipbarEndItem.Loading, + allowSwipeToDismiss = false, + ) + ) + + verify(swipeGestureHandler, never()).addOnGestureDetectedCallback(any(), any()) + } + + @Test + fun swipeToDismiss_true_listensForGesture() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.drawable.ic_cake, contentDescription = null), + Text.Loaded("title text"), + endItem = ChipbarEndItem.Loading, + allowSwipeToDismiss = true, + ) + ) + + verify(swipeGestureHandler).addOnGestureDetectedCallback(any(), any()) + } + + @Test + fun swipeToDismiss_swipeOccurs_viewDismissed() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.drawable.ic_cake, contentDescription = null), + Text.Loaded("title text"), + endItem = ChipbarEndItem.Loading, + allowSwipeToDismiss = true, + ) + ) + val view = getChipbarView() + + val callbackCaptor = argumentCaptor<(MotionEvent) -> Unit>() + verify(swipeGestureHandler).addOnGestureDetectedCallback(any(), capture(callbackCaptor)) + + callbackCaptor.value.invoke(MotionEvent.obtain(0L, 0L, 0, 0f, 0f, 0)) + + verify(windowManager).removeView(view) + } + + @Test + fun swipeToDismiss_viewUpdatedToFalse_swipeOccurs_viewNotDismissed() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.drawable.ic_cake, contentDescription = null), + Text.Loaded("title text"), + endItem = ChipbarEndItem.Loading, + allowSwipeToDismiss = true, + ) + ) + val view = getChipbarView() + val callbackCaptor = argumentCaptor<(MotionEvent) -> Unit>() + verify(swipeGestureHandler).addOnGestureDetectedCallback(any(), capture(callbackCaptor)) + + // WHEN the view is updated to not allow swipe-to-dismiss + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.drawable.ic_cake, contentDescription = null), + Text.Loaded("title text"), + endItem = ChipbarEndItem.Loading, + allowSwipeToDismiss = false, + ) + ) + + // THEN the callback is removed + verify(swipeGestureHandler).removeOnGestureDetectedCallback(any()) + + // And WHEN the old callback is invoked + callbackCaptor.value.invoke(MotionEvent.obtain(0L, 0L, 0, 0f, 0f, 0)) + + // THEN it is ignored and view isn't removed + verify(windowManager, never()).removeView(view) + } + private fun createChipbarInfo( startIcon: Icon, text: Text, endItem: ChipbarEndItem?, vibrationEffect: VibrationEffect? = null, + allowSwipeToDismiss: Boolean = false, ): ChipbarInfo { return ChipbarInfo( TintedIcon(startIcon, tintAttr = null), text, endItem, vibrationEffect, + allowSwipeToDismiss, windowTitle = WINDOW_TITLE, wakeReason = WAKE_REASON, timeoutMs = TIMEOUT, diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/FakeChipbarCoordinator.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/FakeChipbarCoordinator.kt index 4ef4e6ca65408..ffac8f6aabe68 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/FakeChipbarCoordinator.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/FakeChipbarCoordinator.kt @@ -43,6 +43,7 @@ class FakeChipbarCoordinator( powerManager: PowerManager, falsingManager: FalsingManager, falsingCollector: FalsingCollector, + swipeChipbarAwayGestureHandler: SwipeChipbarAwayGestureHandler, viewUtil: ViewUtil, vibratorHelper: VibratorHelper, wakeLockBuilder: WakeLock.Builder, @@ -59,6 +60,7 @@ class FakeChipbarCoordinator( powerManager, falsingManager, falsingCollector, + swipeChipbarAwayGestureHandler, viewUtil, vibratorHelper, wakeLockBuilder, diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandlerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandlerTest.kt new file mode 100644 index 0000000000000..a87a95060a7e2 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/SwipeChipbarAwayGestureHandlerTest.kt @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2023 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.temporarydisplay.chipbar + +import android.graphics.Rect +import android.view.MotionEvent +import android.view.View +import androidx.test.filters.SmallTest +import com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer +import com.android.systemui.SysuiTestCase +import com.android.systemui.util.mockito.any +import com.android.systemui.util.mockito.mock +import com.android.systemui.util.mockito.whenever +import com.google.common.truth.Truth.assertThat +import org.junit.Before +import org.junit.Test + +@SmallTest +class SwipeChipbarAwayGestureHandlerTest : SysuiTestCase() { + + private lateinit var underTest: SwipeChipbarAwayGestureHandler + + @Before + fun setUp() { + underTest = SwipeChipbarAwayGestureHandler(context, mock()) + } + + @Test + fun startOfGestureIsWithinBounds_noViewFetcher_returnsFalse() { + assertThat(underTest.startOfGestureIsWithinBounds(createMotionEvent())).isFalse() + } + + @Test + fun startOfGestureIsWithinBounds_usesViewFetcher_aboveBottom_returnsTrue() { + val view = createMockView() + + underTest.setViewFetcher { view } + + val motionEvent = createMotionEvent(y = VIEW_BOTTOM - 100f) + assertThat(underTest.startOfGestureIsWithinBounds(motionEvent)).isTrue() + } + + @Test + fun startOfGestureIsWithinBounds_usesViewFetcher_slightlyBelowBottom_returnsTrue() { + val view = createMockView() + + underTest.setViewFetcher { view } + + val motionEvent = createMotionEvent(y = VIEW_BOTTOM + 20f) + assertThat(underTest.startOfGestureIsWithinBounds(motionEvent)).isTrue() + } + + @Test + fun startOfGestureIsWithinBounds_usesViewFetcher_tooFarDown_returnsFalse() { + val view = createMockView() + + underTest.setViewFetcher { view } + + val motionEvent = createMotionEvent(y = VIEW_BOTTOM * 4f) + assertThat(underTest.startOfGestureIsWithinBounds(motionEvent)).isFalse() + } + + @Test + fun startOfGestureIsWithinBounds_viewFetcherReset_returnsFalse() { + val view = createMockView() + + underTest.setViewFetcher { view } + + val motionEvent = createMotionEvent(y = VIEW_BOTTOM - 100f) + assertThat(underTest.startOfGestureIsWithinBounds(motionEvent)).isTrue() + + underTest.resetViewFetcher() + assertThat(underTest.startOfGestureIsWithinBounds(motionEvent)).isFalse() + } + + private fun createMotionEvent(y: Float = 0f): MotionEvent { + return MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 0f, y, 0) + } + + private fun createMockView(): View { + return mock().also { + doAnswer { invocation -> + val out: Rect = invocation.getArgument(0) + out.set(0, 0, 0, VIEW_BOTTOM) + null + } + .whenever(it) + .getBoundsOnScreen(any()) + } + } + + private companion object { + const val VIEW_BOTTOM = 455 + } +}