From 6aff7ce3dd623bea9aa6ced8fcc5b1e85155333d Mon Sep 17 00:00:00 2001 From: Nicolo' Mazzucato Date: Fri, 14 Mar 2025 16:04:13 +0000 Subject: Use SystemUIDisplayComponent scope in PerDisplayRepository This allows classes outside the components to use the scope created inside the component. As there are several places outside the display component using this scope, this is an intermediate step in the refactor. Future classes requiring the coroutine scope are expected to be created inside the display subcomponent Bug: 403481022 Test: DisplayScopeRepositoryInstanceProviderTest, DisplayComponentPerDisplayRepositoryTest Flag: NONE - refactor without behaviour changes Change-Id: I95cf815cbfdc3c334ff404434d7ff847a3342330 --- .../dagger/PerDisplayRepositoriesModule.kt | 5 +++- .../android/systemui/dagger/SystemUIModule.java | 2 ++ .../display/dagger/SystemUIDisplaySubcomponent.kt | 4 ++- .../data/repository/DisplayScopeRepository.kt | 29 ++++++++++++---------- 4 files changed, 25 insertions(+), 15 deletions(-) (limited to 'packages/SystemUI/src') diff --git a/packages/SystemUI/src/com/android/systemui/dagger/PerDisplayRepositoriesModule.kt b/packages/SystemUI/src/com/android/systemui/dagger/PerDisplayRepositoriesModule.kt index a8f21885907b..4bd275d50d23 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/PerDisplayRepositoriesModule.kt +++ b/packages/SystemUI/src/com/android/systemui/dagger/PerDisplayRepositoriesModule.kt @@ -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.DisplayComponentRepository import com.android.systemui.display.data.repository.PerDisplayCoroutineScopeRepositoryModule import com.android.systemui.model.SysUIStateInstanceProvider import com.android.systemui.model.SysUiState @@ -27,7 +28,9 @@ import dagger.Module import dagger.Provides /** This module is meant to contain all the code to create the various [PerDisplayRepository<>]. */ -@Module(includes = [PerDisplayCoroutineScopeRepositoryModule::class]) +@Module( + includes = [PerDisplayCoroutineScopeRepositoryModule::class, DisplayComponentRepository::class] +) class PerDisplayRepositoriesModule { @SysUISingleton diff --git a/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java b/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java index edee64e50b53..38c456f03c72 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/SystemUIModule.java @@ -66,6 +66,7 @@ import com.android.systemui.demomode.dagger.DemoModeModule; import com.android.systemui.deviceentry.DeviceEntryModule; import com.android.systemui.display.DisplayModule; import com.android.app.displaylib.PerDisplayRepository; +import com.android.systemui.display.dagger.SystemUIDisplaySubcomponent; import com.android.systemui.doze.dagger.DozeComponent; import com.android.systemui.dreams.dagger.DreamModule; import com.android.systemui.flags.FeatureFlags; @@ -299,6 +300,7 @@ import javax.inject.Named; NavigationBarComponent.class, NotificationRowComponent.class, WindowRootViewComponent.class, + SystemUIDisplaySubcomponent.class, }) public abstract class SystemUIModule { diff --git a/packages/SystemUI/src/com/android/systemui/display/dagger/SystemUIDisplaySubcomponent.kt b/packages/SystemUI/src/com/android/systemui/display/dagger/SystemUIDisplaySubcomponent.kt index f878aa1ea87b..c83b99ca2d93 100644 --- a/packages/SystemUI/src/com/android/systemui/display/dagger/SystemUIDisplaySubcomponent.kt +++ b/packages/SystemUI/src/com/android/systemui/display/dagger/SystemUIDisplaySubcomponent.kt @@ -16,6 +16,7 @@ package com.android.systemui.display.dagger +import com.android.systemui.display.dagger.SystemUIDisplaySubcomponent.PerDisplaySingleton import dagger.BindsInstance import dagger.Subcomponent import javax.inject.Qualifier @@ -31,10 +32,11 @@ import kotlinx.coroutines.CoroutineScope * cancelled in the background, so any teardown logic should be threadsafe. Cancelling on the main * thread is not feasible as it would cause jank. */ +@PerDisplaySingleton @Subcomponent(modules = [PerDisplayCommonModule::class]) interface SystemUIDisplaySubcomponent { - @DisplayAware val displayCoroutineScope: CoroutineScope + @get:DisplayAware val displayCoroutineScope: CoroutineScope @Subcomponent.Factory interface Factory { diff --git a/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayScopeRepository.kt b/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayScopeRepository.kt index ac59b16bfe3c..90dcd018e4fe 100644 --- a/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayScopeRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayScopeRepository.kt @@ -17,45 +17,48 @@ package com.android.systemui.display.data.repository import android.view.Display -import com.android.app.displaylib.PerDisplayInstanceProviderWithTeardown +import com.android.app.displaylib.PerDisplayInstanceProvider 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.display.dagger.SystemUIDisplaySubcomponent +import com.android.systemui.display.dagger.SystemUIDisplaySubcomponent.PerDisplaySingleton import dagger.Module import dagger.Provides import javax.inject.Inject -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.cancel /** * Provides per display instances of [CoroutineScope]. * - * This is used to create a [PerDisplayRepository] of [CoroutineScope] + * This is used to create a [PerDisplayRepository] of [CoroutineScope]. + * + * Note this scope is cancelled when the display is removed, see [DisplayComponentRepository]. This + * class is essentially only needed to reuse the application background scope for the default + * display, and get the display scope from the correct [SystemUIDisplaySubcomponent]. + * + * We should eventually delete this, when all classes using display scoped instances are in the + * correct dagger scope ([PerDisplaySingleton]) */ @SysUISingleton class DisplayScopeRepositoryInstanceProvider @Inject constructor( @Background private val backgroundApplicationScope: CoroutineScope, - @Background private val backgroundDispatcher: CoroutineDispatcher, -) : PerDisplayInstanceProviderWithTeardown { + private val displayComponentRepository: PerDisplayRepository, +) : PerDisplayInstanceProvider { - override fun createInstance(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")) + // The scope is automatically cancelled from the component when the display is removed. + displayComponentRepository[displayId]?.displayCoroutineScope } } - - override fun destroyInstance(instance: CoroutineScope) { - instance.cancel("DisplayContext has been cancelled.") - } } @Module -- cgit v1.2.3-59-g8ed1b