From 84371de99ddf1f8ea8b427dbe6a6e8c220990fcd Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Thu, 27 Feb 2020 10:58:25 -0500 Subject: [PATCH] Remove favorites when app is uninstalled Whenever an ControlsProviderService is removed, its favorites are removed and the new set of favorites are stored. Test: atest ControlsControllerImplTest ControlsBindingControllerImplTest Test: manual remove and update package Fixes: 150137085 Change-Id: I64aba1f2513bbd5d1b171ece036555a530e80939 --- .../controller/ControlsBindingController.kt | 5 ++ .../ControlsBindingControllerImpl.kt | 12 ++++ .../controller/ControlsControllerImpl.kt | 31 ++++++++ .../controls/management/AppAdapter.kt | 3 +- .../management/ControlsListingController.kt | 6 +- .../ControlsListingControllerImpl.kt | 2 +- .../controls/ui/ControlsUiControllerImpl.kt | 3 +- .../ControlsBindingControllerImplTest.kt | 19 +++++ .../controller/ControlsControllerImplTest.kt | 70 +++++++++++++++++++ 9 files changed, 145 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt index 7fae6a3249c65..07488ad836ef9 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt @@ -72,6 +72,11 @@ interface ControlsBindingController : UserAwareController { */ fun unsubscribe() + /** + * Notify this controller that this component has been removed (uninstalled). + */ + fun onComponentRemoved(componentName: ComponentName) + /** * Consumer for load calls. * diff --git a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt index 6ff1bbce672f5..2ed8652330706 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt @@ -186,6 +186,18 @@ open class ControlsBindingControllerImpl @Inject constructor( } } + override fun onComponentRemoved(componentName: ComponentName) { + backgroundExecutor.execute { + synchronized(componentMap) { + val removed = componentMap.remove(Key(componentName, currentUser)) + removed?.let { + it.unbindService() + tokenMap.remove(it.token) + } + } + } + } + override fun toString(): String { return StringBuilder(" ControlsBindingController:\n").apply { append(" refreshing=${refreshing.get()}\n") diff --git a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt index bb665fe3113b6..f4301a0d000a7 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt @@ -37,6 +37,7 @@ import com.android.internal.annotations.VisibleForTesting import com.android.systemui.Dumpable import com.android.systemui.broadcast.BroadcastDispatcher import com.android.systemui.controls.ControlStatus +import com.android.systemui.controls.ControlsServiceInfo import com.android.systemui.controls.management.ControlsListingController import com.android.systemui.controls.ui.ControlsUiController import com.android.systemui.dagger.qualifiers.Background @@ -122,6 +123,7 @@ class ControlsControllerImpl @Inject constructor ( override fun onReceive(context: Context, intent: Intent) { if (intent.action == Intent.ACTION_USER_SWITCHED) { userChanging = true + listingController.removeCallback(listingCallback) val newUser = UserHandle.of(intent.getIntExtra(Intent.EXTRA_USER_HANDLE, sendingUserId)) if (currentUser == newUser) { @@ -152,6 +154,34 @@ class ControlsControllerImpl @Inject constructor ( } } + // Handling of removed components + + /** + * Check if any component has been removed and if so, remove all its favorites. + * + * If some component has been removed, the new set of favorites will also be saved. + */ + private val listingCallback = object : ControlsListingController.ControlsListingCallback { + override fun onServicesUpdated(candidates: List) { + executor.execute { + val candidateComponents = candidates.map(ControlsServiceInfo::componentName) + synchronized(currentFavorites) { + val components = currentFavorites.keys.toSet() // create a copy + components.forEach { + if (it !in candidateComponents) { + currentFavorites.remove(it) + bindingController.onComponentRemoved(it) + } + } + // Check if something has been removed, if so, store the new list + if (components.size > currentFavorites.size) { + persistenceWrapper.storeFavorites(favoritesAsListLocked()) + } + } + } + } + } + init { dumpManager.registerDumpable(javaClass.name, this) if (available) { @@ -186,6 +216,7 @@ class ControlsControllerImpl @Inject constructor ( currentFavorites.getOrPut(it.component, { mutableListOf() }).add(it) } } + listingController.addCallback(listingCallback) } override fun loadForComponent( diff --git a/packages/SystemUI/src/com/android/systemui/controls/management/AppAdapter.kt b/packages/SystemUI/src/com/android/systemui/controls/management/AppAdapter.kt index ac5e0893b526a..8f5d511a82b49 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/management/AppAdapter.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/management/AppAdapter.kt @@ -29,6 +29,7 @@ import androidx.recyclerview.widget.RecyclerView import com.android.settingslib.applications.DefaultAppInfo import com.android.settingslib.widget.CandidateInfo import com.android.systemui.R +import com.android.systemui.controls.ControlsServiceInfo import java.text.Collator import java.util.concurrent.Executor @@ -58,7 +59,7 @@ class AppAdapter( private var listOfServices = emptyList() private val callback = object : ControlsListingController.ControlsListingCallback { - override fun onServicesUpdated(candidates: List) { + override fun onServicesUpdated(candidates: List) { backgroundExecutor.execute { val collator = Collator.getInstance(resources.configuration.locales[0]) val localeComparator = compareBy(collator) { diff --git a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingController.kt b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingController.kt index 8e47f6466955c..efbe7c0bdb382 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingController.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingController.kt @@ -17,7 +17,7 @@ package com.android.systemui.controls.management import android.content.ComponentName -import com.android.settingslib.widget.CandidateInfo +import com.android.systemui.controls.ControlsServiceInfo import com.android.systemui.controls.UserAwareController import com.android.systemui.statusbar.policy.CallbackController @@ -31,7 +31,7 @@ interface ControlsListingController : /** * @return the current list of services that satisfies the [ServiceListing]. */ - fun getCurrentServices(): List + fun getCurrentServices(): List /** * Get the app label for a given component. @@ -45,6 +45,6 @@ interface ControlsListingController : @FunctionalInterface interface ControlsListingCallback { - fun onServicesUpdated(candidates: List) + fun onServicesUpdated(candidates: List) } } diff --git a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingControllerImpl.kt index 882382cc4ade7..0ed7c069a1dd9 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsListingControllerImpl.kt @@ -146,7 +146,7 @@ class ControlsListingControllerImpl @VisibleForTesting constructor( * @return a list of components that satisfy the requirements to be a * [ControlsProviderService] */ - override fun getCurrentServices(): List = + override fun getCurrentServices(): List = availableServices.map { ControlsServiceInfo(context, it) } /** 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 f4fd37557fa47..8d01bceb524d0 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/ui/ControlsUiControllerImpl.kt @@ -42,6 +42,7 @@ import com.android.systemui.controls.management.ControlsProviderSelectorActivity import com.android.systemui.dagger.qualifiers.Background import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.R +import com.android.systemui.controls.ControlsServiceInfo import com.android.systemui.util.concurrency.DelayableExecutor import dagger.Lazy @@ -128,7 +129,7 @@ class ControlsUiControllerImpl @Inject constructor ( get() = controlsController.get().available private val listingCallback = object : ControlsListingController.ControlsListingCallback { - override fun onServicesUpdated(candidates: List) { + override fun onServicesUpdated(candidates: List) { bgExecutor.execute { val collator = Collator.getInstance(context.resources.configuration.locales[0]) val localeComparator = compareBy(collator) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt index 02bfc19e07d4f..007acb678c612 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt @@ -40,6 +40,7 @@ import org.mockito.Mockito.`when` import org.mockito.Mockito.mock import org.mockito.Mockito.never import org.mockito.Mockito.reset +import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations @@ -194,6 +195,24 @@ class ControlsBindingControllerImplTest : SysuiTestCase() { verify(provider2).unbindService() verify(provider1, never()).unbindService() } + + @Test + fun testComponentRemoved_existingIsUnbound() { + controller.bindServices(listOf( + TEST_COMPONENT_NAME_1, + TEST_COMPONENT_NAME_2, + TEST_COMPONENT_NAME_3 + )) + + controller.onComponentRemoved(TEST_COMPONENT_NAME_2) + + executor.runAllReady() + + providers.forEach { + verify(it, if (it.componentName == TEST_COMPONENT_NAME_2) times(1) else never()) + .unbindService() + } + } } class TestableControlsBindingControllerImpl( diff --git a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt index 488e418f61521..e302ca4aad337 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt @@ -22,6 +22,7 @@ import android.content.ComponentName import android.content.Context import android.content.ContextWrapper import android.content.Intent +import android.content.pm.ServiceInfo import android.os.UserHandle import android.provider.Settings import android.service.controls.Control @@ -32,6 +33,7 @@ import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.broadcast.BroadcastDispatcher import com.android.systemui.controls.ControlStatus +import com.android.systemui.controls.ControlsServiceInfo import com.android.systemui.controls.management.ControlsListingController import com.android.systemui.controls.ui.ControlsUiController import com.android.systemui.dump.DumpManager @@ -50,6 +52,7 @@ import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito import org.mockito.Mockito.`when` +import org.mockito.Mockito.inOrder import org.mockito.Mockito.mock import org.mockito.Mockito.never import org.mockito.Mockito.reset @@ -82,6 +85,9 @@ class ControlsControllerImplTest : SysuiTestCase() { ArgumentCaptor @Captor private lateinit var broadcastReceiverCaptor: ArgumentCaptor + @Captor + private lateinit var listingCallbackCaptor: + ArgumentCaptor private lateinit var delayableExecutor: FakeExecutor private lateinit var controller: ControlsControllerImpl @@ -139,6 +145,7 @@ class ControlsControllerImplTest : SysuiTestCase() { assertTrue(controller.available) verify(broadcastDispatcher).registerReceiver( capture(broadcastReceiverCaptor), any(), any(), eq(UserHandle.ALL)) + verify(listingController).addCallback(capture(listingCallbackCaptor)) } private fun builderFromInfo(controlInfo: ControlInfo): Control.StatelessBuilder { @@ -606,4 +613,67 @@ class ControlsControllerImplTest : SysuiTestCase() { assertEquals(2, controller.countFavoritesForComponent(TEST_COMPONENT)) assertEquals(listOrder2, controller.getFavoritesForComponent(TEST_COMPONENT)) } + + @Test + fun testPackageRemoved_noFavorites_noRemovals() { + controller.changeFavoriteStatus(TEST_CONTROL_INFO, true) + + val serviceInfo = mock(ServiceInfo::class.java) + `when`(serviceInfo.componentName).thenReturn(TEST_COMPONENT) + val info = ControlsServiceInfo(mContext, serviceInfo) + + // Don't want to check what happens before this call + reset(persistenceWrapper) + listingCallbackCaptor.value.onServicesUpdated(listOf(info)) + + delayableExecutor.runAllReady() + + verify(bindingController, never()).onComponentRemoved(any()) + + assertEquals(1, controller.getFavoriteControls().size) + assertEquals(TEST_CONTROL_INFO, controller.getFavoriteControls()[0]) + + verify(persistenceWrapper, never()).storeFavorites(ArgumentMatchers.anyList()) + } + + @Test + fun testPackageRemoved_hasFavorites() { + controller.changeFavoriteStatus(TEST_CONTROL_INFO, true) + controller.changeFavoriteStatus(TEST_CONTROL_INFO_2, true) + + val serviceInfo = mock(ServiceInfo::class.java) + `when`(serviceInfo.componentName).thenReturn(TEST_COMPONENT) + val info = ControlsServiceInfo(mContext, serviceInfo) + + // Don't want to check what happens before this call + reset(persistenceWrapper) + listingCallbackCaptor.value.onServicesUpdated(listOf(info)) + + delayableExecutor.runAllReady() + + verify(bindingController).onComponentRemoved(TEST_COMPONENT_2) + + assertEquals(1, controller.getFavoriteControls().size) + assertEquals(TEST_CONTROL_INFO, controller.getFavoriteControls()[0]) + + verify(persistenceWrapper).storeFavorites(ArgumentMatchers.anyList()) + } + + @Test + fun testListingCallbackNotListeningWhileReadingFavorites() { + val intent = Intent(Intent.ACTION_USER_SWITCHED).apply { + putExtra(Intent.EXTRA_USER_HANDLE, otherUser) + } + val pendingResult = mock(BroadcastReceiver.PendingResult::class.java) + `when`(pendingResult.sendingUserId).thenReturn(otherUser) + broadcastReceiverCaptor.value.pendingResult = pendingResult + + broadcastReceiverCaptor.value.onReceive(mContext, intent) + + val inOrder = inOrder(persistenceWrapper, listingController) + + inOrder.verify(listingController).removeCallback(listingCallbackCaptor.value) + inOrder.verify(persistenceWrapper).readFavorites() + inOrder.verify(listingController).addCallback(listingCallbackCaptor.value) + } }