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

3 files changed, 82 insertions, 4 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ConfigurationControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ConfigurationControllerImpl.kt
index 34cd1ceeffc7..110dc90b6291 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ConfigurationControllerImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ConfigurationControllerImpl.kt
@@ -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()
             }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ConfigurationControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ConfigurationControllerImplTest.kt
index fee3ccb21792..b1e62e19dd3b 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ConfigurationControllerImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ConfigurationControllerImplTest.kt
@@ -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()
+    }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarContentInsetsProviderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarContentInsetsProviderTest.kt
index e86676b81f8e..e32260d4c42b 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarContentInsetsProviderTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarContentInsetsProviderTest.kt
@@ -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