From 652801f3390fed5549b907f80d494500096b0531 Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Thu, 27 Apr 2023 18:11:26 +0200 Subject: [PATCH] Fix Toast memory leaks Use weak references for Context in Toast.TN The memory leaks only repro with the legacy (non SystemUi Toasts) Test: atest ToastWindowTest atest ToastPresenterTest atest ToastUITest Bug: 204272000 Change-Id: Ia0112978f866ca4c98e48c41b09849b5d5a49276 --- core/java/android/widget/Toast.java | 27 +++++++---- core/java/android/widget/ToastPresenter.java | 50 ++++++++++++-------- 2 files changed, 48 insertions(+), 29 deletions(-) diff --git a/core/java/android/widget/Toast.java b/core/java/android/widget/Toast.java index 54d19eb793053..ee6ac1297b63b 100644 --- a/core/java/android/widget/Toast.java +++ b/core/java/android/widget/Toast.java @@ -50,6 +50,7 @@ import com.android.internal.annotations.GuardedBy; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.List; @@ -205,7 +206,7 @@ public class Toast { INotificationManager service = getService(); String pkg = mContext.getOpPackageName(); TN tn = mTN; - tn.mNextView = mNextView; + tn.mNextView = new WeakReference<>(mNextView); final boolean isUiContext = mContext.isUiContext(); final int displayId = mContext.getDisplayId(); @@ -622,7 +623,7 @@ public class Toast { @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) View mView; @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P) - View mNextView; + WeakReference mNextView; int mDuration; WindowManager mWM; @@ -632,7 +633,7 @@ public class Toast { private final ToastPresenter mPresenter; @GuardedBy("mCallbacks") - private final List mCallbacks; + private final WeakReference> mCallbacks; /** * Creates a {@link ITransientNotification} object. @@ -649,7 +650,7 @@ public class Toast { mParams = mPresenter.getLayoutParams(); mPackageName = packageName; mToken = token; - mCallbacks = callbacks; + mCallbacks = new WeakReference<>(callbacks); mHandler = new Handler(looper, null) { @Override @@ -685,7 +686,11 @@ public class Toast { private List getCallbacks() { synchronized (mCallbacks) { - return new ArrayList<>(mCallbacks); + if (mCallbacks.get() != null) { + return new ArrayList<>(mCallbacks.get()); + } else { + return new ArrayList<>(); + } } } @@ -721,13 +726,15 @@ public class Toast { if (mHandler.hasMessages(CANCEL) || mHandler.hasMessages(HIDE)) { return; } - if (mView != mNextView) { + if (mNextView != null && mView != mNextView.get()) { // remove the old view if necessary handleHide(); - mView = mNextView; - mPresenter.show(mView, mToken, windowToken, mDuration, mGravity, mX, mY, - mHorizontalMargin, mVerticalMargin, - new CallbackBinder(getCallbacks(), mHandler)); + mView = mNextView.get(); + if (mView != null) { + mPresenter.show(mView, mToken, windowToken, mDuration, mGravity, mX, mY, + mHorizontalMargin, mVerticalMargin, + new CallbackBinder(getCallbacks(), mHandler)); + } } } diff --git a/core/java/android/widget/ToastPresenter.java b/core/java/android/widget/ToastPresenter.java index 7467100838ec2..7cb61fe597db8 100644 --- a/core/java/android/widget/ToastPresenter.java +++ b/core/java/android/widget/ToastPresenter.java @@ -41,6 +41,8 @@ import com.android.internal.R; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; +import java.lang.ref.WeakReference; + /** * Class responsible for toast presentation inside app's process and in system UI. * @@ -87,25 +89,33 @@ public class ToastPresenter { return view; } - private final Context mContext; private final Resources mResources; - private final WindowManager mWindowManager; - private final IAccessibilityManager mAccessibilityManager; + private final WeakReference mWindowManager; + private final WeakReference mAccessibilityManager; private final INotificationManager mNotificationManager; private final String mPackageName; + private final String mContextPackageName; private final WindowManager.LayoutParams mParams; @Nullable private View mView; @Nullable private IBinder mToken; public ToastPresenter(Context context, IAccessibilityManager accessibilityManager, INotificationManager notificationManager, String packageName) { - mContext = context; mResources = context.getResources(); - mWindowManager = context.getSystemService(WindowManager.class); + mWindowManager = new WeakReference<>(context.getSystemService(WindowManager.class)); mNotificationManager = notificationManager; mPackageName = packageName; - mAccessibilityManager = accessibilityManager; + mContextPackageName = context.getPackageName(); mParams = createLayoutParams(); + + // We obtain AccessibilityManager manually via its constructor instead of using method + // AccessibilityManager.getInstance() for 2 reasons: + // 1. We want to be able to inject IAccessibilityManager in tests to verify behavior. + // 2. getInstance() caches the instance for the process even if we pass a different + // context to it. This is problematic for multi-user because callers can pass a context + // created via Context.createContextAsUser(). + mAccessibilityManager = new WeakReference<>( + new AccessibilityManager(context, accessibilityManager, context.getUserId())); } public String getPackageName() { @@ -173,7 +183,7 @@ public class ToastPresenter { params.y = yOffset; params.horizontalMargin = horizontalMargin; params.verticalMargin = verticalMargin; - params.packageName = mContext.getPackageName(); + params.packageName = mContextPackageName; params.hideTimeoutMilliseconds = (duration == Toast.LENGTH_LONG) ? LONG_DURATION_TIMEOUT : SHORT_DURATION_TIMEOUT; params.token = windowToken; @@ -270,8 +280,9 @@ public class ToastPresenter { public void hide(@Nullable ITransientNotificationCallback callback) { checkState(mView != null, "No toast to hide."); - if (mView.getParent() != null) { - mWindowManager.removeViewImmediate(mView); + final WindowManager windowManager = mWindowManager.get(); + if (mView.getParent() != null && windowManager != null) { + windowManager.removeViewImmediate(mView); } try { mNotificationManager.finishToken(mPackageName, mToken); @@ -295,14 +306,11 @@ public class ToastPresenter { * enabled. */ public void trySendAccessibilityEvent(View view, String packageName) { - // We obtain AccessibilityManager manually via its constructor instead of using method - // AccessibilityManager.getInstance() for 2 reasons: - // 1. We want to be able to inject IAccessibilityManager in tests to verify behavior. - // 2. getInstance() caches the instance for the process even if we pass a different - // context to it. This is problematic for multi-user because callers can pass a context - // created via Context.createContextAsUser(). - final AccessibilityManager accessibilityManager = - new AccessibilityManager(mContext, mAccessibilityManager, mContext.getUserId()); + final AccessibilityManager accessibilityManager = mAccessibilityManager.get(); + if (accessibilityManager == null) { + return; + } + if (!accessibilityManager.isEnabled()) { accessibilityManager.removeClient(); return; @@ -320,11 +328,15 @@ public class ToastPresenter { } private void addToastView() { + final WindowManager windowManager = mWindowManager.get(); + if (windowManager == null) { + return; + } if (mView.getParent() != null) { - mWindowManager.removeView(mView); + windowManager.removeView(mView); } try { - mWindowManager.addView(mView, mParams); + windowManager.addView(mView, mParams); } catch (WindowManager.BadTokenException e) { // Since the notification manager service cancels the token right after it notifies us // to cancel the toast there is an inherent race and we may attempt to add a window