Merge "Fix concurrent external vibrations on ExternalVibratorService" into sc-dev am: 1dd6f20e3b

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15243095

Change-Id: If55d15b631ecd8f0da0afba6d2388db9d3405f06

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

@@ -1301,7 +1301,8 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
     }
 
     /** Implementation of {@link IExternalVibratorService} to be triggered on external control. */
-    private final class ExternalVibratorService extends IExternalVibratorService.Stub {
+    @VisibleForTesting
+    final class ExternalVibratorService extends IExternalVibratorService.Stub {
         ExternalVibrationDeathRecipient mCurrentExternalDeathRecipient;
 
         @Override
@@ -1332,6 +1333,7 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
                 return vibHolder.scale;
             }
 
+            ExternalVibrationHolder cancelingExternalVibration = null;
             VibrationThread cancelingVibration = null;
             int scale;
             synchronized (mLock) {
@@ -1350,16 +1352,18 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
                         cancelingVibration = mCurrentVibration;
                     }
                 } else {
+                    // At this point we have an externally controlled vibration playing already.
+                    // Since the interface defines that only one externally controlled vibration can
+                    // play at a time, we need to first mute the ongoing vibration and then return
+                    // a scale from this function for the new one. Ee can be assured that the
+                    // ongoing it will be muted in favor of the new vibration.
+                    //
+                    // Note that this doesn't support multiple concurrent external controls, as we
+                    // would need to mute the old one still if it came from a different controller.
+                    mCurrentExternalVibration.externalVibration.mute();
                     endVibrationLocked(mCurrentExternalVibration, Vibration.Status.CANCELLED);
+                    cancelingExternalVibration = mCurrentExternalVibration;
                 }
-                // At this point we either have an externally controlled vibration playing, or
-                // no vibration playing. Since the interface defines that only one externally
-                // controlled vibration can play at a time, by returning something other than
-                // SCALE_MUTE from this function we can be assured that if we are currently
-                // playing vibration, it will be muted in favor of the new vibration.
-                //
-                // Note that this doesn't support multiple concurrent external controls, as we
-                // would need to mute the old one still if it came from a different controller.
                 mCurrentExternalVibration = new ExternalVibrationHolder(vib);
                 mCurrentExternalDeathRecipient = new ExternalVibrationDeathRecipient();
                 vib.linkToDeath(mCurrentExternalDeathRecipient);
@@ -1376,10 +1380,14 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
                             + "external control", e);
                 }
             }
-            if (DEBUG) {
+            if (cancelingExternalVibration == null) {
-                Slog.d(TAG, "Vibrator going under external control.");
+                // We only need to set external control if it was not already set by another
+                // external vibration.
+                if (DEBUG) {
+                    Slog.d(TAG, "Vibrator going under external control.");
+                }
+                setExternalControl(true);
             }
-            setExternalControl(true);
             if (DEBUG) {
                 Slog.e(TAG, "Playing external vibration: " + vib);
             }

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

@@ -46,6 +46,7 @@ final class FakeVibratorControllerProvider {
     private final List<VibrationEffectSegment> mEffectSegments = new ArrayList<>();
     private final List<Integer> mBraking = new ArrayList<>();
     private final List<Float> mAmplitudes = new ArrayList<>();
+    private final List<Boolean> mExternalControlStates = new ArrayList<>();
     private final Handler mHandler;
     private final FakeNativeWrapper mNativeWrapper;
 
@@ -139,6 +140,7 @@ final class FakeVibratorControllerProvider {
 
         @Override
         public void setExternalControl(boolean enabled) {
+            mExternalControlStates.add(enabled);
         }
 
         @Override
@@ -301,6 +303,11 @@ final class FakeVibratorControllerProvider {
         return new ArrayList<>(mEffectSegments);
     }
 
+    /** Return list of states set for external control to the fake vibrator hardware. */
+    public List<Boolean> getExternalControlStates() {
+        return mExternalControlStates;
+    }
+
     /**
      * Return the {@link PrebakedSegment} effect enabled with given id, or {@code null} if
      * missing or disabled.

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

@@ -35,6 +35,7 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import android.app.AppOpsManager;
@@ -50,8 +51,11 @@ import android.hardware.vibrator.IVibratorManager;
 import android.media.AudioAttributes;
 import android.media.AudioManager;
 import android.os.CombinedVibration;
+import android.os.ExternalVibration;
 import android.os.Handler;
 import android.os.IBinder;
+import android.os.IExternalVibrationController;
+import android.os.IExternalVibratorService;
 import android.os.IVibratorStateListener;
 import android.os.Looper;
 import android.os.PowerManager;
@@ -108,6 +112,8 @@ public class VibratorManagerServiceTest {
     private static final PowerSaveState NORMAL_POWER_STATE = new PowerSaveState.Builder().build();
     private static final PowerSaveState LOW_POWER_STATE = new PowerSaveState.Builder()
             .setBatterySaverEnabled(true).build();
+    private static final AudioAttributes AUDIO_ATTRS =
+            new AudioAttributes.Builder().setUsage(AudioAttributes.USAGE_ALARM).build();
     private static final VibrationAttributes ALARM_ATTRS =
             new VibrationAttributes.Builder().setUsage(VibrationAttributes.USAGE_ALARM).build();
     private static final VibrationAttributes HAPTIC_FEEDBACK_ATTRS =
@@ -136,6 +142,7 @@ public class VibratorManagerServiceTest {
     private TestLooper mTestLooper;
     private FakeVibrator mVibrator;
     private PowerManagerInternal.LowPowerModeListener mRegisteredPowerModeListener;
+    private VibratorManagerService.ExternalVibratorService mExternalVibratorService;
 
     @Before
     public void setUp() throws Exception {
@@ -208,6 +215,9 @@ public class VibratorManagerServiceTest {
 
                     @Override
                     void addService(String name, IBinder service) {
+                        Object serviceInstance = service;
+                        mExternalVibratorService =
+                                (VibratorManagerService.ExternalVibratorService) serviceInstance;
                     }
                 });
     }
@@ -967,6 +977,69 @@ public class VibratorManagerServiceTest {
         assertTrue(waitUntil(s -> !s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
     }
 
+    @Test
+    public void onExternalVibration_setsExternalControl() {
+        mockVibrators(1);
+        mVibratorProviders.get(1).setCapabilities(IVibrator.CAP_EXTERNAL_CONTROL);
+        createSystemReadyService();
+
+        ExternalVibration externalVibration = new ExternalVibration(UID, PACKAGE_NAME, AUDIO_ATTRS,
+                mock(IExternalVibrationController.class));
+        int scale = mExternalVibratorService.onExternalVibrationStart(externalVibration);
+        mExternalVibratorService.onExternalVibrationStop(externalVibration);
+
+        assertEquals(IExternalVibratorService.SCALE_NONE, scale);
+        assertEquals(Arrays.asList(true, false),
+                mVibratorProviders.get(1).getExternalControlStates());
+    }
+
+    @Test
+    public void onExternalVibration_withOngoingExternalVibration_mutesPreviousVibration()
+            throws Exception {
+        mockVibrators(1);
+        mVibratorProviders.get(1).setCapabilities(IVibrator.CAP_EXTERNAL_CONTROL);
+        createSystemReadyService();
+
+        IExternalVibrationController firstController = mock(IExternalVibrationController.class);
+        IExternalVibrationController secondController = mock(IExternalVibrationController.class);
+        ExternalVibration firstVibration = new ExternalVibration(UID, PACKAGE_NAME, AUDIO_ATTRS,
+                firstController);
+        int firstScale = mExternalVibratorService.onExternalVibrationStart(firstVibration);
+
+        ExternalVibration secondVibration = new ExternalVibration(UID, PACKAGE_NAME, AUDIO_ATTRS,
+                secondController);
+        int secondScale = mExternalVibratorService.onExternalVibrationStart(secondVibration);
+
+        assertEquals(IExternalVibratorService.SCALE_NONE, firstScale);
+        assertEquals(IExternalVibratorService.SCALE_NONE, secondScale);
+        verify(firstController).mute();
+        verifyNoMoreInteractions(secondController);
+        // Set external control called only once.
+        assertEquals(Arrays.asList(true), mVibratorProviders.get(1).getExternalControlStates());
+    }
+
+    @Test
+    public void onExternalVibration_withOngoingVibration_cancelsOngoingVibrationImmediately()
+            throws Exception {
+        mockVibrators(1);
+        mVibratorProviders.get(1).setCapabilities(IVibrator.CAP_EXTERNAL_CONTROL,
+                IVibrator.CAP_AMPLITUDE_CONTROL);
+        VibratorManagerService service = createSystemReadyService();
+
+        VibrationEffect effect = VibrationEffect.createOneShot(10 * TEST_TIMEOUT_MILLIS, 100);
+        vibrate(service, effect, HAPTIC_FEEDBACK_ATTRS);
+        assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
+
+        ExternalVibration externalVibration = new ExternalVibration(UID, PACKAGE_NAME, AUDIO_ATTRS,
+                mock(IExternalVibrationController.class));
+        int scale = mExternalVibratorService.onExternalVibrationStart(externalVibration);
+        assertEquals(IExternalVibratorService.SCALE_NONE, scale);
+
+        // Vibration is cancelled.
+        assertTrue(waitUntil(s -> !s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
+        assertEquals(Arrays.asList(true), mVibratorProviders.get(1).getExternalControlStates());
+    }
+
     private VibrationEffectSegment expectedPrebaked(int effectId) {
         return new PrebakedSegment(effectId, false, VibrationEffect.EFFECT_STRENGTH_MEDIUM);
     }