From 2bf7f520867d98b4ae4d08ccc4b4ff5e580fbc61 Mon Sep 17 00:00:00 2001 From: Lais Andrade Date: Wed, 10 Nov 2021 19:03:48 +0000 Subject: [PATCH] Check for the vibrate notification key on delayed vibrations Notifications played with sound have a delayed vibration, in order to match the vibration and sound effects. If the effects are cleared before the vibration starts, or a new one takes its place, then the vibration hould be cancelled because it no longer will be tracked by the NotificationManagerService.mVibrateNotificationKey. This can cause repeating vibrations, like incoming calls, to start and continue vibrating even after the notification was handled, (if the user opens the notification panel before it actually starts vibrating, for example). Fix: 202752953 Test: NotificationManagerServiceTest Change-Id: If3ed4a741e78058a321691acebe735c735542f16 --- .../NotificationManagerService.java | 17 +++++- .../notification/BuzzBeepBlinkTest.java | 58 +++++++++++++++++-- 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index e5df3e7d83e2d..94f74bf5f183e 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -7808,7 +7808,10 @@ public class NotificationManagerService extends SystemService { final int waitMs = mAudioManager.getFocusRampTimeMs( AudioManager.AUDIOFOCUS_GAIN_TRANSIENT_MAY_DUCK, record.getAudioAttributes()); - if (DBG) Slog.v(TAG, "Delaying vibration by " + waitMs + "ms"); + if (DBG) { + Slog.v(TAG, "Delaying vibration for notification " + + record.getKey() + " by " + waitMs + "ms"); + } try { Thread.sleep(waitMs); } catch (InterruptedException e) { } @@ -7816,9 +7819,17 @@ public class NotificationManagerService extends SystemService { // so need to check the notification still valide for vibrate. synchronized (mNotificationLock) { if (mNotificationsByKey.get(record.getKey()) != null) { - vibrate(record, effect, true); + if (record.getKey().equals(mVibrateNotificationKey)) { + vibrate(record, effect, true); + } else { + if (DBG) { + Slog.v(TAG, "No vibration for notification " + + record.getKey() + ": a new notification is " + + "vibrating, or effects were cleared while waiting"); + } + } } else { - Slog.e(TAG, "No vibration for canceled notification : " + Slog.w(TAG, "No vibration for canceled notification " + record.getKey()); } } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java index ea46eab6e8f9c..d593e8000048f 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java @@ -32,7 +32,6 @@ import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyInt; @@ -40,6 +39,7 @@ import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.after; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -48,6 +48,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.annotation.SuppressLint; import android.app.ActivityManager; import android.app.KeyguardManager; import android.app.Notification; @@ -73,7 +74,6 @@ import android.provider.Settings; import android.service.notification.NotificationListenerService; import android.service.notification.StatusBarNotification; import android.test.suitebuilder.annotation.SmallTest; -import android.util.Slog; import android.view.accessibility.AccessibilityEvent; import android.view.accessibility.AccessibilityManager; import android.view.accessibility.IAccessibilityManager; @@ -102,6 +102,7 @@ import java.util.Objects; @SmallTest @RunWith(AndroidJUnit4.class) +@SuppressLint("GuardedBy") // It's ok for this test to access guarded methods from the service. public class BuzzBeepBlinkTest extends UiServiceTestCase { @Mock AudioManager mAudioManager; @@ -156,6 +157,7 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { when(mAudioManager.getRingtonePlayer()).thenReturn(mRingtonePlayer); when(mAudioManager.getStreamVolume(anyInt())).thenReturn(10); when(mAudioManager.getRingerModeInternal()).thenReturn(AudioManager.RINGER_MODE_NORMAL); + when(mAudioManager.getFocusRampTimeMs(anyInt(), any(AudioAttributes.class))).thenReturn(50); when(mUsageStats.isAlertRateLimited(any())).thenReturn(false); when(mVibrator.hasFrequencyControl()).thenReturn(false); when(mKeyguardManager.isDeviceLocked(anyInt())).thenReturn(false); @@ -444,6 +446,11 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { timeout(MAX_VIBRATION_DELAY).times(1)); } + private void verifyDelayedNeverVibrate() { + verify(mVibrator, after(MAX_VIBRATION_DELAY).never()).vibrate(anyInt(), anyString(), any(), + anyString(), any(AudioAttributes.class)); + } + private void verifyVibrate(ArgumentMatcher effectMatcher, VerificationMode verification) { ArgumentCaptor captor = ArgumentCaptor.forClass(AudioAttributes.class); @@ -1588,8 +1595,51 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { // beep wasn't reset verifyNeverBeep(); verifyNeverVibrate(); - verify(mRingtonePlayer, never()).stopAsync(); - verify(mVibrator, never()).cancel(); + verifyNeverStopAudio(); + verifyNeverStopVibrate(); + } + + @Test + public void testRingtoneInsistentBeep_clearEffectsStopsSoundAndVibration() throws Exception { + NotificationChannel ringtoneChannel = + new NotificationChannel("ringtone", "", IMPORTANCE_HIGH); + ringtoneChannel.setSound(Uri.fromParts("a", "b", "c"), + new AudioAttributes.Builder().setUsage(USAGE_NOTIFICATION_RINGTONE).build()); + ringtoneChannel.enableVibration(true); + NotificationRecord ringtoneNotification = getCallRecord(1, ringtoneChannel, true); + mService.addNotification(ringtoneNotification); + assertFalse(mService.shouldMuteNotificationLocked(ringtoneNotification)); + mService.buzzBeepBlinkLocked(ringtoneNotification); + verifyBeepLooped(); + verifyDelayedVibrateLooped(); + + mService.clearSoundLocked(); + mService.clearVibrateLocked(); + + verifyStopAudio(); + verifyStopVibrate(); + } + + @Test + public void testRingtoneInsistentBeep_neverVibratesWhenEffectsClearedBeforeDelay() + throws Exception { + NotificationChannel ringtoneChannel = + new NotificationChannel("ringtone", "", IMPORTANCE_HIGH); + ringtoneChannel.setSound(Uri.fromParts("a", "b", "c"), + new AudioAttributes.Builder().setUsage(USAGE_NOTIFICATION_RINGTONE).build()); + ringtoneChannel.enableVibration(true); + NotificationRecord ringtoneNotification = getCallRecord(1, ringtoneChannel, true); + mService.addNotification(ringtoneNotification); + assertFalse(mService.shouldMuteNotificationLocked(ringtoneNotification)); + mService.buzzBeepBlinkLocked(ringtoneNotification); + verifyBeepLooped(); + verifyNeverVibrate(); + + mService.clearSoundLocked(); + mService.clearVibrateLocked(); + + verifyStopAudio(); + verifyDelayedNeverVibrate(); } @Test