diff --git a/services/core/java/com/android/server/VibratorService.java b/services/core/java/com/android/server/VibratorService.java index 2c83da55f2750..a6a00fbe17abf 100644 --- a/services/core/java/com/android/server/VibratorService.java +++ b/services/core/java/com/android/server/VibratorService.java @@ -129,8 +129,10 @@ public class VibratorService extends IVibratorService.Stub { Slog.d(TAG, "Vibration thread finished with status " + status); } synchronized (mLock) { - mThread = null; - reportFinishVibrationLocked(status); + if (mCurrentVibration != null && mCurrentVibration.id == vibrationId) { + mThread = null; + reportFinishVibrationLocked(status); + } } } } diff --git a/services/core/java/com/android/server/vibrator/VibrationThread.java b/services/core/java/com/android/server/vibrator/VibrationThread.java index a4d888b3f9cf0..c36375ef0af5f 100644 --- a/services/core/java/com/android/server/vibrator/VibrationThread.java +++ b/services/core/java/com/android/server/vibrator/VibrationThread.java @@ -39,8 +39,6 @@ import com.google.android.collect.Lists; import java.util.ArrayList; import java.util.List; import java.util.PriorityQueue; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; /** Plays a {@link Vibration} in dedicated thread. */ // TODO(b/159207608): Make this package-private once vibrator services are moved to this package @@ -76,7 +74,6 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi void onVibrationEnded(long vibrationId, Vibration.Status status); } - private final Object mLock = new Object(); private final WorkSource mWorkSource = new WorkSource(); private final PowerManager.WakeLock mWakeLock; private final IBatteryStats mBatteryStatsService; @@ -84,7 +81,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi private final VibrationCallbacks mCallbacks; private final SparseArray mVibrators; - @GuardedBy("mLock") + @GuardedBy("this") @Nullable private VibrateStep mCurrentVibrateStep; @GuardedBy("this") @@ -147,7 +144,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi /** Notify current vibration that a step has completed on given vibrator. */ public void vibratorComplete(int vibratorId) { - synchronized (mLock) { + synchronized (this) { if (mCurrentVibrateStep != null) { mCurrentVibrateStep.vibratorComplete(vibratorId); } @@ -171,7 +168,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi final int stepCount = steps.size(); for (int i = 0; i < stepCount; i++) { Step step = steps.get(i); - synchronized (mLock) { + synchronized (this) { if (step instanceof VibrateStep) { mCurrentVibrateStep = (VibrateStep) step; } else { @@ -315,27 +312,6 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi } } - /** - * Sleeps until given {@link CountDownLatch} has finished or {@code wakeUpTime} was reached. - * - *

This stops immediately when {@link #cancel()} is called. - */ - private void awaitUntil(CountDownLatch counter, long wakeUpTime) { - synchronized (this) { - long durationRemaining = wakeUpTime - SystemClock.uptimeMillis(); - while (counter.getCount() > 0 && durationRemaining > 0) { - try { - counter.await(durationRemaining, TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - } - if (mForceStop) { - break; - } - durationRemaining = wakeUpTime - SystemClock.uptimeMillis(); - } - } - } - private void noteVibratorOn(long duration) { try { mBatteryStatsService.noteVibratorOn(mVibration.uid, duration); @@ -371,12 +347,10 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi private final class SingleVibrateStep implements VibrateStep { private final VibratorController mVibrator; private final VibrationEffect mEffect; - private final CountDownLatch mCounter; SingleVibrateStep(VibratorController vibrator, VibrationEffect effect) { mVibrator = vibrator; mEffect = effect; - mCounter = new CountDownLatch(1); } @Override @@ -390,7 +364,9 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi return; } mVibrator.off(); - mCounter.countDown(); + synchronized (VibrationThread.this) { + VibrationThread.this.notify(); + } } @Override @@ -408,7 +384,11 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi noteVibratorOn(duration); // Vibration is playing with no need to control amplitudes, just wait for native // callback or timeout. - awaitUntil(mCounter, startTime + duration + CALLBACKS_EXTRA_TIMEOUT); + waitUntil(startTime + duration + CALLBACKS_EXTRA_TIMEOUT); + if (mForceStop) { + mVibrator.off(); + return Vibration.Status.CANCELLED; + } return Vibration.Status.FINISHED; } @@ -499,14 +479,15 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi /** Represent a synchronized vibration step on multiple vibrators. */ private final class SyncedVibrateStep implements VibrateStep { private final SparseArray mEffects; - private final CountDownLatch mActiveVibratorCounter; - private final int mRequiredCapabilities; private final int[] mVibratorIds; + @GuardedBy("VibrationThread.this") + private int mActiveVibratorCounter; + SyncedVibrateStep(SparseArray effects) { mEffects = effects; - mActiveVibratorCounter = new CountDownLatch(mEffects.size()); + mActiveVibratorCounter = mEffects.size(); // TODO(b/159207608): Calculate required capabilities for syncing this step. mRequiredCapabilities = 0; mVibratorIds = new int[effects.size()]; @@ -527,7 +508,11 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi return; } mVibrators.get(vibratorId).off(); - mActiveVibratorCounter.countDown(); + synchronized (VibrationThread.this) { + if (--mActiveVibratorCounter <= 0) { + VibrationThread.this.notify(); + } + } } @Override @@ -556,7 +541,9 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi AmplitudeStep nextStep = step.nextStep(); if (nextStep == null) { // This vibrator has finished playing the effect for this step. - mActiveVibratorCounter.countDown(); + synchronized (VibrationThread.this) { + mActiveVibratorCounter--; + } } else { nextSteps.add(nextStep); } @@ -564,7 +551,16 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi // All OneShot and Waveform effects have finished. Just wait for the other effects // to end via native callbacks before finishing this synced step. - awaitUntil(mActiveVibratorCounter, startTime + timeout + CALLBACKS_EXTRA_TIMEOUT); + synchronized (VibrationThread.this) { + if (mActiveVibratorCounter > 0) { + waitUntil(startTime + timeout + CALLBACKS_EXTRA_TIMEOUT); + } + } + if (mForceStop) { + stopAllVibrators(); + return Vibration.Status.CANCELLED; + } + return Vibration.Status.FINISHED; } finally { if (timeout > 0) { @@ -779,7 +775,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi Slog.d(TAG, "DelayStep of " + mDelay + "ms starting..."); } waitUntil(SystemClock.uptimeMillis() + mDelay); - return Vibration.Status.FINISHED; + return mForceStop ? Vibration.Status.CANCELLED : Vibration.Status.FINISHED; } finally { if (DEBUG) { Slog.d(TAG, "DelayStep done."); diff --git a/services/core/java/com/android/server/vibrator/VibratorController.java b/services/core/java/com/android/server/vibrator/VibratorController.java index 53f52e286fbdf..9dcf12caa55b7 100644 --- a/services/core/java/com/android/server/vibrator/VibratorController.java +++ b/services/core/java/com/android/server/vibrator/VibratorController.java @@ -272,12 +272,18 @@ public final class VibratorController { return 0; } synchronized (mLock) { - mNativeWrapper.compose(effect.getPrimitiveEffects().toArray( - new VibrationEffect.Composition.PrimitiveEffect[0]), vibrationId); + VibrationEffect.Composition.PrimitiveEffect[] primitives = + effect.getPrimitiveEffects().toArray( + new VibrationEffect.Composition.PrimitiveEffect[0]); + mNativeWrapper.compose(primitives, vibrationId); notifyVibratorOnLocked(); // Compose don't actually give us an estimated duration, so we just guess here. - // TODO(b/177807015): use exposed durations from IVibrator here instead - return 20 * effect.getPrimitiveEffects().size(); + long duration = 0; + for (VibrationEffect.Composition.PrimitiveEffect primitive : primitives) { + // TODO(b/177807015): use exposed durations from IVibrator here instead + duration += 20 + primitive.delay; + } + return duration; } } 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 bee739231d3f1..0fce4baf966f1 100644 --- a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java +++ b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java @@ -212,6 +212,60 @@ public class VibrationThreadTest { } } + @Test + public void vibrate_singleVibratorPredefinedCancel_cancelsVibrationImmediately() + throws Exception { + mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS); + + long vibrationId = 1; + VibrationEffect effect = VibrationEffect.startComposition() + .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100) + .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100) + .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100) + .compose(); + VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); + + Thread.sleep(20); + assertTrue(vibrationThread.isAlive()); + assertTrue(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + + // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should + // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately. + Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); + cancellingThread.start(); + + waitForCompletion(vibrationThread, 20); + waitForCompletion(cancellingThread); + + verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); + assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + } + + @Test + public void vibrate_singleVibratorWaveformCancel_cancelsVibrationImmediately() + throws Exception { + mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS); + + long vibrationId = 1; + VibrationEffect effect = VibrationEffect.createWaveform(new long[]{100}, new int[]{100}, 0); + VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); + + Thread.sleep(20); + assertTrue(vibrationThread.isAlive()); + assertTrue(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + + // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should + // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately. + Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); + cancellingThread.start(); + + waitForCompletion(vibrationThread, 20); + waitForCompletion(cancellingThread); + + verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); + assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + } + @Test public void vibrate_singleVibratorPrebaked_runsVibration() throws Exception { mVibratorProviders.get(1).setSupportedEffects(VibrationEffect.EFFECT_THUD); @@ -544,36 +598,70 @@ public class VibrationThreadTest { } @Test - public void vibrate_multipleCancelled_allVibratorsStopped() throws Exception { - mockVibrators(1, 2, 3); + public void vibrate_multiplePredefinedCancel_cancelsVibrationImmediately() throws Exception { + mockVibrators(1, 2); + mVibratorProviders.get(1).setSupportedEffects(VibrationEffect.EFFECT_CLICK); + mVibratorProviders.get(2).setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS); + + long vibrationId = 1; + CombinedVibrationEffect effect = CombinedVibrationEffect.startSynced() + .addVibrator(1, VibrationEffect.get(VibrationEffect.EFFECT_CLICK)) + .addVibrator(2, VibrationEffect.startComposition() + .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100) + .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100) + .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f, 100) + .compose()) + .combine(); + VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); + + Thread.sleep(10); + assertTrue(vibrationThread.isAlive()); + assertTrue(vibrationThread.getVibrators().get(1).isVibrating()); + assertTrue(vibrationThread.getVibrators().get(2).isVibrating()); + + // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should + // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately. + Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); + cancellingThread.start(); + + waitForCompletion(vibrationThread, 20); + waitForCompletion(cancellingThread); + + verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); + assertFalse(vibrationThread.getVibrators().get(1).isVibrating()); + assertFalse(vibrationThread.getVibrators().get(2).isVibrating()); + } + + @Test + public void vibrate_multipleWaveformCancel_cancelsVibrationImmediately() throws Exception { + mockVibrators(1, 2); mVibratorProviders.get(1).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL); mVibratorProviders.get(2).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL); - mVibratorProviders.get(3).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL); long vibrationId = 1; CombinedVibrationEffect effect = CombinedVibrationEffect.startSynced() .addVibrator(1, VibrationEffect.createWaveform( - new long[]{5, 10}, new int[]{1, 2}, 0)) - .addVibrator(2, VibrationEffect.createWaveform( - new long[]{20, 30}, new int[]{3, 4}, 0)) - .addVibrator(3, VibrationEffect.createWaveform( - new long[]{10, 40}, new int[]{5, 6}, 0)) + new long[]{100, 100}, new int[]{1, 2}, 0)) + .addVibrator(2, VibrationEffect.createOneShot(100, 100)) .combine(); - VibrationThread thread = startThreadAndDispatcher(vibrationId, effect); + VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - Thread.sleep(15); - assertTrue(thread.isAlive()); - assertTrue(thread.getVibrators().get(1).isVibrating()); - assertTrue(thread.getVibrators().get(2).isVibrating()); - assertTrue(thread.getVibrators().get(3).isVibrating()); + Thread.sleep(10); + assertTrue(vibrationThread.isAlive()); + assertTrue(vibrationThread.getVibrators().get(1).isVibrating()); + assertTrue(vibrationThread.getVibrators().get(2).isVibrating()); - thread.cancel(); - waitForCompletion(thread); - assertFalse(thread.getVibrators().get(1).isVibrating()); - assertFalse(thread.getVibrators().get(2).isVibrating()); - assertFalse(thread.getVibrators().get(3).isVibrating()); + // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should + // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately. + Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); + cancellingThread.start(); + + waitForCompletion(vibrationThread, 20); + waitForCompletion(cancellingThread); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); + assertFalse(vibrationThread.getVibrators().get(1).isVibrating()); + assertFalse(vibrationThread.getVibrators().get(2).isVibrating()); } @Test @@ -621,11 +709,11 @@ public class VibrationThreadTest { return thread; } - private void waitForCompletion(VibrationThread thread) { + private void waitForCompletion(Thread thread) { waitForCompletion(thread, TEST_TIMEOUT_MILLIS); } - private void waitForCompletion(VibrationThread thread, long timeout) { + private void waitForCompletion(Thread thread, long timeout) { try { thread.join(timeout); } catch (InterruptedException e) {