Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cc32b4c1 authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Expand correct portion of the shade after moving it to a different display 

This fixes the case when one shade element is visible (e.g. QS or notifiactions), but the user swipes down another one in the external display.

Before, the same that was being collapsed was expanded. After, the correct one (based on the status bar touch) is expanded

Bug: 362719719
Bug: 389017437
Test: StatusBarTouchShadeDisplayPolicyTest, ShadeExpandedStateInteractorTest, PhoneStatusBarViewControllerTest
Flag: com.android.systemui.shade_window_goes_around
Change-Id: Ia7b3ab680a71e16bbe3dfe0d35773195342fa5b0
parent 5e1e59ac

packages/SystemUI/multivalentTests/src/com/android/systemui/shade/display/StatusBarTouchShadeDisplayPolicyTest.kt

+65 −5
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.systemui.shade.display 


import android.view.Display

import android.view.Display.TYPE_EXTERNAL

import android.view.MotionEvent

import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase
@@ -28,11 +29,16 @@ import com.android.systemui.display.data.repository.displayRepository 
import com.android.systemui.keyguard.data.repository.fakeKeyguardRepository

import com.android.systemui.kosmos.testScope

import com.android.systemui.kosmos.useUnconfinedTestDispatcher

import com.android.systemui.shade.domain.interactor.notificationElement

import com.android.systemui.shade.domain.interactor.qsElement

import com.android.systemui.shade.domain.interactor.shadeInteractor

import com.android.systemui.testKosmos

import com.google.common.truth.Truth.assertThat

import kotlin.test.Test

import kotlinx.coroutines.test.runTest

import org.junit.runner.RunWith

import org.mockito.kotlin.doReturn

import org.mockito.kotlin.mock



@SmallTest

@RunWith(AndroidJUnit4::class)
@@ -50,9 +56,19 @@ class StatusBarTouchShadeDisplayPolicyTest : SysuiTestCase() { 
            keyguardRepository,

            testScope.backgroundScope,

            shadeOnDefaultDisplayWhenLocked = shadeOnDefaultDisplayWhenLocked,

            shadeInteractor = { kosmos.shadeInteractor },

            { kosmos.qsElement },

            { kosmos.notificationElement },

        )

    }



    private fun createMotionEventForDisplay(displayId: Int, xCoordinate: Float = 0f): MotionEvent {

        return mock<MotionEvent> {

            on { getX() } doReturn xCoordinate

            on { getDisplayId() } doReturn displayId

        }

    }



    @Test

    fun displayId_defaultToDefaultDisplay() {

        val underTest = createUnderTest()
@@ -67,7 +83,7 @@ class StatusBarTouchShadeDisplayPolicyTest : SysuiTestCase() { 
            val displayId by collectLastValue(underTest.displayId)



            displayRepository.addDisplays(display(id = 2, type = TYPE_EXTERNAL))

            underTest.onStatusBarTouched(2)

            underTest.onStatusBarTouched(createMotionEventForDisplay(2), STATUS_BAR_WIDTH)



            assertThat(displayId).isEqualTo(2)

        }
@@ -79,7 +95,7 @@ class StatusBarTouchShadeDisplayPolicyTest : SysuiTestCase() { 
            val displayIds by collectValues(underTest.displayId)

            assertThat(displayIds).isEqualTo(listOf(Display.DEFAULT_DISPLAY))



            underTest.onStatusBarTouched(2)

            underTest.onStatusBarTouched(createMotionEventForDisplay(2), STATUS_BAR_WIDTH)



            // Never set, as 2 was not a display according to the repository.

            assertThat(displayIds).isEqualTo(listOf(Display.DEFAULT_DISPLAY))
@@ -92,7 +108,7 @@ class StatusBarTouchShadeDisplayPolicyTest : SysuiTestCase() { 
            val displayId by collectLastValue(underTest.displayId)



            displayRepository.addDisplays(display(id = 2, type = TYPE_EXTERNAL))

            underTest.onStatusBarTouched(2)

            underTest.onStatusBarTouched(createMotionEventForDisplay(2), STATUS_BAR_WIDTH)



            assertThat(displayId).isEqualTo(2)
@@ -108,7 +124,7 @@ class StatusBarTouchShadeDisplayPolicyTest : SysuiTestCase() { 
            val displayId by collectLastValue(underTest.displayId)



            displayRepository.addDisplays(display(id = 2, type = TYPE_EXTERNAL))

            underTest.onStatusBarTouched(2)

            underTest.onStatusBarTouched(createMotionEventForDisplay(2), STATUS_BAR_WIDTH)



            assertThat(displayId).isEqualTo(2)
@@ -124,7 +140,7 @@ class StatusBarTouchShadeDisplayPolicyTest : SysuiTestCase() { 
            val displayId by collectLastValue(underTest.displayId)



            displayRepository.addDisplays(display(id = 2, type = TYPE_EXTERNAL))

            underTest.onStatusBarTouched(2)

            underTest.onStatusBarTouched(createMotionEventForDisplay(2), STATUS_BAR_WIDTH)



            assertThat(displayId).isEqualTo(2)
@@ -136,4 +152,48 @@ class StatusBarTouchShadeDisplayPolicyTest : SysuiTestCase() { 


            assertThat(displayId).isEqualTo(2)

        }



    @Test

    fun onStatusBarTouched_leftSide_intentSetToNotifications() =

        testScope.runTest {

            val underTest = createUnderTest(shadeOnDefaultDisplayWhenLocked = true)



            underTest.onStatusBarTouched(

                createMotionEventForDisplay(2, STATUS_BAR_WIDTH * 0.1f),

                STATUS_BAR_WIDTH,

            )



            assertThat(underTest.consumeExpansionIntent()).isEqualTo(kosmos.notificationElement)

        }



    @Test

    fun onStatusBarTouched_rightSide_intentSetToQs() =

        testScope.runTest {

            val underTest = createUnderTest(shadeOnDefaultDisplayWhenLocked = true)



            underTest.onStatusBarTouched(

                createMotionEventForDisplay(2, STATUS_BAR_WIDTH * 0.95f),

                STATUS_BAR_WIDTH,

            )



            assertThat(underTest.consumeExpansionIntent()).isEqualTo(kosmos.qsElement)

        }



    @Test

    fun onStatusBarTouched_nullAfterConsumed() =

        testScope.runTest {

            val underTest = createUnderTest(shadeOnDefaultDisplayWhenLocked = true)



            underTest.onStatusBarTouched(

                createMotionEventForDisplay(2, STATUS_BAR_WIDTH * 0.1f),

                STATUS_BAR_WIDTH,

            )

            assertThat(underTest.consumeExpansionIntent()).isEqualTo(kosmos.notificationElement)



            assertThat(underTest.consumeExpansionIntent()).isNull()

        }



    companion object {

        private const val STATUS_BAR_WIDTH = 100

    }

}

packages/SystemUI/multivalentTests/src/com/android/systemui/shade/domain/interactor/ShadeExpandedStateInteractorTest.kt

+2 −4
Original line number Diff line number Diff line
@@ -22,8 +22,6 @@ import com.android.systemui.SysuiTestCase 
import com.android.systemui.flags.EnableSceneContainer

import com.android.systemui.kosmos.testScope

import com.android.systemui.kosmos.useUnconfinedTestDispatcher

import com.android.systemui.shade.domain.interactor.ShadeExpandedStateInteractorImpl.NotificationElement

import com.android.systemui.shade.domain.interactor.ShadeExpandedStateInteractorImpl.QSElement

import com.android.systemui.shade.shadeTestUtil

import com.android.systemui.testKosmos

import com.google.common.truth.Truth.assertThat
@@ -52,7 +50,7 @@ class ShadeExpandedStateInteractorTest : SysuiTestCase() { 


            val element = currentlyExpandedElement.value



            assertThat(element).isInstanceOf(QSElement::class.java)

            assertThat(element).isInstanceOf(QSShadeElement::class.java)

        }



    @Test
@@ -62,7 +60,7 @@ class ShadeExpandedStateInteractorTest : SysuiTestCase() { 


            val element = underTest.currentlyExpandedElement.value



            assertThat(element).isInstanceOf(NotificationElement::class.java)

            assertThat(element).isInstanceOf(NotificationShadeElement::class.java)

        }



    @Test

packages/SystemUI/src/com/android/systemui/shade/display/ShadeDisplayPolicy.kt

+23 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.systemui.shade.display



import com.android.systemui.shade.domain.interactor.ShadeExpandedStateInteractor.ShadeElement

import dagger.Binds

import dagger.Module

import dagger.multibindings.IntoSet
@@ -33,11 +34,33 @@ interface ShadeDisplayPolicy { 
    val displayId: StateFlow<Int>

}



/** Return the latest element the user intended to expand in the shade (notifications or QS). */

interface ShadeExpansionIntent {

    /**

     * Returns the latest element the user intended to expand in the shade (notifications or QS).

     *

     * When the shade moves to a different display (e.g., due to a touch on the status bar of an

     * external display), it's first collapsed and then re-expanded on the target display.

     *

     * If the user was trying to open a specific element (QS or notifications) when the shade was on

     * the original display, that intention might be lost during the collapse/re-expand transition.

     * This is used to preserve the user's intention, ensuring the correct element is expanded on

     * the target display.

     *

     * Note that the expansion intent is kept for a very short amount of time (ideally, just a bit

     * above the time it takes for the shade to collapse)

     */

    fun consumeExpansionIntent(): ShadeElement?

}



@Module

interface ShadeDisplayPolicyModule {



    @Binds fun provideDefaultPolicy(impl: StatusBarTouchShadeDisplayPolicy): ShadeDisplayPolicy



    @Binds

    fun provideShadeExpansionIntent(impl: StatusBarTouchShadeDisplayPolicy): ShadeExpansionIntent



    @IntoSet

    @Binds

    fun provideDefaultDisplayPolicyToSet(impl: DefaultDisplayShadePolicy): ShadeDisplayPolicy

packages/SystemUI/src/com/android/systemui/shade/display/StatusBarTouchShadeDisplayPolicy.kt

+52 −4
Original line number Diff line number Diff line
@@ -18,16 +18,25 @@ package com.android.systemui.shade.display 


import android.util.Log

import android.view.Display

import android.view.MotionEvent

import com.android.app.tracing.coroutines.launchTraced

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.display.data.repository.DisplayRepository

import com.android.systemui.keyguard.data.repository.KeyguardRepository

import com.android.systemui.shade.ShadeOnDefaultDisplayWhenLocked

import com.android.systemui.shade.domain.interactor.NotificationShadeElement

import com.android.systemui.shade.domain.interactor.QSShadeElement

import com.android.systemui.shade.domain.interactor.ShadeExpandedStateInteractor.ShadeElement

import com.android.systemui.shade.domain.interactor.ShadeInteractor

import com.android.systemui.shade.shared.flag.ShadeWindowGoesAround

import dagger.Lazy

import java.util.concurrent.atomic.AtomicReference

import javax.inject.Inject

import kotlin.time.Duration.Companion.seconds

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.Job

import kotlinx.coroutines.delay

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.SharingStarted

import kotlinx.coroutines.flow.StateFlow
@@ -49,14 +58,20 @@ class StatusBarTouchShadeDisplayPolicy 
constructor(

    displayRepository: DisplayRepository,

    keyguardRepository: KeyguardRepository,

    @Background val backgroundScope: CoroutineScope,

    @ShadeOnDefaultDisplayWhenLocked val shadeOnDefaultDisplayWhenLocked: Boolean,

) : ShadeDisplayPolicy {

    @Background private val backgroundScope: CoroutineScope,

    @ShadeOnDefaultDisplayWhenLocked private val shadeOnDefaultDisplayWhenLocked: Boolean,

    private val shadeInteractor: Lazy<ShadeInteractor>,

    private val qsShadeElement: Lazy<QSShadeElement>,

    private val notificationElement: Lazy<NotificationShadeElement>,

) : ShadeDisplayPolicy, ShadeExpansionIntent {

    override val name: String = "status_bar_latest_touch"



    private val currentDisplayId = MutableStateFlow(Display.DEFAULT_DISPLAY)

    private val availableDisplayIds: StateFlow<Set<Int>> = displayRepository.displayIds



    private var latestIntent = AtomicReference<ShadeElement?>()

    private var timeoutJob: Job? = null



    override val displayId: StateFlow<Int> =

        if (shadeOnDefaultDisplayWhenLocked) {

            keyguardRepository.isKeyguardShowing
@@ -75,8 +90,29 @@ constructor( 
    private var removalListener: Job? = null



    /** Called when the status bar on the given display is touched. */

    fun onStatusBarTouched(statusBarDisplayId: Int) {

    fun onStatusBarTouched(event: MotionEvent, statusBarWidth: Int) {

        ShadeWindowGoesAround.isUnexpectedlyInLegacyMode()

        updateShadeDisplayIfNeeded(event)

        updateExpansionIntent(event, statusBarWidth)

    }



    override fun consumeExpansionIntent(): ShadeElement? {

        return latestIntent.getAndSet(null)

    }



    private fun updateExpansionIntent(event: MotionEvent, statusBarWidth: Int) {

        val element = classifyStatusBarEvent(event, statusBarWidth)

        latestIntent.set(element)

        timeoutJob?.cancel()

        timeoutJob =

            backgroundScope.launchTraced("StatusBarTouchDisplayPolicy#intentTimeout") {

                delay(EXPANSION_INTENT_EXPIRY)

                latestIntent.set(null)

            }

    }



    private fun updateShadeDisplayIfNeeded(event: MotionEvent) {

        val statusBarDisplayId = event.displayId

        if (statusBarDisplayId !in availableDisplayIds.value) {

            Log.e(TAG, "Got touch on unknown display $statusBarDisplayId")

            return
@@ -90,6 +126,17 @@ constructor( 
        }

    }



    private fun classifyStatusBarEvent(

        motionEvent: MotionEvent,

        statusbarWidth: Int,

    ): ShadeElement {

        val xPercentage = motionEvent.x / statusbarWidth

        val threshold = shadeInteractor.get().getTopEdgeSplitFraction()

        return if (xPercentage < threshold) {

            notificationElement.get()

        } else qsShadeElement.get()

    }



    private fun monitorDisplayRemovals(): Job {

        return backgroundScope.launchTraced("StatusBarTouchDisplayPolicy#monitorDisplayRemovals") {

            currentDisplayId.subscriptionCount
@@ -112,5 +159,6 @@ constructor( 


    private companion object {

        const val TAG = "StatusBarTouchDisplayPolicy"

        val EXPANSION_INTENT_EXPIRY = 2.seconds

    }

}

packages/SystemUI/src/com/android/systemui/shade/domain/interactor/ShadeDisplaysInteractor.kt

+16 −4
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import com.android.systemui.shade.ShadeDisplayChangeLatencyTracker 
import com.android.systemui.shade.ShadeTraceLogger.logMoveShadeWindowTo

import com.android.systemui.shade.ShadeTraceLogger.traceReparenting

import com.android.systemui.shade.data.repository.ShadeDisplaysRepository

import com.android.systemui.shade.display.ShadeExpansionIntent

import com.android.systemui.shade.shared.flag.ShadeWindowGoesAround

import com.android.window.flags.Flags

import java.util.Optional
@@ -49,6 +50,7 @@ constructor( 
    @Main private val mainThreadContext: CoroutineContext,

    private val shadeDisplayChangeLatencyTracker: ShadeDisplayChangeLatencyTracker,

    shadeExpandedInteractor: Optional<ShadeExpandedStateInteractor>,

    private val shadeExpansionIntent: ShadeExpansionIntent,

) : CoreStartable {



    private val shadeExpandedInteractor =
@@ -90,10 +92,7 @@ constructor( 
            withContext(mainThreadContext) {

                traceReparenting {

                    shadeDisplayChangeLatencyTracker.onShadeDisplayChanging(destinationId)

                    val expandedElement = shadeExpandedInteractor.currentlyExpandedElement.value

                    expandedElement?.collapse(reason = "Shade window move")

                    reparentToDisplayId(id = destinationId)

                    expandedElement?.expand(reason = "Shade window move")

                    collapseAndExpandShadeIfNeeded { reparentToDisplayId(id = destinationId) }

                    checkContextDisplayMatchesExpected(destinationId)

                }

            }
@@ -106,6 +105,18 @@ constructor( 
        }

    }



    private suspend fun collapseAndExpandShadeIfNeeded(wrapped: () -> Unit) {

        val previouslyExpandedElement = shadeExpandedInteractor.currentlyExpandedElement.value

        previouslyExpandedElement?.collapse(reason = COLLAPSE_EXPAND_REASON)



        wrapped()



        // If the user was trying to expand a specific shade element, let's make sure to expand

        // that one. Otherwise, we can just re-expand the previous expanded element.

        shadeExpansionIntent.consumeExpansionIntent()?.expand(COLLAPSE_EXPAND_REASON)

            ?: previouslyExpandedElement?.expand(reason = COLLAPSE_EXPAND_REASON)

    }



    private fun checkContextDisplayMatchesExpected(destinationId: Int) {

        if (shadeContext.displayId != destinationId) {

            Log.wtf(
@@ -125,5 +136,6 @@ constructor( 


    private companion object {

        const val TAG = "ShadeDisplaysInteractor"

        const val COLLAPSE_EXPAND_REASON = "Shade window move"

    }

}