Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix cancelSynced cleanup: stop the vibrator.

Browse Source 
Currently, the VibratorController is not updated when a synced vibration
is cancelled. Consequently, it stays "isVibrating" when the vibration is
finished. This is fixed by doing the same steps cancellation of individual
steps failure for the triggerSynced failure too.

Add new tests to ensure that vibrators have stopped vibrating, and to
stop the looper thread (this is how the issue was found).

Bug: 244428021
Test: atest
Change-Id: I67ffeb170ce28e1f65227079185f1cda75636a14
This commit is contained in:
Simon Bowden
2022-08-31 14:23:32 +00:00
parent a18b921f3b
commit fb2fc1f0c7
2 changed files with 73 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
services/core/java/com/android/server/vibrator/StartSequentialEffectStep.java
View File

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

45
services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java
View File

@@ -121,6 +121,10 @@ import java.util.function.Predicate;
public class VibratorManagerServiceTest { public class VibratorManagerServiceTest {
private static final int TEST_TIMEOUT_MILLIS = 1_000; 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 UID = Process.ROOT_UID;
private static final int VIRTUAL_DISPLAY_ID = 1; private static final int VIRTUAL_DISPLAY_ID = 1;
private static final String PACKAGE_NAME = "package"; private static final String PACKAGE_NAME = "package";
@@ -166,6 +170,7 @@ public class VibratorManagerServiceTest {
private final Map<Integer, FakeVibratorControllerProvider> mVibratorProviders = new HashMap<>(); private final Map<Integer, FakeVibratorControllerProvider> mVibratorProviders = new HashMap<>();
private VibratorManagerService mService;
private Context mContextSpy; private Context mContextSpy;
private TestLooper mTestLooper; private TestLooper mTestLooper;
private FakeVibrator mVibrator; private FakeVibrator mVibrator;
@@ -230,8 +235,27 @@ public class VibratorManagerServiceTest {
@After @After
public void tearDown() throws Exception { 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(PackageManagerInternal.class);
LocalServices.removeServiceForTest(PowerManagerInternal.class); LocalServices.removeServiceForTest(PowerManagerInternal.class);
// Ignore potential exceptions about the looper having never dispatched any messages.
mTestLooper.stopAutoDispatchAndIgnoreExceptions();
} }
private VibratorManagerService createSystemReadyService() { private VibratorManagerService createSystemReadyService() {
@@ -241,7 +265,7 @@ public class VibratorManagerServiceTest {
} }
private VibratorManagerService createService() { private VibratorManagerService createService() {
return new VibratorManagerService( mService = new VibratorManagerService(
mContextSpy, mContextSpy,
new VibratorManagerService.Injector() { new VibratorManagerService.Injector() {
@Override @Override
@@ -278,6 +302,7 @@ public class VibratorManagerServiceTest {
(VibratorManagerService.ExternalVibratorService) serviceInstance; (VibratorManagerService.ExternalVibratorService) serviceInstance;
} }
}); });
return mService;
} }
@Test @Test
@@ -478,6 +503,7 @@ public class VibratorManagerServiceTest {
verify(listeners[0]).onVibrating(eq(true)); verify(listeners[0]).onVibrating(eq(true));
verify(listeners[1]).onVibrating(eq(true)); verify(listeners[1]).onVibrating(eq(true));
verify(listeners[2], never()).onVibrating(eq(true)); verify(listeners[2], never()).onVibrating(eq(true));
cancelVibrate(service);
} }
@Test @Test
@@ -804,6 +830,7 @@ public class VibratorManagerServiceTest {
assertFalse( assertFalse(
mVibratorProviders.get(1).getAllEffectSegments().stream() mVibratorProviders.get(1).getAllEffectSegments().stream()
.anyMatch(segment -> segment instanceof PrebakedSegment)); .anyMatch(segment -> segment instanceof PrebakedSegment));
cancelVibrate(service); // Clean up repeating effect.
} }
@Test @Test
@@ -830,6 +857,8 @@ public class VibratorManagerServiceTest {
// The second vibration should have recorded that the vibrators were turned on. // The second vibration should have recorded that the vibrators were turned on.
verify(mBatteryStatsMock, times(2)).noteVibratorOn(anyInt(), anyLong()); verify(mBatteryStatsMock, times(2)).noteVibratorOn(anyInt(), anyLong());
cancelVibrate(service); // Clean up repeating effect.
} }
@Test @Test
@@ -855,6 +884,8 @@ public class VibratorManagerServiceTest {
assertFalse( assertFalse(
mVibratorProviders.get(1).getAllEffectSegments().stream() mVibratorProviders.get(1).getAllEffectSegments().stream()
.anyMatch(segment -> segment instanceof PrebakedSegment)); .anyMatch(segment -> segment instanceof PrebakedSegment));
cancelVibrate(service); // Clean up long effect.
} }
@Test @Test
@@ -1194,10 +1225,11 @@ public class VibratorManagerServiceTest {
// Vibration is not stopped nearly after updating service. // Vibration is not stopped nearly after updating service.
assertFalse(waitUntil(s -> !s.isVibrating(1), service, 50)); assertFalse(waitUntil(s -> !s.isVibrating(1), service, 50));
cancelVibrate(service);
} }
@Test @Test
public void vibrate_withVitualDisplayChange_ignoreVibrationFromVirtualDisplay() public void vibrate_withVirtualDisplayChange_ignoreVibrationFromVirtualDisplay()
throws Exception { throws Exception {
mockVibrators(1); mockVibrators(1);
VibratorManagerService service = createSystemReadyService(); VibratorManagerService service = createSystemReadyService();
@@ -1223,10 +1255,11 @@ public class VibratorManagerServiceTest {
// Haptic feedback played normally when the virtual display is removed. // Haptic feedback played normally when the virtual display is removed.
assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS)); assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
cancelVibrate(service); // Clean up long-ish effect.
} }
@Test @Test
public void vibrate_withAppsOnVitualDisplayChange_ignoreVibrationFromVirtualDisplay() public void vibrate_withAppsOnVirtualDisplayChange_ignoreVibrationFromVirtualDisplay()
throws Exception { throws Exception {
mockVibrators(1); mockVibrators(1);
VibratorManagerService service = createSystemReadyService(); VibratorManagerService service = createSystemReadyService();
@@ -1251,7 +1284,7 @@ public class VibratorManagerServiceTest {
HAPTIC_FEEDBACK_ATTRS); HAPTIC_FEEDBACK_ATTRS);
// Haptic feedback played normally when the same app no long runs on a virtual display. // 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)); assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
cancelVibrate(service); // Clean up long-ish effect.
} }
@Test @Test
@@ -1935,6 +1968,10 @@ public class VibratorManagerServiceTest {
when(mNativeWrapperMock.getVibratorIds()).thenReturn(vibratorIds); when(mNativeWrapperMock.getVibratorIds()).thenReturn(vibratorIds);
} }
private void cancelVibrate(VibratorManagerService service) {
service.cancelVibrate(VibrationAttributes.USAGE_FILTER_MATCH_ALL, service);
}
private IVibratorStateListener mockVibratorStateListener() { private IVibratorStateListener mockVibratorStateListener() {
IVibratorStateListener listenerMock = mock(IVibratorStateListener.class); IVibratorStateListener listenerMock = mock(IVibratorStateListener.class);
IBinder binderMock = mock(IBinder.class); IBinder binderMock = mock(IBinder.class);