diff --git a/core/java/android/os/SystemVibrator.java b/core/java/android/os/SystemVibrator.java index 0aafaf456a083..6091bf9088d3a 100644 --- a/core/java/android/os/SystemVibrator.java +++ b/core/java/android/os/SystemVibrator.java @@ -50,10 +50,10 @@ public class SystemVibrator extends Vibrator { private final Context mContext; @GuardedBy("mBrokenListeners") - private final ArrayList mBrokenListeners = new ArrayList<>(); + private final ArrayList mBrokenListeners = new ArrayList<>(); @GuardedBy("mRegisteredListeners") - private final ArrayMap + private final ArrayMap mRegisteredListeners = new ArrayMap<>(); private final Object mLock = new Object(); @@ -147,7 +147,7 @@ public class SystemVibrator extends Vibrator { Log.w(TAG, "Failed to add vibrate state listener; no vibrator manager."); return; } - AllVibratorsStateListener delegate = null; + MultiVibratorStateListener delegate = null; try { synchronized (mRegisteredListeners) { // If listener is already registered, reject and return. @@ -155,7 +155,7 @@ public class SystemVibrator extends Vibrator { Log.w(TAG, "Listener already registered."); return; } - delegate = new AllVibratorsStateListener(executor, listener); + delegate = new MultiVibratorStateListener(executor, listener); delegate.register(mVibratorManager); mRegisteredListeners.put(listener, delegate); delegate = null; @@ -181,7 +181,7 @@ public class SystemVibrator extends Vibrator { } synchronized (mRegisteredListeners) { if (mRegisteredListeners.containsKey(listener)) { - AllVibratorsStateListener delegate = mRegisteredListeners.get(listener); + MultiVibratorStateListener delegate = mRegisteredListeners.get(listener); delegate.unregister(mVibratorManager); mRegisteredListeners.remove(listener); } @@ -238,7 +238,7 @@ public class SystemVibrator extends Vibrator { * Tries to unregister individual {@link android.os.Vibrator.OnVibratorStateChangedListener} * that were left registered to vibrators after failures to register them to all vibrators. * - *

This might happen if {@link AllVibratorsStateListener} fails to register to any vibrator + *

This might happen if {@link MultiVibratorStateListener} fails to register to any vibrator * and also fails to unregister any previously registered single listeners to other vibrators. * *

This method never throws {@link RuntimeException} if it fails to unregister again, it will @@ -259,10 +259,10 @@ public class SystemVibrator extends Vibrator { /** Listener for a single vibrator state change. */ private static class SingleVibratorStateListener implements OnVibratorStateChangedListener { - private final AllVibratorsStateListener mAllVibratorsListener; + private final MultiVibratorStateListener mAllVibratorsListener; private final int mVibratorIdx; - SingleVibratorStateListener(AllVibratorsStateListener listener, int vibratorIdx) { + SingleVibratorStateListener(MultiVibratorStateListener listener, int vibratorIdx) { mAllVibratorsListener = listener; mVibratorIdx = vibratorIdx; } @@ -552,8 +552,16 @@ public class SystemVibrator extends Vibrator { } } - /** Listener for all vibrators state change. */ - private static class AllVibratorsStateListener { + /** + * Listener for all vibrators state change. + * + *

This registers a listener to all vibrators to merge the callbacks into a single state + * that is set to true if any individual vibrator is also true, and false otherwise. + * + * @hide + */ + @VisibleForTesting + public static class MultiVibratorStateListener { private final Object mLock = new Object(); private final Executor mExecutor; private final OnVibratorStateChangedListener mDelegate; @@ -567,19 +575,21 @@ public class SystemVibrator extends Vibrator { @GuardedBy("mLock") private int mVibratingMask; - AllVibratorsStateListener(@NonNull Executor executor, + public MultiVibratorStateListener(@NonNull Executor executor, @NonNull OnVibratorStateChangedListener listener) { mExecutor = executor; mDelegate = listener; } - boolean hasRegisteredListeners() { + /** Returns true if at least one listener was registered to an individual vibrator. */ + public boolean hasRegisteredListeners() { synchronized (mLock) { return mVibratorListeners.size() > 0; } } - void register(VibratorManager vibratorManager) { + /** Registers a listener to all individual vibrators in {@link VibratorManager}. */ + public void register(VibratorManager vibratorManager) { int[] vibratorIds = vibratorManager.getVibratorIds(); synchronized (mLock) { for (int i = 0; i < vibratorIds.length; i++) { @@ -603,7 +613,8 @@ public class SystemVibrator extends Vibrator { } } - void unregister(VibratorManager vibratorManager) { + /** Unregisters the listeners from all individual vibrators in {@link VibratorManager}. */ + public void unregister(VibratorManager vibratorManager) { synchronized (mLock) { for (int i = mVibratorListeners.size(); --i >= 0; ) { int vibratorId = mVibratorListeners.keyAt(i); @@ -614,30 +625,44 @@ public class SystemVibrator extends Vibrator { } } - void onVibrating(int vibratorIdx, boolean vibrating) { + /** Callback triggered by {@link SingleVibratorStateListener} for each vibrator. */ + public void onVibrating(int vibratorIdx, boolean vibrating) { mExecutor.execute(() -> { - boolean anyVibrating; + boolean shouldNotifyStateChange; + boolean isAnyVibrating; synchronized (mLock) { + // Bitmask indicating that all vibrators have been initialized. int allInitializedMask = (1 << mVibratorListeners.size()) - 1; - int vibratorMask = 1 << vibratorIdx; - if ((mInitializedMask & vibratorMask) == 0) { - // First state report for this vibrator, set vibrating initial value. - mInitializedMask |= vibratorMask; - mVibratingMask |= vibrating ? vibratorMask : 0; - } else { - // Flip vibrating value, if changed. - boolean prevVibrating = (mVibratingMask & vibratorMask) != 0; - if (prevVibrating != vibrating) { - mVibratingMask ^= vibratorMask; - } + + // Save current global state before processing this vibrator state change. + boolean previousIsAnyVibrating = (mVibratingMask != 0); + boolean previousAreAllInitialized = (mInitializedMask == allInitializedMask); + + // Mark this vibrator as initialized. + int vibratorMask = (1 << vibratorIdx); + mInitializedMask |= vibratorMask; + + // Flip the vibrating bit flag for this vibrator, only if the state is changing. + boolean previousVibrating = (mVibratingMask & vibratorMask) != 0; + if (previousVibrating != vibrating) { + mVibratingMask ^= vibratorMask; } - if (mInitializedMask != allInitializedMask) { - // Wait for all vibrators initial state to be reported before delegating. - return; - } - anyVibrating = mVibratingMask != 0; + + // Check new global state after processing this vibrator state change. + isAnyVibrating = (mVibratingMask != 0); + boolean areAllInitialized = (mInitializedMask == allInitializedMask); + + // Prevent multiple triggers with the same state. + // Trigger once when all vibrators have reported their state, and then only when + // the merged vibrating state changes. + boolean isStateChanging = (previousIsAnyVibrating != isAnyVibrating); + shouldNotifyStateChange = + areAllInitialized && (!previousAreAllInitialized || isStateChanging); + } + // Notify delegate listener outside the lock, only if merged state is changing. + if (shouldNotifyStateChange) { + mDelegate.onVibratorStateChanged(isAnyVibrating); } - mDelegate.onVibratorStateChanged(anyVibrating); }); } } diff --git a/core/tests/coretests/src/android/os/VibratorTest.java b/core/tests/coretests/src/android/os/VibratorTest.java index 7a66befad1a1e..7ebebc965d8d8 100644 --- a/core/tests/coretests/src/android/os/VibratorTest.java +++ b/core/tests/coretests/src/android/os/VibratorTest.java @@ -21,12 +21,17 @@ import static junit.framework.Assert.assertTrue; import static junit.framework.TestCase.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import android.content.ContentResolver; @@ -34,6 +39,7 @@ import android.content.Context; import android.content.ContextWrapper; import android.hardware.vibrator.IVibrator; import android.media.AudioAttributes; +import android.os.test.TestLooper; import android.platform.test.annotations.Presubmit; import androidx.test.InstrumentationRegistry; @@ -46,6 +52,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.junit.MockitoJUnitRunner; /** @@ -65,6 +72,7 @@ public class VibratorTest { private Context mContextSpy; private Vibrator mVibratorSpy; + private TestLooper mTestLooper; @Before public void setUp() { @@ -73,6 +81,7 @@ public class VibratorTest { ContentResolver contentResolver = mSettingsProviderRule.mockContentResolver(mContextSpy); when(mContextSpy.getContentResolver()).thenReturn(contentResolver); mVibratorSpy = spy(new SystemVibrator(mContextSpy)); + mTestLooper = new TestLooper(); } @Test @@ -394,6 +403,108 @@ public class VibratorTest { info.getFrequencyProfile()); } + @Test + public void onVibratorStateChanged_noVibrator_registersNoListenerToVibratorManager() { + VibratorManager mockVibratorManager = mock(VibratorManager.class); + when(mockVibratorManager.getVibratorIds()).thenReturn(new int[0]); + + Vibrator.OnVibratorStateChangedListener mockListener = + mock(Vibrator.OnVibratorStateChangedListener.class); + SystemVibrator.MultiVibratorStateListener multiVibratorListener = + new SystemVibrator.MultiVibratorStateListener( + mTestLooper.getNewExecutor(), mockListener); + + multiVibratorListener.register(mockVibratorManager); + + // Never tries to register a listener to an individual vibrator. + assertFalse(multiVibratorListener.hasRegisteredListeners()); + verify(mockVibratorManager, never()).getVibrator(anyInt()); + } + + @Test + public void onVibratorStateChanged_singleVibrator_forwardsAllCallbacks() { + VibratorManager mockVibratorManager = mock(VibratorManager.class); + when(mockVibratorManager.getVibratorIds()).thenReturn(new int[] { 1 }); + when(mockVibratorManager.getVibrator(anyInt())).thenReturn(NullVibrator.getInstance()); + + Vibrator.OnVibratorStateChangedListener mockListener = + mock(Vibrator.OnVibratorStateChangedListener.class); + SystemVibrator.MultiVibratorStateListener multiVibratorListener = + new SystemVibrator.MultiVibratorStateListener( + mTestLooper.getNewExecutor(), mockListener); + + multiVibratorListener.register(mockVibratorManager); + assertTrue(multiVibratorListener.hasRegisteredListeners()); + + multiVibratorListener.onVibrating(/* vibratorIdx= */ 0, /* vibrating= */ false); + multiVibratorListener.onVibrating(/* vibratorIdx= */ 0, /* vibrating= */ true); + multiVibratorListener.onVibrating(/* vibratorIdx= */ 0, /* vibrating= */ false); + + mTestLooper.dispatchAll(); + + InOrder inOrder = inOrder(mockListener); + inOrder.verify(mockListener).onVibratorStateChanged(eq(false)); + inOrder.verify(mockListener).onVibratorStateChanged(eq(true)); + inOrder.verify(mockListener).onVibratorStateChanged(eq(false)); + inOrder.verifyNoMoreInteractions(); + } + + @Test + public void onVibratorStateChanged_multipleVibrators_triggersOnlyWhenAllVibratorsInitialized() { + VibratorManager mockVibratorManager = mock(VibratorManager.class); + when(mockVibratorManager.getVibratorIds()).thenReturn(new int[] { 1, 2 }); + when(mockVibratorManager.getVibrator(anyInt())).thenReturn(NullVibrator.getInstance()); + + Vibrator.OnVibratorStateChangedListener mockListener = + mock(Vibrator.OnVibratorStateChangedListener.class); + SystemVibrator.MultiVibratorStateListener multiVibratorListener = + new SystemVibrator.MultiVibratorStateListener( + mTestLooper.getNewExecutor(), mockListener); + + multiVibratorListener.register(mockVibratorManager); + assertTrue(multiVibratorListener.hasRegisteredListeners()); + + multiVibratorListener.onVibrating(/* vibratorIdx= */ 0, /* vibrating= */ false); + mTestLooper.dispatchAll(); + verify(mockListener, never()).onVibratorStateChanged(anyBoolean()); + + multiVibratorListener.onVibrating(/* vibratorIdx= */ 1, /* vibrating= */ false); + mTestLooper.dispatchAll(); + verify(mockListener).onVibratorStateChanged(eq(false)); + verifyNoMoreInteractions(mockListener); + } + + @Test + public void onVibratorStateChanged_multipleVibrators_stateChangeIsDeduped() { + VibratorManager mockVibratorManager = mock(VibratorManager.class); + when(mockVibratorManager.getVibratorIds()).thenReturn(new int[] { 1, 2 }); + when(mockVibratorManager.getVibrator(anyInt())).thenReturn(NullVibrator.getInstance()); + + Vibrator.OnVibratorStateChangedListener mockListener = + mock(Vibrator.OnVibratorStateChangedListener.class); + SystemVibrator.MultiVibratorStateListener multiVibratorListener = + new SystemVibrator.MultiVibratorStateListener( + mTestLooper.getNewExecutor(), mockListener); + + multiVibratorListener.register(mockVibratorManager); + assertTrue(multiVibratorListener.hasRegisteredListeners()); + + multiVibratorListener.onVibrating(/* vibratorIdx= */ 0, /* vibrating= */ false); // none + multiVibratorListener.onVibrating(/* vibratorIdx= */ 1, /* vibrating= */ false); // false + multiVibratorListener.onVibrating(/* vibratorIdx= */ 0, /* vibrating= */ true); // true + multiVibratorListener.onVibrating(/* vibratorIdx= */ 1, /* vibrating= */ true); // true + multiVibratorListener.onVibrating(/* vibratorIdx= */ 0, /* vibrating= */ false); // true + multiVibratorListener.onVibrating(/* vibratorIdx= */ 1, /* vibrating= */ false); // false + + mTestLooper.dispatchAll(); + + InOrder inOrder = inOrder(mockListener); + inOrder.verify(mockListener).onVibratorStateChanged(eq(false)); + inOrder.verify(mockListener).onVibratorStateChanged(eq(true)); + inOrder.verify(mockListener).onVibratorStateChanged(eq(false)); + inOrder.verifyNoMoreInteractions(); + } + @Test public void vibrate_withVibrationAttributes_usesGivenAttributes() { VibrationEffect effect = VibrationEffect.get(VibrationEffect.EFFECT_CLICK);