From f9e686b91aeba5701e468675098de21f326d3f11 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 5 Apr 2023 16:39:31 +0000 Subject: [PATCH 1/2] [Central Surfaces] Make LetterboxBackgroundProvider a singleton. See bug for more information. Bug: 277762009 Test: compiles Test: `adb shell dumpsys activity service com.android.systemui/.SystemUIService LetterboxBackgroundProvider` -> dumps letterbox provider info Test: atest LetterboxBackgroundProviderTest Change-Id: Id589c335d19468d3320b8a33ea0ed5d2246e0f5b --- .../dagger/SystemUICoreStartableModule.kt | 4 ++- .../phone/LetterboxBackgroundProvider.kt | 17 +++------- .../statusbar/phone/LetterboxModule.kt | 32 +++++++++++++++++++ .../CentralSurfacesStartableModule.java | 10 ++---- .../phone/LetterboxBackgroundProviderTest.kt | 5 +-- 5 files changed, 42 insertions(+), 26 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxModule.kt diff --git a/packages/SystemUI/src/com/android/systemui/dagger/SystemUICoreStartableModule.kt b/packages/SystemUI/src/com/android/systemui/dagger/SystemUICoreStartableModule.kt index 9bf6b2a5b42b4..20d690e209afe 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/SystemUICoreStartableModule.kt +++ b/packages/SystemUI/src/com/android/systemui/dagger/SystemUICoreStartableModule.kt @@ -48,6 +48,7 @@ import com.android.systemui.settings.dagger.MultiUserUtilsModule import com.android.systemui.shortcut.ShortcutKeyDispatcher import com.android.systemui.statusbar.notification.InstantAppNotifier import com.android.systemui.statusbar.phone.KeyguardLiftController +import com.android.systemui.statusbar.phone.LetterboxModule import com.android.systemui.stylus.StylusUsiPowerStartable import com.android.systemui.temporarydisplay.chipbar.ChipbarCoordinator import com.android.systemui.theme.ThemeOverlayController @@ -66,7 +67,8 @@ import dagger.multibindings.IntoMap */ @Module(includes = [ MultiUserUtilsModule::class, - StartControlsStartableModule::class + StartControlsStartableModule::class, + LetterboxModule::class, ]) abstract class SystemUICoreStartableModule { /** Inject into AuthController. */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt index 276375004f76d..34c7059ec991b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt @@ -22,28 +22,25 @@ import android.graphics.Color import android.os.Handler import android.os.RemoteException import android.view.IWindowManager +import com.android.systemui.CoreStartable import com.android.systemui.Dumpable +import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Background import com.android.systemui.dagger.qualifiers.Main -import com.android.systemui.dump.DumpManager -import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent -import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent.CentralSurfacesScope import java.io.PrintWriter import java.util.concurrent.Executor import javax.inject.Inject /** Responsible for providing information about the background of letterboxed apps. */ -@CentralSurfacesScope +@SysUISingleton class LetterboxBackgroundProvider @Inject constructor( private val windowManager: IWindowManager, @Background private val backgroundExecutor: Executor, - private val dumpManager: DumpManager, private val wallpaperManager: WallpaperManager, @Main private val mainHandler: Handler, -) : CentralSurfacesComponent.Startable, Dumpable { - +) : CoreStartable, Dumpable { @ColorInt var letterboxBackgroundColor: Int = Color.BLACK private set @@ -57,7 +54,6 @@ constructor( } override fun start() { - dumpManager.registerDumpable(javaClass.simpleName, this) fetchBackgroundColorInfo() wallpaperManager.addOnColorsChangedListener(wallpaperColorsListener, mainHandler) } @@ -74,11 +70,6 @@ constructor( } } - override fun stop() { - dumpManager.unregisterDumpable(javaClass.simpleName) - wallpaperManager.removeOnColorsChangedListener(wallpaperColorsListener) - } - override fun dump(pw: PrintWriter, args: Array) { pw.println( """ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxModule.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxModule.kt new file mode 100644 index 0000000000000..2e3f0d0abc0a5 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxModule.kt @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package com.android.systemui.statusbar.phone + +import com.android.systemui.CoreStartable +import dagger.Binds +import dagger.Module +import dagger.multibindings.ClassKey +import dagger.multibindings.IntoMap + +@Module +abstract class LetterboxModule { + @Binds + @IntoMap + @ClassKey(LetterboxBackgroundProvider::class) + abstract fun bindFeature(impl: LetterboxBackgroundProvider): CoreStartable +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java index b0532d773f7bf..07356a2807116 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java @@ -17,16 +17,15 @@ package com.android.systemui.statusbar.phone.dagger; import com.android.systemui.statusbar.phone.LetterboxAppearanceCalculator; -import com.android.systemui.statusbar.phone.LetterboxBackgroundProvider; import com.android.systemui.statusbar.phone.SystemBarAttributesListener; -import java.util.Set; - import dagger.Binds; import dagger.Module; import dagger.multibindings.IntoSet; import dagger.multibindings.Multibinds; +import java.util.Set; + @Module interface CentralSurfacesStartableModule { @Multibinds @@ -41,9 +40,4 @@ interface CentralSurfacesStartableModule { @IntoSet CentralSurfacesComponent.Startable sysBarAttrsListener( SystemBarAttributesListener systemBarAttributesListener); - - @Binds - @IntoSet - CentralSurfacesComponent.Startable letterboxBgProvider( - LetterboxBackgroundProvider letterboxBackgroundProvider); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProviderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProviderTest.kt index a2828d33375b4..1cc0bd3cb36c6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProviderTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProviderTest.kt @@ -25,7 +25,6 @@ import android.testing.AndroidTestingRunner import android.view.IWindowManager import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase -import com.android.systemui.dump.DumpManager import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.eq import com.android.systemui.util.time.FakeSystemClock @@ -52,7 +51,6 @@ class LetterboxBackgroundProviderTest : SysuiTestCase() { @get:Rule var expect: Expect = Expect.create() @Mock private lateinit var windowManager: IWindowManager - @Mock private lateinit var dumpManager: DumpManager @Mock private lateinit var wallpaperManager: WallpaperManager private lateinit var provider: LetterboxBackgroundProvider @@ -65,8 +63,7 @@ class LetterboxBackgroundProviderTest : SysuiTestCase() { setUpWallpaperManager() provider = - LetterboxBackgroundProvider( - windowManager, fakeExecutor, dumpManager, wallpaperManager, mainHandler) + LetterboxBackgroundProvider(windowManager, fakeExecutor, wallpaperManager, mainHandler) } private fun setUpWallpaperManager() { From cb05797cb1fb15ffa175f280ab2b026d194f2c86 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 5 Apr 2023 16:42:15 +0000 Subject: [PATCH 2/2] [Central Surfaces] Make LetterboxAppearanceCalculator a singleton. Bug: 277762009 Test: compiles Test: `adb shell dumpsys activity service com.android.systemui/.SystemUIService LetterboxAppearanceCalculator` -> shows dump Test: atest LeterboxAppearanceCalculatorTest Change-Id: I3713c8ed9a5408283374229ebbf3d47497c716b5 --- .../com/android/systemui/dump/DumpManager.kt | 5 ++++ .../phone/LetterboxAppearanceCalculator.kt | 26 ++++++++----------- .../CentralSurfacesStartableModule.java | 6 ----- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt b/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt index 276a290e22cab..7d1ffca88d9f9 100644 --- a/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt +++ b/packages/SystemUI/src/com/android/systemui/dump/DumpManager.kt @@ -39,6 +39,11 @@ open class DumpManager @Inject constructor() { private val dumpables: MutableMap> = ArrayMap() private val buffers: MutableMap> = ArrayMap() + /** See [registerCriticalDumpable]. */ + fun registerCriticalDumpable(module: Dumpable) { + registerCriticalDumpable(module::class.java.simpleName, module) + } + /** * Registers a dumpable to be called during the CRITICAL section of the bug report. * diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt index 398985402e76a..f7426451fa501 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt @@ -26,10 +26,10 @@ import android.view.WindowInsetsController.Appearance import com.android.internal.statusbar.LetterboxDetails import com.android.internal.util.ContrastColorUtil import com.android.internal.view.AppearanceRegion +import com.android.systemui.Dumpable +import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dump.DumpManager import com.android.systemui.statusbar.core.StatusBarInitializer.OnStatusBarViewInitializedListener -import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent -import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent.CentralSurfacesScope import com.android.systemui.statusbar.phone.fragment.dagger.StatusBarFragmentComponent import java.io.PrintWriter import java.util.Arrays @@ -50,25 +50,21 @@ class LetterboxAppearance( * Responsible for calculating the [Appearance] and [AppearanceRegion] for the status bar when apps * are letterboxed. */ -@CentralSurfacesScope +@SysUISingleton class LetterboxAppearanceCalculator @Inject constructor( private val lightBarController: LightBarController, - private val dumpManager: DumpManager, + dumpManager: DumpManager, private val letterboxBackgroundProvider: LetterboxBackgroundProvider, -) : OnStatusBarViewInitializedListener, CentralSurfacesComponent.Startable { +) : OnStatusBarViewInitializedListener, Dumpable { + + init { + dumpManager.registerCriticalDumpable(this) + } private var statusBarBoundsProvider: StatusBarBoundsProvider? = null - override fun start() { - dumpManager.registerCriticalDumpable(javaClass.simpleName) { pw, _ -> dump(pw) } - } - - override fun stop() { - dumpManager.unregisterDumpable(javaClass.simpleName) - } - private var lastAppearance: Int? = null private var lastAppearanceRegions: Array? = null private var lastLetterboxes: Array? = null @@ -216,8 +212,8 @@ constructor( return this.intersect(other) } - private fun dump(printWriter: PrintWriter) { - printWriter.println( + override fun dump(pw: PrintWriter, args: Array) { + pw.println( """ lastAppearance: ${lastAppearance?.toAppearanceString()} lastAppearanceRegion: ${Arrays.toString(lastAppearanceRegions)}, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java index 07356a2807116..f72e74b77aeac 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesStartableModule.java @@ -16,7 +16,6 @@ package com.android.systemui.statusbar.phone.dagger; -import com.android.systemui.statusbar.phone.LetterboxAppearanceCalculator; import com.android.systemui.statusbar.phone.SystemBarAttributesListener; import dagger.Binds; @@ -31,11 +30,6 @@ interface CentralSurfacesStartableModule { @Multibinds Set multibindStartables(); - @Binds - @IntoSet - CentralSurfacesComponent.Startable letterboxAppearanceCalculator( - LetterboxAppearanceCalculator letterboxAppearanceCalculator); - @Binds @IntoSet CentralSurfacesComponent.Startable sysBarAttrsListener(