Handle errors inflating notifications (and their icons).

On an inflation error, the StatusBarService cleans up, removes / doesn't add
the views, and calls into the StatusBarManagerService, which tells the
NotificationManagerService to remove the notification.

That then calls all the way back into the StatusBarService, but I think being
extra careful is okay.  Throughout the status bar, it's all keyed off of the
IBinder key, so if the app comes in with a good notification while we're
cleaning up, we won't lose the new notification or anything like that.

Change-Id: Iea78a637495a8b67810c214b951d5ddb93becacb

6 files changed, 112 insertions, 55 deletions

diff --git a/core/java/com/android/internal/statusbar/IStatusBarService.aidl b/core/java/com/android/internal/statusbar/IStatusBarService.aidl
index 1f25b37..be3ded1 100644
--- a/core/java/com/android/internal/statusbar/IStatusBarService.aidl
+++ b/core/java/com/android/internal/statusbar/IStatusBarService.aidl
@@ -36,5 +36,6 @@ interface IStatusBarService
             out List<IBinder> notificationKeys, out List<StatusBarNotification> notifications);
     void visibilityChanged(boolean visible);
     void onNotificationClick(String pkg, String tag, int id);
+    void onNotificationError(String pkg, String tag, int id, String message);
     void onClearAllNotifications();
 }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/PhoneStatusBarService.java b/packages/SystemUI/src/com/android/systemui/statusbar/PhoneStatusBarService.java
index ad59a0c..ca589cd 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/PhoneStatusBarService.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/PhoneStatusBarService.java
@@ -373,6 +373,14 @@ public class PhoneStatusBarService extends StatusBarService {
                     oldEntry.content.setOnClickListener(new Launcher(contentIntent,
                                 notification.pkg, notification.tag, notification.id));
                 }
+                // Update the icon.
+                final StatusBarIcon ic = new StatusBarIcon(notification.pkg,
+                        notification.notification.icon, notification.notification.iconLevel,
+                        notification.notification.number);
+                if (!oldEntry.icon.set(ic)) {
+                    handleNotificationError(key, notification, "Couldn't update icon: " + ic);
+                    return;
+                }
             }
             catch (RuntimeException e) {
                 // It failed to add cleanly.  Log, and remove the view from the panel.
@@ -380,9 +388,6 @@ public class PhoneStatusBarService extends StatusBarService {
                 removeNotificationViews(key);
                 addNotificationViews(key, notification);
             }
-            // Update the icon.
-            oldEntry.icon.set(new StatusBarIcon(notification.pkg, notification.notification.icon,
-                    notification.notification.iconLevel, notification.notification.number));
         } else {
             Slog.d(TAG, "not reusing notification");
             removeNotificationViews(key);
@@ -451,8 +456,9 @@ public class PhoneStatusBarService extends StatusBarService {
             exception = e;
         }
         if (expanded == null) {
-            Slog.e(TAG, "couldn't inflate view for package " + notification.pkg, exception);
-            row.setVisibility(View.GONE);
+            String ident = notification.pkg + "/0x" + Integer.toHexString(notification.id);
+            Slog.e(TAG, "couldn't inflate view for notification " + ident);
+            return null;
         } else {
             content.addView(expanded);
             row.setDrawingCacheEnabled(true);
@@ -474,14 +480,23 @@ public class PhoneStatusBarService extends StatusBarService {
         }
         // Construct the expanded view.
         final View[] views = makeNotificationView(notification, parent);
+        if (views == null) {
+            handleNotificationError(key, notification, "Couldn't expand RemoteViews for: "
+                    + notification);
+            return;
+        }
         final View row = views[0];
         final View content = views[1];
         final View expanded = views[2];
         // Construct the icon.
-        StatusBarIconView iconView = new StatusBarIconView(this,
+        final StatusBarIconView iconView = new StatusBarIconView(this,
                 notification.pkg + "/0x" + Integer.toHexString(notification.id));
-        iconView.set(new StatusBarIcon(notification.pkg, notification.notification.icon,
-                    notification.notification.iconLevel, notification.notification.number));
+        final StatusBarIcon ic = new StatusBarIcon(notification.pkg, notification.notification.icon,
+                    notification.notification.iconLevel, notification.notification.number);
+        if (!iconView.set(ic)) {
+            handleNotificationError(key, notification, "Coulding create icon: " + ic);
+            return;
+        }
         // Add the expanded view.
         final int viewIndex = list.add(key, notification, row, content, expanded, iconView);
         parent.addView(row, viewIndex);
@@ -967,6 +982,21 @@ public class PhoneStatusBarService extends StatusBarService {
         }
     }
 
+    /**
+     * Cancel this notification and tell the StatusBarManagerService / NotificationManagerService
+     * about the failure.
+     *
+     * WARNING: this will call back into us.  Don't hold any locks.
+     */
+    void handleNotificationError(IBinder key, StatusBarNotification n, String message) {
+        removeNotification(key);
+        try {
+            mBarService.onNotificationError(n.pkg, n.tag, n.id, message);
+        } catch (RemoteException ex) {
+            // The end is nigh.
+        }
+    }
+
     private class MyTicker extends Ticker {
         MyTicker(Context context, StatusBarView sb) {
             super(context, sb);
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java
index 5eb0d68..bc1e798 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java
@@ -32,7 +32,6 @@ public class StatusBarIconView extends AnimatedImageView {
 
     private StatusBarIcon mIcon;
     @ViewDebug.ExportedProperty private String mSlot;
-    @ViewDebug.ExportedProperty private boolean mError;
 
     public StatusBarIconView(Context context, String slot) {
         super(context);
@@ -52,39 +51,33 @@ public class StatusBarIconView extends AnimatedImageView {
         return a.equals(b);
     }
 
-    public void set(StatusBarIcon icon) {
-        error: {
-            final boolean iconEquals = !mError
-                    && mIcon != null
-                    && streq(mIcon.iconPackage, icon.iconPackage)
-                    && mIcon.iconId == icon.iconId;
-            final boolean levelEquals = !mError
-                    && iconEquals
-                    && mIcon.iconLevel == icon.iconLevel;
-            final boolean visibilityEquals = !mError
-                    && mIcon != null
-                    && mIcon.visible == icon.visible;
-            mError = false;
-            if (!iconEquals) {
-                Drawable drawable = getIcon(icon);
-                if (drawable == null) {
-                    mError = true;
-                    Slog.w(PhoneStatusBarService.TAG, "No icon ID for slot " + mSlot);
-                    break error;
-                }
-                setImageDrawable(drawable);
-            }
-            if (!levelEquals) {
-                setImageLevel(icon.iconLevel);
-            }
-            if (!visibilityEquals) {
-                setVisibility(icon.visible ? VISIBLE : GONE);
+    /**
+     * Returns whether the set succeeded.
+     */
+    public boolean set(StatusBarIcon icon) {
+        final boolean iconEquals = mIcon != null
+                && streq(mIcon.iconPackage, icon.iconPackage)
+                && mIcon.iconId == icon.iconId;
+        final boolean levelEquals = iconEquals
+                && mIcon.iconLevel == icon.iconLevel;
+        final boolean visibilityEquals = mIcon != null
+                && mIcon.visible == icon.visible;
+        if (!iconEquals) {
+            Drawable drawable = getIcon(icon);
+            if (drawable == null) {
+                Slog.w(PhoneStatusBarService.TAG, "No icon for slot " + mSlot);
+                return false;
             }
-            mIcon = icon.clone();
+            setImageDrawable(drawable);
+        }
+        if (!levelEquals) {
+            setImageLevel(icon.iconLevel);
         }
-        if (mError) {
-            setVisibility(GONE);
+        if (!visibilityEquals) {
+            setVisibility(icon.visible ? VISIBLE : GONE);
         }
+        mIcon = icon.clone();
+        return true;
     }
 
     private Drawable getIcon(StatusBarIcon icon) {
@@ -106,7 +99,7 @@ public class StatusBarIconView extends AnimatedImageView {
             try {
                 r = context.getPackageManager().getResourcesForApplication(icon.iconPackage);
             } catch (PackageManager.NameNotFoundException ex) {
-                Slog.e(PhoneStatusBarService.TAG, "Icon package not found: "+icon.iconPackage, ex);
+                Slog.e(PhoneStatusBarService.TAG, "Icon package not found: " + icon.iconPackage);
                 return null;
             }
         } else {
diff --git a/services/java/com/android/server/NotificationManagerService.java b/services/java/com/android/server/NotificationManagerService.java
index b5c2b1b..a51691c 100755
--- a/services/java/com/android/server/NotificationManagerService.java
+++ b/services/java/com/android/server/NotificationManagerService.java
@@ -303,6 +303,12 @@ class NotificationManagerService extends INotificationManager.Stub
                 updateLightsLocked();
             }
         }
+
+        public void onNotificationError(String pkg, String tag, int id, String message) {
+            Slog.d(TAG, "onNotification error pkg=" + pkg + " tag=" + tag + " id=" + id);
+            cancelNotification(pkg, tag, id, 0, 0);
+            // TODO: Tell the activity manager.
+        }
     };
 
     private BroadcastReceiver mIntentReceiver = new BroadcastReceiver() {
diff --git a/services/java/com/android/server/status/StatusBarManagerService.java b/services/java/com/android/server/status/StatusBarManagerService.java
index 0af1ebb..925a6eb 100644
--- a/services/java/com/android/server/status/StatusBarManagerService.java
+++ b/services/java/com/android/server/status/StatusBarManagerService.java
@@ -87,6 +87,7 @@ public class StatusBarManagerService extends IStatusBarService.Stub
         void onClearAll();
         void onNotificationClick(String pkg, String tag, int id);
         void onPanelRevealed();
+        void onNotificationError(String pkg, String tag, int id, String message);
     }
 
     /**
@@ -279,6 +280,12 @@ public class StatusBarManagerService extends IStatusBarService.Stub
         mNotificationCallbacks.onNotificationClick(pkg, tag, id);
     }
 
+    public void onNotificationError(String pkg, String tag, int id, String message) {
+        Slog.d(TAG, "onNotificationError message=" + message);
+        // WARNING: this will call back into us to do the remove.  Don't hold any locks.
+        mNotificationCallbacks.onNotificationError(pkg, tag, id, message);
+    }
+
     public void onClearAllNotifications() {
         mNotificationCallbacks.onClearAll();
     }
diff --git a/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java b/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java
index f96a079..4d071e3 100644
--- a/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java
+++ b/tests/StatusBar/src/com/android/statusbartest/NotificationTestList.java
@@ -123,29 +123,49 @@ public class NotificationTestList extends TestActivity
             }
         },
 
-        new Test("Bad Icon") {
+        new Test("Bad Icon #1 (when=create)") {
             public void run() {
-                mNM.notify(1, new Notification(NotificationTestList.this,
-                            R.layout.chrono_notification, /* not a drawable! */
-                            null, System.currentTimeMillis()-(1000*60*60*24),
-                            "(453) 123-2328",
-                            "", null));
+                Notification n = new Notification(R.layout.chrono_notification /* not an icon */,
+                        null, mActivityCreateTime);
+                n.setLatestEventInfo(NotificationTestList.this, "Persistent #1",
+                            "This is the same notification!!!", makeIntent());
+                mNM.notify(1, n);
             }
         },
 
-        new Test("Bad resource #2") {
-            public void run()
-            {
-                Notification n = new Notification(NotificationTestList.this,
-                            R.drawable.ic_statusbar_missedcall,
-                            null, System.currentTimeMillis()-(1000*60*60*24),
-                            "(453) 123-2328",
-                            "", null);
+        new Test("Bad Icon #1 (when=now)") {
+            public void run() {
+                Notification n = new Notification(R.layout.chrono_notification /* not an icon */,
+                        null, System.currentTimeMillis());
+                n.setLatestEventInfo(NotificationTestList.this, "Persistent #1",
+                            "This is the same notification!!!", makeIntent());
+                mNM.notify(1, n);
+            }
+        },
+
+        new Test("Bad resource #1 (when=create)") {
+            public void run() {
+                Notification n = new Notification(R.drawable.icon2,
+                        null, mActivityCreateTime);
+                n.setLatestEventInfo(NotificationTestList.this, "Persistent #1",
+                            "This is the same notification!!!", makeIntent());
                 n.contentView.setInt(1 /*bogus*/, "bogus method", 666);
-                mNM.notify(2, n);
+                mNM.notify(1, n);
             }
         },
 
+        new Test("Bad resource #1 (when=now)") {
+            public void run() {
+                Notification n = new Notification(R.drawable.icon2,
+                        null, System.currentTimeMillis());
+                n.setLatestEventInfo(NotificationTestList.this, "Persistent #1",
+                            "This is the same notification!!!", makeIntent());
+                n.contentView.setInt(1 /*bogus*/, "bogus method", 666);
+                mNM.notify(1, n);
+            }
+        },
+
+
         new Test("Bad resource #3") {
             public void run()
             {