From 7acb899031eb76a4e784a5c56773eafc369d3997 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Tue, 22 Feb 2022 13:37:30 -0500 Subject: [PATCH] Move SettingObserver#setListening out of frequent path FooterActionsController#setListening is called in the main thread because it updates views. There was a register/unregister content observer in that path that was causing jank. As the setting doesn't change frequently, move the register/unregister to match the view attached/detached. Test: atest FooterActionsController Test: manual Fixes: 220874628 Change-Id: I9b5130eaee71ab8d5eb226a3078631ac7d153e23 --- .../systemui/qs/FooterActionsController.kt | 3 ++- .../qs/FooterActionsControllerTest.kt | 27 ++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt b/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt index a262b8ad2e285..217210a5dafc1 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt @@ -165,6 +165,7 @@ internal class FooterActionsController @Inject constructor( powerMenuLite.visibility = View.GONE } settingsButton.setOnClickListener(onClickListener) + multiUserSetting.isListening = true if (featureFlags.isEnabled(Flags.NEW_FOOTER)) { val securityFooter = securityFooterController.view as DualHeightHorizontalLinearLayout securityFootersContainer?.addView(securityFooter) @@ -215,6 +216,7 @@ internal class FooterActionsController @Inject constructor( override fun onViewDetached() { setListening(false) + multiUserSetting.isListening = false } fun setListening(listening: Boolean) { @@ -222,7 +224,6 @@ internal class FooterActionsController @Inject constructor( return } this.listening = listening - multiUserSetting.isListening = listening if (this.listening) { userInfoController.addCallback(onUserInfoChangedListener) updateView() diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/FooterActionsControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/FooterActionsControllerTest.kt index 8ce50a680e503..f736f26e5c46c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/FooterActionsControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/FooterActionsControllerTest.kt @@ -100,7 +100,9 @@ class FooterActionsControllerTest : LeakCheckedTest() { @After fun tearDown() { - ViewUtils.detachView(view) + if (view.isAttachedToWindow) { + ViewUtils.detachView(view) + } } @Test @@ -139,8 +141,7 @@ class FooterActionsControllerTest : LeakCheckedTest() { @Test fun testMultiUserSwitchUpdatedWhenSettingChanged() { - // When expanded, listening is true - controller.setListening(true) + // Always listening to setting while View is attached testableLooper.processAllMessages() val multiUserSwitch = view.requireViewById(R.id.multi_user_switch) @@ -156,4 +157,24 @@ class FooterActionsControllerTest : LeakCheckedTest() { assertThat(multiUserSwitch.visibility).isEqualTo(View.VISIBLE) } + + @Test + fun testMultiUserSettingNotListenedAfterDetach() { + testableLooper.processAllMessages() + + val multiUserSwitch = view.requireViewById(R.id.multi_user_switch) + assertThat(multiUserSwitch.visibility).isNotEqualTo(View.VISIBLE) + + ViewUtils.detachView(view) + + // The setting is only used as an indicator for whether the view should refresh. The actual + // value of the setting is ignored; isMultiUserEnabled is the source of truth + whenever(multiUserSwitchController.isMultiUserEnabled).thenReturn(true) + + // Changing the value of USER_SWITCHER_ENABLED should cause the view to update + fakeSettings.putIntForUser(Settings.Global.USER_SWITCHER_ENABLED, 1, userTracker.userId) + testableLooper.processAllMessages() + + assertThat(multiUserSwitch.visibility).isNotEqualTo(View.VISIBLE) + } } \ No newline at end of file