commit f430e0a9c8a111138e59347ec0de5e50842a68fa
author Hall Liu <hallliu@google.com> Thu Jun 13 18:59:59 2019 -0700
committer Hall Liu <hallliu@google.com> Fri Jun 14 21:28:02 2019 +0000
tree de8a8b7e2fc09ed8359f0f67da728cbdb242efbf
parent 92d32918d87205238e56fff02c0eedb3c1c5cbe6

Fix ordering issue with vibration

When someone calls stopRinging, also cancel the pending
completablefuture that's supported to start vibration.

Prior to this change, if a user picked up a call really quickly before
the haptics lookup had a chance to complete, stopRinging would get
called before the vibration even started. Then, when the haptics lookup
completes, it'll trigger the vibration to start. With nothing to stop
it, the vibration will continue until the user reboots the device or
receives another call.

Fixes: 134833743
Test: unit, manual
Change-Id: I6968a1ed0693e0eb839b3d9be032d5bed9525f77

src/com/android/server/telecom/Ringer.java

diff --git a/src/com/android/server/telecom/Ringer.java b/src/com/android/server/telecom/Ringer.java
index b5ff31d..2909b72 100644
--- a/src/com/android/server/telecom/Ringer.java
+++ b/src/com/android/server/telecom/Ringer.java
@@ -146,6 +146,8 @@
      */
     private CompletableFuture<Void> mBlockOnRingingFuture = null;
 
+    private CompletableFuture<Void> mVibrateFuture = CompletableFuture.completedFuture(null);
+
     private InCallTonePlayer mCallWaitingPlayer;
     private RingtoneFactory mRingtoneFactory;
 
@@ -324,8 +326,7 @@
         }
 
         if (hapticsFuture != null) {
-            CompletableFuture<Void> vibrateFuture =
-                    hapticsFuture.thenAccept(isUsingAudioCoupledHaptics -> {
+           mVibrateFuture = hapticsFuture.thenAccept(isUsingAudioCoupledHaptics -> {
                 if (!isUsingAudioCoupledHaptics || !mIsHapticPlaybackSupportedByDevice) {
                     Log.i(this, "startRinging: fileHasHaptics=%b, hapticsSupported=%b",
                             isUsingAudioCoupledHaptics, mIsHapticPlaybackSupportedByDevice);
@@ -337,10 +338,7 @@
                 }
             });
             if (mBlockOnRingingFuture != null) {
-                vibrateFuture.thenCompose( v -> {
-                    mBlockOnRingingFuture.complete(null);
-                    return null;
-                });
+                mVibrateFuture.whenComplete((v, e) -> mBlockOnRingingFuture.complete(null));
             }
         } else {
             if (mBlockOnRingingFuture != null) {
@@ -438,6 +436,12 @@
 
         mRingtonePlayer.stop();
 
+        // If we haven't started vibrating because we were waiting for the haptics info, cancel
+        // it and don't vibrate at all.
+        if (mVibrateFuture != null) {
+            mVibrateFuture.cancel(true);
+        }
+
         if (mIsVibrating) {
             Log.addEvent(mVibratingCall, LogUtils.Events.STOP_VIBRATOR);
             mVibrator.cancel();

tests/src/com/android/server/telecom/tests/RingerTest.java

diff --git a/tests/src/com/android/server/telecom/tests/RingerTest.java b/tests/src/com/android/server/telecom/tests/RingerTest.java
index 5434acc..75e89bc 100644
--- a/tests/src/com/android/server/telecom/tests/RingerTest.java
+++ b/tests/src/com/android/server/telecom/tests/RingerTest.java
@@ -24,6 +24,7 @@
 import android.media.VolumeShaper;
 import android.net.Uri;
 import android.os.Bundle;
+import android.os.Looper;
 import android.os.Parcel;
 import android.os.VibrationEffect;
 import android.os.Vibrator;
@@ -302,6 +303,31 @@
 
     @SmallTest
     @Test
+    public void testStopRingingBeforeHapticsLookupComplete() throws Exception {
+        enableVibrationWhenRinging();
+        Ringtone mockRingtone = mock(Ringtone.class);
+        when(mockRingtoneFactory.getRingtone(nullable(Call.class))).thenReturn(mockRingtone);
+        when(mockAudioManager.getRingerModeInternal()).thenReturn(AudioManager.RINGER_MODE_NORMAL);
+
+        mRingerUnderTest.startRinging(mockCall1, false);
+        // Make sure we haven't started the vibrator yet, but have started ringing.
+        verify(mockRingtonePlayer).play(nullable(RingtoneFactory.class), nullable(Call.class),
+                nullable(VolumeShaper.Configuration.class), anyBoolean());
+        verify(mockVibrator, never()).vibrate(nullable(VibrationEffect.class),
+                nullable(AudioAttributes.class));
+        // Simulate something stopping the ringer
+        mRingerUnderTest.stopRinging();
+        verify(mockRingtonePlayer).stop();
+        verify(mockVibrator, never()).cancel();
+        // Simulate the haptics computation finishing
+        mFuture.complete(false);
+        // Then make sure that we don't actually start vibrating.
+        verify(mockVibrator, never()).vibrate(nullable(VibrationEffect.class),
+                nullable(AudioAttributes.class));
+    }
+
+    @SmallTest
+    @Test
     public void testCustomVibrationForRingtone() throws Exception {
         mRingerUnderTest.startCallWaiting(mockCall1);
         Ringtone mockRingtone = mock(Ringtone.class);