Fix a bug that prevented notification reinflate on user switch.

ViewConfigCoordinator now relies on a consistent 'isSwitchingUser' signal to guarantee that change events that are coalesced are always triggered, because the state is all tracked within this class. Bug: 227880269 Test: atest ViewConfigCoordinatorTest Change-Id: I36f9723e3673aa9079c1f467cc812b076a3271d1
2022-06-12 10:51:48 +00:00
parent a53b933831
commit 85713436ff
2 changed files with 251 additions and 7 deletions
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt
@@ -16,15 +16,18 @@

 package com.android.systemui.statusbar.notification.collection.coordinator

+import android.util.Log
 import com.android.internal.widget.MessagingGroup
 import com.android.internal.widget.MessagingMessage
 import com.android.keyguard.KeyguardUpdateMonitor
+import com.android.keyguard.KeyguardUpdateMonitorCallback
 import com.android.systemui.statusbar.NotificationLockscreenUserManager
 import com.android.systemui.statusbar.NotificationLockscreenUserManager.UserChangedListener
 import com.android.systemui.statusbar.notification.collection.NotifPipeline
 import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope
 import com.android.systemui.statusbar.notification.row.NotificationGutsManager
 import com.android.systemui.statusbar.policy.ConfigurationController
+import com.android.systemui.util.Compile
 import javax.inject.Inject

 /**
@@ -38,24 +41,52 @@ class ViewConfigCoordinator @Inject internal constructor(
    private val mLockscreenUserManager: NotificationLockscreenUserManager,
    private val mGutsManager: NotificationGutsManager,
    private val mKeyguardUpdateMonitor: KeyguardUpdateMonitor
-) : Coordinator, UserChangedListener, ConfigurationController.ConfigurationListener {
+) : Coordinator, ConfigurationController.ConfigurationListener {

+    private var mIsSwitchingUser = false
    private var mReinflateNotificationsOnUserSwitched = false
    private var mDispatchUiModeChangeOnUserSwitched = false
    private var mPipeline: NotifPipeline? = null

+    private val mKeyguardUpdateCallback = object : KeyguardUpdateMonitorCallback() {
+        override fun onUserSwitching(userId: Int) {
+            log { "ViewConfigCoordinator.onUserSwitching(userId=$userId)" }
+            mIsSwitchingUser = true
+        }
+
+        override fun onUserSwitchComplete(userId: Int) {
+            log { "ViewConfigCoordinator.onUserSwitchComplete(userId=$userId)" }
+            mIsSwitchingUser = false
+            applyChangesOnUserSwitched()
+        }
+    }
+
+    private val mUserChangedListener = object : UserChangedListener {
+        override fun onUserChanged(userId: Int) {
+            log { "ViewConfigCoordinator.onUserChanged(userId=$userId)" }
+            applyChangesOnUserSwitched()
+        }
+    }
+
    override fun attach(pipeline: NotifPipeline) {
        mPipeline = pipeline
        if (pipeline.isNewPipelineEnabled) {
-            mLockscreenUserManager.addUserChangedListener(this)
+            mLockscreenUserManager.addUserChangedListener(mUserChangedListener)
            mConfigurationController.addCallback(this)
+            mKeyguardUpdateMonitor.registerCallback(mKeyguardUpdateCallback)
        }
    }

    override fun onDensityOrFontScaleChanged() {
+        log {
+            val keyguardIsSwitchingUser = mKeyguardUpdateMonitor.isSwitchingUser
+            "ViewConfigCoordinator.onDensityOrFontScaleChanged()" +
+                    " isSwitchingUser=$mIsSwitchingUser" +
+                    " keyguardUpdateMonitor.isSwitchingUser=$keyguardIsSwitchingUser"
+        }
        MessagingMessage.dropCache()
        MessagingGroup.dropCache()
-        if (!mKeyguardUpdateMonitor.isSwitchingUser) {
+        if (!mIsSwitchingUser) {
            updateNotificationsOnDensityOrFontScaleChanged()
        } else {
            mReinflateNotificationsOnUserSwitched = true
@@ -63,7 +94,13 @@ class ViewConfigCoordinator @Inject internal constructor(
    }

    override fun onUiModeChanged() {
-        if (!mKeyguardUpdateMonitor.isSwitchingUser) {
+        log {
+            val keyguardIsSwitchingUser = mKeyguardUpdateMonitor.isSwitchingUser
+            "ViewConfigCoordinator.onUiModeChanged()" +
+                    " isSwitchingUser=$mIsSwitchingUser" +
+                    " keyguardUpdateMonitor.isSwitchingUser=$keyguardIsSwitchingUser"
+        }
+        if (!mIsSwitchingUser) {
            updateNotificationsOnUiModeChanged()
        } else {
            mDispatchUiModeChangeOnUserSwitched = true
@@ -74,7 +111,7 @@ class ViewConfigCoordinator @Inject internal constructor(
        onDensityOrFontScaleChanged()
    }

-    override fun onUserChanged(userId: Int) {
+    private fun applyChangesOnUserSwitched() {
        if (mReinflateNotificationsOnUserSwitched) {
            updateNotificationsOnDensityOrFontScaleChanged()
            mReinflateNotificationsOnUserSwitched = false
@@ -86,9 +123,9 @@ class ViewConfigCoordinator @Inject internal constructor(
    }

    private fun updateNotificationsOnUiModeChanged() {
+        log { "ViewConfigCoordinator.updateNotificationsOnUiModeChanged()" }
        mPipeline?.allNotifs?.forEach { entry ->
-            val row = entry.row
-            row?.onUiModeChanged()
+            entry.row?.onUiModeChanged()
        }
    }

@@ -101,4 +138,13 @@ class ViewConfigCoordinator @Inject internal constructor(
            }
        }
    }
+
+    private inline fun log(message: () -> String) {
+        if (DEBUG) Log.d(TAG, message())
+    }
+
+    companion object {
+        private const val TAG = "ViewConfigCoordinator"
+        private val DEBUG = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.DEBUG)
+    }
 }
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinatorTest.kt
@@ -0,0 +1,198 @@
+/*
+ * 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.notification.collection.coordinator
+
+import android.testing.AndroidTestingRunner
+import android.testing.TestableLooper.RunWithLooper
+import androidx.test.filters.SmallTest
+import com.android.keyguard.KeyguardUpdateMonitor
+import com.android.keyguard.KeyguardUpdateMonitorCallback
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.statusbar.NotificationLockscreenUserManager
+import com.android.systemui.statusbar.NotificationLockscreenUserManager.UserChangedListener
+import com.android.systemui.statusbar.notification.collection.NotifPipeline
+import com.android.systemui.statusbar.notification.collection.NotificationEntry
+import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow
+import com.android.systemui.statusbar.notification.row.NotificationGutsManager
+import com.android.systemui.statusbar.policy.ConfigurationController
+import com.android.systemui.util.mockito.mock
+import com.android.systemui.util.mockito.withArgCaptor
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mockito.clearInvocations
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.verifyNoMoreInteractions
+import org.mockito.Mockito.`when` as whenever
+
+@SmallTest
+@RunWith(AndroidTestingRunner::class)
+@RunWithLooper
+class ViewConfigCoordinatorTest : SysuiTestCase() {
+    private lateinit var coordinator: ViewConfigCoordinator
+
+    // Captured
+    private lateinit var userChangedListener: UserChangedListener
+    private lateinit var configurationListener: ConfigurationController.ConfigurationListener
+    private lateinit var keyguardUpdateMonitorCallback: KeyguardUpdateMonitorCallback
+
+    // Mocks
+    private val entry: NotificationEntry = mock()
+    private val row: ExpandableNotificationRow = mock()
+    private val pipeline: NotifPipeline = mock()
+    private val configurationController: ConfigurationController = mock()
+    private val lockscreenUserManager: NotificationLockscreenUserManager = mock()
+    private val gutsManager: NotificationGutsManager = mock()
+    private val keyguardUpdateMonitor: KeyguardUpdateMonitor = mock()
+
+    @Before
+    fun setUp() {
+        whenever(pipeline.isNewPipelineEnabled).thenReturn(true)
+        whenever(pipeline.allNotifs).thenReturn(listOf(entry))
+        whenever(entry.row).thenReturn(row)
+        coordinator = ViewConfigCoordinator(
+            configurationController,
+            lockscreenUserManager,
+            gutsManager,
+            keyguardUpdateMonitor)
+        coordinator.attach(pipeline)
+        userChangedListener = withArgCaptor {
+            verify(lockscreenUserManager).addUserChangedListener(capture())
+        }
+        configurationListener = withArgCaptor {
+            verify(configurationController).addCallback(capture())
+        }
+        keyguardUpdateMonitorCallback = withArgCaptor {
+            verify(keyguardUpdateMonitor).registerCallback(capture())
+        }
+    }
+
+    @Test
+    fun uiModeChangePropagatesToRow() {
+        configurationListener.onUiModeChanged()
+        verify(entry).row
+        verify(row).onUiModeChanged()
+        verifyNoMoreInteractions(entry, row)
+    }
+
+    @Test
+    fun themeChangePropagatesToEntry() {
+        configurationListener.onThemeChanged()
+        verify(entry).onDensityOrFontScaleChanged()
+        verify(entry).areGutsExposed()
+        verifyNoMoreInteractions(entry, row)
+    }
+
+    @Test
+    fun densityChangePropagatesToEntry() {
+        configurationListener.onDensityOrFontScaleChanged()
+        verify(entry).onDensityOrFontScaleChanged()
+        verify(entry).areGutsExposed()
+        verifyNoMoreInteractions(entry, row)
+    }
+
+    @Test
+    fun switchingUserDefersChangesWithUserChangedEventAfter() {
+        // GIVEN switching users
+        keyguardUpdateMonitorCallback.onUserSwitching(10)
+
+        // WHEN configuration changes happen
+        configurationListener.onUiModeChanged()
+        configurationListener.onDensityOrFontScaleChanged()
+        configurationListener.onThemeChanged()
+
+        // VERIFY no changes are propagated
+        verifyNoMoreInteractions(entry, row)
+
+        // WHEN user switch completes
+        keyguardUpdateMonitorCallback.onUserSwitchComplete(10)
+
+        // VERIFY the changes propagate
+        verify(entry).row
+        verify(row).onUiModeChanged()
+        verify(entry).onDensityOrFontScaleChanged()
+        verify(entry).areGutsExposed()
+        verifyNoMoreInteractions(entry, row)
+        clearInvocations(entry, row)
+
+        // WHEN user change happens after the switching window
+        userChangedListener.onUserChanged(10)
+
+        // VERIFY user change itself does not re-trigger updates
+        verifyNoMoreInteractions(entry, row)
+    }
+
+    @Test
+    fun switchingUserDefersChangesWithUserChangedEventDuring() {
+        // GIVEN switching users
+        keyguardUpdateMonitorCallback.onUserSwitching(10)
+
+        // WHEN configuration changes happen
+        configurationListener.onUiModeChanged()
+        configurationListener.onDensityOrFontScaleChanged()
+        configurationListener.onThemeChanged()
+
+        // VERIFY no changes are propagated
+        verifyNoMoreInteractions(entry, row)
+
+        // WHEN user change happens during the switching window
+        userChangedListener.onUserChanged(10)
+
+        // VERIFY the changes propagate
+        verify(entry).row
+        verify(row).onUiModeChanged()
+        verify(entry).onDensityOrFontScaleChanged()
+        verify(entry).areGutsExposed()
+        verifyNoMoreInteractions(entry, row)
+        clearInvocations(entry, row)
+
+        // WHEN user switch completes
+        keyguardUpdateMonitorCallback.onUserSwitchComplete(10)
+
+        // VERIFY the switching window closing does not re-propagate
+        verifyNoMoreInteractions(entry, row)
+    }
+
+    @Test
+    fun switchingUserDefersChangesWithUserChangedEventBefore() {
+        // WHEN user change happens before configuration changes or switching window
+        userChangedListener.onUserChanged(10)
+
+        // VERIFY no changes happen
+        verifyNoMoreInteractions(entry, row)
+
+        // WHEN switching users then configuration changes happen
+        keyguardUpdateMonitorCallback.onUserSwitching(10)
+
+        configurationListener.onUiModeChanged()
+        configurationListener.onDensityOrFontScaleChanged()
+        configurationListener.onThemeChanged()
+
+        // VERIFY no changes happen
+        verifyNoMoreInteractions(entry, row)
+
+        // WHEN user switch completes
+        keyguardUpdateMonitorCallback.onUserSwitchComplete(10)
+
+        // VERIFY the changes propagate
+        verify(entry).row
+        verify(row).onUiModeChanged()
+        verify(entry).onDensityOrFontScaleChanged()
+        verify(entry).areGutsExposed()
+        verifyNoMoreInteractions(entry, row)
+        clearInvocations(entry, row)
+    }
+}