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

1 files changed, 10 insertions, 13 deletions

diff --git a/services/java/com/android/server/NotificationManagerService.java b/services/java/com/android/server/NotificationManagerService.java
index 3bebf91a922c..dc43f9279527 100644
--- a/services/java/com/android/server/NotificationManagerService.java
+++ b/services/java/com/android/server/NotificationManagerService.java
@@ -237,7 +237,7 @@ public class NotificationManagerService extends INotificationManager.Stub
             try {
                 listener.onNotificationPosted(sbn);
             } catch (RemoteException ex) {
-                // not there?
+                Log.e(TAG, "unable to notify listener (posted): " + listener, ex);
             }
         }
 
@@ -246,7 +246,7 @@ public class NotificationManagerService extends INotificationManager.Stub
             try {
                 listener.onNotificationRemoved(sbn);
             } catch (RemoteException ex) {
-                // not there?
+                Log.e(TAG, "unable to notify listener (removed): " + listener, ex);
             }
         }
 
@@ -285,14 +285,7 @@ public class NotificationManagerService extends INotificationManager.Stub
 
         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 @@ public class NotificationManagerService extends INotificationManager.Stub
      * 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 @@ public class NotificationManagerService extends INotificationManager.Stub
      * 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);
                 }});
         }
     }