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

Commit 7d67bd4f authored by Dan Sandler's avatar Dan Sandler
Browse files

Improved notification interruptiveness calculation. 

Apparently comparing Spannables is dangerous because
the various Span classes do not implement .equals() in any
meaningful way, so all CharSequences must be converted to
flat Strings before being compared.

Lots of additional debug code remains, for the next time we
don't understand why an innocuous notification update
appears to be interruptive.

Test: atest com.android.server.notification.NotificationManagerServiceTest
      atest com.android.server.notification.NotificationTest
Bug: 78643290
Change-Id: I1c282238687f28b5b197e28a4b878dc697049f4d
parent 82db2614

core/java/android/app/Notification.java

+45 −12
Original line number Diff line number Diff line
@@ -6374,6 +6374,8 @@ public class Notification implements Parcelable 


        /**

         * @hide

         * Note that we aren't actually comparing the contents of the bitmaps here, so this

         * is only doing a cursory inspection. Bitmaps of equal size will appear the same.

         */

        @Override

        public boolean areNotificationsVisiblyDifferent(Style other) {
@@ -6381,7 +6383,20 @@ public class Notification implements Parcelable 
                return true;

            }

            BigPictureStyle otherS = (BigPictureStyle) other;

            return !Objects.equals(getBigPicture(), otherS.getBigPicture());

            return areBitmapsObviouslyDifferent(getBigPicture(), otherS.getBigPicture());

        }



        private static boolean areBitmapsObviouslyDifferent(Bitmap a, Bitmap b) {

            if (a == b) {

                return false;

            }

            if (a == null || b == null) {

                return true;

            }

            return a.getWidth() != b.getWidth()

                    || a.getHeight() != b.getHeight()

                    || a.getConfig() != b.getConfig()

                    || a.getGenerationId() != b.getGenerationId();

        }

    }
@@ -6526,6 +6541,7 @@ public class Notification implements Parcelable 


        /**

         * @hide

         * Spans are ignored when comparing text for visual difference.

         */

        @Override

        public boolean areNotificationsVisiblyDifferent(Style other) {
@@ -6533,7 +6549,7 @@ public class Notification implements Parcelable 
                return true;

            }

            BigTextStyle newS = (BigTextStyle) other;

            return !Objects.equals(getBigText(), newS.getBigText());

            return !Objects.equals(String.valueOf(getBigText()), String.valueOf(newS.getBigText()));

        }



        static void applyBigTextContentView(Builder builder,
@@ -6900,6 +6916,7 @@ public class Notification implements Parcelable 


        /**

         * @hide

         * Spans are ignored when comparing text for visual difference.

         */

        @Override

        public boolean areNotificationsVisiblyDifferent(Style other) {
@@ -6910,10 +6927,7 @@ public class Notification implements Parcelable 
            List<MessagingStyle.Message> oldMs = getMessages();

            List<MessagingStyle.Message> newMs = newS.getMessages();



            if (oldMs == null) {

                oldMs = new ArrayList<>();

            }

            if (newMs == null) {

            if (oldMs == null || newMs == null) {

                newMs = new ArrayList<>();

            }
@@ -6924,16 +6938,20 @@ public class Notification implements Parcelable 
            for (int i = 0; i < n; i++) {

                MessagingStyle.Message oldM = oldMs.get(i);

                MessagingStyle.Message newM = newMs.get(i);

                if (!Objects.equals(oldM.getText(), newM.getText())) {

                if (!Objects.equals(

                        String.valueOf(oldM.getText()),

                        String.valueOf(newM.getText()))) {

                    return true;

                }

                if (!Objects.equals(oldM.getDataUri(), newM.getDataUri())) {

                    return true;

                }

                CharSequence oldSender = oldM.getSenderPerson() == null ? oldM.getSender()

                        : oldM.getSenderPerson().getName();

                CharSequence newSender = newM.getSenderPerson() == null ? newM.getSender()

                        : newM.getSenderPerson().getName();

                String oldSender = String.valueOf(oldM.getSenderPerson() == null

                        ? oldM.getSender()

                        : oldM.getSenderPerson().getName());

                String newSender = String.valueOf(newM.getSenderPerson() == null

                        ? newM.getSender()

                        : newM.getSenderPerson().getName());

                if (!Objects.equals(oldSender, newSender)) {

                    return true;

                }
@@ -7533,7 +7551,22 @@ public class Notification implements Parcelable 
                return true;

            }

            InboxStyle newS = (InboxStyle) other;

            return !Objects.equals(getLines(), newS.getLines());



            final ArrayList<CharSequence> myLines = getLines();

            final ArrayList<CharSequence> newLines = newS.getLines();

            final int n = myLines.size();

            if (n != newLines.size()) {

                return true;

            }



            for (int i = 0; i < n; i++) {

                if (!Objects.equals(

                        String.valueOf(myLines.get(i)),

                        String.valueOf(newLines.get(i)))) {

                    return true;

                }

            }

            return false;

        }



        private void handleInboxImageMargin(RemoteViews contentView, int id, boolean first,

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

+62 −6
Original line number Diff line number Diff line
@@ -243,6 +243,9 @@ public class NotificationManagerService extends SystemService { 
    public static final boolean ENABLE_CHILD_NOTIFICATIONS

            = SystemProperties.getBoolean("debug.child_notifs", true);



    static final boolean DEBUG_INTERRUPTIVENESS = SystemProperties.getBoolean(

            "debug.notification.interruptiveness", false);



    static final int MAX_PACKAGE_NOTIFICATIONS = 50;

    static final float DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE = 5f;
@@ -4437,7 +4440,7 @@ public class NotificationManagerService extends SystemService { 
                    if (index < 0) {

                        mNotificationList.add(r);

                        mUsageStats.registerPostedByApp(r);

                        r.setInterruptive(true);

                        r.setInterruptive(isVisuallyInterruptive(null, r));

                    } else {

                        old = mNotificationList.get(index);

                        mNotificationList.set(index, r);
@@ -4516,31 +4519,75 @@ public class NotificationManagerService extends SystemService { 
    @GuardedBy("mNotificationLock")

    @VisibleForTesting

    protected boolean isVisuallyInterruptive(NotificationRecord old, NotificationRecord r) {

        if (old == null) {

            if (DEBUG_INTERRUPTIVENESS) {

                Log.v(TAG, "INTERRUPTIVENESS: "

                        +  r.getKey() + " is interruptive: new notification");

            }

            return true;

        }



        Notification oldN = old.sbn.getNotification();

        Notification newN = r.sbn.getNotification();



        if (oldN.extras == null || newN.extras == null) {

            if (DEBUG_INTERRUPTIVENESS) {

                Log.v(TAG, "INTERRUPTIVENESS: "

                        +  r.getKey() + " is not interruptive: no extras");

            }

            return false;

        }



        // Ignore visual interruptions from foreground services because users

        // consider them one 'session'. Count them for everything else.

        if (r != null && (r.sbn.getNotification().flags & FLAG_FOREGROUND_SERVICE) != 0) {

            if (DEBUG_INTERRUPTIVENESS) {

                Log.v(TAG, "INTERRUPTIVENESS: "

                        +  r.getKey() + " is not interruptive: foreground service");

            }

            return false;

        }



        if (!Objects.equals(oldN.extras.get(Notification.EXTRA_TITLE),

                newN.extras.get(Notification.EXTRA_TITLE))) {

        final String oldTitle = String.valueOf(oldN.extras.get(Notification.EXTRA_TITLE));

        final String newTitle = String.valueOf(newN.extras.get(Notification.EXTRA_TITLE));

        if (!Objects.equals(oldTitle, newTitle)) {

            if (DEBUG_INTERRUPTIVENESS) {

                Log.v(TAG, "INTERRUPTIVENESS: "

                        +  r.getKey() + " is interruptive: changed title");

                Log.v(TAG, "INTERRUPTIVENESS: " + String.format("   old title: %s (%s@0x%08x)",

                        oldTitle, oldTitle.getClass(), oldTitle.hashCode()));

                Log.v(TAG, "INTERRUPTIVENESS: " + String.format("   new title: %s (%s@0x%08x)",

                        newTitle, newTitle.getClass(), newTitle.hashCode()));

            }

            return true;

        }

        if (!Objects.equals(oldN.extras.get(Notification.EXTRA_TEXT),

                newN.extras.get(Notification.EXTRA_TEXT))) {

        // Do not compare Spannables (will always return false); compare unstyled Strings

        final String oldText = String.valueOf(oldN.extras.get(Notification.EXTRA_TEXT));

        final String newText = String.valueOf(newN.extras.get(Notification.EXTRA_TEXT));

        if (!Objects.equals(oldText, newText)) {

            if (DEBUG_INTERRUPTIVENESS) {

                Log.v(TAG, "INTERRUPTIVENESS: "

                        + r.getKey() + " is interruptive: changed text");

                Log.v(TAG, "INTERRUPTIVENESS: " + String.format("   old text: %s (%s@0x%08x)",

                        oldText, oldText.getClass(), oldText.hashCode()));

                Log.v(TAG, "INTERRUPTIVENESS: " + String.format("   new text: %s (%s@0x%08x)",

                        newText, newText.getClass(), newText.hashCode()));

            }

            return true;

        }

        if (oldN.extras.containsKey(Notification.EXTRA_PROGRESS) && newN.hasCompletedProgress()) {

        if (oldN.hasCompletedProgress() != newN.hasCompletedProgress()) {

            if (DEBUG_INTERRUPTIVENESS) {

                Log.v(TAG, "INTERRUPTIVENESS: "

                    +  r.getKey() + " is interruptive: completed progress");

            }

            return true;

        }

        // Actions

        if (Notification.areActionsVisiblyDifferent(oldN, newN)) {

            if (DEBUG_INTERRUPTIVENESS) {

                Log.v(TAG, "INTERRUPTIVENESS: "

                        +  r.getKey() + " is interruptive: changed actions");

            }

            return true;

        }
@@ -4550,16 +4597,25 @@ public class NotificationManagerService extends SystemService { 


            // Style based comparisons

            if (Notification.areStyledNotificationsVisiblyDifferent(oldB, newB)) {

                if (DEBUG_INTERRUPTIVENESS) {

                    Log.v(TAG, "INTERRUPTIVENESS: "

                            +  r.getKey() + " is interruptive: styles differ");

                }

                return true;

            }



            // Remote views

            if (Notification.areRemoteViewsChanged(oldB, newB)) {

                if (DEBUG_INTERRUPTIVENESS) {

                    Log.v(TAG, "INTERRUPTIVENESS: "

                            +  r.getKey() + " is interruptive: remoteviews differ");

                }

                return true;

            }

        } catch (Exception e) {

            Slog.w(TAG, "error recovering builder", e);

        }



        return false;

    }

services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java

+127 −0
Original line number Diff line number Diff line
@@ -104,6 +104,7 @@ import android.testing.AndroidTestingRunner; 
import android.testing.TestableContext;

import android.testing.TestableLooper;

import android.testing.TestableLooper.RunWithLooper;

import android.text.Html;

import android.util.ArrayMap;

import android.util.AtomicFile;
@@ -2730,6 +2731,56 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertTrue(mService.isVisuallyInterruptive(r1, r2));

    }



    @Test

    public void testVisualDifference_inboxStyle() {

        Notification.Builder nb1 = new Notification.Builder(mContext, "")

                .setStyle(new Notification.InboxStyle()

                    .addLine("line1").addLine("line2"));

        StatusBarNotification sbn1 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb1.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r1 =

                new NotificationRecord(mContext, sbn1, mock(NotificationChannel.class));



        Notification.Builder nb2 = new Notification.Builder(mContext, "")

                .setStyle(new Notification.InboxStyle()

                        .addLine("line1").addLine("line2_changed"));

        StatusBarNotification sbn2 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb2.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r2 =

                new NotificationRecord(mContext, sbn2, mock(NotificationChannel.class));



        assertTrue(mService.isVisuallyInterruptive(r1, r2)); // line 2 changed unnoticed



        Notification.Builder nb3 = new Notification.Builder(mContext, "")

                .setStyle(new Notification.InboxStyle()

                        .addLine("line1"));

        StatusBarNotification sbn3 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb3.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r3 =

                new NotificationRecord(mContext, sbn3, mock(NotificationChannel.class));



        assertTrue(mService.isVisuallyInterruptive(r1, r3)); // line 2 removed unnoticed



        Notification.Builder nb4 = new Notification.Builder(mContext, "")

                .setStyle(new Notification.InboxStyle()

                        .addLine("line1").addLine("line2").addLine("line3"));

        StatusBarNotification sbn4 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb4.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r4 =

                new NotificationRecord(mContext, sbn4, mock(NotificationChannel.class));



        assertTrue(mService.isVisuallyInterruptive(r1, r4)); // line 3 added unnoticed



        Notification.Builder nb5 = new Notification.Builder(mContext, "")

            .setContentText("not an inbox");

        StatusBarNotification sbn5 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb5.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r5 =

                new NotificationRecord(mContext, sbn5, mock(NotificationChannel.class));



        assertTrue(mService.isVisuallyInterruptive(r1, r5)); // changed Styles, went unnoticed

    }



    @Test

    public void testVisualDifference_diffText() {

        Notification.Builder nb1 = new Notification.Builder(mContext, "")
@@ -2749,6 +2800,63 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertTrue(mService.isVisuallyInterruptive(r1, r2));

    }



    @Test

    public void testVisualDifference_sameText() {

        Notification.Builder nb1 = new Notification.Builder(mContext, "")

                .setContentText("foo");

        StatusBarNotification sbn1 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb1.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r1 =

                new NotificationRecord(mContext, sbn1, mock(NotificationChannel.class));



        Notification.Builder nb2 = new Notification.Builder(mContext, "")

                .setContentText("foo");

        StatusBarNotification sbn2 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb2.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r2 =

                new NotificationRecord(mContext, sbn2, mock(NotificationChannel.class));



        assertFalse(mService.isVisuallyInterruptive(r1, r2));

    }



    @Test

    public void testVisualDifference_sameTextButStyled() {

        Notification.Builder nb1 = new Notification.Builder(mContext, "")

                .setContentText(Html.fromHtml("<b>foo</b>"));

        StatusBarNotification sbn1 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb1.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r1 =

                new NotificationRecord(mContext, sbn1, mock(NotificationChannel.class));



        Notification.Builder nb2 = new Notification.Builder(mContext, "")

                .setContentText(Html.fromHtml("<b>foo</b>"));

        StatusBarNotification sbn2 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb2.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r2 =

                new NotificationRecord(mContext, sbn2, mock(NotificationChannel.class));



        assertFalse(mService.isVisuallyInterruptive(r1, r2));

    }



    @Test

    public void testVisualDifference_diffTextButStyled() {

        Notification.Builder nb1 = new Notification.Builder(mContext, "")

                .setContentText(Html.fromHtml("<b>foo</b>"));

        StatusBarNotification sbn1 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb1.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r1 =

                new NotificationRecord(mContext, sbn1, mock(NotificationChannel.class));



        Notification.Builder nb2 = new Notification.Builder(mContext, "")

                .setContentText(Html.fromHtml("<b>bar</b>"));

        StatusBarNotification sbn2 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb2.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r2 =

                new NotificationRecord(mContext, sbn2, mock(NotificationChannel.class));



        assertTrue(mService.isVisuallyInterruptive(r1, r2));

    }



    @Test

    public void testVisualDifference_diffProgress() {

        Notification.Builder nb1 = new Notification.Builder(mContext, "")
@@ -2787,6 +2895,25 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertFalse(mService.isVisuallyInterruptive(r1, r2));

    }



    @Test

    public void testVisualDifference_sameProgressStillDone() {

        Notification.Builder nb1 = new Notification.Builder(mContext, "")

                .setProgress(100, 100, false);

        StatusBarNotification sbn1 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb1.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r1 =

                new NotificationRecord(mContext, sbn1, mock(NotificationChannel.class));



        Notification.Builder nb2 = new Notification.Builder(mContext, "")

                .setProgress(100, 100, false);

        StatusBarNotification sbn2 = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,

                nb2.build(), new UserHandle(mUid), null, 0);

        NotificationRecord r2 =

                new NotificationRecord(mContext, sbn2, mock(NotificationChannel.class));



        assertFalse(mService.isVisuallyInterruptive(r1, r2));

    }



    @Test

    public void testHideAndUnhideNotificationsOnSuspendedPackageBroadcast() {

        // post 2 notification from this package

services/tests/uiservicestests/src/com/android/server/notification/NotificationTest.java

+7 −2
Original line number Diff line number Diff line
@@ -221,10 +221,15 @@ public class NotificationTest extends UiServiceTestCase { 


    @Test

    public void testBigPictureChange() {

        Bitmap bitA = mock(Bitmap.class);

        when(bitA.getGenerationId()).thenReturn(100);

        Bitmap bitB = mock(Bitmap.class);

        when(bitB.getGenerationId()).thenReturn(200);



        Notification.Builder nBigPic1 = new Notification.Builder(mContext, "test")

                .setStyle(new Notification.BigPictureStyle().bigPicture(mock(Bitmap.class)));

                .setStyle(new Notification.BigPictureStyle().bigPicture(bitA));

        Notification.Builder nBigPic2 = new Notification.Builder(mContext, "test")

                .setStyle(new Notification.BigPictureStyle().bigPicture(mock(Bitmap.class)));

                .setStyle(new Notification.BigPictureStyle().bigPicture(bitB));



        assertTrue(Notification.areStyledNotificationsVisiblyDifferent(nBigPic1, nBigPic2));

    }