From c5ef3541fcc5056a85c23c61b4eeba1cfebc2503 Mon Sep 17 00:00:00 2001 From: Christian Go??llner Date: Thu, 28 Jul 2022 17:05:08 +0200 Subject: [PATCH] [StatusBar][Letterbox] Retrieve letterbox background info from WM Before this change, we were assuming that the background was always black. Now we retrieve the actual values from WindowManager. This is used to determine the color of the status bar icons that are positioned on the background. Fixes: 238607453 Test: Manually Test: LetterboxBackgroundProviderTest.kt Test: LetterboxAppearanceCalculatorTest.kt Change-Id: I9dd9c95b35c5ca7ce20ee670e073cbad578ab5d9 --- .../phone/LetterboxAppearanceCalculator.kt | 16 +-- .../phone/LetterboxBackgroundProvider.kt | 74 +++++++++++++ .../CentralSurfacesStartableModule.java | 6 ++ .../LetterboxAppearanceCalculatorTest.kt | 27 ++++- .../phone/LetterboxBackgroundProviderTest.kt | 100 ++++++++++++++++++ 5 files changed, 215 insertions(+), 8 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProviderTest.kt 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 70ec13b14abd0..4496607a7db27 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculator.kt @@ -17,7 +17,6 @@ package com.android.systemui.statusbar.phone import android.annotation.ColorInt -import android.graphics.Color import android.graphics.Rect import android.view.InsetsFlags import android.view.ViewDebug @@ -39,7 +38,13 @@ import javax.inject.Inject class LetterboxAppearance( @Appearance val appearance: Int, val appearanceRegions: Array -) +) { + override fun toString(): String { + val appearanceString = + ViewDebug.flagsToString(InsetsFlags::class.java, "appearance", appearance) + return "LetterboxAppearance{$appearanceString, ${appearanceRegions.contentToString()}}" + } +} /** * Responsible for calculating the [Appearance] and [AppearanceRegion] for the status bar when apps @@ -51,6 +56,7 @@ class LetterboxAppearanceCalculator constructor( private val lightBarController: LightBarController, private val dumpManager: DumpManager, + private val letterboxBackgroundProvider: LetterboxBackgroundProvider, ) : OnStatusBarViewInitializedListener, CentralSurfacesComponent.Startable { private var statusBarBoundsProvider: StatusBarBoundsProvider? = null @@ -184,13 +190,11 @@ constructor( @ColorInt private fun outerLetterboxBackgroundColor(): Int { - // TODO(b/238607453): retrieve this information from WindowManager. - return Color.BLACK + return letterboxBackgroundProvider.letterboxBackgroundColor } private fun isOuterLetterboxMultiColored(): Boolean { - // TODO(b/238607453): retrieve this information from WindowManager. - return false + return letterboxBackgroundProvider.isLetterboxBackgroundMultiColored } private fun getEndSideIconsBounds(): Rect { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt new file mode 100644 index 0000000000000..96b9aca9c64c2 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProvider.kt @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2022 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 android.annotation.ColorInt +import android.graphics.Color +import android.os.RemoteException +import android.view.IWindowManager +import com.android.systemui.Dumpable +import com.android.systemui.dagger.qualifiers.Background +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 +class LetterboxBackgroundProvider +@Inject +constructor( + private val windowManager: IWindowManager, + @Background private val backgroundExecutor: Executor, + private val dumpManager: DumpManager, +) : CentralSurfacesComponent.Startable, Dumpable { + + @ColorInt + var letterboxBackgroundColor: Int = Color.BLACK + private set + + var isLetterboxBackgroundMultiColored: Boolean = false + private set + + override fun start() { + dumpManager.registerDumpable(javaClass.simpleName, this) + + // Using a background executor, as binder calls to IWindowManager are blocking + backgroundExecutor.execute { + try { + isLetterboxBackgroundMultiColored = windowManager.isLetterboxBackgroundMultiColored + letterboxBackgroundColor = windowManager.letterboxBackgroundColorInArgb + } catch (e: RemoteException) { + e.rethrowFromSystemServer() + } + } + } + + override fun stop() { + dumpManager.unregisterDumpable(javaClass.simpleName) + } + + override fun dump(pw: PrintWriter, args: Array) { + pw.println( + """ + letterboxBackgroundColor: ${Color.valueOf(letterboxBackgroundColor)} + isLetterboxBackgroundMultiColored: $isLetterboxBackgroundMultiColored + """.trimIndent()) + } +} 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 d57e6a7914897..b0532d773f7bf 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,6 +17,7 @@ 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; @@ -40,4 +41,9 @@ 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/LetterboxAppearanceCalculatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculatorTest.kt index 65401356b3e3b..c0243dc537b54 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxAppearanceCalculatorTest.kt @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.phone +import android.graphics.Color import android.graphics.Rect import android.testing.AndroidTestingRunner import android.view.WindowInsetsController @@ -24,6 +25,7 @@ import androidx.test.filters.SmallTest import com.android.internal.statusbar.LetterboxDetails import com.android.internal.view.AppearanceRegion import com.android.systemui.SysuiTestCase +import com.android.systemui.dump.DumpManager import com.android.systemui.statusbar.phone.fragment.dagger.StatusBarFragmentComponent import com.google.common.truth.Expect import com.google.common.truth.Truth.assertThat @@ -33,7 +35,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock import org.mockito.Mockito.`when` as whenever -import com.android.systemui.dump.DumpManager import org.mockito.MockitoAnnotations @RunWith(AndroidTestingRunner::class) @@ -56,6 +57,7 @@ class LetterboxAppearanceCalculatorTest : SysuiTestCase() { @Mock private lateinit var statusBarBoundsProvider: StatusBarBoundsProvider @Mock private lateinit var statusBarFragmentComponent: StatusBarFragmentComponent @Mock private lateinit var dumpManager: DumpManager + @Mock private lateinit var letterboxBackgroundProvider: LetterboxBackgroundProvider private lateinit var calculator: LetterboxAppearanceCalculator @@ -63,8 +65,12 @@ class LetterboxAppearanceCalculatorTest : SysuiTestCase() { fun setUp() { MockitoAnnotations.initMocks(this) whenever(statusBarFragmentComponent.boundsProvider).thenReturn(statusBarBoundsProvider) - calculator = LetterboxAppearanceCalculator(lightBarController, dumpManager) + calculator = + LetterboxAppearanceCalculator( + lightBarController, dumpManager, letterboxBackgroundProvider) calculator.onStatusBarViewInitialized(statusBarFragmentComponent) + whenever(letterboxBackgroundProvider.letterboxBackgroundColor).thenReturn(Color.BLACK) + whenever(letterboxBackgroundProvider.isLetterboxBackgroundMultiColored).thenReturn(false) } @Test @@ -99,6 +105,23 @@ class LetterboxAppearanceCalculatorTest : SysuiTestCase() { expect.that(letterboxAppearance.appearanceRegions).isEqualTo(TEST_APPEARANCE_REGIONS) } + @Test + fun getLetterboxAppearance_noOverlap_BackgroundMultiColor_returnsAppearanceWithScrim() { + whenever(letterboxBackgroundProvider.isLetterboxBackgroundMultiColored).thenReturn(true) + whenever(statusBarBoundsProvider.visibleStartSideBounds).thenReturn(Rect(0, 0, 100, 100)) + whenever(statusBarBoundsProvider.visibleEndSideBounds).thenReturn(Rect(200, 0, 300, 100)) + val letterbox = letterboxWithInnerBounds(Rect(101, 0, 199, 100)) + + val letterboxAppearance = + calculator.getLetterboxAppearance( + TEST_APPEARANCE, TEST_APPEARANCE_REGIONS, arrayOf(letterbox)) + + expect + .that(letterboxAppearance.appearance) + .isEqualTo(TEST_APPEARANCE or APPEARANCE_SEMI_TRANSPARENT_STATUS_BARS) + expect.that(letterboxAppearance.appearanceRegions).isEqualTo(TEST_APPEARANCE_REGIONS) + } + @Test fun getLetterboxAppearance_noOverlap_returnsAppearanceWithoutScrim() { whenever(statusBarBoundsProvider.visibleStartSideBounds).thenReturn(Rect(0, 0, 100, 100)) 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 new file mode 100644 index 0000000000000..44325dddd1114 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LetterboxBackgroundProviderTest.kt @@ -0,0 +1,100 @@ +/* + * Copyright (C) 2022 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 android.graphics.Color +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.time.FakeSystemClock +import com.google.common.truth.Truth.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.`when` as whenever +import org.mockito.MockitoAnnotations + +@RunWith(AndroidTestingRunner::class) +@SmallTest +class LetterboxBackgroundProviderTest : SysuiTestCase() { + + private val fakeSystemClock = FakeSystemClock() + private val fakeExecutor = FakeExecutor(fakeSystemClock) + + @Mock private lateinit var windowManager: IWindowManager + @Mock private lateinit var dumpManager: DumpManager + + private lateinit var provider: LetterboxBackgroundProvider + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + + provider = LetterboxBackgroundProvider(windowManager, fakeExecutor, dumpManager) + } + + @Test + fun letterboxBackgroundColor_defaultValue_returnsBlack() { + assertThat(provider.letterboxBackgroundColor).isEqualTo(Color.BLACK) + } + + @Test + fun letterboxBackgroundColor_afterOnStart_executorNotDone_returnsDefaultValue() { + whenever(windowManager.letterboxBackgroundColorInArgb).thenReturn(Color.RED) + + provider.start() + + assertThat(provider.letterboxBackgroundColor).isEqualTo(Color.BLACK) + } + + @Test + fun letterboxBackgroundColor_afterOnStart_executorDone_returnsValueFromWindowManager() { + whenever(windowManager.letterboxBackgroundColorInArgb).thenReturn(Color.RED) + + provider.start() + fakeExecutor.runAllReady() + + assertThat(provider.letterboxBackgroundColor).isEqualTo(Color.RED) + } + + @Test + fun isLetterboxBackgroundMultiColored_defaultValue_returnsFalse() { + assertThat(provider.isLetterboxBackgroundMultiColored).isEqualTo(false) + } + @Test + fun isLetterboxBackgroundMultiColored_afterOnStart_executorNotDone_returnsDefaultValue() { + whenever(windowManager.isLetterboxBackgroundMultiColored).thenReturn(true) + + provider.start() + + assertThat(provider.isLetterboxBackgroundMultiColored).isFalse() + } + + @Test + fun isBackgroundMultiColored_afterOnStart_executorDone_returnsValueFromWindowManager() { + whenever(windowManager.isLetterboxBackgroundMultiColored).thenReturn(true) + + provider.start() + fakeExecutor.runAllReady() + + assertThat(provider.isLetterboxBackgroundMultiColored).isTrue() + } +}