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

Commit 99742734 authored by Robert Carr's avatar Robert Carr
Browse files

Fix toast lifetime 

In NotificationManagerService#cancelToast we have been calling
WindowManagerService#removeWindowToken with 'removeWindows'=true. This
is allowing for Surface destruction without any sort of synchronization
from the client. Before the call to removeWindowToken we are emitting
a one-way hide call to the Toast client. As a solution to the lifetime
issue we have the client callback to let us know it has processed the
hide call (and thus stopped the ViewRoot). On the server side we also instate
a timeout. This mirrors the app stop timeout. All codepaths I could find
leading to this sort of situation where a client is still rendering
in to a toast following the total duration expiring seem to indicate a hung
client UI thread.

Bug: 62536731
Bug: 70530552
Test: Manual. go/wm-smoke
Change-Id: I89643b3c3a9fa42423b498c1bd3a422a7959aaaf
parent 82409585

core/java/android/app/INotificationManager.aidl

+2 −0
Original line number Diff line number Diff line
@@ -45,6 +45,8 @@ interface INotificationManager 
    void clearData(String pkg, int uid, boolean fromApp);

    void enqueueToast(String pkg, ITransientNotification callback, int duration);

    void cancelToast(String pkg, ITransientNotification callback);

    void finishToken(String pkg, ITransientNotification callback);



    void enqueueNotificationWithTag(String pkg, String opPkg, String tag, int id,

            in Notification notification, int userId);

    void cancelNotificationWithTag(String pkg, String tag, int id, int userId);

core/java/android/widget/Toast.java

+8 −0
Original line number Diff line number Diff line
@@ -531,6 +531,14 @@ public class Toast { 
                    mWM.removeViewImmediate(mView);

                }





                // Now that we've removed the view it's safe for the server to release

                // the resources.

                try {

                    getService().finishToken(mPackageName, this);

                } catch (RemoteException e) {

                }



                mView = null;

            }

        }

services/core/java/com/android/server/notification/NotificationManagerService.java

+65 −8
Original line number Diff line number Diff line
@@ -229,11 +229,12 @@ public class NotificationManagerService extends SystemService { 
    static final float DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE = 5f;



    // message codes

    static final int MESSAGE_TIMEOUT = 2;

    static final int MESSAGE_DURATION_REACHED = 2;

    static final int MESSAGE_SAVE_POLICY_FILE = 3;

    static final int MESSAGE_SEND_RANKING_UPDATE = 4;

    static final int MESSAGE_LISTENER_HINTS_CHANGED = 5;

    static final int MESSAGE_LISTENER_NOTIFICATION_FILTER_CHANGED = 6;

    static final int MESSAGE_FINISH_TOKEN_TIMEOUT = 7;



    // ranking thread messages

    private static final int MESSAGE_RECONSIDER_RANKING = 1000;
@@ -1889,6 +1890,25 @@ public class NotificationManagerService extends SystemService { 
            }

        }



        @Override

        public void finishToken(String pkg, ITransientNotification callback) {

            synchronized (mToastQueue) {

                long callingId = Binder.clearCallingIdentity();

                try {

                    int index = indexOfToastLocked(pkg, callback);

                    if (index >= 0) {

                        ToastRecord record = mToastQueue.get(index);

                        finishTokenLocked(record.token);

                    } else {

                        Slog.w(TAG, "Toast already killed. pkg=" + pkg

                                + " callback=" + callback);

                    }

                } finally {

                    Binder.restoreCallingIdentity(callingId);

                }

            }

        }



        @Override

        public void enqueueNotificationWithTag(String pkg, String opPkg, String tag, int id,

                Notification notification, int userId) throws RemoteException {
@@ -4578,7 +4598,7 @@ public class NotificationManagerService extends SystemService { 
            if (DBG) Slog.d(TAG, "Show pkg=" + record.pkg + " callback=" + record.callback);

            try {

                record.callback.show(record.token);

                scheduleTimeoutLocked(record);

                scheduleDurationReachedLocked(record);

                return;

            } catch (RemoteException e) {

                Slog.w(TAG, "Object died trying to show notification " + record.callback
@@ -4611,7 +4631,15 @@ public class NotificationManagerService extends SystemService { 
        }



        ToastRecord lastToast = mToastQueue.remove(index);

        mWindowManagerInternal.removeWindowToken(lastToast.token, true, DEFAULT_DISPLAY);



        mWindowManagerInternal.removeWindowToken(lastToast.token, false /* removeWindows */,

                DEFAULT_DISPLAY);

        // We passed 'false' for 'removeWindows' so that the client has time to stop

        // rendering (as hide above is a one-way message), otherwise we could crash

        // a client which was actively using a surface made from the token. However

        // we need to schedule a timeout to make sure the token is eventually killed

        // one way or another.

        scheduleKillTokenTimeout(lastToast.token);



        keepProcessAliveIfNeededLocked(record.pid);

        if (mToastQueue.size() > 0) {
@@ -4622,16 +4650,26 @@ public class NotificationManagerService extends SystemService { 
        }

    }



    void finishTokenLocked(IBinder t) {

        mHandler.removeCallbacksAndMessages(t);

        // We pass 'true' for 'removeWindows' to let the WindowManager destroy any

        // remaining surfaces as either the client has called finishToken indicating

        // it has successfully removed the views, or the client has timed out

        // at which point anything goes.

        mWindowManagerInternal.removeWindowToken(t, true /* removeWindows */,

                DEFAULT_DISPLAY);

    }



    @GuardedBy("mToastQueue")

    private void scheduleTimeoutLocked(ToastRecord r)

    private void scheduleDurationReachedLocked(ToastRecord r)

    {

        mHandler.removeCallbacksAndMessages(r);

        Message m = Message.obtain(mHandler, MESSAGE_TIMEOUT, r);

        Message m = Message.obtain(mHandler, MESSAGE_DURATION_REACHED, r);

        long delay = r.duration == Toast.LENGTH_LONG ? LONG_DELAY : SHORT_DELAY;

        mHandler.sendMessageDelayed(m, delay);

    }



    private void handleTimeout(ToastRecord record)

    private void handleDurationReached(ToastRecord record)

    {

        if (DBG) Slog.d(TAG, "Timeout pkg=" + record.pkg + " callback=" + record.callback);

        synchronized (mToastQueue) {
@@ -4642,6 +4680,22 @@ public class NotificationManagerService extends SystemService { 
        }

    }



    @GuardedBy("mToastQueue")

    private void scheduleKillTokenTimeout(IBinder token)

    {

        mHandler.removeCallbacksAndMessages(token);

        Message m = Message.obtain(mHandler, MESSAGE_FINISH_TOKEN_TIMEOUT, token);

        mHandler.sendMessageDelayed(m, 5);

    }



    private void handleKillTokenTimeout(IBinder token)

    {

        if (DBG) Slog.d(TAG, "Kill Token Timeout token=" + token);

        synchronized (mToastQueue) {

            finishTokenLocked(token);

        }

    }



    @GuardedBy("mToastQueue")

    int indexOfToastLocked(String pkg, ITransientNotification callback)

    {
@@ -4839,8 +4893,11 @@ public class NotificationManagerService extends SystemService { 
        {

            switch (msg.what)

            {

                case MESSAGE_TIMEOUT:

                    handleTimeout((ToastRecord)msg.obj);

                case MESSAGE_DURATION_REACHED:

                    handleDurationReached((ToastRecord)msg.obj);

                    break;

                case MESSAGE_FINISH_TOKEN_TIMEOUT:

                    handleKillTokenTimeout((IBinder)msg.obj);

                    break;

                case MESSAGE_SAVE_POLICY_FILE:

                    handleSavePolicyFile();