From 04f83eb5f059b1bda0f1025d2445d3f01f110318 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Tue, 22 Jan 2019 10:38:40 -0500 Subject: [PATCH] Convert PrivacyItemController to Dependency This makes sure that PIC is a Singleton and prevents memory leak. Bug: 121388507 Test: atest PrivacyItemControllerTest Change-Id: Ib5c2a8790157034e1937c8037650ac047478d005 --- .../src/com/android/systemui/Dependency.java | 4 + .../systemui/privacy/PrivacyItemController.kt | 48 ++++++++++-- .../systemui/qs/QuickStatusBarHeader.java | 7 +- .../statusbar/phone/PhoneStatusBarPolicy.java | 6 +- .../privacy/PrivacyItemControllerTest.kt | 76 +++++++++++++++++-- 5 files changed, 123 insertions(+), 18 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/Dependency.java b/packages/SystemUI/src/com/android/systemui/Dependency.java index d99f234c26c82..fece94e69a3d4 100644 --- a/packages/SystemUI/src/com/android/systemui/Dependency.java +++ b/packages/SystemUI/src/com/android/systemui/Dependency.java @@ -44,6 +44,7 @@ import com.android.systemui.plugins.PluginDependencyProvider; import com.android.systemui.plugins.VolumeDialogController; import com.android.systemui.power.EnhancedEstimates; import com.android.systemui.power.PowerUI; +import com.android.systemui.privacy.PrivacyItemController; import com.android.systemui.recents.OverviewProxyService; import com.android.systemui.shared.plugins.PluginManager; import com.android.systemui.statusbar.AmbientPulseManager; @@ -278,6 +279,7 @@ public class Dependency extends SystemUI { @Inject Lazy mSensorPrivacyManager; @Inject Lazy mAutoHideController; @Inject Lazy mForegroundServiceNotificationListener; + @Inject Lazy mPrivacyItemController; @Inject @Named(BG_LOOPER_NAME) Lazy mBgLooper; @Inject @Named(BG_HANDLER_NAME) Lazy mBgHandler; @Inject @Named(MAIN_HANDLER_NAME) Lazy mMainHandler; @@ -452,6 +454,8 @@ public class Dependency extends SystemUI { mProviders.put(ForegroundServiceNotificationListener.class, mForegroundServiceNotificationListener::get); mProviders.put(ClockManager.class, mClockManager::get); + mProviders.put(PrivacyItemController.class, mPrivacyItemController::get); + // TODO(b/118592525): to support multi-display , we start to add something which is // per-display, while others may be global. I think it's time to add diff --git a/packages/SystemUI/src/com/android/systemui/privacy/PrivacyItemController.kt b/packages/SystemUI/src/com/android/systemui/privacy/PrivacyItemController.kt index b218e80693b13..2339fae2d1ee8 100644 --- a/packages/SystemUI/src/com/android/systemui/privacy/PrivacyItemController.kt +++ b/packages/SystemUI/src/com/android/systemui/privacy/PrivacyItemController.kt @@ -30,8 +30,12 @@ import com.android.systemui.Dependency import com.android.systemui.appops.AppOpItem import com.android.systemui.appops.AppOpsController import com.android.systemui.R +import java.lang.ref.WeakReference +import javax.inject.Inject +import javax.inject.Singleton -class PrivacyItemController(val context: Context, val callback: Callback) { +@Singleton +class PrivacyItemController @Inject constructor(val context: Context) { companion object { val OPS = intArrayOf(AppOpsManager.OP_CAMERA, @@ -56,9 +60,10 @@ class PrivacyItemController(val context: Context, val callback: Callback) { private val uiHandler = Dependency.get(Dependency.MAIN_HANDLER) private var listening = false val systemApp = PrivacyApplication(context.getString(R.string.device_services), context) + private val callbacks = mutableListOf>() private val notifyChanges = Runnable { - callback.privacyChanged(privacyList) + callbacks.forEach { it.get()?.privacyChanged(privacyList) } } private val updateListAndNotifyChanges = Runnable { @@ -88,8 +93,8 @@ class PrivacyItemController(val context: Context, val callback: Callback) { registerReceiver() } - init { - registerReceiver() + private fun unregisterReceiver() { + context.unregisterReceiver(userSwitcherReceiver) } private fun registerReceiver() { @@ -108,17 +113,41 @@ class PrivacyItemController(val context: Context, val callback: Callback) { bgHandler.post(updateListAndNotifyChanges) } - fun setListening(listen: Boolean) { + @VisibleForTesting + internal fun setListening(listen: Boolean) { if (listening == listen) return listening = listen if (listening) { appOpsController.addCallback(OPS, cb) + registerReceiver() update(true) } else { appOpsController.removeCallback(OPS, cb) + unregisterReceiver() } } + private fun addCallback(callback: WeakReference) { + callbacks.add(callback) + if (callbacks.isNotEmpty() && !listening) setListening(true) + // Notify this callback if we didn't set to listening + else uiHandler.post(NotifyChangesToCallback(callback.get(), privacyList)) + } + + private fun removeCallback(callback: WeakReference) { + // Removes also if the callback is null + callbacks.removeIf { it.get()?.equals(callback.get()) ?: true } + if (callbacks.isEmpty()) setListening(false) + } + + fun addCallback(callback: Callback) { + addCallback(WeakReference(callback)) + } + + fun removeCallback(callback: Callback) { + removeCallback(WeakReference(callback)) + } + private fun updatePrivacyList() { privacyList = currentUserIds.flatMap { appOpsController.getActiveAppOpsForUser(it) } .mapNotNull { toPrivacyItem(it) }.distinct() @@ -149,4 +178,13 @@ class PrivacyItemController(val context: Context, val callback: Callback) { } } } + + private class NotifyChangesToCallback( + private val callback: Callback?, + private val list: List + ) : Runnable { + override fun run() { + callback?.privacyChanged(list) + } + } } \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeader.java b/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeader.java index 75ab5dfd2977d..2d64ecd99c880 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeader.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeader.java @@ -180,14 +180,14 @@ public class QuickStatusBarHeader extends RelativeLayout implements public QuickStatusBarHeader(@Named(VIEW_CONTEXT) Context context, AttributeSet attrs, NextAlarmController nextAlarmController, ZenModeController zenModeController, BatteryController batteryController, StatusBarIconController statusBarIconController, - ActivityStarter activityStarter) { + ActivityStarter activityStarter, PrivacyItemController privacyItemController) { super(context, attrs); mAlarmController = nextAlarmController; mZenController = zenModeController; mBatteryController = batteryController; mStatusBarIconController = statusBarIconController; mActivityStarter = activityStarter; - mPrivacyItemController = new PrivacyItemController(context, mPICCallback); + mPrivacyItemController = privacyItemController; mShownCount = getStoredShownCount(); } @@ -512,7 +512,6 @@ public class QuickStatusBarHeader extends RelativeLayout implements return; } mHeaderQsPanel.setListening(listening); - mPrivacyItemController.setListening(listening); mListening = listening; if (listening) { @@ -520,9 +519,11 @@ public class QuickStatusBarHeader extends RelativeLayout implements mAlarmController.addCallback(this); mContext.registerReceiver(mRingerReceiver, new IntentFilter(AudioManager.INTERNAL_RINGER_MODE_CHANGED_ACTION)); + mPrivacyItemController.addCallback(mPICCallback); } else { mZenController.removeCallback(this); mAlarmController.removeCallback(this); + mPrivacyItemController.removeCallback(mPICCallback); mContext.unregisterReceiver(mRingerReceiver); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java index 43c35f11d9e56..18711c0d1ae31 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBarPolicy.java @@ -173,7 +173,7 @@ public class PhoneStatusBarPolicy implements Callback, Callbacks, mProvisionedController = Dependency.get(DeviceProvisionedController.class); mKeyguardMonitor = Dependency.get(KeyguardMonitor.class); mLocationController = Dependency.get(LocationController.class); - mPrivacyItemController = new PrivacyItemController(mContext, this); + mPrivacyItemController = Dependency.get(PrivacyItemController.class); mSlotCast = context.getString(com.android.internal.R.string.status_bar_cast); mSlotHotspot = context.getString(com.android.internal.R.string.status_bar_hotspot); @@ -266,7 +266,7 @@ public class PhoneStatusBarPolicy implements Callback, Callbacks, mNextAlarmController.addCallback(mNextAlarmCallback); mDataSaver.addCallback(this); mKeyguardMonitor.addCallback(this); - mPrivacyItemController.setListening(true); + mPrivacyItemController.addCallback(this); SysUiServiceProvider.getComponent(mContext, CommandQueue.class).addCallback(this); ActivityManagerWrapper.getInstance().registerTaskStackListener(mTaskListener); @@ -294,7 +294,7 @@ public class PhoneStatusBarPolicy implements Callback, Callbacks, mNextAlarmController.removeCallback(mNextAlarmCallback); mDataSaver.removeCallback(this); mKeyguardMonitor.removeCallback(this); - mPrivacyItemController.setListening(false); + mPrivacyItemController.removeCallback(this); SysUiServiceProvider.getComponent(mContext, CommandQueue.class).removeCallback(this); mContext.unregisterReceiver(mIntentReceiver); diff --git a/packages/SystemUI/tests/src/com/android/systemui/privacy/PrivacyItemControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/privacy/PrivacyItemControllerTest.kt index e6d7ee7407d4a..98bf3c2743a8f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/privacy/PrivacyItemControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/privacy/PrivacyItemControllerTest.kt @@ -45,9 +45,12 @@ import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito.atLeastOnce import org.mockito.Mockito.doReturn +import org.mockito.Mockito.mock import org.mockito.Mockito.never +import org.mockito.Mockito.reset import org.mockito.Mockito.spy import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoMoreInteractions import org.mockito.MockitoAnnotations @RunWith(AndroidTestingRunner::class) @@ -73,6 +76,8 @@ class PrivacyItemControllerTest : SysuiTestCase() { private lateinit var userManager: UserManager @Captor private lateinit var argCaptor: ArgumentCaptor> + @Captor + private lateinit var argCaptorCallback: ArgumentCaptor private lateinit var testableLooper: TestableLooper private lateinit var privacyItemController: PrivacyItemController @@ -95,7 +100,16 @@ class PrivacyItemControllerTest : SysuiTestCase() { } })).`when`(userManager).getProfiles(anyInt()) - privacyItemController = PrivacyItemController(mContext, callback) + privacyItemController = PrivacyItemController(mContext) + } + + @Test + fun testSetListeningTrueByAddingCallback() { + privacyItemController.addCallback(callback) + verify(appOpsController).addCallback(eq(PrivacyItemController.OPS), + any(AppOpsController.Callback::class.java)) + testableLooper.processAllMessages() + verify(callback).privacyChanged(anyList()) } @Test @@ -103,8 +117,6 @@ class PrivacyItemControllerTest : SysuiTestCase() { privacyItemController.setListening(true) verify(appOpsController).addCallback(eq(PrivacyItemController.OPS), any(AppOpsController.Callback::class.java)) - testableLooper.processAllMessages() - verify(callback).privacyChanged(anyList()) } @Test @@ -121,7 +133,7 @@ class PrivacyItemControllerTest : SysuiTestCase() { AppOpItem(AppOpsManager.OP_CAMERA, TEST_UID, "", 1))) .`when`(appOpsController).getActiveAppOpsForUser(anyInt()) - privacyItemController.setListening(true) + privacyItemController.addCallback(callback) testableLooper.processAllMessages() verify(callback).privacyChanged(capture(argCaptor)) assertEquals(1, argCaptor.value.size) @@ -131,7 +143,7 @@ class PrivacyItemControllerTest : SysuiTestCase() { fun testSystemApps() { doReturn(listOf(AppOpItem(AppOpsManager.OP_COARSE_LOCATION, SYSTEM_UID, TEST_PACKAGE_NAME, 0))).`when`(appOpsController).getActiveAppOpsForUser(anyInt()) - privacyItemController.setListening(true) + privacyItemController.addCallback(callback) testableLooper.processAllMessages() verify(callback).privacyChanged(capture(argCaptor)) assertEquals(1, argCaptor.value.size) @@ -142,8 +154,8 @@ class PrivacyItemControllerTest : SysuiTestCase() { @Test fun testRegisterReceiver_allUsers() { val spiedContext = spy(mContext) - val itemController = PrivacyItemController(spiedContext, callback) - + val itemController = PrivacyItemController(spiedContext) + itemController.setListening(true) verify(spiedContext, atLeastOnce()).registerReceiverAsUser( eq(itemController.userSwitcherReceiver), eq(UserHandle.ALL), any(), eq(null), eq(null)) @@ -170,4 +182,54 @@ class PrivacyItemControllerTest : SysuiTestCase() { Intent(Intent.ACTION_MANAGED_PROFILE_REMOVED)) verify(userManager).getProfiles(anyInt()) } + + @Test + fun testAddMultipleCallbacks() { + val otherCallback = mock(PrivacyItemController.Callback::class.java) + privacyItemController.addCallback(callback) + testableLooper.processAllMessages() + verify(callback).privacyChanged(anyList()) + + privacyItemController.addCallback(otherCallback) + testableLooper.processAllMessages() + verify(otherCallback).privacyChanged(anyList()) + // Adding a callback should not unnecessarily call previous ones + verifyNoMoreInteractions(callback) + } + + @Test + fun testMultipleCallbacksAreUpdated() { + doReturn(emptyList()).`when`(appOpsController).getActiveAppOpsForUser(anyInt()) + + val otherCallback = mock(PrivacyItemController.Callback::class.java) + privacyItemController.addCallback(callback) + privacyItemController.addCallback(otherCallback) + testableLooper.processAllMessages() + reset(callback) + reset(otherCallback) + + verify(appOpsController).addCallback(any(), capture(argCaptorCallback)) + argCaptorCallback.value.onActiveStateChanged(0, TEST_UID, "", true) + testableLooper.processAllMessages() + verify(callback).privacyChanged(anyList()) + verify(otherCallback).privacyChanged(anyList()) + } + + @Test + fun testRemoveCallback() { + doReturn(emptyList()).`when`(appOpsController).getActiveAppOpsForUser(anyInt()) + val otherCallback = mock(PrivacyItemController.Callback::class.java) + privacyItemController.addCallback(callback) + privacyItemController.addCallback(otherCallback) + testableLooper.processAllMessages() + reset(callback) + reset(otherCallback) + + verify(appOpsController).addCallback(any(), capture(argCaptorCallback)) + privacyItemController.removeCallback(callback) + argCaptorCallback.value.onActiveStateChanged(0, TEST_UID, "", true) + testableLooper.processAllMessages() + verify(callback, never()).privacyChanged(anyList()) + verify(otherCallback).privacyChanged(anyList()) + } } \ No newline at end of file