From ec4b69a99189b22f8817edeea6e1302c0a8489c4 Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Fri, 18 Feb 2022 12:41:25 -0500 Subject: [PATCH] Ensure that GADLite is destroyed. The FooterActionsController asks for a new GlobalActionsDialogLite each time it is attached, and then that it properly destroyes the GADLite each time it is detached. GADLite registers listeners each time it is constructed. Prior to this change, GADLite was constructed with each FooterActionsController, but never had its #destroy method called. For the sake of semantic correctness, we never reuse the GAD and instead create new one when it is needed. Fixes: 219535221 Test: manual && atest SystemUITests Change-Id: Iab9aeb294e5995944a7c619013adccb8b36a3df9 --- .../systemui/qs/FooterActionsController.kt | 10 +++- .../qs/FooterActionsControllerTest.kt | 49 ++++++++++++++++--- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt b/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt index 217210a5dafc1..27da6f3d2d2ce 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/FooterActionsController.kt @@ -50,6 +50,7 @@ import com.android.systemui.util.ViewController import com.android.systemui.util.settings.GlobalSettings import javax.inject.Inject import javax.inject.Named +import javax.inject.Provider /** * Manages [FooterActionsView] behaviour, both when it's placed in QS or QQS (split shade). @@ -69,7 +70,7 @@ internal class FooterActionsController @Inject constructor( private val fgsManagerFooterController: QSFgsManagerFooter, private val falsingManager: FalsingManager, private val metricsLogger: MetricsLogger, - private val globalActionsDialog: GlobalActionsDialogLite, + private val globalActionsDialogProvider: Provider, private val uiEventLogger: UiEventLogger, @Named(PM_LITE_ENABLED) private val showPMLiteButton: Boolean, private val globalSetting: GlobalSettings, @@ -77,6 +78,8 @@ internal class FooterActionsController @Inject constructor( private val featureFlags: FeatureFlags ) : ViewController(view) { + private var globalActionsDialog: GlobalActionsDialogLite? = null + private var lastExpansion = -1f private var listening: Boolean = false @@ -131,7 +134,7 @@ internal class FooterActionsController @Inject constructor( startSettingsActivity() } else if (v === powerMenuLite) { uiEventLogger.log(GlobalActionsDialogLite.GlobalActionsEvent.GA_OPEN_QS) - globalActionsDialog.showOrHideDialog(false, true, v) + globalActionsDialog?.showOrHideDialog(false, true, v) } } @@ -158,6 +161,7 @@ internal class FooterActionsController @Inject constructor( @VisibleForTesting public override fun onViewAttached() { + globalActionsDialog = globalActionsDialogProvider.get() if (showPMLiteButton) { powerMenuLite.visibility = View.VISIBLE powerMenuLite.setOnClickListener(onClickListener) @@ -215,6 +219,8 @@ internal class FooterActionsController @Inject constructor( } override fun onViewDetached() { + globalActionsDialog?.destroy() + globalActionsDialog = null setListening(false) multiUserSetting.isListening = false } 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 f736f26e5c46c..7b17c36aaf165 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/FooterActionsControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/FooterActionsControllerTest.kt @@ -33,12 +33,15 @@ import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.Mock import org.mockito.Mockito +import org.mockito.Mockito.never +import org.mockito.Mockito.reset import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations +import javax.inject.Provider import org.mockito.Mockito.`when` as whenever @SmallTest -@TestableLooper.RunWithLooper +@TestableLooper.RunWithLooper(setAsMainLooper = true) @RunWith(AndroidTestingRunner::class) class FooterActionsControllerTest : LeakCheckedTest() { @Mock @@ -56,6 +59,8 @@ class FooterActionsControllerTest : LeakCheckedTest() { @Mock private lateinit var multiUserSwitchController: MultiUserSwitchController @Mock + private lateinit var globalActionsDialogProvider: Provider + @Mock private lateinit var globalActionsDialog: GlobalActionsDialogLite @Mock private lateinit var uiEventLogger: UiEventLogger @@ -83,15 +88,11 @@ class FooterActionsControllerTest : LeakCheckedTest() { whenever(multiUserSwitchControllerFactory.create(any())) .thenReturn(multiUserSwitchController) whenever(featureFlags.isEnabled(Flags.NEW_FOOTER)).thenReturn(false) + whenever(globalActionsDialogProvider.get()).thenReturn(globalActionsDialog) - view = LayoutInflater.from(context) - .inflate(R.layout.footer_actions, null) as FooterActionsView + view = inflateView() - controller = FooterActionsController(view, multiUserSwitchControllerFactory, - activityStarter, userManager, userTracker, userInfoController, - deviceProvisionedController, securityFooterController, fgsManagerController, - falsingManager, metricsLogger, globalActionsDialog, uiEventLogger, - showPMLiteButton = true, fakeSettings, Handler(testableLooper.looper), featureFlags) + controller = constructFooterActionsController(view) controller.init() ViewUtils.attachView(view) // View looper is the testable looper associated with the test @@ -177,4 +178,36 @@ class FooterActionsControllerTest : LeakCheckedTest() { assertThat(multiUserSwitch.visibility).isNotEqualTo(View.VISIBLE) } + + @Test + fun testCleanUpGAD() { + reset(globalActionsDialogProvider) + whenever(globalActionsDialogProvider.get()).thenReturn(globalActionsDialog) + val view = inflateView() + controller = constructFooterActionsController(view) + controller.init() + verify(globalActionsDialogProvider, never()).get() + + // GAD is constructed during attachment + ViewUtils.attachView(view) + testableLooper.processAllMessages() + verify(globalActionsDialogProvider).get() + + ViewUtils.detachView(view) + testableLooper.processAllMessages() + verify(globalActionsDialog).destroy() + } + + private fun inflateView(): FooterActionsView { + return LayoutInflater.from(context) + .inflate(R.layout.footer_actions, null) as FooterActionsView + } + + private fun constructFooterActionsController(view: FooterActionsView): FooterActionsController { + return FooterActionsController(view, multiUserSwitchControllerFactory, + activityStarter, userManager, userTracker, userInfoController, + deviceProvisionedController, securityFooterController, fgsManagerController, + falsingManager, metricsLogger, globalActionsDialogProvider, uiEventLogger, + showPMLiteButton = true, fakeSettings, Handler(testableLooper.looper), featureFlags) + } } \ No newline at end of file