Merge "Increase timeout on VibrationThread to wait for vibrator callbacks" into sc-dev (1a208a73) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/vibrator/VibrationThread.java

+107 −54

Original line number	Diff line number	Diff line
		@@ -59,7 +59,7 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		* Extra timeout added to the end of each vibration step to ensure it finishes even when
		* vibrator callbacks are lost.
		*/
		private static final long CALLBACKS_EXTRA_TIMEOUT = 100;
		private static final long CALLBACKS_EXTRA_TIMEOUT = 1_000;

		/** Threshold to prevent the ramp off steps from trying to set extremely low amplitudes. */
		private static final float RAMP_OFF_AMPLITUDE_MIN = 1e-3f;
		@@ -341,6 +341,8 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		private final PriorityQueue<Step> mNextSteps = new PriorityQueue<>();
		@GuardedBy("mLock")
		private final Queue<Step> mPendingOnVibratorCompleteSteps = new LinkedList<>();
		@GuardedBy("mLock")
		private final Queue<Integer> mNotifiedVibrators = new LinkedList<>();

		@GuardedBy("mLock")
		private int mPendingVibrateSteps;
		@@ -348,6 +350,8 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		private int mConsumedStartVibrateSteps;
		@GuardedBy("mLock")
		private int mSuccessfulVibratorOnSteps;
		@GuardedBy("mLock")
		private boolean mWaitToProcessVibratorCallbacks;

		public void offer(@NonNull Step step) {
		synchronized (mLock) {
		@@ -398,14 +402,16 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		/**
		* 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
		*/
		public void consumeNext() {
		// 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();
		try {
		Step nextStep = pollNext();
		if (nextStep != null) {
		// This might turn on the vibrator and have a HAL latency. Execute this outside any
		// lock to avoid blocking other interactions with the thread.
		// 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) {
		if (nextStep.getVibratorOnDuration() > 0) {
		@@ -423,28 +429,25 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		mNextSteps.addAll(nextSteps);
		}
		}
		} finally {
		synchronized (mLock) {
		processVibratorCallbacks();
		}
		}
		}

		/**
		* 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()}.
		* Notify the vibrator completion.
		*
		* <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 will be anticipated by this method, in no particular order.
		*/
		@GuardedBy("mLock")
		public void notifyVibratorComplete(int vibratorId) {
		Iterator<Step> it = mNextSteps.iterator();
		while (it.hasNext()) {
		Step step = it.next();
		if (step.shouldPlayWhenVibratorComplete(vibratorId)) {
		it.remove();
		mPendingOnVibratorCompleteSteps.offer(step);
		break;
		}
		mNotifiedVibrators.offer(vibratorId);
		if (!mWaitToProcessVibratorCallbacks) {
		// No step is being played or cancelled now, process the callback right away.
		processVibratorCallbacks();
		}
		}

		@@ -455,6 +458,10 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		* {@link Step#cancel()}.
		*/
		public void cancel() {
		// 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();
		try {
		List<Step> cleanUpSteps = new ArrayList<>();
		Step step;
		while ((step = pollNext()) != null) {
		@@ -465,6 +472,11 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		mPendingVibrateSteps = 0;
		mNextSteps.addAll(cleanUpSteps);
		}
		} finally {
		synchronized (mLock) {
		processVibratorCallbacks();
		}
		}
		}

		/**
		@@ -473,6 +485,9 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		* <p>This will remove and trigger {@link Step#cancelImmediately()} in all steps, in order.
		*/
		public void cancelImmediately() {
		// Vibrator callbacks should wait until all steps from the queue are properly cancelled.
		markWaitToProcessVibratorCallbacks();
		try {
		Step step;
		while ((step = pollNext()) != null) {
		// This might turn off the vibrator and have a HAL latency. Execute this outside
		@@ -482,6 +497,11 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		synchronized (mLock) {
		mPendingVibrateSteps = 0;
		}
		} finally {
		synchronized (mLock) {
		processVibratorCallbacks();
		}
		}
		}

		@Nullable
		@@ -494,6 +514,39 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		return mNextSteps.poll();
		}
		}

		private void markWaitToProcessVibratorCallbacks() {
		synchronized (mLock) {
		mWaitToProcessVibratorCallbacks = true;
		}
		}

		/**
		* 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 will be anticipated by this method, in no particular order.
		*/
		@GuardedBy("mLock")
		private void processVibratorCallbacks() {
		mWaitToProcessVibratorCallbacks = false;
		while (!mNotifiedVibrators.isEmpty()) {
		int vibratorId = mNotifiedVibrators.poll();
		Iterator<Step> it = mNextSteps.iterator();
		while (it.hasNext()) {
		Step step = it.next();
		if (step.shouldPlayWhenVibratorComplete(vibratorId)) {
		it.remove();
		mPendingOnVibratorCompleteSteps.offer(step);
		break;
		}
		}
		}
		}
		}

		/**
		@@ -894,9 +947,9 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
		// Vibration was not started, so just skip the played segments and keep timings.
		return skipToNextSteps(segmentsPlayed);
		}
		long nextVibratorOffTimeout =
		SystemClock.uptimeMillis() + mVibratorOnResult + CALLBACKS_EXTRA_TIMEOUT;
		return nextSteps(nextVibratorOffTimeout, nextVibratorOffTimeout, segmentsPlayed);
		long nextStartTime = SystemClock.uptimeMillis() + mVibratorOnResult;
		long nextVibratorOffTimeout = nextStartTime + CALLBACKS_EXTRA_TIMEOUT;
		return nextSteps(nextStartTime, nextVibratorOffTimeout, segmentsPlayed);
		}

		/**

services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java

+39 −2

Original line number	Diff line number	Diff line
		@@ -58,6 +58,7 @@ import androidx.test.InstrumentationRegistry;

		import com.android.internal.app.IBatteryStats;

		import org.junit.After;
		import org.junit.Before;
		import org.junit.Rule;
		import org.junit.Test;
		@@ -106,6 +107,7 @@ public class VibrationThreadTest {
		private DeviceVibrationEffectAdapter mEffectAdapter;
		private PowerManager.WakeLock mWakeLock;
		private TestLooper mTestLooper;
		private TestLooperAutoDispatcher mCustomTestLooperDispatcher;

		@Before
		public void setUp() throws Exception {
		@@ -121,6 +123,13 @@ public class VibrationThreadTest {
		mockVibrators(VIBRATOR_ID);
		}

		@After
		public void tearDown() {
		if (mCustomTestLooperDispatcher != null) {
		mCustomTestLooperDispatcher.cancel();
		}
		}

		@Test
		public void vibrate_noVibrator_ignoresVibration() {
		mVibratorProviders.clear();
		@@ -508,7 +517,7 @@ public class VibrationThreadTest {
		.addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f)
		.addPrimitive(VibrationEffect.Composition.PRIMITIVE_TICK, 0.5f)
		.addEffect(VibrationEffect.get(VibrationEffect.EFFECT_CLICK))
		.addEffect(VibrationEffect.get(VibrationEffect.EFFECT_CLICK), 10)
		.addEffect(VibrationEffect.get(VibrationEffect.EFFECT_CLICK), /* delay= */ 100)
		.compose();
		VibrationThread thread = startThreadAndDispatcher(vibrationId, effect);
		waitForCompletion(thread);
		@@ -1290,7 +1299,9 @@ public class VibrationThreadTest {
		thread.vibratorComplete(answer.getArgument(0));
		return null;
		}).when(mControllerCallbacks).onComplete(anyInt(), eq(vib.id));
		mTestLooper.startAutoDispatch();
		// TestLooper.AutoDispatchThread has a fixed 1s duration. Use a custom auto-dispatcher.
		mCustomTestLooperDispatcher = new TestLooperAutoDispatcher(mTestLooper);
		mCustomTestLooperDispatcher.start();
		thread.start();
		return thread;
		}
		@@ -1316,6 +1327,7 @@ public class VibrationThreadTest {
		} catch (InterruptedException e) {
		}
		assertFalse(thread.isAlive());
		mCustomTestLooperDispatcher.cancel();
		mTestLooper.dispatchAll();
		}

		@@ -1364,4 +1376,29 @@ public class VibrationThreadTest {
		verify(mThreadCallbacks).onVibrationCompleted(eq(vibrationId), eq(expectedStatus));
		verify(mThreadCallbacks).onVibratorsReleased();
		}

		private final class TestLooperAutoDispatcher extends Thread {
		private final TestLooper mTestLooper;
		private boolean mCancelled;

		TestLooperAutoDispatcher(TestLooper testLooper) {
		mTestLooper = testLooper;
		}

		@Override
		public void run() {
		while (!mCancelled) {
		mTestLooper.dispatchAll();
		try {
		Thread.sleep(10);
		} catch (InterruptedException e) {
		return;
		}
		}
		}

		public void cancel() {
		mCancelled = true;
		}
		}
		}