Merge "Refactor CoroutineScopeStore to PerDisplayRepository<CoroutineScope>" into main (191fda06) · Commits · e / os / android_frameworks_base

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.DisplaysWithDecorationsRepository
		@@ -78,8 +76,6 @@ interface DisplayModule {
		focusedDisplayRepository: FocusedDisplayRepositoryImpl
		): FocusedDisplayRepository

		@Binds fun displayScopeRepository(impl: DisplayScopeRepositoryImpl): DisplayScopeRepository

		@Binds
		fun displayWindowPropertiesRepository(
		impl: DisplayWindowPropertiesRepositoryImpl
		@@ -98,20 +94,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,