From 4f0f9d2d856723ef0dd07ef6bfbb5cde2e94ad87 Mon Sep 17 00:00:00 2001 From: Will Brockman Date: Sun, 23 Feb 2020 21:18:11 -0500 Subject: [PATCH] Start InstanceIDs at 1. Protobufs make it easy to confuse default and unset values. Test: atest NotificationManagerServiceTest Fixes: 149760215 Change-Id: Ia27f33cb8331960369f86e75ac3cfa148dad887a --- .../internal/logging/InstanceIdSequence.java | 10 ++++----- .../logging/InstanceIdSequenceFake.java | 6 +++--- .../NotificationManagerServiceTest.java | 21 ++++++++++--------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/core/java/com/android/internal/logging/InstanceIdSequence.java b/core/java/com/android/internal/logging/InstanceIdSequence.java index aa507e538944e..34643105b9658 100644 --- a/core/java/com/android/internal/logging/InstanceIdSequence.java +++ b/core/java/com/android/internal/logging/InstanceIdSequence.java @@ -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)); } /** diff --git a/services/tests/uiservicestests/src/com/android/internal/logging/InstanceIdSequenceFake.java b/services/tests/uiservicestests/src/com/android/internal/logging/InstanceIdSequenceFake.java index 1629ef00b65a1..6e7df05e8afe1 100644 --- a/services/tests/uiservicestests/src/com/android/internal/logging/InstanceIdSequenceFake.java +++ b/services/tests/uiservicestests/src/com/android/internal/logging/InstanceIdSequenceFake.java @@ -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++); } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index d0283f78338da..47075f92d254e 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -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