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

Commit e4e53bea authored by Treehugger Robot's avatar Treehugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Fix recursive loop in DisplayComponent lifecycle listeners" into main

parents 9925bb05 f2248c3a
Loading
Loading
Loading
Loading
+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() }
    }

+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)

+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
+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,