Merge "Allow upcoming vibration when current one if being cancelled"

5 files changed, 69 insertions, 25 deletions

diff --git a/services/core/java/com/android/server/vibrator/VibrationStepConductor.java b/services/core/java/com/android/server/vibrator/VibrationStepConductor.java
index 8ac4fd4860bb..141be702ee13 100644
--- a/services/core/java/com/android/server/vibrator/VibrationStepConductor.java
+++ b/services/core/java/com/android/server/vibrator/VibrationStepConductor.java
@@ -65,6 +65,9 @@ final class VibrationStepConductor implements IBinder.DeathRecipient {
     public final DeviceVibrationEffectAdapter deviceEffectAdapter;
     public final VibrationThread.VibratorManagerHooks vibratorManagerHooks;
 
+    // Not guarded by lock because they're not modified by this conductor, it's used here only to
+    // check immutable attributes. The status and other mutable states are changed by the service or
+    // by the vibrator steps.
     private final Vibration mVibration;
     private final SparseArray<VibratorController> mVibrators = new SparseArray<>();
 
@@ -412,6 +415,16 @@ final class VibrationStepConductor implements IBinder.DeathRecipient {
         }
     }
 
+    /** Returns true if a cancellation signal was sent via {@link #notifyCancelled}. */
+    public boolean wasNotifiedToCancel() {
+        if (Build.IS_DEBUGGABLE) {
+            expectIsVibrationThread(false);
+        }
+        synchronized (mLock) {
+            return mSignalCancel != null;
+        }
+    }
+
     @GuardedBy("mLock")
     private boolean hasPendingNotifySignalLocked() {
         if (Build.IS_DEBUGGABLE) {
diff --git a/services/core/java/com/android/server/vibrator/VibratorManagerService.java b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
index 8514e272e250..8613b5027d57 100644
--- a/services/core/java/com/android/server/vibrator/VibratorManagerService.java
+++ b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
@@ -864,8 +864,8 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
         }
 
         Vibration currentVibration = mCurrentVibration.getVibration();
-        if (currentVibration.hasEnded()) {
-            // Current vibration is finishing up, it should not block incoming vibrations.
+        if (currentVibration.hasEnded() || mCurrentVibration.wasNotifiedToCancel()) {
+            // Current vibration has ended or is cancelling, should not block incoming vibrations.
             return null;
         }
 
diff --git a/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java b/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java
index 235849c1cd8b..c484f457faea 100644
--- a/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java
+++ b/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java
@@ -53,7 +53,8 @@ final class FakeVibratorControllerProvider {
 
     private boolean mIsAvailable = true;
     private boolean mIsInfoLoadSuccessful = true;
-    private long mLatency;
+    private long mOnLatency;
+    private long mOffLatency;
     private int mOffCount;
 
     private int mCapabilities;
@@ -97,7 +98,7 @@ final class FakeVibratorControllerProvider {
         public long on(long milliseconds, long vibrationId) {
             recordEffectSegment(vibrationId, new StepSegment(VibrationEffect.DEFAULT_AMPLITUDE,
                     /* frequencyHz= */ 0, (int) milliseconds));
-            applyLatency();
+            applyLatency(mOnLatency);
             scheduleListener(milliseconds, vibrationId);
             return milliseconds;
         }
@@ -105,12 +106,13 @@ final class FakeVibratorControllerProvider {
         @Override
         public void off() {
             mOffCount++;
+            applyLatency(mOffLatency);
         }
 
         @Override
         public void setAmplitude(float amplitude) {
             mAmplitudes.add(amplitude);
-            applyLatency();
+            applyLatency(mOnLatency);
         }
 
         @Override
@@ -121,7 +123,7 @@ final class FakeVibratorControllerProvider {
             }
             recordEffectSegment(vibrationId,
                     new PrebakedSegment((int) effect, false, (int) strength));
-            applyLatency();
+            applyLatency(mOnLatency);
             scheduleListener(EFFECT_DURATION, vibrationId);
             return EFFECT_DURATION;
         }
@@ -141,7 +143,7 @@ final class FakeVibratorControllerProvider {
                 duration += EFFECT_DURATION + primitive.getDelay();
                 recordEffectSegment(vibrationId, primitive);
             }
-            applyLatency();
+            applyLatency(mOnLatency);
             scheduleListener(duration, vibrationId);
             return duration;
         }
@@ -154,7 +156,7 @@ final class FakeVibratorControllerProvider {
                 recordEffectSegment(vibrationId, primitive);
             }
             recordBraking(vibrationId, braking);
-            applyLatency();
+            applyLatency(mOnLatency);
             scheduleListener(duration, vibrationId);
             return duration;
         }
@@ -193,10 +195,10 @@ final class FakeVibratorControllerProvider {
             return mIsInfoLoadSuccessful;
         }
 
-        private void applyLatency() {
+        private void applyLatency(long latencyMillis) {
             try {
-                if (mLatency > 0) {
-                    Thread.sleep(mLatency);
+                if (latencyMillis > 0) {
+                    Thread.sleep(latencyMillis);
                 }
             } catch (InterruptedException e) {
             }
@@ -240,10 +242,15 @@ final class FakeVibratorControllerProvider {
 
     /**
      * Sets the latency this controller should fake for turning the vibrator hardware on or setting
-     * it's vibration amplitude.
+     * the vibration amplitude.
      */
-    public void setLatency(long millis) {
-        mLatency = millis;
+    public void setOnLatency(long millis) {
+        mOnLatency = millis;
+    }
+
+    /** Sets the latency this controller should fake for turning the vibrator off. */
+    public void setOffLatency(long millis) {
+        mOffLatency = millis;
     }
 
     /** Set the capabilities of the fake vibrator hardware. */
diff --git a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java
index a15e4b0c74a0..fc830a9f61ad 100644
--- a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java
+++ b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java
@@ -1159,7 +1159,7 @@ public class VibrationThreadTest {
 
         // 25% of the first waveform step will be spent on the native on() call.
         // 25% of each waveform step will be spent on the native setAmplitude() call..
-        mVibratorProviders.get(VIBRATOR_ID).setLatency(stepDuration / 4);
+        mVibratorProviders.get(VIBRATOR_ID).setOnLatency(stepDuration / 4);
         mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
 
         int stepCount = totalDuration / stepDuration;
@@ -1190,7 +1190,7 @@ public class VibrationThreadTest {
         fakeVibrator.setSupportedEffects(VibrationEffect.EFFECT_CLICK);
 
         long latency = 5_000; // 5s
-        fakeVibrator.setLatency(latency);
+        fakeVibrator.setOnLatency(latency);
 
         long vibrationId = 1;
         VibrationEffect effect = VibrationEffect.get(VibrationEffect.EFFECT_CLICK);
@@ -1204,8 +1204,7 @@ public class VibrationThreadTest {
         // fail at waitForCompletion(cancellingThread).
         Thread cancellingThread = new Thread(
                 () -> conductor.notifyCancelled(
-                        new Vibration.EndInfo(
-                                Vibration.Status.CANCELLED_BY_USER),
+                        new Vibration.EndInfo(Vibration.Status.CANCELLED_BY_USER),
                         /* immediate= */ false));
         cancellingThread.start();
 
diff --git a/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java
index c46fecd1a55e..c83afb74ccba 100644
--- a/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java
@@ -826,13 +826,40 @@ public class VibratorManagerServiceTest {
         // The second vibration shouldn't have recorded that the vibrators were turned on.
         verify(mBatteryStatsMock, times(1)).noteVibratorOn(anyInt(), anyLong());
         // No segment played is the prebaked CLICK from the second vibration.
-        assertFalse(
-                mVibratorProviders.get(1).getAllEffectSegments().stream()
-                        .anyMatch(segment -> segment instanceof PrebakedSegment));
+        assertFalse(mVibratorProviders.get(1).getAllEffectSegments().stream()
+                .anyMatch(PrebakedSegment.class::isInstance));
         cancelVibrate(service);  // Clean up repeating effect.
     }
 
     @Test
+    public void vibrate_withOngoingRepeatingVibrationBeingCancelled_playsAfterPreviousIsCancelled()
+            throws Exception {
+        mockVibrators(1);
+        FakeVibratorControllerProvider fakeVibrator = mVibratorProviders.get(1);
+        fakeVibrator.setOffLatency(50); // Add latency so cancellation is slow.
+        fakeVibrator.setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+        fakeVibrator.setSupportedEffects(VibrationEffect.EFFECT_CLICK);
+        VibratorManagerService service = createSystemReadyService();
+
+        VibrationEffect repeatingEffect = VibrationEffect.createWaveform(
+                new long[]{10, 10_000}, new int[]{255, 0}, 1);
+        vibrate(service, repeatingEffect, ALARM_ATTRS);
+
+        // VibrationThread will start this vibration async, wait until the off waveform step.
+        assertTrue(waitUntil(s -> fakeVibrator.getOffCount() > 0, service, TEST_TIMEOUT_MILLIS));
+
+        // Cancel vibration right before requesting a new one.
+        // This should trigger slow IVibrator.off before setting the vibration status to cancelled.
+        cancelVibrate(service);
+        vibrateAndWaitUntilFinished(service, VibrationEffect.get(VibrationEffect.EFFECT_CLICK),
+                ALARM_ATTRS);
+
+        // Check that second vibration was played.
+        assertTrue(fakeVibrator.getAllEffectSegments().stream()
+                .anyMatch(PrebakedSegment.class::isInstance));
+    }
+
+    @Test
     public void vibrate_withNewRepeatingVibration_cancelsOngoingEffect() throws Exception {
         mockVibrators(1);
         mVibratorProviders.get(1).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
@@ -880,10 +907,8 @@ public class VibratorManagerServiceTest {
         // The second vibration shouldn't have recorded that the vibrators were turned on.
         verify(mBatteryStatsMock, times(1)).noteVibratorOn(anyInt(), anyLong());
         // The second vibration shouldn't have played any prebaked segment.
-        assertFalse(
-                mVibratorProviders.get(1).getAllEffectSegments().stream()
-                        .anyMatch(segment -> segment instanceof PrebakedSegment));
-
+        assertFalse(mVibratorProviders.get(1).getAllEffectSegments().stream()
+                .anyMatch(PrebakedSegment.class::isInstance));
         cancelVibrate(service);  // Clean up long effect.
     }