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

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

Initialize enabledDisplays with the correct displays 

It could have happened that the list of displays was not up to date when DisplayRepository was being initialized.
This makes a binder call to get them from the ui thread once DisplayRepository class is instantiated to ensure correctness immediately (and not eventual, as before)

It would be possible to also have correctness immediately avoiding the binder call using SharedFlows, but we banned them due to performance concerns. The ui thread binder call here happens once per process start, so it shouldn't be that problematic.

Fixes: 355364683
Test: DisplayRepositoryTest
Flag: com.android.systemui.enable_efficient_display_repository
Change-Id: If379d754fa378eaaeddb29fc5f94f5536f0b5c01
parent 06bb60b5

packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayRepository.kt

+10 −3
Original line number Diff line number Diff line
@@ -160,7 +160,10 @@ constructor( 
                    .stateIn(

                        bgApplicationScope,

                        SharingStarted.WhileSubscribed(),

                        emptySet(),

                        // This is necessary because there might be multiple displays, and we could

                        // have missed events for those added before this process or flow started.

                        // Note it causes a binder call from the main thread (it's traced).

                        getDisplays().map { display -> display.displayId }.toSet(),

                    )

            } else {

                oldEnabledDisplays.map { enabledDisplaysSet ->
@@ -186,8 +189,12 @@ constructor( 
                .stateIn(

                    bgApplicationScope,

                    started = SharingStarted.WhileSubscribed(),

                    initialValue = setOf(defaultDisplay)

                )

                    // This triggers a single binder call on the UI thread per process. The

                    // alternative would be to use sharedFlows, but they are prohibited due to

                    // performance concerns.

                    // Ultimately, this is a trade-off between a one-time UI thread binder call and

                    // the constant overhead of sharedFlows.

                    initialValue = getDisplays())

        } else {

            oldEnabledDisplays

        }

packages/SystemUI/tests/src/com/android/systemui/display/data/repository/DisplayRepositoryTest.kt

+13 −0
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import androidx.test.filters.SmallTest 
import com.android.systemui.SysuiTestCase

import com.android.systemui.coroutines.FlowValue

import com.android.systemui.coroutines.collectLastValue

import com.android.systemui.coroutines.collectValues

import com.android.systemui.util.mockito.kotlinArgumentCaptor

import com.android.systemui.util.mockito.mock

import com.android.systemui.util.mockito.whenever
@@ -456,8 +457,20 @@ class DisplayRepositoryTest : SysuiTestCase() { 
            assertThat(value?.ids()).containsExactly(DEFAULT_DISPLAY)

        }



    @Test

    fun displayFlow_emitsCorrectDisplaysAtFirst() =

        testScope.runTest {

            setDisplays(0, 1, 2)



            val values: List<Set<Display>> by collectValues(displayRepository.displays)



            assertThat(values.toIdSets()).containsExactly(setOf(0, 1, 2))

        }



    private fun Iterable<Display>.ids(): List<Int> = map { it.displayId }



    private fun Iterable<Set<Display>>.toIdSets(): List<Set<Int>> = map { it.ids().toSet() }



    // Wrapper to capture the displayListener.

    private fun TestScope.latestDisplayFlowValue(): FlowValue<Set<Display>?> {

        val flowValue = collectLastValue(displayRepository.displays)