commit 1a497d3a2b1496c12949e47e55f8e46d8f585be5
author Daniel Sandler <dsandler@android.com> Thu Apr 18 14:52:45 2013 -0400
committer Daniel Sandler <dsandler@android.com> Mon Apr 22 15:14:55 2013 -0400
tree f94fde92b7408ac80ff1a2682297e703caad94de
parent 5d1a182a8a2dd9613ef3b1f2de7b6a3d690ae890

Fix concurrency issues when parceling StatusBarNotifications.

Protip: Don't mess with Bundles after you've sent them off
for parceling in an RPC.

Note that this change reduces the payload size of
StatusBarNotification objects received in
onNotificationRemoved() callbacks; it scrubs out the
RemoteViews and Bitmaps just as the NoMan's internal archive
does. [You don't really need that information anyway when
hearing about a removed notification; most likely all you
need are the other slots on StatusBarNotification, but
nulling the whole Notification object breaks a lot of
clients.]

Bug: 8616295
Change-Id: Ic899045f2352b96dcf064d3e9e51dad52629aea3

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

diff --git a/services/java/com/android/server/NotificationManagerService.java b/services/java/com/android/server/NotificationManagerService.java
index 3bebf91..dc43f92 100644
--- a/services/java/com/android/server/NotificationManagerService.java
+++ b/services/java/com/android/server/NotificationManagerService.java
@@ -237,7 +237,7 @@
             try {
                 listener.onNotificationPosted(sbn);
             } catch (RemoteException ex) {
-                // not there?
+                Log.e(TAG, "unable to notify listener (posted): " + listener, ex);
             }
         }
 
@@ -246,7 +246,7 @@
             try {
                 listener.onNotificationRemoved(sbn);
             } catch (RemoteException ex) {
-                // not there?
+                Log.e(TAG, "unable to notify listener (removed): " + listener, ex);
             }
         }
 
@@ -285,14 +285,7 @@
 
         public void record(StatusBarNotification nr) {
             // Nuke heavy parts of notification before storing in archive
-            nr.notification.tickerView = null;
-            nr.notification.contentView = null;
-            nr.notification.bigContentView = null;
-            nr.notification.largeIcon = null;
-            final Bundle extras = nr.notification.extras;
-            extras.remove(Notification.EXTRA_LARGE_ICON);
-            extras.remove(Notification.EXTRA_LARGE_ICON_BIG);
-            extras.remove(Notification.EXTRA_PICTURE);
+            nr.notification.lightenPayload();
 
             if (mBuffer.size() == BUFFER_SIZE) {
                 mBuffer.removeFirst();
@@ -741,7 +734,8 @@
      * asynchronously notify all listeners about a new notification
      */
     private void notifyPostedLocked(NotificationRecord n) {
-        final StatusBarNotification sbn = n.sbn;
+        // make a copy in case changes are made to the underlying Notification object
+        final StatusBarNotification sbn = n.sbn.clone();
         for (final NotificationListenerInfo info : mListeners) {
             mHandler.post(new Runnable() {
                 @Override
@@ -755,12 +749,15 @@
      * asynchronously notify all listeners about a removed notification
      */
     private void notifyRemovedLocked(NotificationRecord n) {
-        final StatusBarNotification sbn = n.sbn;
+        // make a copy in case changes are made to the underlying Notification object
+        // NOTE: this copy is lightweight: it doesn't include heavyweight parts of the notification
+        final StatusBarNotification sbn_light = n.sbn.cloneLight();
+
         for (final NotificationListenerInfo info : mListeners) {
             mHandler.post(new Runnable() {
                 @Override
                 public void run() {
-                    info.notifyRemovedIfUserMatch(sbn);
+                    info.notifyRemovedIfUserMatch(sbn_light);
                 }});
         }
     }