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

Commit 4f0f9d2d authored by Will Brockman's avatar Will Brockman
Browse files

Start InstanceIDs at 1. 

Protobufs make it easy to confuse default and unset values.

Test: atest NotificationManagerServiceTest
Fixes: 149760215
Change-Id: Ia27f33cb8331960369f86e75ac3cfa148dad887a
parent c6616838

core/java/com/android/internal/logging/InstanceIdSequence.java

+5 −5
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ import java.security.SecureRandom; 
import java.util.Random;



/**

 * Generates random InstanceIds in range [0, instanceIdMax) for passing to

 * Generates random InstanceIds in range [1, instanceIdMax] for passing to

 * UiEventLogger.logWithInstanceId(). Holds a SecureRandom, which self-seeds on

 * first use; try to give it a long lifetime. Safe for concurrent use.

 */
@@ -34,12 +34,12 @@ public class InstanceIdSequence { 
    private final Random mRandom = new SecureRandom();



    /**

     * Constructs a sequence with identifiers [0, instanceIdMax).  Capped at INSTANCE_ID_MAX.

     * Constructs a sequence with identifiers [1, instanceIdMax].  Capped at INSTANCE_ID_MAX.

     * @param instanceIdMax Limiting value of identifiers. Normally positive: otherwise you get

     *                      an all-zero sequence.

     *                      an all-1 sequence.

     */

    public InstanceIdSequence(int instanceIdMax) {

        mInstanceIdMax = min(max(0, instanceIdMax), InstanceId.INSTANCE_ID_MAX);

        mInstanceIdMax = min(max(1, instanceIdMax), InstanceId.INSTANCE_ID_MAX);

    }



    /**
@@ -47,7 +47,7 @@ public class InstanceIdSequence { 
     * @return new InstanceId

     */

    public InstanceId newInstanceId() {

        return newInstanceIdInternal(mRandom.nextInt(mInstanceIdMax));

        return newInstanceIdInternal(1 + mRandom.nextInt(mInstanceIdMax));

    }



    /**

services/tests/uiservicestests/src/com/android/internal/logging/InstanceIdSequenceFake.java

+3 −3
Original line number Diff line number Diff line
@@ -17,7 +17,7 @@ 
package com.android.internal.logging;



/**

 * A fake implementation of InstanceIdSequence that returns 0, 1, 2, ...

 * A fake implementation of InstanceIdSequence that returns 1, 2, ...

 */

public class InstanceIdSequenceFake extends InstanceIdSequence {
@@ -25,13 +25,13 @@ public class InstanceIdSequenceFake extends InstanceIdSequence { 
        super(instanceIdMax);

    }



    private int mNextId = 0;

    private int mNextId = 1;



    @Override

    public InstanceId newInstanceId() {

        synchronized (this) {

            if (mNextId >= mInstanceIdMax) {

                mNextId = 0;

                mNextId = 1;

            }

            return newInstanceIdInternal(mNextId++);

        }

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

+11 −10
Original line number Diff line number Diff line
@@ -153,7 +153,6 @@ import android.widget.RemoteViews; 
import androidx.annotation.Nullable;

import androidx.test.InstrumentationRegistry;



import com.android.internal.R;

import com.android.internal.config.sysui.SystemUiDeviceConfigFlags;

import com.android.internal.logging.InstanceIdSequence;

import com.android.internal.logging.InstanceIdSequenceFake;
@@ -1164,7 +1163,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertEquals(PKG, call.r.getSbn().getPackageName());

        assertEquals(0, call.r.getSbn().getId());

        assertEquals(tag, call.r.getSbn().getTag());

        assertEquals(0, call.getInstanceId());  // Fake instance IDs are assigned in order

        assertEquals(1, call.getInstanceId());  // Fake instance IDs are assigned in order

    }



    @Test
@@ -1186,14 +1185,14 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertEquals(

                NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_POSTED,

                mNotificationRecordLogger.get(0).event);

        assertEquals(0, mNotificationRecordLogger.get(0).getInstanceId());

        assertEquals(1, mNotificationRecordLogger.get(0).getInstanceId());



        assertTrue(mNotificationRecordLogger.get(1).shouldLogReported);

        assertEquals(

                NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_UPDATED,

                mNotificationRecordLogger.get(1).event);

        // Instance ID doesn't change on update of an active notification

        assertEquals(0, mNotificationRecordLogger.get(1).getInstanceId());

        assertEquals(1, mNotificationRecordLogger.get(1).getInstanceId());

    }



    @Test
@@ -1248,19 +1247,19 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
                NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_POSTED,

                mNotificationRecordLogger.get(0).event);

        assertTrue(mNotificationRecordLogger.get(0).shouldLogReported);

        assertEquals(0, mNotificationRecordLogger.get(0).getInstanceId());

        assertEquals(1, mNotificationRecordLogger.get(0).getInstanceId());



        assertEquals(

                NotificationRecordLogger.NotificationCancelledEvent.NOTIFICATION_CANCEL_APP_CANCEL,

                mNotificationRecordLogger.get(1).event);

        assertEquals(0, mNotificationRecordLogger.get(1).getInstanceId());

        assertEquals(1, mNotificationRecordLogger.get(1).getInstanceId());



        assertEquals(

                NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_POSTED,

                mNotificationRecordLogger.get(2).event);

        assertTrue(mNotificationRecordLogger.get(2).shouldLogReported);

        // New instance ID because notification was canceled before re-post

        assertEquals(1, mNotificationRecordLogger.get(2).getInstanceId());

        assertEquals(2, mNotificationRecordLogger.get(2).getInstanceId());

    }



    @Test
@@ -3453,6 +3452,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
    @Test

    public void testStats_dismissalSurface() throws Exception {

        final NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);

        r.getSbn().setInstanceId(mNotificationInstanceIdSequence.newInstanceId());

        mService.addNotification(r);



        final NotificationVisibility nv = NotificationVisibility.obtain(r.getKey(), 0, 1, true);
@@ -3470,7 +3470,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertEquals(

                NotificationRecordLogger.NotificationCancelledEvent.NOTIFICATION_CANCEL_USER_AOD,

                mNotificationRecordLogger.get(0).event);

        assertEquals(0, mNotificationRecordLogger.get(0).getInstanceId());

        assertEquals(1, mNotificationRecordLogger.get(0).getInstanceId());

    }



    @Test
@@ -4344,6 +4344,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        final NotificationRecord r = generateNotificationRecord(

                mTestNotificationChannel, 1, null, true);

        r.setTextChanged(true);

        r.getSbn().setInstanceId(mNotificationInstanceIdSequence.newInstanceId());

        mService.addNotification(r);



        mService.mNotificationDelegate.onNotificationVisibilityChanged(new NotificationVisibility[]
@@ -4353,7 +4354,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertEquals(1, mNotificationRecordLogger.getCalls().size());

        assertEquals(NotificationRecordLogger.NotificationEvent.NOTIFICATION_OPEN,

                mNotificationRecordLogger.get(0).event);

        assertEquals(0, mNotificationRecordLogger.get(0).getInstanceId());

        assertEquals(1, mNotificationRecordLogger.get(0).getInstanceId());



        mService.mNotificationDelegate.onNotificationVisibilityChanged(

                new NotificationVisibility[]{},
@@ -4364,7 +4365,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { 
        assertEquals(2, mNotificationRecordLogger.getCalls().size());

        assertEquals(NotificationRecordLogger.NotificationEvent.NOTIFICATION_CLOSE,

                mNotificationRecordLogger.get(1).event);

        assertEquals(0, mNotificationRecordLogger.get(1).getInstanceId());

        assertEquals(1, mNotificationRecordLogger.get(1).getInstanceId());

    }



    @Test