Merge "Fix cancelSynced cleanup: stop the vibrator."

This commit is contained in:
Simon Bowden
2022-09-06 08:45:58 +00:00
committed by Android (Google) Code Review
2 changed files with 73 additions and 33 deletions

View File

@@ -192,41 +192,44 @@ final class StartSequentialEffectStep extends Step {
// delivered asynchronously but enqueued until the step processing is finished.
boolean hasPrepared = false;
boolean hasTriggered = false;
boolean hasFailed = false;
long maxDuration = 0;
try {
hasPrepared = conductor.vibratorManagerHooks.prepareSyncedVibration(
effectMapping.getRequiredSyncCapabilities(),
effectMapping.getVibratorIds());
hasPrepared = conductor.vibratorManagerHooks.prepareSyncedVibration(
effectMapping.getRequiredSyncCapabilities(),
effectMapping.getVibratorIds());
for (AbstractVibratorStep step : steps) {
long duration = startVibrating(step, nextSteps);
if (duration < 0) {
// One vibrator has failed, fail this entire sync attempt.
return maxDuration = -1;
}
maxDuration = Math.max(maxDuration, duration);
for (AbstractVibratorStep step : steps) {
long duration = startVibrating(step, nextSteps);
if (duration < 0) {
// One vibrator has failed, fail this entire sync attempt.
hasFailed = true;
break;
}
maxDuration = Math.max(maxDuration, duration);
}
// Check if sync was prepared and if any step was accepted by a vibrator,
// otherwise there is nothing to trigger here.
if (hasPrepared && maxDuration > 0) {
hasTriggered = conductor.vibratorManagerHooks.triggerSyncedVibration(
getVibration().id);
}
return maxDuration;
} finally {
if (hasPrepared && !hasTriggered) {
// Trigger has failed or all steps were ignored by the vibrators.
conductor.vibratorManagerHooks.cancelSyncedVibration();
nextSteps.clear();
} else if (maxDuration < 0) {
// Some vibrator failed without being prepared so other vibrators might be
// active. Cancel and remove every pending step from output list.
for (int i = nextSteps.size() - 1; i >= 0; i--) {
nextSteps.remove(i).cancelImmediately();
}
// Check if sync was prepared and if any step was accepted by a vibrator,
// otherwise there is nothing to trigger here.
if (hasPrepared && !hasFailed && maxDuration > 0) {
hasTriggered = conductor.vibratorManagerHooks.triggerSyncedVibration(getVibration().id);
hasFailed &= hasTriggered;
}
if (hasFailed) {
// Something failed, possibly after other vibrators were activated.
// Cancel and remove every pending step from output list.
for (int i = nextSteps.size() - 1; i >= 0; i--) {
nextSteps.remove(i).cancelImmediately();
}
}
// Cancel the preparation if trigger failed or all
if (hasPrepared && !hasTriggered) {
// Trigger has failed or was skipped, so abort the synced vibration.
conductor.vibratorManagerHooks.cancelSyncedVibration();
}
return hasFailed ? -1 : maxDuration;
}
private long startVibrating(AbstractVibratorStep step, List<Step> nextSteps) {

View File

@@ -121,6 +121,10 @@ import java.util.function.Predicate;
public class VibratorManagerServiceTest {
private static final int TEST_TIMEOUT_MILLIS = 1_000;
// Time to allow for a cancellation to complete (notably including system ramp down), but not so
// long that tests easily get really slow or flaky. If a vibration is close to this, it should
// be cancelled in the body of the individual test.
private static final int CLEANUP_TIMEOUT_MILLIS = 100;
private static final int UID = Process.ROOT_UID;
private static final int VIRTUAL_DISPLAY_ID = 1;
private static final String PACKAGE_NAME = "package";
@@ -166,6 +170,7 @@ public class VibratorManagerServiceTest {
private final Map<Integer, FakeVibratorControllerProvider> mVibratorProviders = new HashMap<>();
private VibratorManagerService mService;
private Context mContextSpy;
private TestLooper mTestLooper;
private FakeVibrator mVibrator;
@@ -230,8 +235,27 @@ public class VibratorManagerServiceTest {
@After
public void tearDown() throws Exception {
if (mService != null) {
// Wait until all vibrators have stopped vibrating, with a bit of flexibility for tests
// that just do a click or have cancelled at the end (waiting for ramp-down).
//
// Note: if a test is flaky here, check whether a VibrationEffect duration is close to
// CLEANUP_TIMEOUT_MILLIS - in which case it's probably best to just cancel that effect
// explicitly at the end of the test case (rather than letting it run and race flakily).
assertTrue(waitUntil(s -> {
for (int vibratorId : mService.getVibratorIds()) {
if (s.isVibrating(vibratorId)) {
return false;
}
}
return true;
}, mService, CLEANUP_TIMEOUT_MILLIS));
}
LocalServices.removeServiceForTest(PackageManagerInternal.class);
LocalServices.removeServiceForTest(PowerManagerInternal.class);
// Ignore potential exceptions about the looper having never dispatched any messages.
mTestLooper.stopAutoDispatchAndIgnoreExceptions();
}
private VibratorManagerService createSystemReadyService() {
@@ -241,7 +265,7 @@ public class VibratorManagerServiceTest {
}
private VibratorManagerService createService() {
return new VibratorManagerService(
mService = new VibratorManagerService(
mContextSpy,
new VibratorManagerService.Injector() {
@Override
@@ -278,6 +302,7 @@ public class VibratorManagerServiceTest {
(VibratorManagerService.ExternalVibratorService) serviceInstance;
}
});
return mService;
}
@Test
@@ -478,6 +503,7 @@ public class VibratorManagerServiceTest {
verify(listeners[0]).onVibrating(eq(true));
verify(listeners[1]).onVibrating(eq(true));
verify(listeners[2], never()).onVibrating(eq(true));
cancelVibrate(service);
}
@Test
@@ -804,6 +830,7 @@ public class VibratorManagerServiceTest {
assertFalse(
mVibratorProviders.get(1).getAllEffectSegments().stream()
.anyMatch(segment -> segment instanceof PrebakedSegment));
cancelVibrate(service); // Clean up repeating effect.
}
@Test
@@ -830,6 +857,8 @@ public class VibratorManagerServiceTest {
// The second vibration should have recorded that the vibrators were turned on.
verify(mBatteryStatsMock, times(2)).noteVibratorOn(anyInt(), anyLong());
cancelVibrate(service); // Clean up repeating effect.
}
@Test
@@ -855,6 +884,8 @@ public class VibratorManagerServiceTest {
assertFalse(
mVibratorProviders.get(1).getAllEffectSegments().stream()
.anyMatch(segment -> segment instanceof PrebakedSegment));
cancelVibrate(service); // Clean up long effect.
}
@Test
@@ -1194,10 +1225,11 @@ public class VibratorManagerServiceTest {
// Vibration is not stopped nearly after updating service.
assertFalse(waitUntil(s -> !s.isVibrating(1), service, 50));
cancelVibrate(service);
}
@Test
public void vibrate_withVitualDisplayChange_ignoreVibrationFromVirtualDisplay()
public void vibrate_withVirtualDisplayChange_ignoreVibrationFromVirtualDisplay()
throws Exception {
mockVibrators(1);
VibratorManagerService service = createSystemReadyService();
@@ -1223,10 +1255,11 @@ public class VibratorManagerServiceTest {
// Haptic feedback played normally when the virtual display is removed.
assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
cancelVibrate(service); // Clean up long-ish effect.
}
@Test
public void vibrate_withAppsOnVitualDisplayChange_ignoreVibrationFromVirtualDisplay()
public void vibrate_withAppsOnVirtualDisplayChange_ignoreVibrationFromVirtualDisplay()
throws Exception {
mockVibrators(1);
VibratorManagerService service = createSystemReadyService();
@@ -1251,7 +1284,7 @@ public class VibratorManagerServiceTest {
HAPTIC_FEEDBACK_ATTRS);
// Haptic feedback played normally when the same app no long runs on a virtual display.
assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
cancelVibrate(service); // Clean up long-ish effect.
}
@Test
@@ -1935,6 +1968,10 @@ public class VibratorManagerServiceTest {
when(mNativeWrapperMock.getVibratorIds()).thenReturn(vibratorIds);
}
private void cancelVibrate(VibratorManagerService service) {
service.cancelVibrate(VibrationAttributes.USAGE_FILTER_MATCH_ALL, service);
}
private IVibratorStateListener mockVibratorStateListener() {
IVibratorStateListener listenerMock = mock(IVibratorStateListener.class);
IBinder binderMock = mock(IBinder.class);