From 730e06c5ac0086e0bc791a669a581f3fc5cb1c6b Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Wed, 16 Jun 2021 11:54:57 -0400 Subject: [PATCH] Remove listeners when destroying GlobalActionsDialog. Fixes memory leaks. Bug: 191150828 Test: atest SystemUITests Change-Id: I04c2acf528c48eba17a828017de5f70424c09e51 --- .../globalactions/GlobalActionsDialog.java | 128 ++++++++++++------ .../GlobalActionsDialogLite.java | 62 +++++---- .../globalactions/GlobalActionsImpl.java | 14 +- .../systemui/qs/QSFooterViewController.java | 2 +- .../GlobalActionsDialogLiteTest.java | 5 + .../GlobalActionsDialogTest.java | 5 + 6 files changed, 144 insertions(+), 72 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java index 9577aa0828580..4f5d71a3cb62a 100644 --- a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java +++ b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialog.java @@ -30,6 +30,7 @@ import android.app.admin.DevicePolicyManager; import android.app.trust.TrustManager; import android.content.Context; import android.content.DialogInterface; +import android.content.pm.PackageManager; import android.content.res.Resources; import android.database.ContentObserver; import android.graphics.drawable.Drawable; @@ -113,47 +114,100 @@ public class GlobalActionsDialog extends GlobalActionsDialogLite @VisibleForTesting boolean mShowLockScreenCards = false; + private final KeyguardStateController.Callback mKeyguardStateControllerListener = + new KeyguardStateController.Callback() { + @Override + public void onUnlockedChanged() { + if (mDialog != null) { + ActionsDialog dialog = (ActionsDialog) mDialog; + boolean unlocked = mKeyguardStateController.isUnlocked(); + if (dialog.mWalletViewController != null) { + dialog.mWalletViewController.onDeviceLockStateChanged(!unlocked); + } + + if (unlocked) { + dialog.hideLockMessage(); + } + } + } + }; + + private final ContentObserver mSettingsObserver = new ContentObserver(mMainHandler) { + @Override + public void onChange(boolean selfChange) { + onPowerMenuLockScreenSettingsChanged(); + } + }; + /** * @param context everything needs a context :( */ @Inject - public GlobalActionsDialog(Context context, GlobalActionsManager windowManagerFuncs, - AudioManager audioManager, IDreamManager iDreamManager, - DevicePolicyManager devicePolicyManager, LockPatternUtils lockPatternUtils, + public GlobalActionsDialog( + Context context, + GlobalActionsManager windowManagerFuncs, + AudioManager audioManager, + IDreamManager iDreamManager, + DevicePolicyManager devicePolicyManager, + LockPatternUtils lockPatternUtils, BroadcastDispatcher broadcastDispatcher, TelephonyListenerManager telephonyListenerManager, - GlobalSettings globalSettings, SecureSettings secureSettings, - @Nullable Vibrator vibrator, @Main Resources resources, - ConfigurationController configurationController, ActivityStarter activityStarter, - KeyguardStateController keyguardStateController, UserManager userManager, - TrustManager trustManager, IActivityManager iActivityManager, - @Nullable TelecomManager telecomManager, MetricsLogger metricsLogger, - NotificationShadeDepthController depthController, SysuiColorExtractor colorExtractor, + GlobalSettings globalSettings, + SecureSettings secureSettings, + @Nullable Vibrator vibrator, + @Main Resources resources, + ConfigurationController configurationController, + ActivityStarter activityStarter, + KeyguardStateController keyguardStateController, + UserManager userManager, + TrustManager trustManager, + IActivityManager iActivityManager, + @Nullable TelecomManager telecomManager, + MetricsLogger metricsLogger, + NotificationShadeDepthController depthController, + SysuiColorExtractor colorExtractor, IStatusBarService statusBarService, NotificationShadeWindowController notificationShadeWindowController, IWindowManager iWindowManager, @Background Executor backgroundExecutor, UiEventLogger uiEventLogger, - RingerModeTracker ringerModeTracker, SysUiState sysUiState, @Main Handler handler, + RingerModeTracker ringerModeTracker, + SysUiState sysUiState, + @Main Handler handler, + PackageManager packageManager, StatusBar statusBar) { - super(context, windowManagerFuncs, - audioManager, iDreamManager, - devicePolicyManager, lockPatternUtils, - broadcastDispatcher, telephonyListenerManager, - globalSettings, secureSettings, - vibrator, resources, + super(context, + windowManagerFuncs, + audioManager, + iDreamManager, + devicePolicyManager, + lockPatternUtils, + broadcastDispatcher, + telephonyListenerManager, + globalSettings, + secureSettings, + vibrator, + resources, configurationController, - keyguardStateController, userManager, - trustManager, iActivityManager, - telecomManager, metricsLogger, - depthController, colorExtractor, + keyguardStateController, + userManager, + trustManager, + iActivityManager, + telecomManager, + metricsLogger, + depthController, + colorExtractor, statusBarService, notificationShadeWindowController, iWindowManager, backgroundExecutor, uiEventLogger, - ringerModeTracker, sysUiState, handler, statusBar); + ringerModeTracker, + sysUiState, + handler, + packageManager, + statusBar); mLockPatternUtils = lockPatternUtils; mKeyguardStateController = keyguardStateController; @@ -163,34 +217,22 @@ public class GlobalActionsDialog extends GlobalActionsDialogLite mNotificationShadeWindowController = notificationShadeWindowController; mSysUiState = sysUiState; mActivityStarter = activityStarter; - keyguardStateController.addCallback(new KeyguardStateController.Callback() { - @Override - public void onUnlockedChanged() { - if (mDialog != null) { - ActionsDialog dialog = (ActionsDialog) mDialog; - boolean unlocked = mKeyguardStateController.isUnlocked(); - if (dialog.mWalletViewController != null) { - dialog.mWalletViewController.onDeviceLockStateChanged(!unlocked); - } - if (unlocked) { - dialog.hideLockMessage(); - } - } - } - }); + mKeyguardStateController.addCallback(mKeyguardStateControllerListener); // Listen for changes to show pay on the power menu while locked onPowerMenuLockScreenSettingsChanged(); mGlobalSettings.registerContentObserver( Settings.Secure.getUriFor(Settings.Secure.POWER_MENU_LOCKED_SHOW_CONTENT), false /* notifyForDescendants */, - new ContentObserver(handler) { - @Override - public void onChange(boolean selfChange) { - onPowerMenuLockScreenSettingsChanged(); - } - }); + mSettingsObserver); + } + + @Override + public void destroy() { + super.destroy(); + mKeyguardStateController.removeCallback(mKeyguardStateControllerListener); + mGlobalSettings.unregisterContentObserver(mSettingsObserver); } /** diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java index a0a5a6293d9c8..f1f7c71161555 100644 --- a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java +++ b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java @@ -177,6 +177,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene private final IDreamManager mDreamManager; private final DevicePolicyManager mDevicePolicyManager; private final LockPatternUtils mLockPatternUtils; + private final TelephonyListenerManager mTelephonyListenerManager; private final KeyguardStateController mKeyguardStateController; private final BroadcastDispatcher mBroadcastDispatcher; protected final GlobalSettings mGlobalSettings; @@ -307,24 +308,37 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene * @param context everything needs a context :( */ @Inject - public GlobalActionsDialogLite(Context context, GlobalActionsManager windowManagerFuncs, - AudioManager audioManager, IDreamManager iDreamManager, - DevicePolicyManager devicePolicyManager, LockPatternUtils lockPatternUtils, + public GlobalActionsDialogLite( + Context context, + GlobalActionsManager windowManagerFuncs, + AudioManager audioManager, + IDreamManager iDreamManager, + DevicePolicyManager devicePolicyManager, + LockPatternUtils lockPatternUtils, BroadcastDispatcher broadcastDispatcher, TelephonyListenerManager telephonyListenerManager, - GlobalSettings globalSettings, SecureSettings secureSettings, - @Nullable Vibrator vibrator, @Main Resources resources, + GlobalSettings globalSettings, + SecureSettings secureSettings, + @Nullable Vibrator vibrator, + @Main Resources resources, ConfigurationController configurationController, - KeyguardStateController keyguardStateController, UserManager userManager, - TrustManager trustManager, IActivityManager iActivityManager, - @Nullable TelecomManager telecomManager, MetricsLogger metricsLogger, - NotificationShadeDepthController depthController, SysuiColorExtractor colorExtractor, + KeyguardStateController keyguardStateController, + UserManager userManager, + TrustManager trustManager, + IActivityManager iActivityManager, + @Nullable TelecomManager telecomManager, + MetricsLogger metricsLogger, + NotificationShadeDepthController depthController, + SysuiColorExtractor colorExtractor, IStatusBarService statusBarService, NotificationShadeWindowController notificationShadeWindowController, IWindowManager iWindowManager, @Background Executor backgroundExecutor, UiEventLogger uiEventLogger, - RingerModeTracker ringerModeTracker, SysUiState sysUiState, @Main Handler handler, + RingerModeTracker ringerModeTracker, + SysUiState sysUiState, + @Main Handler handler, + PackageManager packageManager, StatusBar statusBar) { mContext = context; mWindowManagerFuncs = windowManagerFuncs; @@ -332,6 +346,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene mDreamManager = iDreamManager; mDevicePolicyManager = devicePolicyManager; mLockPatternUtils = lockPatternUtils; + mTelephonyListenerManager = telephonyListenerManager; mKeyguardStateController = keyguardStateController; mBroadcastDispatcher = broadcastDispatcher; mGlobalSettings = globalSettings; @@ -353,7 +368,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene mRingerModeTracker = ringerModeTracker; mSysUiState = sysUiState; mMainHandler = handler; - mSmallestScreenWidthDp = mContext.getResources().getConfiguration().smallestScreenWidthDp; + mSmallestScreenWidthDp = resources.getConfiguration().smallestScreenWidthDp; mStatusBar = statusBar; // receive broadcasts @@ -363,11 +378,10 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene filter.addAction(TelephonyManager.ACTION_EMERGENCY_CALLBACK_MODE_CHANGED); mBroadcastDispatcher.registerReceiver(mBroadcastReceiver, filter); - mHasTelephony = - context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_TELEPHONY); + mHasTelephony = packageManager.hasSystemFeature(PackageManager.FEATURE_TELEPHONY); // get notified of phone state changes - telephonyListenerManager.addServiceStateListener(mPhoneStateListener); + mTelephonyListenerManager.addServiceStateListener(mPhoneStateListener); mGlobalSettings.registerContentObserver( Settings.Global.getUriFor(Settings.Global.AIRPLANE_MODE_ON), true, mAirplaneModeObserver); @@ -387,6 +401,16 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene mConfigurationController.addCallback(this); } + /** + * Clean up callbacks + */ + public void destroy() { + mBroadcastDispatcher.unregisterReceiver(mBroadcastReceiver); + mTelephonyListenerManager.removeServiceStateListener(mPhoneStateListener); + mGlobalSettings.unregisterContentObserver(mAirplaneModeObserver); + mConfigurationController.removeCallback(this); + } + protected Context getContext() { return mContext; } @@ -686,14 +710,6 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene mDialog.refreshDialog(); } } - - /** - * Clean up callbacks - */ - public void destroy() { - mConfigurationController.removeCallback(this); - } - /** * Implements {@link GlobalActionsPanelPlugin.Callbacks#dismissGlobalActionsMenu()}, which is * called when the quick access wallet requests dismissal. @@ -2015,7 +2031,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene } }; - private ContentObserver mAirplaneModeObserver = new ContentObserver(mMainHandler) { + private final ContentObserver mAirplaneModeObserver = new ContentObserver(mMainHandler) { @Override public void onChange(boolean selfChange) { onAirplaneModeChanged(); diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsImpl.java b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsImpl.java index 178a74cecc2ec..e37d3d586ccc9 100644 --- a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsImpl.java +++ b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsImpl.java @@ -32,7 +32,6 @@ import android.widget.TextView; import com.android.internal.R; import com.android.keyguard.KeyguardUpdateMonitor; import com.android.settingslib.Utils; -import com.android.systemui.Dependency; import com.android.systemui.plugins.GlobalActions; import com.android.systemui.scrim.ScrimDrawable; import com.android.systemui.statusbar.BlurUtils; @@ -52,19 +51,24 @@ public class GlobalActionsImpl implements GlobalActions, CommandQueue.Callbacks private final KeyguardStateController mKeyguardStateController; private final DeviceProvisionedController mDeviceProvisionedController; private final BlurUtils mBlurUtils; + private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; private final CommandQueue mCommandQueue; private GlobalActionsDialogLite mGlobalActionsDialog; private boolean mDisabled; @Inject public GlobalActionsImpl(Context context, CommandQueue commandQueue, - Lazy globalActionsDialogLazy, BlurUtils blurUtils) { + Lazy globalActionsDialogLazy, BlurUtils blurUtils, + KeyguardStateController keyguardStateController, + DeviceProvisionedController deviceProvisionedController, + KeyguardUpdateMonitor keyguardUpdateMonitor) { mContext = context; mGlobalActionsDialogLazy = globalActionsDialogLazy; - mKeyguardStateController = Dependency.get(KeyguardStateController.class); - mDeviceProvisionedController = Dependency.get(DeviceProvisionedController.class); + mKeyguardStateController = keyguardStateController; + mDeviceProvisionedController = deviceProvisionedController; mCommandQueue = commandQueue; mBlurUtils = blurUtils; + mKeyguardUpdateMonitor = keyguardUpdateMonitor; mCommandQueue.addCallback(this); } @@ -83,7 +87,7 @@ public class GlobalActionsImpl implements GlobalActions, CommandQueue.Callbacks mGlobalActionsDialog = mGlobalActionsDialogLazy.get(); mGlobalActionsDialog.showOrHideDialog(mKeyguardStateController.isShowing(), mDeviceProvisionedController.isDeviceProvisioned()); - Dependency.get(KeyguardUpdateMonitor.class).requestFaceAuth(); + mKeyguardUpdateMonitor.requestFaceAuth(); } @Override diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSFooterViewController.java b/packages/SystemUI/src/com/android/systemui/qs/QSFooterViewController.java index a981b6283764e..929aedae67067 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSFooterViewController.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSFooterViewController.java @@ -74,7 +74,7 @@ public class QSFooterViewController extends ViewController impleme private final PageIndicator mPageIndicator; private final View mPowerMenuLite; private final boolean mShowPMLiteButton; - private GlobalActionsDialogLite mGlobalActionsDialog; + private final GlobalActionsDialogLite mGlobalActionsDialog; private final UiEventLogger mUiEventLogger; private final UserInfoController.OnUserInfoChangedListener mOnUserInfoChangedListener = diff --git a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java index 64aba145c6e2e..bcc4ee09b2eca 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java @@ -30,6 +30,7 @@ import static org.mockito.Mockito.when; import android.app.IActivityManager; import android.app.admin.DevicePolicyManager; import android.app.trust.TrustManager; +import android.content.pm.PackageManager; import android.content.res.Resources; import android.graphics.Color; import android.media.AudioManager; @@ -108,6 +109,7 @@ public class GlobalActionsDialogLiteTest extends SysuiTestCase { @Mock private RingerModeTracker mRingerModeTracker; @Mock private RingerModeLiveData mRingerModeLiveData; @Mock private SysUiState mSysUiState; + @Mock private PackageManager mPackageManager; @Mock private Handler mHandler; @Mock private UserContextProvider mUserContextProvider; @Mock private StatusBar mStatusBar; @@ -122,6 +124,8 @@ public class GlobalActionsDialogLiteTest extends SysuiTestCase { when(mRingerModeTracker.getRingerMode()).thenReturn(mRingerModeLiveData); when(mUserContextProvider.getUserContext()).thenReturn(mContext); + when(mResources.getConfiguration()).thenReturn( + getContext().getResources().getConfiguration()); mGlobalActionsDialogLite = new GlobalActionsDialogLite(mContext, mWindowManagerFuncs, @@ -152,6 +156,7 @@ public class GlobalActionsDialogLiteTest extends SysuiTestCase { mRingerModeTracker, mSysUiState, mHandler, + mPackageManager, mStatusBar ); mGlobalActionsDialogLite.setZeroDialogPressDelayForTesting(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogTest.java b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogTest.java index c543470634878..e5c104e7d377a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogTest.java @@ -33,6 +33,7 @@ import static org.mockito.Mockito.when; import android.app.IActivityManager; import android.app.admin.DevicePolicyManager; import android.app.trust.TrustManager; +import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.content.res.Resources; import android.graphics.Color; @@ -124,6 +125,7 @@ public class GlobalActionsDialogTest extends SysuiTestCase { @Mock GlobalActionsPanelPlugin.PanelViewController mWalletController; @Mock private Handler mHandler; @Mock private UserTracker mUserTracker; + @Mock private PackageManager mPackageManager; @Mock private SecureSettings mSecureSettings; @Mock private StatusBar mStatusBar; @@ -136,6 +138,8 @@ public class GlobalActionsDialogTest extends SysuiTestCase { allowTestableLooperAsMainThread(); when(mRingerModeTracker.getRingerMode()).thenReturn(mRingerModeLiveData); + when(mResources.getConfiguration()).thenReturn( + getContext().getResources().getConfiguration()); mGlobalActionsDialog = new GlobalActionsDialog(mContext, mWindowManagerFuncs, @@ -167,6 +171,7 @@ public class GlobalActionsDialogTest extends SysuiTestCase { mRingerModeTracker, mSysUiState, mHandler, + mPackageManager, mStatusBar ); mGlobalActionsDialog.setZeroDialogPressDelayForTesting();