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

Commit 6ac893b2 authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Propagate new shade displayId only after display change succeeded

This refactors the meaning of displayId in ShadeDisplaysRepository.

Before: displayId was changing before the shade was moved. So if listeners were listening to it and getting values from the context/resoruces, they would have been wrong

Now: there is a pendingDisplayId, and displayId is only changed after the move succeeded, so listeners to it can expect to have correct resources when it changes.

This will be needed in follow up cls.

Bug: 362719719
Bug: 398011576
Test: ShadeDisplaysRepositoryTest, ShadePrimaryDisplayCommandTest, ShadeDisplaysInteractorTest
Flag: com.android.systemui.shade_window_goes_around
Change-Id: If95ab47471a0914e6b7c2f7959d1c3f76a5deeee
parent e01dcc1b
Loading
Loading
Loading
Loading
+16 −3
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ class ShadeDisplaysRepositoryTest : SysuiTestCase() {
    fun policy_changing_propagatedFromTheLatestPolicy() =
        testScope.runTest {
            val underTest = createUnderTest()
            val displayIds by collectValues(underTest.displayId)
            val displayIds by collectValues(underTest.pendingDisplayId)

            assertThat(displayIds).containsExactly(0)

@@ -98,7 +98,7 @@ class ShadeDisplaysRepositoryTest : SysuiTestCase() {
                DEVELOPMENT_SHADE_DISPLAY_AWARENESS,
                FakeShadeDisplayPolicy.name,
            )
            val displayId by collectLastValue(underTest.displayId)
            val displayId by collectLastValue(underTest.pendingDisplayId)

            displayRepository.addDisplay(displayId = 1)

@@ -161,7 +161,7 @@ class ShadeDisplaysRepositoryTest : SysuiTestCase() {
                FakeShadeDisplayPolicy.name,
            )

            val displayId by collectLastValue(underTest.displayId)
            val displayId by collectLastValue(underTest.pendingDisplayId)

            displayRepository.addDisplays(display(id = 2, type = TYPE_EXTERNAL))
            FakeShadeDisplayPolicy.setDisplayId(2)
@@ -176,4 +176,17 @@ class ShadeDisplaysRepositoryTest : SysuiTestCase() {

            assertThat(displayId).isEqualTo(2)
        }

    @Test
    fun onDisplayChangedSucceeded_displayIdChanges() =
        testScope.runTest {
            val underTest = createUnderTest()
            val displayId by collectLastValue(underTest.displayId)

            assertThat(displayId).isEqualTo(Display.DEFAULT_DISPLAY)

            underTest.onDisplayChangedSucceeded(1)

            assertThat(displayId).isEqualTo(1)
        }
}
+2 −2
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ class ShadePrimaryDisplayCommandTest : SysuiTestCase() {
    @Test
    fun commandShadeDisplayOverride_resetsDisplayId() =
        testScope.runTest {
            val displayId by collectLastValue(shadeDisplaysRepository.displayId)
            val displayId by collectLastValue(shadeDisplaysRepository.pendingDisplayId)
            assertThat(displayId).isEqualTo(Display.DEFAULT_DISPLAY)

            val newDisplayId = 2
@@ -87,7 +87,7 @@ class ShadePrimaryDisplayCommandTest : SysuiTestCase() {
    @Test
    fun commandShadeDisplayOverride_anyExternalDisplay_notOnDefaultAnymore() =
        testScope.runTest {
            val displayId by collectLastValue(shadeDisplaysRepository.displayId)
            val displayId by collectLastValue(shadeDisplaysRepository.pendingDisplayId)
            assertThat(displayId).isEqualTo(Display.DEFAULT_DISPLAY)
            val newDisplayId = 2
            displayRepository.addDisplay(displayId = newDisplayId)
+8 −8
Original line number Diff line number Diff line
@@ -22,7 +22,6 @@ import android.view.Display
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.common.ui.data.repository.configurationRepository
import com.android.systemui.common.ui.data.repository.fakeConfigurationRepository
import com.android.systemui.kosmos.testScope
import com.android.systemui.kosmos.useUnconfinedTestDispatcher
@@ -82,7 +81,7 @@ class ShadeDisplaysInteractorTest : SysuiTestCase() {
    fun start_shadeInCorrectPosition_notAddedOrRemoved() =
        testScope.runTest {
            whenever(display.displayId).thenReturn(0)
            positionRepository.setDisplayId(0)
            positionRepository.setPendingDisplayId(0)

            underTest.start()

@@ -93,18 +92,19 @@ class ShadeDisplaysInteractorTest : SysuiTestCase() {
    fun start_shadeInWrongPosition_changes() =
        testScope.runTest {
            whenever(display.displayId).thenReturn(0)
            positionRepository.setDisplayId(1)
            positionRepository.setPendingDisplayId(1)

            underTest.start()

            verify(shadeContext).reparentToDisplay(eq(1))
            assertThat(positionRepository.displayId.value).isEqualTo(1)
        }

    @Test
    fun start_shadeInWrongPosition_logsStartToLatencyTracker() =
        testScope.runTest {
            whenever(display.displayId).thenReturn(0)
            positionRepository.setDisplayId(1)
            positionRepository.setPendingDisplayId(1)

            underTest.start()

@@ -115,7 +115,7 @@ class ShadeDisplaysInteractorTest : SysuiTestCase() {
    fun start_shadeInWrongPosition_someNotificationsVisible_hiddenThenShown() =
        testScope.runTest {
            whenever(display.displayId).thenReturn(0)
            positionRepository.setDisplayId(1)
            positionRepository.setPendingDisplayId(1)
            activeNotificationRepository.setActiveNotifs(1)

            underTest.start()
@@ -129,7 +129,7 @@ class ShadeDisplaysInteractorTest : SysuiTestCase() {
    fun start_shadeInWrongPosition_someNotificationsVisible_waitsForInflationsBeforeShowingNssl() =
        testScope.runTest {
            whenever(display.displayId).thenReturn(0)
            positionRepository.setDisplayId(1)
            positionRepository.setPendingDisplayId(1)
            activeNotificationRepository.setActiveNotifs(1)

            val endRebinding = notificationRebindingTracker.trackRebinding("test")
@@ -155,7 +155,7 @@ class ShadeDisplaysInteractorTest : SysuiTestCase() {
    fun start_shadeInWrongPosition_noNotifications_nsslNotHidden() =
        testScope.runTest {
            whenever(display.displayId).thenReturn(0)
            positionRepository.setDisplayId(1)
            positionRepository.setPendingDisplayId(1)
            activeNotificationRepository.setActiveNotifs(0)

            underTest.start()
@@ -170,7 +170,7 @@ class ShadeDisplaysInteractorTest : SysuiTestCase() {
    fun start_shadeInWrongPosition_waitsUntilMovedToDisplayReceived() =
        testScope.runTest {
            whenever(display.displayId).thenReturn(0)
            positionRepository.setDisplayId(1)
            positionRepository.setPendingDisplayId(1)
            activeNotificationRepository.setActiveNotifs(1)

            underTest.start()
+13 −1
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Main
import com.android.systemui.res.R
import com.android.systemui.scene.ui.view.WindowRootView
import com.android.systemui.shade.data.repository.MutableShadeDisplaysRepository
import com.android.systemui.shade.data.repository.ShadeDisplaysRepository
import com.android.systemui.shade.data.repository.ShadeDisplaysRepositoryImpl
import com.android.systemui.shade.display.ShadeDisplayPolicyModule
@@ -205,7 +206,18 @@ object ShadeDisplayAwareModule {

    @SysUISingleton
    @Provides
    fun provideShadePositionRepository(impl: ShadeDisplaysRepositoryImpl): ShadeDisplaysRepository {
    fun provideShadePositionRepository(
        impl: MutableShadeDisplaysRepository
    ): ShadeDisplaysRepository {
        ShadeWindowGoesAround.isUnexpectedlyInLegacyMode()
        return impl
    }

    @SysUISingleton
    @Provides
    fun provideMutableShadePositionRepository(
        impl: ShadeDisplaysRepositoryImpl
    ): MutableShadeDisplaysRepository {
        ShadeWindowGoesAround.isUnexpectedlyInLegacyMode()
        return impl
    }
+13 −1
Original line number Diff line number Diff line
@@ -22,16 +22,28 @@ import com.android.systemui.shade.display.ShadeDisplayPolicy
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow

class FakeShadeDisplayRepository : ShadeDisplaysRepository {
class FakeShadeDisplayRepository : MutableShadeDisplaysRepository {
    private val _displayId = MutableStateFlow(Display.DEFAULT_DISPLAY)
    private val _pendingDisplayId = MutableStateFlow(Display.DEFAULT_DISPLAY)

    fun setDisplayId(displayId: Int) {
        _displayId.value = displayId
    }

    fun setPendingDisplayId(displayId: Int) {
        _pendingDisplayId.value = displayId
    }

    override fun onDisplayChangedSucceeded(displayId: Int) {
        setDisplayId(displayId)
    }

    override val displayId: StateFlow<Int>
        get() = _displayId

    override val pendingDisplayId: StateFlow<Int>
        get() = _pendingDisplayId

    override val currentPolicy: ShadeDisplayPolicy
        get() = FakeShadeDisplayPolicy
}
Loading