Merge "Increase timeout on VibrationThread to wait for vibrator callbacks"... (3052c303) · Commits · e / os / android_frameworks_base

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

+107 −54

Original line number	Original line	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		* Extra timeout added to the end of each vibration step to ensure it finishes even when
	* vibrator callbacks are lost.		* 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. */		/** Threshold to prevent the ramp off steps from trying to set extremely low amplitudes. */
	private static final float RAMP_OFF_AMPLITUDE_MIN = 1e-3f;		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<>();		private final PriorityQueue<Step> mNextSteps = new PriorityQueue<>();
	@GuardedBy("mLock")		@GuardedBy("mLock")
	private final Queue<Step> mPendingOnVibratorCompleteSteps = new LinkedList<>();		private final Queue<Step> mPendingOnVibratorCompleteSteps = new LinkedList<>();
			@GuardedBy("mLock")
			private final Queue<Integer> mNotifiedVibrators = new LinkedList<>();

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

	public void offer(@NonNull Step step) {		public void offer(@NonNull Step step) {
	synchronized (mLock) {		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		* Play and remove the step at the top of this queue, and also adds the next steps generated
	* to be played next.		* to be played next.
	*
	* @return the number of steps played
	*/		*/
	public void consumeNext() {		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();		Step nextStep = pollNext();
	if (nextStep != null) {		if (nextStep != null) {
	// This might turn on the vibrator and have a HAL latency. Execute this outside any		// This might turn on the vibrator and have a HAL latency. Execute this outside
	// lock to avoid blocking other interactions with the thread.		// any lock to avoid blocking other interactions with the thread.
	List<Step> nextSteps = nextStep.play();		List<Step> nextSteps = nextStep.play();
	synchronized (mLock) {		synchronized (mLock) {
	if (nextStep.getVibratorOnDuration() > 0) {		if (nextStep.getVibratorOnDuration() > 0) {
	@@ -423,28 +429,25 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
	mNextSteps.addAll(nextSteps);		mNextSteps.addAll(nextSteps);
	}		}
	}		}
			} finally {
			synchronized (mLock) {
			processVibratorCallbacks();
			}
			}
	}		}

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

	@@ -455,6 +458,10 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
	* {@link Step#cancel()}.		* {@link Step#cancel()}.
	*/		*/
	public void 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<>();		List<Step> cleanUpSteps = new ArrayList<>();
	Step step;		Step step;
	while ((step = pollNext()) != null) {		while ((step = pollNext()) != null) {
	@@ -465,6 +472,11 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
	mPendingVibrateSteps = 0;		mPendingVibrateSteps = 0;
	mNextSteps.addAll(cleanUpSteps);		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.		* <p>This will remove and trigger {@link Step#cancelImmediately()} in all steps, in order.
	*/		*/
	public void cancelImmediately() {		public void cancelImmediately() {
			// Vibrator callbacks should wait until all steps from the queue are properly cancelled.
			markWaitToProcessVibratorCallbacks();
			try {
	Step step;		Step step;
	while ((step = pollNext()) != null) {		while ((step = pollNext()) != null) {
	// This might turn off the vibrator and have a HAL latency. Execute this outside		// 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) {		synchronized (mLock) {
	mPendingVibrateSteps = 0;		mPendingVibrateSteps = 0;
	}		}
			} finally {
			synchronized (mLock) {
			processVibratorCallbacks();
			}
			}
	}		}

	@Nullable		@Nullable
	@@ -494,6 +514,39 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
	return mNextSteps.poll();		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.		// Vibration was not started, so just skip the played segments and keep timings.
	return skipToNextSteps(segmentsPlayed);		return skipToNextSteps(segmentsPlayed);
	}		}
	long nextVibratorOffTimeout =		long nextStartTime = SystemClock.uptimeMillis() + mVibratorOnResult;
	SystemClock.uptimeMillis() + mVibratorOnResult + CALLBACKS_EXTRA_TIMEOUT;		long nextVibratorOffTimeout = nextStartTime + CALLBACKS_EXTRA_TIMEOUT;
	return nextSteps(nextVibratorOffTimeout, nextVibratorOffTimeout, segmentsPlayed);		return nextSteps(nextStartTime, nextVibratorOffTimeout, segmentsPlayed);
	}		}

	/**		/**

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

+39 −2

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

	import com.android.internal.app.IBatteryStats;		import com.android.internal.app.IBatteryStats;

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

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

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

	@Test		@Test
	public void vibrate_noVibrator_ignoresVibration() {		public void vibrate_noVibrator_ignoresVibration() {
	mVibratorProviders.clear();		mVibratorProviders.clear();
	@@ -508,7 +517,7 @@ public class VibrationThreadTest {
	.addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f)		.addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 1f)
	.addPrimitive(VibrationEffect.Composition.PRIMITIVE_TICK, 0.5f)		.addPrimitive(VibrationEffect.Composition.PRIMITIVE_TICK, 0.5f)
	.addEffect(VibrationEffect.get(VibrationEffect.EFFECT_CLICK))		.addEffect(VibrationEffect.get(VibrationEffect.EFFECT_CLICK))
	.addEffect(VibrationEffect.get(VibrationEffect.EFFECT_CLICK), 10)		.addEffect(VibrationEffect.get(VibrationEffect.EFFECT_CLICK), /* delay= */ 100)
	.compose();		.compose();
	VibrationThread thread = startThreadAndDispatcher(vibrationId, effect);		VibrationThread thread = startThreadAndDispatcher(vibrationId, effect);
	waitForCompletion(thread);		waitForCompletion(thread);
	@@ -1290,7 +1299,9 @@ public class VibrationThreadTest {
	thread.vibratorComplete(answer.getArgument(0));		thread.vibratorComplete(answer.getArgument(0));
	return null;		return null;
	}).when(mControllerCallbacks).onComplete(anyInt(), eq(vib.id));		}).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();		thread.start();
	return thread;		return thread;
	}		}
	@@ -1316,6 +1327,7 @@ public class VibrationThreadTest {
	} catch (InterruptedException e) {		} catch (InterruptedException e) {
	}		}
	assertFalse(thread.isAlive());		assertFalse(thread.isAlive());
			mCustomTestLooperDispatcher.cancel();
	mTestLooper.dispatchAll();		mTestLooper.dispatchAll();
	}		}

	@@ -1364,4 +1376,29 @@ public class VibrationThreadTest {
	verify(mThreadCallbacks).onVibrationCompleted(eq(vibrationId), eq(expectedStatus));		verify(mThreadCallbacks).onVibrationCompleted(eq(vibrationId), eq(expectedStatus));
	verify(mThreadCallbacks).onVibratorsReleased();		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;
			}
			}
	}		}