From c4aee98a62f400dd9f6f964d26d739d409212775 Mon Sep 17 00:00:00 2001 From: John Spurlock Date: Wed, 12 Feb 2014 12:12:26 -0500 Subject: Improve error handling in listener services. Check explicitly for null listeners in NMS, throwing IllegalArgumentException (on the small list of exceptions that survive RPC boundaries) with a message. Normally this situation is caused by listeners that attempt to perform NM-related actions before they are bound. Check for this case in the base NLS class and avoid the call to NM if we know it will fail. Although it's tempting to throw an IllegalStateException on the client side, preserve the existing semantics for backwards-compatibility purposes. That is, silently fail (or return null) - and provide a log warning. Bug:12805707 Change-Id: I0d92fd0d460a8592e8a23fd8fd718ae2ba3bd4c7 --- .../service/notification/NotificationListenerService.java | 11 +++++++++++ .../server/notification/NotificationManagerService.java | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/core/java/android/service/notification/NotificationListenerService.java b/core/java/android/service/notification/NotificationListenerService.java index 2e0e59bc91c9..cf862b872aa3 100644 --- a/core/java/android/service/notification/NotificationListenerService.java +++ b/core/java/android/service/notification/NotificationListenerService.java @@ -112,6 +112,7 @@ public abstract class NotificationListenerService extends Service { * {@link android.app.NotificationManager#notify(String, int, android.app.Notification)}. */ public final void cancelNotification(String pkg, String tag, int id) { + if (!isBound()) return; try { getNotificationInterface().cancelNotificationFromListener(mWrapper, pkg, tag, id); } catch (android.os.RemoteException ex) { @@ -131,6 +132,7 @@ public abstract class NotificationListenerService extends Service { * {@see #cancelNotification(String, String, int)} */ public final void cancelAllNotifications() { + if (!isBound()) return; try { getNotificationInterface().cancelAllNotificationsFromListener(mWrapper); } catch (android.os.RemoteException ex) { @@ -145,6 +147,7 @@ public abstract class NotificationListenerService extends Service { * @return An array of active notifications. */ public StatusBarNotification[] getActiveNotifications() { + if (!isBound()) return null; try { return getNotificationInterface().getActiveNotificationsFromListener(mWrapper); } catch (android.os.RemoteException ex) { @@ -161,6 +164,14 @@ public abstract class NotificationListenerService extends Service { return mWrapper; } + private boolean isBound() { + if (mWrapper == null) { + Log.w(TAG, "Notification listener service not yet bound."); + return false; + } + return true; + } + private class INotificationListenerWrapper extends INotificationListener.Stub { @Override public void onNotificationPosted(StatusBarNotification sbn) { diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 6ee89891a84b..2ba6c711bce3 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -725,7 +725,14 @@ public class NotificationManagerService extends SystemService { // -- APIs to support listeners clicking/clearing notifications -- + private void checkNullListener(INotificationListener listener) { + if (listener == null) { + throw new IllegalArgumentException("Listener must not be null"); + } + } + private NotificationListenerInfo checkListenerToken(INotificationListener listener) { + checkNullListener(listener); final IBinder token = listener.asBinder(); final int N = mListeners.size(); for (int i=0; i