Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fb2fc1f0 authored by Simon Bowden's avatar Simon Bowden
Browse files

Fix cancelSynced cleanup: stop the vibrator. 

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
parent a18b921f

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

+33 −30
Original line number Diff line number Diff line
@@ -192,8 +192,8 @@ 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());
@@ -202,31 +202,34 @@ final class StartSequentialEffectStep extends Step { 
            long duration = startVibrating(step, nextSteps);

            if (duration < 0) {

                // One vibrator has failed, fail this entire sync attempt.

                    return maxDuration = -1;

                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);

        if (hasPrepared && !hasFailed && maxDuration > 0) {

            hasTriggered = conductor.vibratorManagerHooks.triggerSyncedVibration(getVibration().id);

            hasFailed &= hasTriggered;

        }

            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.



        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) {

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

+41 −4
Original line number Diff line number Diff line
@@ -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);