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

Commit b8f53ee8 authored by Svet Ganov's avatar Svet Ganov Committed by Svetoslav Ganov
Browse files

Notification listener and ranker callbacks on binder threads.

The callbacks for the notification listener and notification
ranker were delivered on binder threads which is problematic
becuase: 1) permission checks and app ops checks would fail
unless the app developer knows to clear binder calling id and
restore it after that; 2) developers need to synchronize their
implementation as they get callbacks on different threads (
arguably callbacks should not be concurrent); 3) this doesn't
follow the pattern in the platform;

Also the code delivering callbacks was catching Throwable which
we shouldn't do in general and also masks bugs in the listener
or ranker implementation. Now that the callbacks are offloaded
to the main listener/ranker thread system code should not be
guarding against Throwable to handle exceptions propagated
over binder calls.

bug:26704777

Change-Id: I171fb41bbe25e6105dd05e4166193dbcec594f82
parent a52f6a17
Loading
Loading
Loading
Loading
+119 −45
Original line number Diff line number Diff line
@@ -18,18 +18,17 @@ package android.service.notification;

import android.annotation.SdkConstant;
import android.annotation.SystemApi;
import android.app.INotificationManager;
import android.app.Notification;
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
import android.net.Uri;
import android.os.Handler;
import android.os.IBinder;
import android.os.Parcel;
import android.os.Parcelable;
import android.os.Looper;
import android.os.Message;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.util.Log;
import com.android.internal.os.SomeArgs;

/**
 * A service that helps the user manage notifications by modifying the
@@ -125,8 +124,24 @@ public abstract class NotificationAssistantService extends NotificationListenerS
        }
    }

    private Handler mHandler;

    /** @hide */
    @Override
    public void registerAsSystemService(Context context, ComponentName componentName,
            int currentUser) throws RemoteException {
        super.registerAsSystemService(context, componentName, currentUser);
        mHandler = new MyHandler(getContext().getMainLooper());
    }

    @Override
    protected void attachBaseContext(Context base) {
        super.attachBaseContext(base);
        mHandler = new MyHandler(getContext().getMainLooper());
    }

    @Override
    public IBinder onBind(Intent intent) {
    public final IBinder onBind(Intent intent) {
        if (mWrapper == null) {
            mWrapper = new NotificationAssistantWrapper();
        }
@@ -198,8 +213,7 @@ public abstract class NotificationAssistantService extends NotificationListenerS
     * @param key the notification key
     * @param adjustment the new importance with an explanation
     */
    public final void adjustImportance(String key, Adjustment adjustment)
    {
    public final void adjustImportance(String key, Adjustment adjustment) {
        if (!isBound()) return;
        try {
            getNotificationInterface().setImportanceFromAssistant(mWrapper, key,
@@ -212,7 +226,7 @@ public abstract class NotificationAssistantService extends NotificationListenerS
    private class NotificationAssistantWrapper extends NotificationListenerWrapper {
        @Override
        public void onNotificationEnqueued(IStatusBarNotificationHolder sbnHolder,
                                           int importance, boolean user) throws RemoteException {
                int importance, boolean user) {
            StatusBarNotification sbn;
            try {
                sbn = sbnHolder.get();
@@ -221,54 +235,114 @@ public abstract class NotificationAssistantService extends NotificationListenerS
                return;
            }

            try {
                Adjustment adjustment =
                    NotificationAssistantService.this.onNotificationEnqueued(sbn, importance, user);
                if (adjustment != null) {
                    adjustImportance(sbn.getKey(), adjustment);
                }
            } catch (Throwable t) {
                Log.w(TAG, "Error running onNotificationEnqueued", t);
            }
            SomeArgs args = SomeArgs.obtain();
            args.arg1 = sbn;
            args.argi1 = importance;
            args.argi2 = user ? 1 : 0;
            mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_ENQUEUED,
                    args).sendToTarget();
        }

        @Override
        public void onNotificationVisibilityChanged(String key, long time, boolean visible)
                throws RemoteException {
            try {
                NotificationAssistantService.this.onNotificationVisibilityChanged(key, time,
                        visible);
            } catch (Throwable t) {
                Log.w(TAG, "Error running onNotificationVisibilityChanged", t);
            }
        public void onNotificationVisibilityChanged(String key, long time, boolean visible) {
            SomeArgs args = SomeArgs.obtain();
            args.arg1 = key;
            args.arg2 = time;
            args.argi1 = visible ? 1 : 0;
            mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_VISIBILITY_CHANGED,
                    args).sendToTarget();
        }

        @Override
        public void onNotificationClick(String key, long time) throws RemoteException {
            try {
                NotificationAssistantService.this.onNotificationClick(key, time);
            } catch (Throwable t) {
                Log.w(TAG, "Error running onNotificationClick", t);
        public void onNotificationClick(String key, long time) {
            SomeArgs args = SomeArgs.obtain();
            args.arg1 = key;
            args.arg2 = time;
            mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_CLICK,
                    args).sendToTarget();
        }

        @Override
        public void onNotificationActionClick(String key, long time, int actionIndex) {
            SomeArgs args = SomeArgs.obtain();
            args.arg1 = key;
            args.arg2 = time;
            args.argi1 = actionIndex;
            mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_ACTION_CLICK,
                    args).sendToTarget();
        }

        @Override
        public void onNotificationActionClick(String key, long time, int actionIndex)
                throws RemoteException {
            try {
                NotificationAssistantService.this.onNotificationActionClick(key, time, actionIndex);
            } catch (Throwable t) {
                Log.w(TAG, "Error running onNotificationActionClick", t);
        public void onNotificationRemovedReason(String key, long time, int reason) {
            SomeArgs args = SomeArgs.obtain();
            args.arg1 = key;
            args.arg2 = time;
            args.argi1 = reason;
            mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_REMOVED_REASON,
                    args).sendToTarget();
        }
    }

    private final class MyHandler extends Handler {
        public static final int MSG_ON_NOTIFICATION_ENQUEUED = 1;
        public static final int MSG_ON_NOTIFICATION_VISIBILITY_CHANGED = 2;
        public static final int MSG_ON_NOTIFICATION_CLICK = 3;
        public static final int MSG_ON_NOTIFICATION_ACTION_CLICK = 4;
        public static final int MSG_ON_NOTIFICATION_REMOVED_REASON = 5;

        public MyHandler(Looper looper) {
            super(looper, null, false);
        }

        @Override
        public void onNotificationRemovedReason(String key, long time, int reason)
                throws RemoteException {
            try {
                NotificationAssistantService.this.onNotificationRemoved(key, time, reason);
            } catch (Throwable t) {
                Log.w(TAG, "Error running onNotificationRemoved", t);
        public void handleMessage(Message msg) {
            switch (msg.what) {
                case MSG_ON_NOTIFICATION_ENQUEUED: {
                    SomeArgs args = (SomeArgs) msg.obj;
                    StatusBarNotification sbn = (StatusBarNotification) args.arg1;
                    final int importance = args.argi1;
                    final boolean user = args.argi2 == 1;
                    args.recycle();
                    Adjustment adjustment = onNotificationEnqueued(sbn, importance, user);
                    if (adjustment != null) {
                        adjustImportance(sbn.getKey(), adjustment);
                    }
                } break;

                case MSG_ON_NOTIFICATION_VISIBILITY_CHANGED: {
                    SomeArgs args = (SomeArgs) msg.obj;
                    final String key = (String) args.arg1;
                    final long time = (long) args.arg2;
                    final boolean visible = args.argi1 == 1;
                    args.recycle();
                    onNotificationVisibilityChanged(key, time, visible);
                } break;

                case MSG_ON_NOTIFICATION_CLICK: {
                    SomeArgs args = (SomeArgs) msg.obj;
                    final String key = (String) args.arg1;
                    final long time = (long) args.arg2;
                    args.recycle();
                    onNotificationClick(key, time);
                } break;

                case MSG_ON_NOTIFICATION_ACTION_CLICK: {
                    SomeArgs args = (SomeArgs) msg.obj;
                    final String key = (String) args.arg1;
                    final long time = (long) args.arg2;
                    final int actionIndex = args.argi1;
                    args.recycle();
                    onNotificationActionClick(key, time, actionIndex);
                } break;

                case MSG_ON_NOTIFICATION_REMOVED_REASON: {
                    SomeArgs args = (SomeArgs) msg.obj;
                    final String key = (String) args.arg1;
                    final long time = (long) args.arg2;
                    final int reason = args.argi1;
                    args.recycle();
                    onNotificationRemoved(key, time, reason);
                } break;
            }
        }
    }
+116 −48
Original line number Diff line number Diff line
@@ -16,6 +16,10 @@

package android.service.notification;

import android.os.Handler;
import android.os.Looper;
import android.os.Message;

import android.annotation.IntDef;
import android.annotation.SystemApi;
import android.annotation.SdkConstant;
@@ -43,7 +47,8 @@ import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
import android.widget.RemoteViews;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.os.SomeArgs;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
@@ -163,8 +168,14 @@ public abstract class NotificationListenerService extends Service {
    @SystemApi
    public static final int TRIM_LIGHT = 1;

    private final Object mLock = new Object();

    private Handler mHandler;

    /** @hide */
    protected NotificationListenerWrapper mWrapper = null;

    @GuardedBy("mLock")
    private RankingMap mRankingMap;

    private INotificationManager mNoMan;
@@ -195,6 +206,12 @@ public abstract class NotificationListenerService extends Service {
    public static final String CATEGORY_VR_NOTIFICATIONS =
        "android.intent.category.vr.notifications";

    @Override
    protected void attachBaseContext(Context base) {
        super.attachBaseContext(base);
        mHandler = new MyHandler(getMainLooper());
    }

    /**
     * Implement this method to learn about new notifications as they are posted by apps.
     *
@@ -642,8 +659,10 @@ public abstract class NotificationListenerService extends Service {
     * @return A {@link RankingMap} object providing access to ranking information
     */
    public RankingMap getCurrentRanking() {
        synchronized (mLock) {
            return mRankingMap;
        }
    }

    @Override
    public IBinder onBind(Intent intent) {
@@ -684,6 +703,7 @@ public abstract class NotificationListenerService extends Service {
        INotificationManager noMan = getNotificationInterface();
        noMan.registerListener(mWrapper, componentName, currentUser);
        mCurrentUser = currentUser;
        mHandler = new MyHandler(context.getMainLooper());
    }

    /**
@@ -710,7 +730,7 @@ public abstract class NotificationListenerService extends Service {
     * <P>The service should wait for the {@link #onListenerConnected()} event
     * before performing any operations.
     */
    public static final void requestRebind(ComponentName componentName)
    public static void requestRebind(ComponentName componentName)
            throws RemoteException {
        INotificationManager noMan = INotificationManager.Stub.asInterface(
                ServiceManager.getService(Context.NOTIFICATION_SERVICE));
@@ -782,7 +802,6 @@ public abstract class NotificationListenerService extends Service {
            }

            try {
                Notification notification = sbn.getNotification();
                // convert icon metadata to legacy format for older clients
                createLegacyIconExtras(sbn.getNotification());
                maybePopulateRemoteViews(sbn.getNotification());
@@ -794,20 +813,23 @@ public abstract class NotificationListenerService extends Service {
            }

            // protect subclass from concurrent modifications of (@link mNotificationKeys}.
            synchronized (mWrapper) {
                applyUpdate(update);
                try {
            synchronized (mLock) {
                applyUpdateLocked(update);
                if (sbn != null) {
                        NotificationListenerService.this.onNotificationPosted(sbn, mRankingMap);
                    SomeArgs args = SomeArgs.obtain();
                    args.arg1 = sbn;
                    args.arg2 = mRankingMap;
                    mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_POSTED,
                            args).sendToTarget();
                } else {
                    // still pass along the ranking map, it may contain other information
                        NotificationListenerService.this.onNotificationRankingUpdate(mRankingMap);
                    }
                } catch (Throwable t) {
                    Log.w(TAG, "Error running onNotificationPosted", t);
                    mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_RANKING_UPDATE,
                            mRankingMap).sendToTarget();
                }
            }

        }

        @Override
        public void onNotificationRemoved(IStatusBarNotificationHolder sbnHolder,
                NotificationRankingUpdate update) {
@@ -819,56 +841,48 @@ public abstract class NotificationListenerService extends Service {
                return;
            }
            // protect subclass from concurrent modifications of (@link mNotificationKeys}.
            synchronized (mWrapper) {
                applyUpdate(update);
                try {
                    NotificationListenerService.this.onNotificationRemoved(sbn, mRankingMap);
                } catch (Throwable t) {
                    Log.w(TAG, "Error running onNotificationRemoved", t);
                }
            synchronized (mLock) {
                applyUpdateLocked(update);
                SomeArgs args = SomeArgs.obtain();
                args.arg1 = sbn;
                args.arg2 = mRankingMap;
                mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_REMOVED,
                        args).sendToTarget();
            }

        }

        @Override
        public void onListenerConnected(NotificationRankingUpdate update) {
            // protect subclass from concurrent modifications of (@link mNotificationKeys}.
            synchronized (mWrapper) {
                applyUpdate(update);
                try {
                    NotificationListenerService.this.onListenerConnected();
                } catch (Throwable t) {
                    Log.w(TAG, "Error running onListenerConnected", t);
                }
            synchronized (mLock) {
                applyUpdateLocked(update);
            }
            mHandler.obtainMessage(MyHandler.MSG_ON_LISTENER_CONNECTED).sendToTarget();
        }

        @Override
        public void onNotificationRankingUpdate(NotificationRankingUpdate update)
                throws RemoteException {
            // protect subclass from concurrent modifications of (@link mNotificationKeys}.
            synchronized (mWrapper) {
                applyUpdate(update);
                try {
                    NotificationListenerService.this.onNotificationRankingUpdate(mRankingMap);
                } catch (Throwable t) {
                    Log.w(TAG, "Error running onNotificationRankingUpdate", t);
                }
            synchronized (mLock) {
                applyUpdateLocked(update);
                mHandler.obtainMessage(MyHandler.MSG_ON_NOTIFICATION_RANKING_UPDATE,
                        mRankingMap).sendToTarget();
            }

        }

        @Override
        public void onListenerHintsChanged(int hints) throws RemoteException {
            try {
                NotificationListenerService.this.onListenerHintsChanged(hints);
            } catch (Throwable t) {
                Log.w(TAG, "Error running onListenerHintsChanged", t);
            }
            mHandler.obtainMessage(MyHandler.MSG_ON_LISTENER_HINTS_CHANGED,
                    hints, 0).sendToTarget();
        }

        @Override
        public void onInterruptionFilterChanged(int interruptionFilter) throws RemoteException {
            try {
                NotificationListenerService.this.onInterruptionFilterChanged(interruptionFilter);
            } catch (Throwable t) {
                Log.w(TAG, "Error running onInterruptionFilterChanged", t);
            }
            mHandler.obtainMessage(MyHandler.MSG_ON_INTERRUPTION_FILTER_CHANGED,
                    interruptionFilter, 0).sendToTarget();
        }

        @Override
@@ -901,11 +915,12 @@ public abstract class NotificationListenerService extends Service {
        }
    }

    private void applyUpdate(NotificationRankingUpdate update) {
    private void applyUpdateLocked(NotificationRankingUpdate update) {
        mRankingMap = new RankingMap(update);
    }

    private Context getContext() {
    /** @hide */
    protected Context getContext() {
        if (mSystemContext != null) {
            return mSystemContext;
        }
@@ -1289,4 +1304,57 @@ public abstract class NotificationListenerService extends Service {
            }
        };
    }

    private final class MyHandler extends Handler {
        public static final int MSG_ON_NOTIFICATION_POSTED = 1;
        public static final int MSG_ON_NOTIFICATION_REMOVED = 2;
        public static final int MSG_ON_LISTENER_CONNECTED = 3;
        public static final int MSG_ON_NOTIFICATION_RANKING_UPDATE = 4;
        public static final int MSG_ON_LISTENER_HINTS_CHANGED = 5;
        public static final int MSG_ON_INTERRUPTION_FILTER_CHANGED = 6;

        public MyHandler(Looper looper) {
            super(looper, null, false);
        }

        @Override
        public void handleMessage(Message msg) {
            switch (msg.what) {
                case MSG_ON_NOTIFICATION_POSTED: {
                    SomeArgs args = (SomeArgs) msg.obj;
                    StatusBarNotification sbn = (StatusBarNotification) args.arg1;
                    RankingMap rankingMap = (RankingMap) args.arg2;
                    args.recycle();
                    onNotificationPosted(sbn, rankingMap);
                } break;

                case MSG_ON_NOTIFICATION_REMOVED: {
                    SomeArgs args = (SomeArgs) msg.obj;
                    StatusBarNotification sbn = (StatusBarNotification) args.arg1;
                    RankingMap rankingMap = (RankingMap) args.arg2;
                    args.recycle();
                    onNotificationRemoved(sbn, rankingMap);
                } break;

                case MSG_ON_LISTENER_CONNECTED: {
                    onListenerConnected();
                } break;

                case MSG_ON_NOTIFICATION_RANKING_UPDATE: {
                    RankingMap rankingMap = (RankingMap) msg.obj;
                    onNotificationRankingUpdate(rankingMap);
                } break;

                case MSG_ON_LISTENER_HINTS_CHANGED: {
                    final int hints = msg.arg1;
                    onListenerHintsChanged(hints);
                } break;

                case MSG_ON_INTERRUPTION_FILTER_CHANGED: {
                    final int interruptionFilter = msg.arg1;
                    onInterruptionFilterChanged(interruptionFilter);
                } break;
            }
        }
    }
}