diff options
| -rw-r--r-- | services/core/java/com/android/server/vibrator/VibrationThread.java | 110 | ||||
| -rw-r--r-- | services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java | 58 |
2 files changed, 120 insertions, 48 deletions
diff --git a/services/core/java/com/android/server/vibrator/VibrationThread.java b/services/core/java/com/android/server/vibrator/VibrationThread.java index bc61478ec1b5..b3f1bcd444cc 100644 --- a/services/core/java/com/android/server/vibrator/VibrationThread.java +++ b/services/core/java/com/android/server/vibrator/VibrationThread.java @@ -45,8 +45,10 @@ import com.android.internal.util.FrameworkStatsLog; import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.PriorityQueue; +import java.util.Queue; /** Plays a {@link Vibration} in dedicated thread. */ final class VibrationThread extends Thread implements IBinder.DeathRecipient { @@ -171,7 +173,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { Slog.d(TAG, "Synced vibration complete reported by vibrator manager"); } for (int i = 0; i < mVibrators.size(); i++) { - mStepQueue.consumeOnVibratorComplete(mVibrators.keyAt(i)); + mStepQueue.notifyVibratorComplete(mVibrators.keyAt(i)); } mLock.notify(); } @@ -183,7 +185,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { if (DEBUG) { Slog.d(TAG, "Vibration complete reported by vibrator " + vibratorId); } - mStepQueue.consumeOnVibratorComplete(vibratorId); + mStepQueue.notifyVibratorComplete(vibratorId); mLock.notify(); } } @@ -192,26 +194,25 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "playVibration"); try { CombinedVibration.Sequential effect = toSequential(mVibration.getEffect()); - int stepsPlayed = 0; - - synchronized (mLock) { - mStepQueue.offer(new StartVibrateStep(effect)); - Step topOfQueue; + mStepQueue.offer(new StartVibrateStep(effect)); - while ((topOfQueue = mStepQueue.peek()) != null) { - long waitTime = topOfQueue.calculateWaitTime(); - if (waitTime <= 0) { - stepsPlayed += mStepQueue.consume(); - } else { + int stepsPlayed = 0; + while (!mStepQueue.isEmpty()) { + long waitTime = mStepQueue.calculateWaitTime(); + if (waitTime <= 0) { + stepsPlayed += mStepQueue.consumeNext(); + } else { + synchronized (mLock) { try { mLock.wait(waitTime); - } catch (InterruptedException e) { } - } - if (mForceStop) { - mStepQueue.cancel(); - return Vibration.Status.CANCELLED; + } catch (InterruptedException e) { + } } } + if (mForceStop) { + mStepQueue.cancel(); + return Vibration.Status.CANCELLED; + } } // Some effects might be ignored because the specified vibrator don't exist or doesn't @@ -295,54 +296,75 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { private final class StepQueue { @GuardedBy("mLock") private final PriorityQueue<Step> mNextSteps = new PriorityQueue<>(); - @GuardedBy("mLock") + private final Queue<Step> mPendingOnVibratorCompleteSteps = new LinkedList<>(); + public void offer(@NonNull Step step) { - mNextSteps.offer(step); + synchronized (mLock) { + mNextSteps.offer(step); + } } - @GuardedBy("mLock") - @Nullable - public Step peek() { - return mNextSteps.peek(); + public boolean isEmpty() { + synchronized (mLock) { + return mPendingOnVibratorCompleteSteps.isEmpty() && mNextSteps.isEmpty(); + } + } + + /** Returns the time in millis to wait before calling {@link #consumeNext()}. */ + public long calculateWaitTime() { + Step nextStep; + synchronized (mLock) { + if (!mPendingOnVibratorCompleteSteps.isEmpty()) { + // Steps anticipated by vibrator complete callback should be played right away. + return 0; + } + nextStep = mNextSteps.peek(); + } + return nextStep == null ? 0 : nextStep.calculateWaitTime(); } /** - * Play and remove the step at the top of this queue, and also adds the next steps - * generated to be played next. + * Play and remove the step at the top of this queue, and also adds the next steps generated + * to be played next. * * @return the number of steps played */ - @GuardedBy("mLock") - public int consume() { - Step nextStep = mNextSteps.poll(); + public int consumeNext() { + Step nextStep = pollNext(); if (nextStep != null) { - mNextSteps.addAll(nextStep.play()); + // This might turn on the vibrator and have a HAL latency. Execute this outside any + // lock to avoid blocking other interactions with the thread. + List<Step> nextSteps = nextStep.play(); + synchronized (mLock) { + mNextSteps.addAll(nextSteps); + } return 1; } return 0; } /** - * Play and remove the step in this queue that should be anticipated by the vibrator - * completion callback. + * Notify the step in this queue that should be anticipated by the vibrator completion + * callback and keep it separate to be consumed by {@link #consumeNext()}. + * + * <p>This is a lightweight method that do not trigger any operation from {@link + * VibratorController}, so it can be called directly from a native callback. * * <p>This assumes only one of the next steps is waiting on this given vibrator, so the - * first step found is played by this method, in no particular order. + * first step found will be anticipated by this method, in no particular order. */ @GuardedBy("mLock") - public void consumeOnVibratorComplete(int vibratorId) { + public void notifyVibratorComplete(int vibratorId) { Iterator<Step> it = mNextSteps.iterator(); - List<Step> nextSteps = EMPTY_STEP_LIST; while (it.hasNext()) { Step step = it.next(); if (step.shouldPlayWhenVibratorComplete(vibratorId)) { it.remove(); - nextSteps = step.play(); + mPendingOnVibratorCompleteSteps.offer(step); break; } } - mNextSteps.addAll(nextSteps); } /** @@ -350,13 +372,25 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { * * <p>This will remove and trigger {@link Step#cancel()} in all steps, in order. */ - @GuardedBy("mLock") public void cancel() { Step step; - while ((step = mNextSteps.poll()) != null) { + while ((step = pollNext()) != null) { + // This might turn off the vibrator and have a HAL latency. Execute this outside + // any lock to avoid blocking other interactions with the thread. step.cancel(); } } + + @Nullable + private Step pollNext() { + synchronized (mLock) { + // Prioritize the steps anticipated by a vibrator complete callback. + if (!mPendingOnVibratorCompleteSteps.isEmpty()) { + return mPendingOnVibratorCompleteSteps.poll(); + } + return mNextSteps.poll(); + } + } } /** 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 5743982171cb..ac3e05d27962 100644 --- a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java +++ b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java @@ -85,12 +85,17 @@ public class VibrationThreadTest { private static final String PACKAGE_NAME = "package"; private static final VibrationAttributes ATTRS = new VibrationAttributes.Builder().build(); - @Rule public MockitoRule mMockitoRule = MockitoJUnit.rule(); - - @Mock private VibrationThread.VibrationCallbacks mThreadCallbacks; - @Mock private VibratorController.OnVibrationCompleteListener mControllerCallbacks; - @Mock private IBinder mVibrationToken; - @Mock private IBatteryStats mIBatteryStatsMock; + @Rule + public MockitoRule mMockitoRule = MockitoJUnit.rule(); + + @Mock + private VibrationThread.VibrationCallbacks mThreadCallbacks; + @Mock + private VibratorController.OnVibrationCompleteListener mControllerCallbacks; + @Mock + private IBinder mVibrationToken; + @Mock + private IBatteryStats mIBatteryStatsMock; private final Map<Integer, FakeVibratorControllerProvider> mVibratorProviders = new HashMap<>(); private PowerManager.WakeLock mWakeLock; @@ -253,7 +258,7 @@ public class VibrationThreadTest { Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); cancellingThread.start(); - waitForCompletion(vibrationThread, 20); + waitForCompletion(vibrationThread, /* timeout= */ 50); waitForCompletion(cancellingThread); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); @@ -278,7 +283,7 @@ public class VibrationThreadTest { Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); cancellingThread.start(); - waitForCompletion(vibrationThread, 20); + waitForCompletion(vibrationThread, /* timeout= */ 50); waitForCompletion(cancellingThread); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); @@ -844,6 +849,39 @@ public class VibrationThreadTest { delay < maxDelay); } + @LargeTest + @Test + public void vibrate_cancelSlowVibrator_cancelIsNotBlockedByVibrationThread() throws Exception { + FakeVibratorControllerProvider fakeVibrator = mVibratorProviders.get(VIBRATOR_ID); + fakeVibrator.setSupportedEffects(VibrationEffect.EFFECT_CLICK); + + long latency = 5_000; // 5s + fakeVibrator.setLatency(latency); + + long vibrationId = 1; + VibrationEffect effect = VibrationEffect.get(VibrationEffect.EFFECT_CLICK); + VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); + + assertTrue(waitUntil( + t -> !fakeVibrator.getEffectSegments().isEmpty(), + vibrationThread, TEST_TIMEOUT_MILLIS)); + assertTrue(vibrationThread.isAlive()); + + // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should + // fail at waitForCompletion(cancellingThread). + Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); + cancellingThread.start(); + + // Cancelling the vibration should be fast and return right away, even if the thread is + // stuck at the slow call to the vibrator. + waitForCompletion(cancellingThread, /* timeout= */ 50); + + // After the vibrator call ends the vibration is cancelled and the vibrator is turned off. + waitForCompletion(vibrationThread, /* timeout= */ latency + TEST_TIMEOUT_MILLIS); + verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); + assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + } + @Test public void vibrate_multiplePredefinedCancel_cancelsVibrationImmediately() throws Exception { mockVibrators(1, 2); @@ -870,7 +908,7 @@ public class VibrationThreadTest { Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); cancellingThread.start(); - waitForCompletion(vibrationThread, 20); + waitForCompletion(vibrationThread, /* timeout= */ 50); waitForCompletion(cancellingThread); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); @@ -902,7 +940,7 @@ public class VibrationThreadTest { Thread cancellingThread = new Thread(() -> vibrationThread.cancel()); cancellingThread.start(); - waitForCompletion(vibrationThread, 20); + waitForCompletion(vibrationThread, /* timeout= */ 50); waitForCompletion(cancellingThread); verify(mThreadCallbacks).onVibrationEnded(eq(vibrationId), eq(Vibration.Status.CANCELLED)); |