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

Commit 74151e8e authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Refactor CoroutineScopeStore to PerDisplayRepository<CoroutineScope> 

This will help in a follow up refactor of the keyguard

Bug: 362719719
Bug: 397680831
Bug: 403481022
Test: DisplayScopeRepositoryInstanceProviderTest
Flag: NONE - small refactor but no behaviour changes.
Change-Id: I2f29693def2219ca670cb5ca0691d49fb702c9fc
parent 39f6dcef

packages/SystemUI/multivalentTests/src/com/android/systemui/display/data/repository/DisplayScopeRepositoryImplTest.kt→packages/SystemUI/multivalentTests/src/com/android/systemui/display/data/repository/DisplayScopeRepositoryInstanceProviderTest.kt

+8 −45
Original line number Diff line number Diff line
@@ -16,7 +16,6 @@ 


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



import android.platform.test.annotations.EnableFlags

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

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase
@@ -24,77 +23,41 @@ import com.android.systemui.kosmos.applicationCoroutineScope 
import com.android.systemui.kosmos.testDispatcher

import com.android.systemui.kosmos.testScope

import com.android.systemui.kosmos.useUnconfinedTestDispatcher

import com.android.systemui.statusbar.core.StatusBarConnectedDisplays

import com.android.systemui.testKosmos

import com.google.common.truth.Truth.assertThat

import kotlinx.coroutines.isActive

import kotlinx.coroutines.test.runTest

import org.junit.Before

import org.junit.Test

import org.junit.runner.RunWith



@EnableFlags(StatusBarConnectedDisplays.FLAG_NAME)

@RunWith(AndroidJUnit4::class)

@SmallTest

class DisplayScopeRepositoryImplTest : SysuiTestCase() {

class DisplayScopeRepositoryInstanceProviderTest : SysuiTestCase() {



    private val kosmos = testKosmos().useUnconfinedTestDispatcher()

    private val testScope = kosmos.testScope

    private val fakeDisplayRepository = kosmos.displayRepository



    private val repo =

        DisplayScopeRepositoryImpl(

    private val underTest =

        DisplayScopeRepositoryInstanceProvider(

            kosmos.applicationCoroutineScope,

            kosmos.testDispatcher,

            fakeDisplayRepository,

        )



    @Before

    fun setUp() {

        repo.start()

    }



    @Test

    fun scopeForDisplay_multipleCallsForSameDisplayId_returnsSameInstance() {

        val scopeForDisplay = repo.scopeForDisplay(displayId = 1)



        assertThat(repo.scopeForDisplay(displayId = 1)).isSameInstanceAs(scopeForDisplay)

    }



    @Test

    fun scopeForDisplay_differentDisplayId_returnsNewInstance() {

        val scopeForDisplay1 = repo.scopeForDisplay(displayId = 1)

        val scopeForDisplay2 = repo.scopeForDisplay(displayId = 2)



        assertThat(scopeForDisplay1).isNotSameInstanceAs(scopeForDisplay2)

    }



    @Test

    fun scopeForDisplay_activeByDefault() =

    fun createInstance_activeByDefault() =

        testScope.runTest {

            val scopeForDisplay = repo.scopeForDisplay(displayId = 1)

            val scopeForDisplay = underTest.createInstance(displayId = 1)



            assertThat(scopeForDisplay.isActive).isTrue()

        }



    @Test

    fun scopeForDisplay_afterDisplayRemoved_scopeIsCancelled() =

    fun destroyInstance_afterDisplayRemoved_scopeIsCancelled() =

        testScope.runTest {

            val scopeForDisplay = repo.scopeForDisplay(displayId = 1)

            val scopeForDisplay = underTest.createInstance(displayId = 1)



            fakeDisplayRepository.removeDisplay(displayId = 1)

            underTest.destroyInstance(scopeForDisplay)



            assertThat(scopeForDisplay.isActive).isFalse()

        }



    @Test

    fun scopeForDisplay_afterDisplayRemoved_returnsNewInstance() =

        testScope.runTest {

            val initialScope = repo.scopeForDisplay(displayId = 1)



            fakeDisplayRepository.removeDisplay(displayId = 1)



            val newScope = repo.scopeForDisplay(displayId = 1)

            assertThat(newScope).isNotSameInstanceAs(initialScope)

        }

}

packages/SystemUI/src/com/android/systemui/dagger/PerDisplayRepositoriesModule.kt

+2 −1
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.systemui.dagger 
import com.android.app.displaylib.DefaultDisplayOnlyInstanceRepositoryImpl

import com.android.app.displaylib.PerDisplayInstanceRepositoryImpl

import com.android.app.displaylib.PerDisplayRepository

import com.android.systemui.display.data.repository.PerDisplayCoroutineScopeRepositoryModule

import com.android.systemui.model.SysUIStateInstanceProvider

import com.android.systemui.model.SysUiState

import com.android.systemui.shade.shared.flag.ShadeWindowGoesAround
@@ -26,7 +27,7 @@ import dagger.Module 
import dagger.Provides



/** This module is meant to contain all the code to create the various [PerDisplayRepository<>]. */

@Module

@Module(includes = [PerDisplayCoroutineScopeRepositoryModule::class])

class PerDisplayRepositoriesModule {



    @SysUISingleton

packages/SystemUI/src/com/android/systemui/display/DisplayModule.kt

+0 −18
Original line number Diff line number Diff line
@@ -29,8 +29,6 @@ import com.android.systemui.display.data.repository.DeviceStateRepository 
import com.android.systemui.display.data.repository.DeviceStateRepositoryImpl

import com.android.systemui.display.data.repository.DisplayRepository

import com.android.systemui.display.data.repository.DisplayRepositoryImpl

import com.android.systemui.display.data.repository.DisplayScopeRepository

import com.android.systemui.display.data.repository.DisplayScopeRepositoryImpl

import com.android.systemui.display.data.repository.DisplayWindowPropertiesRepository

import com.android.systemui.display.data.repository.DisplayWindowPropertiesRepositoryImpl

import com.android.systemui.display.data.repository.FocusedDisplayRepository
@@ -76,8 +74,6 @@ interface DisplayModule { 
        focusedDisplayRepository: FocusedDisplayRepositoryImpl

    ): FocusedDisplayRepository



    @Binds fun displayScopeRepository(impl: DisplayScopeRepositoryImpl): DisplayScopeRepository



    @Binds

    fun displayWindowPropertiesRepository(

        impl: DisplayWindowPropertiesRepositoryImpl
@@ -91,20 +87,6 @@ interface DisplayModule { 
    fun bindDisplayLibBackground(@Background bgScope: CoroutineScope): CoroutineScope



    companion object {

        @Provides

        @SysUISingleton

        @IntoMap

        @ClassKey(DisplayScopeRepositoryImpl::class)

        fun displayScopeRepoCoreStartable(

            repoImplLazy: Lazy<DisplayScopeRepositoryImpl>

        ): CoreStartable {

            return if (StatusBarConnectedDisplays.isEnabled) {

                repoImplLazy.get()

            } else {

                CoreStartable.NOP

            }

        }



        @Provides

        @SysUISingleton

        @IntoMap

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

+31 −33
Original line number Diff line number Diff line
@@ -17,60 +17,58 @@ 
package com.android.systemui.display.data.repository



import android.view.Display

import com.android.systemui.CoreStartable

import com.android.app.displaylib.PerDisplayInstanceProviderWithTeardown

import com.android.app.displaylib.PerDisplayInstanceRepositoryImpl

import com.android.app.displaylib.PerDisplayRepository

import com.android.systemui.coroutines.newTracingContext

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.statusbar.core.StatusBarConnectedDisplays

import java.util.concurrent.ConcurrentHashMap

import dagger.Module

import dagger.Provides

import javax.inject.Inject

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.cancel

import com.android.app.tracing.coroutines.launchTraced as launch



/**

 * Provides per display instances of [CoroutineScope]. These will remain active as long as the

 * display is connected, and automatically cancelled when the display is removed.

 * Provides per display instances of [CoroutineScope].

 *

 * This is used to create a [PerDisplayRepository] of [CoroutineScope]

 */

interface DisplayScopeRepository {

    fun scopeForDisplay(displayId: Int): CoroutineScope

}



@SysUISingleton

class DisplayScopeRepositoryImpl

class DisplayScopeRepositoryInstanceProvider

@Inject

constructor(

    @Background private val backgroundApplicationScope: CoroutineScope,

    @Background private val backgroundDispatcher: CoroutineDispatcher,

    private val displayRepository: DisplayRepository,

) : DisplayScopeRepository, CoreStartable {



    private val perDisplayScopes = ConcurrentHashMap<Int, CoroutineScope>()



    override fun scopeForDisplay(displayId: Int): CoroutineScope {

        return perDisplayScopes.computeIfAbsent(displayId) { createScopeForDisplay(displayId) }

    }



    override fun start() {

        StatusBarConnectedDisplays.unsafeAssertInNewMode()

        backgroundApplicationScope.launch {

            displayRepository.displayRemovalEvent.collect { displayId ->

                val scope = perDisplayScopes.remove(displayId)

                scope?.cancel("Display $displayId has been removed.")

            }

        }

    }

) : PerDisplayInstanceProviderWithTeardown<CoroutineScope> {



    private fun createScopeForDisplay(displayId: Int): CoroutineScope {

    override fun createInstance(displayId: Int): CoroutineScope {

        return if (displayId == Display.DEFAULT_DISPLAY) {

            // The default display is connected all the time, therefore we can optimise by reusing

            // the application scope, and don't need to create a new scope.

            backgroundApplicationScope

        } else {

            CoroutineScope(

                backgroundDispatcher + newTracingContext("DisplayScope$displayId")

            )

            CoroutineScope(backgroundDispatcher + newTracingContext("DisplayScope$displayId"))

        }

    }



    override fun destroyInstance(instance: CoroutineScope) {

        instance.cancel("DisplayContext has been cancelled.")

    }

}



@Module

object PerDisplayCoroutineScopeRepositoryModule {

    @SysUISingleton

    @Provides

    fun provideDisplayCoroutineScopeRepository(

        repositoryFactory: PerDisplayInstanceRepositoryImpl.Factory<CoroutineScope>,

        instanceProvider: DisplayScopeRepositoryInstanceProvider,

    ): PerDisplayRepository<CoroutineScope> {

        return repositoryFactory.create(

            debugName = "CoroutineScopePerDisplayRepo",

            instanceProvider,

        )

    }

}

packages/SystemUI/src/com/android/systemui/statusbar/core/MultiDisplayStatusBarOrchestratorStore.kt

+4 −4
Original line number Diff line number Diff line
@@ -16,10 +16,10 @@ 


package com.android.systemui.statusbar.core



import com.android.app.displaylib.PerDisplayRepository

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.display.data.repository.DisplayRepository

import com.android.systemui.display.data.repository.DisplayScopeRepository

import com.android.systemui.statusbar.data.repository.StatusBarModeRepositoryStore

import com.android.systemui.statusbar.data.repository.StatusBarPerDisplayStoreImpl

import com.android.systemui.statusbar.phone.AutoHideControllerStore
@@ -40,7 +40,7 @@ constructor( 
    private val statusBarModeRepositoryStore: StatusBarModeRepositoryStore,

    private val initializerStore: StatusBarInitializerStore,

    private val autoHideControllerStore: AutoHideControllerStore,

    private val displayScopeRepository: DisplayScopeRepository,

    private val displayScopeRepository: PerDisplayRepository<CoroutineScope>,

    private val statusBarWindowStateRepositoryStore: StatusBarWindowStateRepositoryStore,

) :

    StatusBarPerDisplayStoreImpl<StatusBarOrchestrator>(
@@ -59,10 +59,10 @@ constructor( 
        val statusBarWindowController =

            statusBarWindowControllerStore.forDisplay(displayId) ?: return null

        val autoHideController = autoHideControllerStore.forDisplay(displayId) ?: return null

        val displayScope = displayScopeRepository[displayId] ?: return null

        return factory.create(

            displayId,

            // TODO: b/398825844 - Handle nullness to prevent leaking CoroutineScope.

            displayScopeRepository.scopeForDisplay(displayId),

            displayScope,

            statusBarWindowStateRepositoryStore.forDisplay(displayId),

            statusBarModeRepository,

            statusBarInitializer,