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

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

Ensure connected display ids in sysui are correct on boot 

This makes the initial list of connected displays include all the ones connected already.

In this way, even after boot or sysui restarts the internal state of DisplayRepository is always coherent.

+ Fix display manager call on object construction: now getDisplays is getting called only if anyone subscribed to any flow.

+ Execute registration for display events on a background coroutine dispatcher

Bug: 298023961
Test: DisplayRepositoryTest
Change-Id: I4354314e6adcbc8b2b58e52959328d6a4a93aa69
parent 72dbcc03

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

+48 −21
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.systemui.display.data.repository



import android.hardware.display.DisplayManager

import android.hardware.display.DisplayManager.DISPLAY_CATEGORY_ALL_INCLUDING_DISABLED

import android.hardware.display.DisplayManager.DisplayListener

import android.hardware.display.DisplayManager.EVENT_FLAG_DISPLAY_ADDED

import android.hardware.display.DisplayManager.EVENT_FLAG_DISPLAY_CHANGED
@@ -95,7 +96,8 @@ constructor( 
    @Background backgroundCoroutineDispatcher: CoroutineDispatcher

) : DisplayRepository {

    // Displays are enabled only after receiving them in [onDisplayAdded]

    private val allDisplayEvents: Flow<DisplayEvent> = conflatedCallbackFlow {

    private val allDisplayEvents: Flow<DisplayEvent> =

        conflatedCallbackFlow {

                val callback =

                    object : DisplayListener {

                        override fun onDisplayAdded(displayId: Int) {
@@ -110,13 +112,20 @@ constructor( 
                            trySend(DisplayEvent.Changed(displayId))

                        }

                    }

                // Triggers an initial event when subscribed. This is needed to avoid getDisplays to

                // be called when this class is constructed, but only when someone subscribes to

                // this flow.

                trySend(DisplayEvent.Changed(Display.DEFAULT_DISPLAY))

                displayManager.registerDisplayListener(

                    callback,

                    backgroundHandler,

            EVENT_FLAG_DISPLAY_ADDED or EVENT_FLAG_DISPLAY_CHANGED or EVENT_FLAG_DISPLAY_REMOVED,

                    EVENT_FLAG_DISPLAY_ADDED or

                        EVENT_FLAG_DISPLAY_CHANGED or

                        EVENT_FLAG_DISPLAY_REMOVED,

                )

                awaitClose { displayManager.unregisterDisplayListener(callback) }

            }

            .flowOn(backgroundCoroutineDispatcher)



    override val displayChangeEvent: Flow<Int> =

        allDisplayEvents.filter { it is DisplayEvent.Changed }.map { it.displayId }
@@ -128,7 +137,9 @@ constructor( 
            .stateIn(

                applicationScope,

                started = SharingStarted.WhileSubscribed(),

                initialValue = getDisplays()

                // To avoid getting displays on this object construction, they are get after the

                // first event. allDisplayEvents emits a changed event when we subscribe to it.

                initialValue = emptySet()

            )



    private fun getDisplays(): Set<Display> =
@@ -146,12 +157,23 @@ constructor( 


    private val ignoredDisplayIds = MutableStateFlow<Set<Int>>(emptySet())



    private fun getInitialConnectedDisplays(): Set<Int> =

        displayManager

            .getDisplays(DISPLAY_CATEGORY_ALL_INCLUDING_DISABLED)

            .map { it.displayId }

            .toSet()

            .also {

                if (DEBUG) {

                    Log.d(TAG, "getInitialConnectedDisplays: $it")

                }

            }



    /* keeps connected displays until they are disconnected. */

    private val connectedDisplayIds: StateFlow<Set<Int>> =

        conflatedCallbackFlow {

                val connectedIds = getInitialConnectedDisplays().toMutableSet()

                val callback =

                    object : DisplayConnectionListener {

                        private val connectedIds = mutableSetOf<Int>()

                        override fun onDisplayConnected(id: Int) {

                            if (DEBUG) {

                                Log.d(TAG, "display with id=$id connected.")
@@ -170,6 +192,7 @@ constructor( 
                            trySend(connectedIds.toSet())

                        }

                    }

                trySend(connectedIds.toSet())

                displayManager.registerDisplayListener(

                    callback,

                    backgroundHandler,
@@ -183,6 +206,10 @@ constructor( 
            .stateIn(

                applicationScope,

                started = SharingStarted.WhileSubscribed(),

                // The initial value is set to empty, but connected displays are gathered as soon as

                // the flow starts being collected. This is to ensure the call to get displays (an

                // IPC) happens in the background instead of when this object

                // is instantiated.

                initialValue = emptySet()

            )

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

+37 −8
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import android.os.Looper 
import android.testing.AndroidTestingRunner

import android.testing.TestableLooper

import android.view.Display

import android.view.Display.TYPE_EXTERNAL

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.android.systemui.coroutines.FlowValue
@@ -62,6 +63,7 @@ class DisplayRepositoryTest : SysuiTestCase() { 
    @Before

    fun setup() {

        setDisplays(emptyList())

        setAllDisplaysIncludingDisabled()

        displayRepository =

            DisplayRepositoryImpl(

                displayManager,
@@ -70,6 +72,7 @@ class DisplayRepositoryTest : SysuiTestCase() { 
                UnconfinedTestDispatcher()

            )

        verify(displayManager, never()).registerDisplayListener(any(), any())

        verify(displayManager, never()).getDisplays(any())

    }



    @Test
@@ -350,6 +353,22 @@ class DisplayRepositoryTest : SysuiTestCase() { 
            assertThat(pendingDisplay).isNotNull()

        }



    @Test

    fun initialState_onePendingDisplayOnBoot_notNull() =

        testScope.runTest {

            // 1 is not enabled, but just connected. It should be seen as pending

            setAllDisplaysIncludingDisabled(0, 1)

            setDisplays(0) // 0 is enabled.

            verify(displayManager, never()).getDisplays(any())



            val pendingDisplay by collectLastValue(displayRepository.pendingDisplay)



            verify(displayManager).getDisplays(any())



            assertThat(pendingDisplay).isNotNull()

            assertThat(pendingDisplay!!.id).isEqualTo(1)

        }



    @Test

    fun onPendingDisplay_internalDisplay_ignored() =

        testScope.runTest {
@@ -365,7 +384,7 @@ class DisplayRepositoryTest : SysuiTestCase() { 
        testScope.runTest {

            val pendingDisplay by lastPendingDisplay()



            sendOnDisplayConnected(1, Display.TYPE_EXTERNAL)

            sendOnDisplayConnected(1, TYPE_EXTERNAL)

            sendOnDisplayConnected(2, Display.TYPE_INTERNAL)



            assertThat(pendingDisplay!!.id).isEqualTo(1)
@@ -416,7 +435,7 @@ class DisplayRepositoryTest : SysuiTestCase() { 
        whenever(displayManager.getDisplay(eq(id))).thenReturn(null)

    }



    private fun sendOnDisplayConnected(id: Int, displayType: Int = Display.TYPE_EXTERNAL) {

    private fun sendOnDisplayConnected(id: Int, displayType: Int = TYPE_EXTERNAL) {

        val mockDisplay = display(id = id, type = displayType)

        whenever(displayManager.getDisplay(eq(id))).thenReturn(mockDisplay)

        connectedDisplayListener.value.onDisplayConnected(id)
@@ -424,15 +443,25 @@ class DisplayRepositoryTest : SysuiTestCase() { 


    private fun setDisplays(displays: List<Display>) {

        whenever(displayManager.displays).thenReturn(displays.toTypedArray())

        displays.forEach { display ->

            whenever(displayManager.getDisplay(eq(display.displayId))).thenReturn(display)

        }



    private fun setDisplays(vararg ids: Int) {

        setDisplays(ids.map { display(it) })

    }



    private fun display(id: Int): Display {

        return mock<Display>().also { mockDisplay ->

            whenever(mockDisplay.displayId).thenReturn(id)

    private fun setAllDisplaysIncludingDisabled(vararg ids: Int) {

        val displays = ids.map { display(type = TYPE_EXTERNAL, id = it) }.toTypedArray()

        whenever(

                displayManager.getDisplays(

                    eq(DisplayManager.DISPLAY_CATEGORY_ALL_INCLUDING_DISABLED)

                )

            )

            .thenReturn(displays)

        displays.forEach { display ->

            whenever(displayManager.getDisplay(eq(display.displayId))).thenReturn(display)

        }

    }



    private fun setDisplays(vararg ids: Int) {

        setDisplays(ids.map { display(type = TYPE_EXTERNAL, id = it) })

    }

}