From ad20c09c804099b5d8a6eb0550e10d05b9e16ca3 Mon Sep 17 00:00:00 2001 From: Simon Bowden Date: Mon, 21 Feb 2022 20:14:30 +0000 Subject: [PATCH 1/2] Move VibrationStepConductor locking out of VibrationThread. Add thread assertions to all VibrationStepConductor methods to make it clear where they're expected to run from. The ones not running from VibrationThread are not expected to change or execute steps - just to signal. With this, we can actually reduce the locking to only lock state that's used by multiple threads, but I'll do that in a follow-up. Bug: 193792066 Test: presubmit, manual Change-Id: I671517b9493ce142756a6d2544b8b4ffa5067fcb --- .../vibrator/StartSequentialEffectStep.java | 7 +- .../vibrator/VibrationStepConductor.java | 172 ++++++++++++++---- .../server/vibrator/VibrationThread.java | 61 +++---- 3 files changed, 170 insertions(+), 70 deletions(-) diff --git a/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java b/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java index b8885e81dbe26..080a36cb2a6e9 100644 --- a/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java +++ b/services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java @@ -55,13 +55,15 @@ final class StartSequentialEffectStep extends Step { private long mVibratorsOnMaxDuration; + /** Start a sequential effect at the beginning. */ StartSequentialEffectStep(VibrationStepConductor conductor, CombinedVibration.Sequential effect) { this(conductor, SystemClock.uptimeMillis() + effect.getDelays().get(0), effect, /* index= */ 0); } - StartSequentialEffectStep(VibrationStepConductor conductor, long startTime, + /** Continue a SequentialEffect from the specified index. */ + private StartSequentialEffectStep(VibrationStepConductor conductor, long startTime, CombinedVibration.Sequential effect, int index) { super(conductor, startTime); sequentialEffect = effect; @@ -123,8 +125,7 @@ final class StartSequentialEffectStep extends Step { /** * Create the next {@link StartSequentialEffectStep} to play this sequential effect, starting at - * the - * time this method is called, or null if sequence is complete. + * the time this method is called, or null if sequence is complete. */ @Nullable Step nextStep() { diff --git a/services/core/java/com/android/server/vibrator/VibrationStepConductor.java b/services/core/java/com/android/server/vibrator/VibrationStepConductor.java index 51691fbcdf5a3..02d1c8141ec92 100644 --- a/services/core/java/com/android/server/vibrator/VibrationStepConductor.java +++ b/services/core/java/com/android/server/vibrator/VibrationStepConductor.java @@ -18,9 +18,9 @@ package com.android.server.vibrator; import android.annotation.NonNull; import android.annotation.Nullable; +import android.os.Build; import android.os.CombinedVibration; import android.os.VibrationEffect; -import android.os.WorkSource; import android.os.vibrator.PrebakedSegment; import android.os.vibrator.PrimitiveSegment; import android.os.vibrator.RampSegment; @@ -42,6 +42,9 @@ import java.util.Queue; * dispatch of callbacks. */ final class VibrationStepConductor { + private static final boolean DEBUG = VibrationThread.DEBUG; + private static final String TAG = VibrationThread.TAG; + /** * Extra timeout added to the end of each vibration step to ensure it finishes even when * vibrator callbacks are lost. @@ -51,14 +54,13 @@ final class VibrationStepConductor { static final float RAMP_OFF_AMPLITUDE_MIN = 1e-3f; static final List EMPTY_STEP_LIST = new ArrayList<>(); - final Object mLock = new Object(); + private final Object mLock = new Object(); // Used within steps. public final VibrationSettings vibrationSettings; public final DeviceVibrationEffectAdapter deviceEffectAdapter; public final VibrationThread.VibratorManagerHooks vibratorManagerHooks; - private final WorkSource mWorkSource; private final Vibration mVibration; private final SparseArray mVibrators = new SparseArray<>(); @@ -72,7 +74,7 @@ final class VibrationStepConductor { @GuardedBy("mLock") private int mPendingVibrateSteps; @GuardedBy("mLock") - private int mConsumedStartVibrateSteps; + private int mRemainingStartSequentialEffectSteps; @GuardedBy("mLock") private int mSuccessfulVibratorOnSteps; @GuardedBy("mLock") @@ -86,7 +88,6 @@ final class VibrationStepConductor { this.vibrationSettings = vibrationSettings; this.deviceEffectAdapter = effectAdapter; this.vibratorManagerHooks = vibratorManagerHooks; - this.mWorkSource = new WorkSource(mVibration.uid); CombinedVibration effect = vib.getEffect(); for (int i = 0; i < availableVibrators.size(); i++) { @@ -100,6 +101,9 @@ final class VibrationStepConductor { AbstractVibratorStep nextVibrateStep(long startTime, VibratorController controller, VibrationEffect.Composed effect, int segmentIndex, long previousStepVibratorOffTimeout) { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } if (segmentIndex >= effect.getSegments().size()) { segmentIndex = effect.getRepeatIndex(); } @@ -127,25 +131,33 @@ final class VibrationStepConductor { } public void initializeForEffect(@NonNull CombinedVibration.Sequential vibration) { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + synchronized (mLock) { mPendingVibrateSteps++; + // This count is decremented at the completion of the step, so we don't subtract one. + mRemainingStartSequentialEffectSteps = vibration.getEffects().size(); mNextSteps.offer(new StartSequentialEffectStep(this, vibration)); } } public Vibration getVibration() { + // No thread assertion: immutable return mVibration; } - public WorkSource getWorkSource() { - return mWorkSource; - } - SparseArray getVibrators() { + // No thread assertion: immutable return mVibrators; } public boolean isFinished() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + synchronized (mLock) { return mPendingOnVibratorCompleteSteps.isEmpty() && mNextSteps.isEmpty(); } @@ -155,10 +167,14 @@ final class VibrationStepConductor { * Calculate the {@link Vibration.Status} based on the current queue state and the expected * number of {@link StartSequentialEffectStep} to be played. */ - public Vibration.Status calculateVibrationStatus(int expectedStartVibrateSteps) { + public Vibration.Status calculateVibrationStatus() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + synchronized (mLock) { if (mPendingVibrateSteps > 0 - || mConsumedStartVibrateSteps < expectedStartVibrateSteps) { + || mRemainingStartSequentialEffectSteps > 0) { return Vibration.Status.RUNNING; } if (mSuccessfulVibratorOnSteps > 0) { @@ -169,15 +185,39 @@ final class VibrationStepConductor { } } - /** Returns the time in millis to wait before calling {@link #runNextStep()}. */ - @GuardedBy("mLock") - public long getWaitMillisBeforeNextStepLocked() { - if (!mPendingOnVibratorCompleteSteps.isEmpty()) { - // Steps resumed by vibrator complete callback should be played right away. - return 0; + /** + * Blocks until the next step is due to run. The wait here may be interrupted by calling + * {@link #notifyWakeUp} or other "notify" methods. + * + *

This method returns false if the next step is ready to run now. If the method returns + * true, then some waiting was done, but may have been interrupted by a wakeUp. + * + * @return true if the method waited at all, or false if a step is ready to run now. + */ + public boolean waitUntilNextStepIsDue() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + + synchronized (mLock) { + if (!mPendingOnVibratorCompleteSteps.isEmpty()) { + // Steps resumed by vibrator complete callback should be played right away. + return false; + } + Step nextStep = mNextSteps.peek(); + if (nextStep == null) { + return false; + } + long waitMillis = nextStep.calculateWaitTime(); + if (waitMillis <= 0) { + return false; + } + try { + mLock.wait(waitMillis); + } catch (InterruptedException e) { + } + return true; } - Step nextStep = mNextSteps.peek(); - return nextStep == null ? 0 : nextStep.calculateWaitTime(); } /** @@ -185,6 +225,10 @@ final class VibrationStepConductor { * to be played next. */ public void runNextStep() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + // Vibrator callbacks should wait until the polled step is played and the next steps are // added back to the queue, so they can handle the callback. markWaitToProcessVibratorCallbacks(); @@ -199,7 +243,7 @@ final class VibrationStepConductor { mSuccessfulVibratorOnSteps++; } if (nextStep instanceof StartSequentialEffectStep) { - mConsumedStartVibrateSteps++; + mRemainingStartSequentialEffectSteps--; } if (!nextStep.isCleanUp()) { mPendingVibrateSteps--; @@ -218,39 +262,71 @@ final class VibrationStepConductor { } /** - * Notify the vibrator completion. + * Wake up the execution thread, which may be waiting until the next step is due. + * The caller is responsible for diverting VibrationThread execution. * - *

This is a lightweight method that do not trigger any operation from {@link - * VibratorController}, so it can be called directly from a native callback. + *

At the moment this is used after the signal is set that a cancellation needs to be + * processed. The actual cancellation will be invoked from the VibrationThread. */ + public void notifyWakeUp() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(false); + } + + synchronized (mLock) { + mLock.notify(); + } + } + @GuardedBy("mLock") - private void notifyVibratorCompleteLocked(int vibratorId) { + private void markVibratorCompleteLocked(int vibratorId) { mCompletionNotifiedVibrators.offer(vibratorId); if (!mWaitToProcessVibratorCompleteCallbacks) { // No step is being played or cancelled now, process the callback right away. processVibratorCompleteCallbacksLocked(); } + // mLock.notify() is done outside this method to ensure it's only done once when + // multiple vibrators are notified. } + /** + * Notify the conductor that a vibrator has completed its work. + * + *

This is a lightweight method intended to be called directly via native callbacks. + * The state update is recorded for processing on the main execution thread (VibrationThread). + */ public void notifyVibratorComplete(int vibratorId) { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(false); + } + synchronized (mLock) { - if (VibrationThread.DEBUG) { - Slog.d(VibrationThread.TAG, - "Vibration complete reported by vibrator " + vibratorId); + if (DEBUG) { + Slog.d(TAG, "Vibration complete reported by vibrator " + vibratorId); } - notifyVibratorCompleteLocked(vibratorId); + markVibratorCompleteLocked(vibratorId); mLock.notify(); } } + /** + * Notify that a VibratorManager sync operation has completed. + * + *

This is a lightweight method intended to be called directly via native callbacks. + * The state update is recorded for processing on the main execution thread + * (VibrationThread). + */ public void notifySyncedVibrationComplete() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(false); + } + synchronized (mLock) { - if (VibrationThread.DEBUG) { - Slog.d(VibrationThread.TAG, - "Synced vibration complete reported by vibrator manager"); + if (DEBUG) { + Slog.d(TAG, "Synced vibration complete reported by vibrator manager"); } for (int i = 0; i < mVibrators.size(); i++) { - notifyVibratorCompleteLocked(mVibrators.keyAt(i)); + markVibratorCompleteLocked(mVibrators.keyAt(i)); } mLock.notify(); } @@ -263,6 +339,10 @@ final class VibrationStepConductor { * {@link Step#cancel()}. */ public void cancel() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + // Vibrator callbacks should wait until all steps from the queue are properly cancelled // and clean up steps are added back to the queue, so they can handle the callback. markWaitToProcessVibratorCallbacks(); @@ -290,6 +370,10 @@ final class VibrationStepConductor { *

This will remove and trigger {@link Step#cancelImmediately()} in all steps, in order. */ public void cancelImmediately() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + // Vibrator callbacks should wait until all steps from the queue are properly cancelled. markWaitToProcessVibratorCallbacks(); try { @@ -311,6 +395,10 @@ final class VibrationStepConductor { @Nullable private Step pollNext() { + if (Build.IS_DEBUGGABLE) { + expectIsVibrationThread(true); + } + synchronized (mLock) { // Prioritize the steps resumed by a vibrator complete callback. if (!mPendingOnVibratorCompleteSteps.isEmpty()) { @@ -338,6 +426,12 @@ final class VibrationStepConductor { */ @GuardedBy("mLock") private void processVibratorCompleteCallbacksLocked() { + if (Build.IS_DEBUGGABLE) { + // TODO: ensure this method is only called on the vibration thread. Currently it + // can be invoked on the completion callback paths. + //expectIsVibrationThread(true); + } + mWaitToProcessVibratorCompleteCallbacks = false; while (!mCompletionNotifiedVibrators.isEmpty()) { int vibratorId = mCompletionNotifiedVibrators.poll(); @@ -352,4 +446,18 @@ final class VibrationStepConductor { } } } + + /** + * This check is used for debugging and documentation to indicate the thread that's expected + * to invoke a given public method on this class. Most methods are only invoked by + * VibrationThread, which is where all the steps and HAL calls should be made. Other threads + * should only signal to the execution flow being run by VibrationThread. + */ + private void expectIsVibrationThread(boolean isVibrationThread) { + if ((Thread.currentThread() instanceof VibrationThread) != isVibrationThread) { + Slog.wtfStack("VibrationStepConductor", + "Thread caller assertion failed, expected isVibrationThread=" + + isVibrationThread); + } + } } diff --git a/services/core/java/com/android/server/vibrator/VibrationThread.java b/services/core/java/com/android/server/vibrator/VibrationThread.java index f2cd8c3ec3f80..066aca1a61f36 100644 --- a/services/core/java/com/android/server/vibrator/VibrationThread.java +++ b/services/core/java/com/android/server/vibrator/VibrationThread.java @@ -22,6 +22,7 @@ import android.os.PowerManager; import android.os.Process; import android.os.RemoteException; import android.os.Trace; +import android.os.WorkSource; import android.util.Slog; import android.util.SparseArray; @@ -136,12 +137,14 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { /** Runs the VibrationThread ensuring that the wake lock is acquired and released. */ private void runWithWakeLock() { - mWakeLock.setWorkSource(mStepConductor.getWorkSource()); + WorkSource workSource = new WorkSource(mStepConductor.getVibration().uid); + mWakeLock.setWorkSource(workSource); mWakeLock.acquire(); try { runWithWakeLockAndDeathLink(); } finally { mWakeLock.release(); + mWakeLock.setWorkSource(null); } } @@ -178,12 +181,10 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { return; } mStop = true; - synchronized (mStepConductor.mLock) { - if (DEBUG) { - Slog.d(TAG, "Vibration cancelled"); - } - mStepConductor.mLock.notify(); + if (DEBUG) { + Slog.d(TAG, "Vibration cancelled"); } + mStepConductor.notifyWakeUp(); } /** Cancel current vibration and shuts off the vibrators immediately. */ @@ -192,13 +193,11 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { // Already forced the thread to stop, wait for it to finish. return; } - mStop = mForceStop = true; - synchronized (mStepConductor.mLock) { - if (DEBUG) { - Slog.d(TAG, "Vibration cancelled immediately"); - } - mStepConductor.mLock.notify(); + if (DEBUG) { + Slog.d(TAG, "Vibration cancelled immediately"); } + mStop = mForceStop = true; + mStepConductor.notifyWakeUp(); } /** Notify current vibration that a synced step has completed. */ @@ -227,25 +226,14 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { try { CombinedVibration.Sequential sequentialEffect = toSequential(mStepConductor.getVibration().getEffect()); - final int sequentialEffectSize = sequentialEffect.getEffects().size(); mStepConductor.initializeForEffect(sequentialEffect); while (!mStepConductor.isFinished()) { - long waitMillisBeforeNextStep; - synchronized (mStepConductor.mLock) { - waitMillisBeforeNextStep = mStepConductor.getWaitMillisBeforeNextStepLocked(); - if (waitMillisBeforeNextStep > 0) { - try { - mStepConductor.mLock.wait(waitMillisBeforeNextStep); - } catch (InterruptedException e) { - } - } - } - // Only run the next vibration step if we didn't have to wait in this loop. - // If we waited then the queue may have changed or the wait could have been - // interrupted by a cancel call, so loop again to re-evaluate the scheduling of - // the queue top element. - if (waitMillisBeforeNextStep <= 0) { + // Skip wait and next step if mForceStop already happened. + boolean waited = mForceStop || mStepConductor.waitUntilNextStepIsDue(); + // If we waited, don't run the next step, but instead re-evaluate cancellation + // status + if (!waited) { if (DEBUG) { Slog.d(TAG, "Play vibration consuming next step..."); } @@ -253,8 +241,17 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { // blocking the thread. mStepConductor.runNextStep(); } + + if (mForceStop) { + // Cancel every step and stop playing them right away, even clean-up steps. + mStepConductor.cancelImmediately(); + clientVibrationCompleteIfNotAlready(Vibration.Status.CANCELLED); + break; + } + Vibration.Status status = mStop ? Vibration.Status.CANCELLED - : mStepConductor.calculateVibrationStatus(sequentialEffectSize); + : mStepConductor.calculateVibrationStatus(); + // This block can only run once due to mCalledVibrationCompleteCallback. if (status != Vibration.Status.RUNNING && !mCalledVibrationCompleteCallback) { // First time vibration stopped running, start clean-up tasks and notify // callback immediately. @@ -263,12 +260,6 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { mStepConductor.cancel(); } } - if (mForceStop) { - // Cancel every step and stop playing them right away, even clean-up steps. - mStepConductor.cancelImmediately(); - clientVibrationCompleteIfNotAlready(Vibration.Status.CANCELLED); - break; - } } } finally { Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR); From 0709a084c96152e87fbdefdf618a458183f364f6 Mon Sep 17 00:00:00 2001 From: Simon Bowden Date: Mon, 21 Feb 2022 20:14:30 +0000 Subject: [PATCH 2/2] Don't expose vibrator controllers via VibrationThread. Also move "toSequential" into VibrationStepConductor - there's no reason for that to be outside. Bug: 193792066 Test: atest Change-Id: I3de6343871ac5269daab9698b66563b4fc394a68 --- .../vibrator/VibrationStepConductor.java | 19 +++- .../server/vibrator/VibrationThread.java | 22 +--- .../server/vibrator/VibrationThreadTest.java | 100 +++++++++--------- 3 files changed, 67 insertions(+), 74 deletions(-) diff --git a/services/core/java/com/android/server/vibrator/VibrationStepConductor.java b/services/core/java/com/android/server/vibrator/VibrationStepConductor.java index 02d1c8141ec92..764813c90e688 100644 --- a/services/core/java/com/android/server/vibrator/VibrationStepConductor.java +++ b/services/core/java/com/android/server/vibrator/VibrationStepConductor.java @@ -16,7 +16,6 @@ package com.android.server.vibrator; -import android.annotation.NonNull; import android.annotation.Nullable; import android.os.Build; import android.os.CombinedVibration; @@ -130,16 +129,17 @@ final class VibrationStepConductor { previousStepVibratorOffTimeout); } - public void initializeForEffect(@NonNull CombinedVibration.Sequential vibration) { + /** Called when this conductor is going to be started running by the VibrationThread. */ + public void prepareToStart() { if (Build.IS_DEBUGGABLE) { expectIsVibrationThread(true); } - + CombinedVibration.Sequential sequentialEffect = toSequential(mVibration.getEffect()); synchronized (mLock) { mPendingVibrateSteps++; // This count is decremented at the completion of the step, so we don't subtract one. - mRemainingStartSequentialEffectSteps = vibration.getEffects().size(); - mNextSteps.offer(new StartSequentialEffectStep(this, vibration)); + mRemainingStartSequentialEffectSteps = sequentialEffect.getEffects().size(); + mNextSteps.offer(new StartSequentialEffectStep(this, sequentialEffect)); } } @@ -447,6 +447,15 @@ final class VibrationStepConductor { } } + private static CombinedVibration.Sequential toSequential(CombinedVibration effect) { + if (effect instanceof CombinedVibration.Sequential) { + return (CombinedVibration.Sequential) effect; + } + return (CombinedVibration.Sequential) CombinedVibration.startSequential() + .addNext(effect) + .combine(); + } + /** * This check is used for debugging and documentation to indicate the thread that's expected * to invoke a given public method on this class. Most methods are only invoked by diff --git a/services/core/java/com/android/server/vibrator/VibrationThread.java b/services/core/java/com/android/server/vibrator/VibrationThread.java index 066aca1a61f36..3fef7f2a747ef 100644 --- a/services/core/java/com/android/server/vibrator/VibrationThread.java +++ b/services/core/java/com/android/server/vibrator/VibrationThread.java @@ -16,7 +16,6 @@ package com.android.server.vibrator; -import android.os.CombinedVibration; import android.os.IBinder; import android.os.PowerManager; import android.os.Process; @@ -26,8 +25,6 @@ import android.os.WorkSource; import android.util.Slog; import android.util.SparseArray; -import com.android.internal.annotations.VisibleForTesting; - import java.util.NoSuchElementException; /** Plays a {@link Vibration} in dedicated thread. */ @@ -106,11 +103,6 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { return mStepConductor.getVibration(); } - @VisibleForTesting - SparseArray getVibrators() { - return mStepConductor.getVibrators(); - } - @Override public void binderDied() { if (DEBUG) { @@ -224,9 +216,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { private void playVibration() { Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "playVibration"); try { - CombinedVibration.Sequential sequentialEffect = - toSequential(mStepConductor.getVibration().getEffect()); - mStepConductor.initializeForEffect(sequentialEffect); + mStepConductor.prepareToStart(); while (!mStepConductor.isFinished()) { // Skip wait and next step if mForceStop already happened. @@ -265,14 +255,4 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient { Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR); } } - - private static CombinedVibration.Sequential toSequential(CombinedVibration effect) { - if (effect instanceof CombinedVibration.Sequential) { - return (CombinedVibration.Sequential) effect; - } - return (CombinedVibration.Sequential) CombinedVibration.startSequential() - .addNext(effect) - .combine(); - } - } 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 01e306e744fb5..3d24a814c7cde 100644 --- a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java +++ b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java @@ -113,6 +113,9 @@ public class VibrationThreadTest { private TestLooper mTestLooper; private TestLooperAutoDispatcher mCustomTestLooperDispatcher; + // Setup from the providers when VibrationThread is initialized. + private SparseArray mControllers; + @Before public void setUp() throws Exception { mTestLooper = new TestLooper(); @@ -178,7 +181,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(10)), mVibratorProviders.get(VIBRATOR_ID).getEffectSegments()); @@ -197,7 +200,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(10)), mVibratorProviders.get(VIBRATOR_ID).getEffectSegments()); @@ -219,7 +222,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(15)), mVibratorProviders.get(VIBRATOR_ID).getEffectSegments()); @@ -243,7 +246,7 @@ public class VibrationThreadTest { thread, TEST_TIMEOUT_MILLIS)); // Vibration still running after 2 cycles. assertTrue(thread.isAlive()); - assertTrue(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertTrue(mControllers.get(VIBRATOR_ID).isVibrating()); thread.cancel(); waitForCompletion(thread); @@ -251,7 +254,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOn(eq(UID), anyLong()); verify(mManagerHooks).noteVibratorOff(eq(UID)); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); List playedAmplitudes = fakeVibrator.getAmplitudes(); assertFalse(fakeVibrator.getEffectSegments().isEmpty()); @@ -280,7 +283,7 @@ public class VibrationThreadTest { waitForCompletion(thread); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(1000)), fakeVibrator.getEffectSegments()); } @@ -302,7 +305,7 @@ public class VibrationThreadTest { waitForCompletion(thread); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(5550)), fakeVibrator.getEffectSegments()); } @@ -325,7 +328,7 @@ public class VibrationThreadTest { waitForCompletion(thread); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(2, fakeVibrator.getEffectSegments().size()); // First time turn vibrator ON for minimum of 1s. assertEquals(1000L, fakeVibrator.getEffectSegments().get(0).getDuration()); @@ -350,7 +353,7 @@ public class VibrationThreadTest { .compose(); VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - assertTrue(waitUntil(t -> t.getVibrators().get(VIBRATOR_ID).isVibrating(), vibrationThread, + assertTrue(waitUntil(t -> mControllers.get(VIBRATOR_ID).isVibrating(), vibrationThread, TEST_TIMEOUT_MILLIS)); assertTrue(vibrationThread.isAlive()); @@ -363,7 +366,7 @@ public class VibrationThreadTest { waitForCompletion(cancellingThread); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); } @Test @@ -375,7 +378,7 @@ public class VibrationThreadTest { VibrationEffect effect = VibrationEffect.createWaveform(new long[]{100}, new int[]{100}, 0); VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - assertTrue(waitUntil(t -> t.getVibrators().get(VIBRATOR_ID).isVibrating(), vibrationThread, + assertTrue(waitUntil(t -> mControllers.get(VIBRATOR_ID).isVibrating(), vibrationThread, TEST_TIMEOUT_MILLIS)); assertTrue(vibrationThread.isAlive()); @@ -388,7 +391,7 @@ public class VibrationThreadTest { waitForCompletion(cancellingThread); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); } @Test @@ -404,7 +407,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedPrebaked(VibrationEffect.EFFECT_THUD)), mVibratorProviders.get(VIBRATOR_ID).getEffectSegments()); @@ -427,7 +430,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(10)), mVibratorProviders.get(VIBRATOR_ID).getEffectSegments()); @@ -466,7 +469,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList( expectedPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1, 0), expectedPrimitive(VibrationEffect.Composition.PRIMITIVE_TICK, 0.5f, 0)), @@ -536,7 +539,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks, times(4)).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList( expectedOneShot(10), expectedPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1, 0), @@ -573,7 +576,7 @@ public class VibrationThreadTest { verify(mManagerHooks).noteVibratorOff(eq(UID)); verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList( expectedRamp(/* amplitude= */ 1, /* frequencyHz= */ 150, /* duration= */ 10), expectedRamp(/* startAmplitude= */ 1, /* endAmplitude= */ 0, @@ -626,11 +629,11 @@ public class VibrationThreadTest { TEST_TIMEOUT_MILLIS)); // Vibration still running after 2 cycles. assertTrue(thread.isAlive()); - assertTrue(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertTrue(mControllers.get(VIBRATOR_ID).isVibrating()); thread.binderDied(); waitForCompletion(thread); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); } @@ -667,7 +670,7 @@ public class VibrationThreadTest { verify(mControllerCallbacks).onComplete(eq(VIBRATOR_ID), eq(vibrationId)); verify(mControllerCallbacks, never()).onComplete(eq(2), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); assertEquals(Arrays.asList(expectedPrebaked(VibrationEffect.EFFECT_TICK)), mVibratorProviders.get(VIBRATOR_ID).getEffectSegments()); @@ -692,9 +695,9 @@ public class VibrationThreadTest { verify(mControllerCallbacks).onComplete(eq(2), eq(vibrationId)); verify(mControllerCallbacks).onComplete(eq(3), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(1).isVibrating()); - assertFalse(thread.getVibrators().get(2).isVibrating()); - assertFalse(thread.getVibrators().get(3).isVibrating()); + assertFalse(mControllers.get(1).isVibrating()); + assertFalse(mControllers.get(2).isVibrating()); + assertFalse(mControllers.get(3).isVibrating()); VibrationEffectSegment expected = expectedPrebaked(VibrationEffect.EFFECT_CLICK); assertEquals(Arrays.asList(expected), mVibratorProviders.get(1).getEffectSegments()); @@ -731,10 +734,10 @@ public class VibrationThreadTest { verify(mControllerCallbacks).onComplete(eq(3), eq(vibrationId)); verify(mControllerCallbacks).onComplete(eq(4), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(1).isVibrating()); - assertFalse(thread.getVibrators().get(2).isVibrating()); - assertFalse(thread.getVibrators().get(3).isVibrating()); - assertFalse(thread.getVibrators().get(4).isVibrating()); + assertFalse(mControllers.get(1).isVibrating()); + assertFalse(mControllers.get(2).isVibrating()); + assertFalse(mControllers.get(3).isVibrating()); + assertFalse(mControllers.get(4).isVibrating()); assertEquals(Arrays.asList(expectedPrebaked(VibrationEffect.EFFECT_CLICK)), mVibratorProviders.get(1).getEffectSegments()); @@ -782,9 +785,9 @@ public class VibrationThreadTest { batteryVerifier.verify(mManagerHooks).noteVibratorOff(eq(UID)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(1).isVibrating()); - assertFalse(thread.getVibrators().get(2).isVibrating()); - assertFalse(thread.getVibrators().get(3).isVibrating()); + assertFalse(mControllers.get(1).isVibrating()); + assertFalse(mControllers.get(2).isVibrating()); + assertFalse(mControllers.get(3).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(10)), mVibratorProviders.get(1).getEffectSegments()); @@ -941,9 +944,9 @@ public class VibrationThreadTest { // All vibrators are turned on in parallel. assertTrue(waitUntil( - t -> t.getVibrators().get(1).isVibrating() - && t.getVibrators().get(2).isVibrating() - && t.getVibrators().get(3).isVibrating(), + t -> mControllers.get(1).isVibrating() + && mControllers.get(2).isVibrating() + && mControllers.get(3).isVibrating(), thread, TEST_TIMEOUT_MILLIS)); waitForCompletion(thread); @@ -954,9 +957,9 @@ public class VibrationThreadTest { verify(mControllerCallbacks).onComplete(eq(2), eq(vibrationId)); verify(mControllerCallbacks).onComplete(eq(3), eq(vibrationId)); verifyCallbacksTriggered(vibrationId, Vibration.Status.FINISHED); - assertFalse(thread.getVibrators().get(1).isVibrating()); - assertFalse(thread.getVibrators().get(2).isVibrating()); - assertFalse(thread.getVibrators().get(3).isVibrating()); + assertFalse(mControllers.get(1).isVibrating()); + assertFalse(mControllers.get(2).isVibrating()); + assertFalse(mControllers.get(3).isVibrating()); assertEquals(Arrays.asList(expectedOneShot(25)), mVibratorProviders.get(1).getEffectSegments()); @@ -1031,7 +1034,7 @@ public class VibrationThreadTest { // After the vibrator call ends the vibration is cancelled and the vibrator is turned off. waitForCompletion(vibrationThread, /* timeout= */ latency + TEST_TIMEOUT_MILLIS); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); } @Test @@ -1051,7 +1054,7 @@ public class VibrationThreadTest { .combine(); VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - assertTrue(waitUntil(t -> t.getVibrators().get(2).isVibrating(), vibrationThread, + assertTrue(waitUntil(t -> mControllers.get(2).isVibrating(), vibrationThread, TEST_TIMEOUT_MILLIS)); assertTrue(vibrationThread.isAlive()); @@ -1064,8 +1067,8 @@ public class VibrationThreadTest { waitForCompletion(cancellingThread); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(vibrationThread.getVibrators().get(1).isVibrating()); - assertFalse(vibrationThread.getVibrators().get(2).isVibrating()); + assertFalse(mControllers.get(1).isVibrating()); + assertFalse(mControllers.get(2).isVibrating()); } @Test @@ -1082,8 +1085,8 @@ public class VibrationThreadTest { .combine(); VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - assertTrue(waitUntil(t -> t.getVibrators().get(1).isVibrating() - && t.getVibrators().get(2).isVibrating(), + assertTrue(waitUntil(t -> mControllers.get(1).isVibrating() + && mControllers.get(2).isVibrating(), vibrationThread, TEST_TIMEOUT_MILLIS)); assertTrue(vibrationThread.isAlive()); @@ -1096,8 +1099,8 @@ public class VibrationThreadTest { waitForCompletion(cancellingThread); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); - assertFalse(vibrationThread.getVibrators().get(1).isVibrating()); - assertFalse(vibrationThread.getVibrators().get(2).isVibrating()); + assertFalse(mControllers.get(1).isVibrating()); + assertFalse(mControllers.get(2).isVibrating()); } @Test @@ -1106,7 +1109,7 @@ public class VibrationThreadTest { VibrationEffect effect = VibrationEffect.createWaveform(new long[]{5}, new int[]{100}, 0); VibrationThread thread = startThreadAndDispatcher(vibrationId, effect); - assertTrue(waitUntil(t -> t.getVibrators().get(VIBRATOR_ID).isVibrating(), thread, + assertTrue(waitUntil(t -> mControllers.get(VIBRATOR_ID).isVibrating(), thread, TEST_TIMEOUT_MILLIS)); assertTrue(thread.isAlive()); @@ -1117,7 +1120,7 @@ public class VibrationThreadTest { verify(mVibrationToken).unlinkToDeath(same(thread), eq(0)); verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED); assertFalse(mVibratorProviders.get(VIBRATOR_ID).getEffectSegments().isEmpty()); - assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating()); + assertFalse(mControllers.get(VIBRATOR_ID).isVibrating()); } @Test @@ -1188,7 +1191,7 @@ public class VibrationThreadTest { long vibrationId = 1; VibrationEffect effect = VibrationEffect.createOneShot(10_000, 240); VibrationThread thread = startThreadAndDispatcher(vibrationId, effect); - assertTrue(waitUntil(t -> t.getVibrators().get(VIBRATOR_ID).isVibrating(), thread, + assertTrue(waitUntil(t -> mControllers.get(VIBRATOR_ID).isVibrating(), thread, TEST_TIMEOUT_MILLIS)); thread.cancel(); waitForCompletion(thread); @@ -1295,8 +1298,9 @@ public class VibrationThreadTest { } private VibrationThread startThreadAndDispatcher(Vibration vib) { + mControllers = createVibratorControllers(); VibrationThread thread = new VibrationThread(vib, mVibrationSettings, mEffectAdapter, - createVibratorControllers(), mWakeLock, mManagerHooks); + mControllers, mWakeLock, mManagerHooks); doAnswer(answer -> { thread.vibratorComplete(answer.getArgument(0)); return null;