Update ConfigurationController to not store maxBounds by reference.

See bug for more information, but tl;dr is: Since maxBounds was directly
referencing the configuration's bounds, it got updated before
`onConfigurationChanged` was triggered. This meant listeners never got
the `onMaxBoundsChanged` callback, so `PrivacyViewDotController` never
updated its bounds and has the incorrect bounds.

Bug: 245799099
Bug: 256754780
Bug: 259105114
Test: manual: Verify privacy dot shows up correctly in all displays
Test: atest ConfigurationControllerImplTest

Change-Id: Ic3968b89a240f28630eff756ca0a7eaacbf5dee2
This commit is contained in:
Caitlin Shkuratov
2022-11-14 18:53:20 +00:00
parent 2c34290e87
commit be5a6e5c51
3 changed files with 82 additions and 4 deletions

View File

@@ -33,7 +33,7 @@ class ConfigurationControllerImpl @Inject constructor(context: Context) : Config
private val lastConfig = Configuration()
private var density: Int = 0
private var smallestScreenWidth: Int = 0
private var maxBounds: Rect? = null
private var maxBounds = Rect()
private var fontScale: Float = 0.toFloat()
private val inCarMode: Boolean
private var uiMode: Int = 0
@@ -92,7 +92,11 @@ class ConfigurationControllerImpl @Inject constructor(context: Context) : Config
val maxBounds = newConfig.windowConfiguration.maxBounds
if (maxBounds != this.maxBounds) {
this.maxBounds = maxBounds
// Update our internal rect to have the same bounds, instead of using
// `this.maxBounds = maxBounds` directly. Setting it directly means that `maxBounds`
// would be a direct reference to windowConfiguration.maxBounds, so the if statement
// above would always fail. See b/245799099 for more information.
this.maxBounds.set(maxBounds)
listeners.filterForEach({ this.listeners.contains(it) }) {
it.onMaxBoundsChanged()
}

View File

@@ -14,10 +14,12 @@
package com.android.systemui.statusbar.phone
import android.content.res.Configuration
import androidx.test.filters.SmallTest
import android.testing.AndroidTestingRunner
import com.android.systemui.SysuiTestCase
import com.android.systemui.statusbar.policy.ConfigurationController.ConfigurationListener
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.doAnswer
@@ -29,8 +31,7 @@ import org.mockito.Mockito.verify
@SmallTest
class ConfigurationControllerImplTest : SysuiTestCase() {
private val mConfigurationController =
com.android.systemui.statusbar.phone.ConfigurationControllerImpl(mContext)
private val mConfigurationController = ConfigurationControllerImpl(mContext)
@Test
fun testThemeChange() {
@@ -57,4 +58,52 @@ class ConfigurationControllerImplTest : SysuiTestCase() {
verify(listener).onThemeChanged()
verify(listener2, never()).onThemeChanged()
}
@Test
fun maxBoundsChange_newConfigObject_listenerNotified() {
val config = mContext.resources.configuration
config.windowConfiguration.setMaxBounds(0, 0, 200, 200)
mConfigurationController.onConfigurationChanged(config)
val listener = object : ConfigurationListener {
var triggered: Boolean = false
override fun onMaxBoundsChanged() {
triggered = true
}
}
mConfigurationController.addCallback(listener)
// WHEN a new configuration object with new bounds is sent
val newConfig = Configuration()
newConfig.windowConfiguration.setMaxBounds(0, 0, 100, 100)
mConfigurationController.onConfigurationChanged(newConfig)
// THEN the listener is notified
assertThat(listener.triggered).isTrue()
}
// Regression test for b/245799099
@Test
fun maxBoundsChange_sameObject_listenerNotified() {
val config = mContext.resources.configuration
config.windowConfiguration.setMaxBounds(0, 0, 200, 200)
mConfigurationController.onConfigurationChanged(config)
val listener = object : ConfigurationListener {
var triggered: Boolean = false
override fun onMaxBoundsChanged() {
triggered = true
}
}
mConfigurationController.addCallback(listener)
// WHEN the existing config is updated with new bounds
config.windowConfiguration.setMaxBounds(0, 0, 100, 100)
mConfigurationController.onConfigurationChanged(config)
// THEN the listener is notified
assertThat(listener.triggered).isTrue()
}
}

View File

@@ -513,6 +513,31 @@ class StatusBarContentInsetsProviderTest : SysuiTestCase() {
assertThat(firstDisplayInsetsFirstCall).isEqualTo(firstDisplayInsetsSecondCall)
}
// Regression test for b/245799099
@Test
fun onMaxBoundsChanged_listenerNotified() {
// Start out with an existing configuration with bounds
configuration.windowConfiguration.setMaxBounds(0, 0, 100, 100)
configurationController.onConfigurationChanged(configuration)
val provider = StatusBarContentInsetsProvider(contextMock, configurationController,
mock(DumpManager::class.java))
val listener = object : StatusBarContentInsetsChangedListener {
var triggered = false
override fun onStatusBarContentInsetsChanged() {
triggered = true
}
}
provider.addCallback(listener)
// WHEN the config is updated with new bounds
configuration.windowConfiguration.setMaxBounds(0, 0, 456, 789)
configurationController.onConfigurationChanged(configuration)
// THEN the listener is notified
assertThat(listener.triggered).isTrue()
}
private fun givenDisplay(screenBounds: Rect, displayUniqueId: String) {
`when`(display.uniqueId).thenReturn(displayUniqueId)
configuration.windowConfiguration.maxBounds = screenBounds