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

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

Fix recursive loop in DisplayComponent lifecycle listeners 

DisplayComponentInstanceProviderTest was calling the lifecycle "start" method during the instance creation, but the start method was trying to access the instance itself, causing a recursive loop.

This adds another callback to initialize the instance after it is created and cached.

Bug: 433203587
Flag: NONE - bugfix
Test: PerDisplayInstanceRepositoryImplTest, DisplayComponentInstanceProviderTest
Change-Id: I44859166c8f22ff63459236efc16c3ef4fb48d0a
parent 73a41ca0

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

+15 −1
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import com.android.systemui.testKosmos 
import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.kotlin.mock

import org.mockito.kotlin.never

import org.mockito.kotlin.verify



@RunWith(AndroidJUnit4::class)
@@ -36,7 +37,7 @@ class DisplayComponentInstanceProviderTest : SysuiTestCase() { 
        DisplayComponentInstanceProvider(kosmos.fakeSysuiDisplayComponentFactory)



    @Test

    fun createInstance_notifiesLifecycleListenersWithStart() {

    fun createInstance_doesNotNotifyLifecycleListenersWithStart() {

        val lifecycleListeners =

            (1..10).map { mock<SystemUIDisplaySubcomponent.LifecycleListener>() }
@@ -44,6 +45,19 @@ class DisplayComponentInstanceProviderTest : SysuiTestCase() { 


        underTest.createInstance(displayId = 123)



        lifecycleListeners.forEach { verify(it, never()).start() }

    }



    @Test

    fun setupInstance_notifiesLifecycleListenersWithSetupInstance() {

        val lifecycleListeners =

            (1..10).map { mock<SystemUIDisplaySubcomponent.LifecycleListener>() }



        kosmos.sysUiDefaultDisplaySubcomponentLifecycleListeners += lifecycleListeners



        val instance = underTest.createInstance(displayId = 123)!!

        underTest.setupInstance(instance)



        lifecycleListeners.forEach { verify(it).start() }

    }

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

+52 −1
Original line number Diff line number Diff line
@@ -19,9 +19,11 @@ package com.android.systemui.display.data.repository 
import android.view.Display

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

import androidx.test.filters.SmallTest

import com.android.app.displaylib.PerDisplayInstanceProviderWithSetup

import com.android.app.displaylib.PerDisplayInstanceRepositoryImpl

import com.android.systemui.SysuiTestCase

import com.android.systemui.dump.dumpManager

import com.android.systemui.kosmos.runTest

import com.android.systemui.kosmos.testScope

import com.android.systemui.kosmos.useUnconfinedTestDispatcher

import com.android.systemui.testKosmos
@@ -43,7 +45,7 @@ class PerDisplayInstanceRepositoryImplTest : SysuiTestCase() { 
    private val testScope = kosmos.testScope

    private val fakeDisplayRepository = kosmos.displayRepository

    private val fakePerDisplayInstanceProviderWithTeardown =

        kosmos.fakePerDisplayInstanceProviderWithTeardown

        kosmos.fakePerDisplayInstanceProviderWithSetupAndTeardown

    private val lifecycleManager = kosmos.fakeDisplayInstanceLifecycleManager



    private val underTest: PerDisplayInstanceRepositoryImpl<TestPerDisplayInstance> =
@@ -86,6 +88,26 @@ class PerDisplayInstanceRepositoryImplTest : SysuiTestCase() { 
    fun forDisplay_nonExistingDisplayId_returnsNull() =

        testScope.runTest { assertThat(underTest[NON_EXISTING_DISPLAY_ID]).isNull() }



    @Test

    fun forDisplay_afterDisplayCreated_setupInvoked() =

        testScope.runTest {

            val instance = underTest[NON_DEFAULT_DISPLAY_ID]



            assertThat(fakePerDisplayInstanceProviderWithTeardown.created).containsExactly(instance)

        }



    @Test

    fun forDisplay_calledMultipleTimes_setupInvokedOnce() =

        testScope.runTest {

            val instance = underTest[NON_DEFAULT_DISPLAY_ID]



            assertThat(fakePerDisplayInstanceProviderWithTeardown.created).containsExactly(instance)



            val instanceAgain = underTest[NON_DEFAULT_DISPLAY_ID]



            assertThat(fakePerDisplayInstanceProviderWithTeardown.created).containsExactly(instance)

        }



    @Test

    fun forDisplay_afterDisplayRemoved_destroyInstanceInvoked() =

        testScope.runTest {
@@ -208,6 +230,35 @@ class PerDisplayInstanceRepositoryImplTest : SysuiTestCase() { 
            assertThat(instance).isSameInstanceAs(defaultInstance)

        }



    @Test

    fun setupInstance_instanceAvailableFromRepositoryDuringSetup() =

        kosmos.runTest {

            lateinit var perDisplayRepo: PerDisplayInstanceRepositoryImpl<TestPerDisplayInstance>

            var instanceSetUp: TestPerDisplayInstance? = null

            perDisplayRepo =

                PerDisplayInstanceRepositoryImpl(

                    debugName = "fakePerDisplayInstanceRepository",

                    instanceProvider =

                        object : PerDisplayInstanceProviderWithSetup<TestPerDisplayInstance> {

                            override fun setupInstance(instance: TestPerDisplayInstance) {

                                assertThat(perDisplayRepo[NON_DEFAULT_DISPLAY_ID])

                                    .isEqualTo(instance)

                                instanceSetUp = instance

                            }



                            override fun createInstance(displayId: Int): TestPerDisplayInstance? {

                                return TestPerDisplayInstance(displayId)

                            }

                        },

                    lifecycleManager = null,

                    testScope.backgroundScope,

                    displayRepository,

                    perDisplayDumpHelper,

                )



            assertThat(perDisplayRepo[NON_DEFAULT_DISPLAY_ID]).isEqualTo(instanceSetUp)

        }



    private fun createDisplay(displayId: Int): Display =

        display(type = Display.TYPE_INTERNAL, id = displayId)

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

+27 −11
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.systemui.display.data.repository



import android.util.Log

import com.android.app.displaylib.PerDisplayInstanceProviderWithSetup

import com.android.app.displaylib.PerDisplayInstanceProviderWithTeardown

import com.android.app.displaylib.PerDisplayInstanceRepositoryImpl

import com.android.app.displaylib.PerDisplayRepository
@@ -32,18 +34,20 @@ import kotlinx.coroutines.cancel 
class DisplayComponentInstanceProvider

@Inject

constructor(private val componentFactory: SystemUIDisplaySubcomponent.Factory) :

    PerDisplayInstanceProviderWithTeardown<SystemUIDisplaySubcomponent> {

    PerDisplayInstanceProviderWithTeardown<SystemUIDisplaySubcomponent>,

    PerDisplayInstanceProviderWithSetup<SystemUIDisplaySubcomponent> {



    override fun createInstance(displayId: Int): SystemUIDisplaySubcomponent? =

        runCatching {

                componentFactory.create(displayId).also { subComponent ->

                    subComponent.lifecycleListeners.forEachTraced(

                        "Notifying listeners of a display component creation"

                    ) {

                        it.start()

                    }

                }

        try {

            componentFactory.create(displayId)

        } catch (e: Exception) {

            Log.e(

                TAG,

                "DisplayComponentInstanceProvider cannot create instance for display $displayId",

                e,

            )

            null

        }

            .getOrNull()



    override fun destroyInstance(instance: SystemUIDisplaySubcomponent) {

        traceSection("Destroying a display component instance") {
@@ -55,6 +59,18 @@ constructor(private val componentFactory: SystemUIDisplaySubcomponent.Factory) : 
            it.stop()

        }

    }



    override fun setupInstance(instance: SystemUIDisplaySubcomponent) {

        instance.lifecycleListeners.forEachTraced(

            "Notifying listeners of a display component creation"

        ) {

            it.start()

        }

    }



    companion object {

        private const val TAG = "DisplayComponentInstanceProvider"

    }

}



@Module

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

+12 −5
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.systemui.display.data.repository 


import com.android.app.displaylib.DisplayInstanceLifecycleManager

import com.android.app.displaylib.FakeDisplayInstanceLifecycleManager

import com.android.app.displaylib.PerDisplayInstanceProviderWithSetup

import com.android.app.displaylib.PerDisplayInstanceProviderWithTeardown

import com.android.app.displaylib.PerDisplayInstanceRepositoryImpl

import com.android.systemui.dump.dumpManager
@@ -54,21 +55,27 @@ val Kosmos.fakePerDisplayStore by 
        )

    }



class FakePerDisplayInstanceProviderWithTeardown :

    PerDisplayInstanceProviderWithTeardown<TestPerDisplayInstance> {

class FakePerDisplayInstanceProviderWithSetupAndTeardown :

    PerDisplayInstanceProviderWithTeardown<TestPerDisplayInstance>,

    PerDisplayInstanceProviderWithSetup<TestPerDisplayInstance> {

    val destroyed = mutableListOf<TestPerDisplayInstance>()

    val created = mutableListOf<TestPerDisplayInstance>()



    override fun destroyInstance(instance: TestPerDisplayInstance) {

        destroyed += instance

    }



    override fun setupInstance(instance: TestPerDisplayInstance) {

        created += instance

    }



    override fun createInstance(displayId: Int): TestPerDisplayInstance? {

        return TestPerDisplayInstance(displayId)

    }

}



val Kosmos.fakePerDisplayInstanceProviderWithTeardown by

    Kosmos.Fixture { FakePerDisplayInstanceProviderWithTeardown() }

val Kosmos.fakePerDisplayInstanceProviderWithSetupAndTeardown by

    Kosmos.Fixture { FakePerDisplayInstanceProviderWithSetupAndTeardown() }



val Kosmos.perDisplayDumpHelper by Kosmos.Fixture { PerDisplayRepoDumpHelper(dumpManager) }

val Kosmos.fakeDisplayInstanceLifecycleManager by
@@ -79,7 +86,7 @@ val Kosmos.fakePerDisplayInstanceRepository by 
        { lifecycleManager: DisplayInstanceLifecycleManager? ->

            PerDisplayInstanceRepositoryImpl(

                debugName = "fakePerDisplayInstanceRepository",

                instanceProvider = fakePerDisplayInstanceProviderWithTeardown,

                instanceProvider = fakePerDisplayInstanceProviderWithSetupAndTeardown,

                lifecycleManager,

                testScope.backgroundScope,

                displayRepository,