From efa0a41ffec7b791de5bafe01b60d63afe7ce253 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Mon, 13 Feb 2023 12:11:58 -0500 Subject: [PATCH] Specify the parent viewgroup that should hide controls Without this, launching ControlsActivtiy while it's running may cause to two instances of it existing for a brief moment. As the new instance will call `show` before the old instance calls `hide`, this will cause everything to be hidden (and all listeners removed). Instead, specify what parent view is calling hide. This makes it so we can decide whether to completely hide `ControlsUiControllerImpl` or just remove the views from the old parent. Test: atest ControlsUiControllerImplTest Test: manual, launch from LS and then from QS Test: heapdump: old ControlsActivity is released Bug: 267949140 Change-Id: I86a323cf25e5da81e610a21c4426872ef8684480 --- .../systemui/controls/ui/ControlsActivity.kt | 3 +- .../controls/ui/ControlsUiController.kt | 8 ++++- .../controls/ui/ControlsUiControllerImpl.kt | 33 ++++++++++++------- .../ui/ControlsUiControllerImplTest.kt | 25 +++++++++++++- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsActivity.kt b/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsActivity.kt index d8d8c0ead06a8..3a3f9b4e52656 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsActivity.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsActivity.kt @@ -134,7 +134,8 @@ class ControlsActivity @Inject constructor( super.onStop() mExitToDream = false - uiController.hide() + // parent is set in onStart, so the field is initialized when we get here + uiController.hide(parent) controlsSettingsDialogManager.closeDialog() } diff --git a/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiController.kt b/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiController.kt index c1cec9dd0f943..58673bb6f567b 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiController.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiController.kt @@ -31,7 +31,13 @@ interface ControlsUiController { } fun show(parent: ViewGroup, onDismiss: Runnable, activityContext: Context) - fun hide() + + /** + * Hide the controls content if it's attached to this parent. + */ + fun hide(parent: ViewGroup) + + val isShowing: Boolean /** * Returns the preferred activity to start, depending on if the user has favorited any diff --git a/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiControllerImpl.kt index 58f4835a01ee6..9405c602caf77 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiControllerImpl.kt @@ -168,6 +168,9 @@ class ControlsUiControllerImpl @Inject constructor ( private lateinit var activityContext: Context private lateinit var listingCallback: ControlsListingController.ControlsListingCallback + override val isShowing: Boolean + get() = !hidden + init { dumpManager.registerDumpable(javaClass.name, this) } @@ -727,21 +730,27 @@ class ControlsUiControllerImpl @Inject constructor ( controlActionCoordinator.closeDialogs() } - override fun hide() { - hidden = true + override fun hide(parent: ViewGroup) { + // We need to check for the parent because it's possible that we have started showing in a + // different activity. In that case, make sure to only clear things associated with the + // passed parent + if (parent == this.parent) { + Log.d(ControlsUiController.TAG, "hide()") + hidden = true - closeDialogs(true) - controlsController.get().unsubscribe() - taskViewController?.dismiss() - taskViewController = null + closeDialogs(true) + controlsController.get().unsubscribe() + taskViewController?.dismiss() + taskViewController = null + controlsById.clear() + controlViewsById.clear() + + controlsListingController.get().removeCallback(listingCallback) + + if (!retainCache) RenderInfo.clearCache() + } parent.removeAllViews() - controlsById.clear() - controlViewsById.clear() - - controlsListingController.get().removeCallback(listingCallback) - - if (!retainCache) RenderInfo.clearCache() } override fun onRefreshState(componentName: ComponentName, controls: List) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/controls/ui/ControlsUiControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/controls/ui/ControlsUiControllerImplTest.kt index 85f9961bf449e..aa90e2a45f107 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/controls/ui/ControlsUiControllerImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/controls/ui/ControlsUiControllerImplTest.kt @@ -29,6 +29,7 @@ import android.testing.TestableLooper import android.util.AttributeSet import android.view.LayoutInflater import android.view.View +import android.view.ViewGroup import android.widget.FrameLayout import androidx.test.filters.SmallTest import com.android.systemui.R @@ -328,7 +329,7 @@ class ControlsUiControllerImplTest : SysuiTestCase() { ) .isTrue() - underTest.hide() + underTest.hide(parent) clearInvocations(controlsListingController, taskViewFactory) controlsSettingsRepository.setAllowActionOnTrivialControlsInLockscreen(false) @@ -387,6 +388,28 @@ class ControlsUiControllerImplTest : SysuiTestCase() { assertThat(underTest.resolveActivity()).isEqualTo(ControlsActivity::class.java) } + @Test + fun testRemoveViewsOnlyForParentPassedInHide() { + underTest.show(parent, {}, context) + parent.addView(View(context)) + + val mockParent: ViewGroup = mock() + + underTest.hide(mockParent) + + verify(mockParent).removeAllViews() + assertThat(parent.childCount).isGreaterThan(0) + } + + @Test + fun testHideDifferentParentDoesntCancelListeners() { + underTest.show(parent, {}, context) + underTest.hide(mock()) + + verify(controlsController, never()).unsubscribe() + verify(controlsListingController, never()).removeCallback(any()) + } + private fun setUpPanel(panel: SelectedItem.PanelItem): ControlsServiceInfo { val activity = ComponentName("pkg", "activity") sharedPreferences