Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fb220a6f authored by Valentin Iftime's avatar Valentin Iftime
Browse files

Remove WeakReference for Toast next view 

 This avoids the view being GCed in low memory situations.
 Only the @Deprecated setView API uses mNextViev so this applies to
 a limited usage base.

 In some stress-test situations, there may be false reports of memory leaks (LeakCanary).
 The root-cause is that the NMS reference to ITransientNotification/TN is not cleaned up immediately after
 the binder call enqueueToast returns - in the case where the maximum number of toasts has been queued for a
 package.

 New unit tests for Toast: android.widget.ToastTest

Flag: android.widget.flags.toast_no_weakref
Test: atest android.widget.ToastTest
Test: atest ToastWindowTest ToastPresenterTest ToastUITest ToastTest NotificationManagerServiceTest
Bug: 321732224
Change-Id: I4aea918911731fdd2ba7c46009a518d30b5caa37
parent 7ca67e0f

core/java/android/app/INotificationManager.aidl

+2 −2
Original line number Diff line number Diff line
@@ -50,8 +50,8 @@ interface INotificationManager 
    void cancelAllNotifications(String pkg, int userId);



    void clearData(String pkg, int uid, boolean fromApp);

    void enqueueTextToast(String pkg, IBinder token, CharSequence text, int duration, boolean isUiContext, int displayId, @nullable ITransientNotificationCallback callback);

    void enqueueToast(String pkg, IBinder token, ITransientNotification callback, int duration, boolean isUiContext, int displayId);

    boolean enqueueTextToast(String pkg, IBinder token, CharSequence text, int duration, boolean isUiContext, int displayId, @nullable ITransientNotificationCallback callback);

    boolean enqueueToast(String pkg, IBinder token, ITransientNotification callback, int duration, boolean isUiContext, int displayId);

    void cancelToast(String pkg, IBinder token);

    void finishToken(String pkg, IBinder token);

core/java/android/widget/Toast.java

+98 −19
Original line number Diff line number Diff line
@@ -45,8 +45,10 @@ import android.util.Log; 
import android.view.View;

import android.view.WindowManager;

import android.view.accessibility.IAccessibilityManager;

import android.widget.flags.Flags;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;



import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;
@@ -205,27 +207,41 @@ public class Toast { 
        INotificationManager service = getService();

        String pkg = mContext.getOpPackageName();

        TN tn = mTN;

        tn.mNextView = new WeakReference<>(mNextView);

        if (Flags.toastNoWeakref()) {

            tn.mNextView = mNextView;

        } else {

            tn.mNextViewWeakRef = new WeakReference<>(mNextView);

        }

        final boolean isUiContext = mContext.isUiContext();

        final int displayId = mContext.getDisplayId();



        boolean wasEnqueued = false;

        try {

            if (Compatibility.isChangeEnabled(CHANGE_TEXT_TOASTS_IN_THE_SYSTEM)) {

                if (mNextView != null) {

                    // It's a custom toast

                    service.enqueueToast(pkg, mToken, tn, mDuration, isUiContext, displayId);

                    wasEnqueued = service.enqueueToast(pkg, mToken, tn, mDuration, isUiContext,

                            displayId);

                } else {

                    // It's a text toast

                    ITransientNotificationCallback callback =

                            new CallbackBinder(mCallbacks, mHandler);

                    service.enqueueTextToast(pkg, mToken, mText, mDuration, isUiContext, displayId,

                            callback);

                    wasEnqueued = service.enqueueTextToast(pkg, mToken, mText, mDuration,

                            isUiContext, displayId, callback);

                }

            } else {

                service.enqueueToast(pkg, mToken, tn, mDuration, isUiContext, displayId);

                wasEnqueued = service.enqueueToast(pkg, mToken, tn, mDuration, isUiContext,

                        displayId);

            }

        } catch (RemoteException e) {

            // Empty

        } finally {

            if (Flags.toastNoWeakref()) {

                if (!wasEnqueued) {

                    tn.mNextViewWeakRef = null;

                    tn.mNextView = null;

                }

            }

        }

    }
@@ -581,6 +597,16 @@ public class Toast { 
        }

    }



    /**

     * Get the Toast.TN ITransientNotification object

     * @return TN

     * @hide

     */

    @VisibleForTesting

    public TN getTn() {

        return mTN;

    }



    // =======================================================================================

    // All the gunk below is the interaction with the Notification Service, which handles

    // the proper ordering of these system-wide.
@@ -599,7 +625,11 @@ public class Toast { 
        return sService;

    }



    private static class TN extends ITransientNotification.Stub {

    /**

     * @hide

     */

    @VisibleForTesting

    public static class TN extends ITransientNotification.Stub {

        @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P)

        private final WindowManager.LayoutParams mParams;
@@ -620,7 +650,9 @@ public class Toast { 
        @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P)

        View mView;

        @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P)

        WeakReference<View> mNextView;

        WeakReference<View> mNextViewWeakRef;

        @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P)

        View mNextView;

        int mDuration;



        WindowManager mWM;
@@ -662,14 +694,22 @@ public class Toast { 
                            handleHide();

                            // Don't do this in handleHide() because it is also invoked by

                            // handleShow()

                            if (Flags.toastNoWeakref()) {

                                mNextView = null;

                            } else {

                                mNextViewWeakRef = null;

                            }

                            break;

                        }

                        case CANCEL: {

                            handleHide();

                            // Don't do this in handleHide() because it is also invoked by

                            // handleShow()

                            if (Flags.toastNoWeakref()) {

                                mNextView = null;

                            } else {

                                mNextViewWeakRef = null;

                            }

                            try {

                                getService().cancelToast(mPackageName, mToken);

                            } catch (RemoteException e) {
@@ -716,17 +756,38 @@ public class Toast { 
        }



        public void handleShow(IBinder windowToken) {

            if (localLOGV) Log.v(TAG, "HANDLE SHOW: " + this + " mView=" + mView

            if (Flags.toastNoWeakref()) {

                if (localLOGV) {

                    Log.v(TAG, "HANDLE SHOW: " + this + " mView=" + mView

                            + " mNextView=" + mNextView);

                }

            } else {

                if (localLOGV) {

                    Log.v(TAG, "HANDLE SHOW: " + this + " mView=" + mView

                            + " mNextView=" + mNextViewWeakRef);

                }

            }

            // If a cancel/hide is pending - no need to show - at this point

            // the window token is already invalid and no need to do any work.

            if (mHandler.hasMessages(CANCEL) || mHandler.hasMessages(HIDE)) {

                return;

            }

            if (mNextView != null && mView != mNextView.get()) {

            if (Flags.toastNoWeakref()) {

                if (mNextView != null && mView != mNextView) {

                    // remove the old view if necessary

                    handleHide();

                    mView = mNextView;

                    if (mView != null) {

                        mPresenter.show(mView, mToken, windowToken, mDuration, mGravity, mX, mY,

                                mHorizontalMargin, mVerticalMargin,

                                new CallbackBinder(getCallbacks(), mHandler));

                    }

                }

            } else {

                if (mNextViewWeakRef != null && mView != mNextViewWeakRef.get()) {

                    // remove the old view if necessary

                    handleHide();

                mView = mNextView.get();

                    mView = mNextViewWeakRef.get();

                    if (mView != null) {

                        mPresenter.show(mView, mToken, windowToken, mDuration, mGravity, mX, mY,

                                mHorizontalMargin, mVerticalMargin,
@@ -734,6 +795,7 @@ public class Toast { 
                    }

                }

            }

        }



        @UnsupportedAppUsage

        public void handleHide() {
@@ -745,6 +807,23 @@ public class Toast { 
                mView = null;

            }

        }



        /**

         * Get the next view to show for enqueued toasts

         * Custom toast views are deprecated.

         * @see #setView(View)

         *

         * @return next view

         * @hide

         */

        @VisibleForTesting

        public View getNextView() {

            if (Flags.toastNoWeakref()) {

                return mNextView;

            } else {

                return (mNextViewWeakRef != null) ? mNextViewWeakRef.get() : null;

            }

        }

    }



    /**

core/java/android/widget/flags/notification_widget_flags.aconfig

+11 −1
Original line number Diff line number Diff line
@@ -27,3 +27,13 @@ flag { 
    purpose: PURPOSE_BUGFIX

  }

}



flag {

  name: "toast_no_weakref"

  namespace: "systemui"

  description: "Do not use WeakReference for custom view Toast"

  bug: "321732224"

  metadata {

    purpose: PURPOSE_BUGFIX

  }

}

core/tests/mockingcoretests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ android_test { 
        "platform-test-annotations",

        "truth",

        "testables",

        "flag-junit",

    ],



    libs: [

core/tests/mockingcoretests/src/android/widget/OWNERS

0 → 100644
+1 −0
Original line number Diff line number Diff line 
include /services/core/java/com/android/server/notification/OWNERS
 
 No newline at end of file