From c8525ecbe3863f8d6dc17a688dbb9668597c4a8a Mon Sep 17 00:00:00 2001 From: Robert Snoeberger Date: Mon, 20 Jul 2020 14:18:08 -0400 Subject: [PATCH] Prevent NotificationSwipeHelper from retaining notification It has been observed that NotificationSwipeHelper can retain an ExpandableNotificationRow in memory after the notification has been dismissed. To prevent this, this change breaks the reference chain between the NotificationSwipeHelper and the NotificationMenuRow. A weak reference is used to refer to the NotificationMenuRow in the NotificationSwipeHelper. Fixes: 157506961 Bug: 161119435 Test: manual - Swipe to dismiss notifiation. Take heap dump. Verify that NotificationMenuRow isn't retained view NotificationSwipeHelper. Change-Id: Ibe445c31494b592f657b5ae94e4a7ee478a40070 --- .../stack/NotificationSwipeHelper.java | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java index d7d09e05c2386..d3b8a8cd20932 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSwipeHelper.java @@ -34,6 +34,8 @@ import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.notification.row.ExpandableView; +import java.lang.ref.WeakReference; + class NotificationSwipeHelper extends SwipeHelper implements NotificationSwipeActionHelper { @VisibleForTesting @@ -47,7 +49,11 @@ class NotificationSwipeHelper extends SwipeHelper implements NotificationSwipeAc private static final long SWIPE_MENU_TIMING = 200; - private NotificationMenuRowPlugin mCurrMenuRow; + // Hold a weak ref to the menu row so that it isn't accidentally retained in memory. The + // lifetime of the row should be the same as the ActivatableView, which is owned by the + // NotificationStackScrollLayout. If the notification isn't in the notification shade, then it + // isn't possible to swipe it and, so, this class doesn't need to "help." + private WeakReference mCurrMenuRowRef; private boolean mIsExpanded; private boolean mPulsing; @@ -82,11 +88,17 @@ class NotificationSwipeHelper extends SwipeHelper implements NotificationSwipeAc return mMenuExposedView; } - public void setCurrentMenuRow(NotificationMenuRowPlugin menuRow) { - mCurrMenuRow = menuRow; + @VisibleForTesting + void setCurrentMenuRow(NotificationMenuRowPlugin menuRow) { + mCurrMenuRowRef = menuRow != null ? new WeakReference(menuRow) : null; } - public NotificationMenuRowPlugin getCurrentMenuRow() { return mCurrMenuRow; } + public NotificationMenuRowPlugin getCurrentMenuRow() { + if (mCurrMenuRowRef == null) { + return null; + } + return mCurrMenuRowRef.get(); + } @VisibleForTesting protected Handler getHandler() { return mHandler; } @@ -102,8 +114,9 @@ class NotificationSwipeHelper extends SwipeHelper implements NotificationSwipeAc @Override protected void onChildSnappedBack(View animView, float targetLeft) { - if (mCurrMenuRow != null && targetLeft == 0) { - mCurrMenuRow.resetMenu(); + final NotificationMenuRowPlugin menuRow = getCurrentMenuRow(); + if (menuRow != null && targetLeft == 0) { + menuRow.resetMenu(); clearCurrentMenuRow(); } } @@ -129,10 +142,11 @@ class NotificationSwipeHelper extends SwipeHelper implements NotificationSwipeAc @VisibleForTesting protected void initializeRow(SwipeableView row) { if (row.hasFinishedInitialization()) { - mCurrMenuRow = row.createMenu(); - if (mCurrMenuRow != null) { - mCurrMenuRow.setMenuClickListener(mMenuListener); - mCurrMenuRow.onTouchStart(); + final NotificationMenuRowPlugin menuRow = row.createMenu(); + setCurrentMenuRow(menuRow); + if (menuRow != null) { + menuRow.setMenuClickListener(mMenuListener); + menuRow.onTouchStart(); } } }