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

Commit 1a497d3a authored by Daniel Sandler's avatar Daniel Sandler
Browse files

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
parent 5d1a182a

core/java/android/app/Notification.java

+33 −8
Original line number Diff line number Diff line
@@ -635,11 +635,16 @@ public class Notification implements Parcelable 
    @Override

    public Notification clone() {

        Notification that = new Notification();

        cloneInto(that);

        cloneInto(that, true);

        return that;

    }



    private void cloneInto(Notification that) {

    /**

     * Copy all (or if heavy is false, all except Bitmaps and RemoteViews) members

     * of this into that.

     * @hide

     */

    public void cloneInto(Notification that, boolean heavy) {

        that.when = this.when;

        that.icon = this.icon;

        that.number = this.number;
@@ -652,13 +657,13 @@ public class Notification implements Parcelable 
        if (this.tickerText != null) {

            that.tickerText = this.tickerText.toString();

        }

        if (this.tickerView != null) {

        if (heavy && this.tickerView != null) {

            that.tickerView = this.tickerView.clone();

        }

        if (this.contentView != null) {

        if (heavy && this.contentView != null) {

            that.contentView = this.contentView.clone();

        }

        if (this.largeIcon != null) {

        if (heavy && this.largeIcon != null) {

            that.largeIcon = Bitmap.createBitmap(this.largeIcon);

        }

        that.iconLevel = this.iconLevel;
@@ -690,7 +695,6 @@ public class Notification implements Parcelable 


        if (this.extras != null) {

            that.extras = new Bundle(this.extras);



        }



        if (this.actions != null) {
@@ -700,9 +704,30 @@ public class Notification implements Parcelable 
            }

        }



        if (this.bigContentView != null) {

        if (heavy && this.bigContentView != null) {

            that.bigContentView = this.bigContentView.clone();

        }



        if (!heavy) {

            that.lightenPayload(); // will clean out extras

        }

    }



    /**

     * Removes heavyweight parts of the Notification object for archival or for sending to

     * listeners when the full contents are not necessary.

     * @hide

     */

    public final void lightenPayload() {

        tickerView = null;

        contentView = null;

        bigContentView = null;

        largeIcon = null;

        if (extras != null) {

            extras.remove(Notification.EXTRA_LARGE_ICON);

            extras.remove(Notification.EXTRA_LARGE_ICON_BIG);

            extras.remove(Notification.EXTRA_PICTURE);

        }

    }



    public int describeContents() {
@@ -1706,7 +1731,7 @@ public class Notification implements Parcelable 
         * @hide

         */

        public Notification buildInto(Notification n) {

            build().cloneInto(n);

            build().cloneInto(n, true);

            return n;

        }

    }

core/java/android/service/notification/NotificationListenerService.java

+10 −3
Original line number Diff line number Diff line
@@ -55,10 +55,17 @@ public abstract class NotificationListenerService extends Service { 
     * <P>

     * This might occur because the user has dismissed the notification using system UI (or another

     * notification listener) or because the app has withdrawn the notification.

     * <P>

     * NOTE: The {@link StatusBarNotification} object you receive will be "light"; that is, the

     * {@link StatusBarNotification#notification} member may be missing some heavyweight

     * fields such as {@link android.app.Notification#contentView} and

     * {@link android.app.Notification#largeIcon}. However, all other fields on

     * {@link StatusBarNotification}, sufficient to match this call with a prior call to

     * {@link #onNotificationPosted(StatusBarNotification)}, will be intact.

     *

     * @param sbn A data structure encapsulating the original {@link android.app.Notification}

     *            object as well as its identifying information (tag and id) and source

     *            (package name).

     * @param sbn A data structure encapsulating at least the original information (tag and id)

     *            and source (package name) used to post the {@link android.app.Notification} that

     *            was just removed.

     */

    public abstract void onNotificationRemoved(StatusBarNotification sbn);

core/java/android/service/notification/StatusBarNotification.java

+11 −0
Original line number Diff line number Diff line
@@ -152,6 +152,17 @@ public class StatusBarNotification implements Parcelable { 
        }

    };



    /**

     * @hide

     */

    public StatusBarNotification cloneLight() {

        final Notification no = new Notification();

        this.notification.cloneInto(no, false); // light copy

        return new StatusBarNotification(this.pkg, this.basePkg,

                this.id, this.tag, this.uid, this.initialPid,

                this.score, no, this.user, this.postTime);

    }



    @Override

    public StatusBarNotification clone() {

        return new StatusBarNotification(this.pkg, this.basePkg,

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

+10 −13
Original line number Diff line number Diff line
@@ -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);

                }});

        }

    }