From 81b893734f05d01bed829f25948c826d118a627d Mon Sep 17 00:00:00 2001
From: Lais Andrade <lsandrade@google.com>
Date: Fri, 27 Aug 2021 16:35:18 +0100
Subject: [PATCH] Allow blocking transaction to IExternalVibrationController

Clear android stability warnings for calls to mute/unmute the external
vibration controlled by the audio framework via the
IExternalVibrationController implementation.

Fix: 197793087
Test: N/A
Change-Id: Ife1c8e59bfe5764316da8eb8b32fc054be7ff213
---
 core/java/android/os/ExternalVibration.java   | 29 ++++++++++++++-----
 .../vibrator/VibratorManagerService.java      |  4 +--
 .../vibrator/VibratorManagerServiceTest.java  |  3 +-
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/core/java/android/os/ExternalVibration.java b/core/java/android/os/ExternalVibration.java
index 7fd02116814b5..0686dd6894998 100644
--- a/core/java/android/os/ExternalVibration.java
+++ b/core/java/android/os/ExternalVibration.java
@@ -42,25 +42,38 @@ public class ExternalVibration implements Parcelable {
     // boundaries.
     @NonNull
     private IBinder mToken;
-
     public ExternalVibration(int uid, @NonNull String pkg, @NonNull AudioAttributes attrs,
             @NonNull IExternalVibrationController controller) {
+        this(uid, pkg, attrs, controller, new Binder());
+    }
+
+    private ExternalVibration(int uid, @NonNull String pkg, @NonNull AudioAttributes attrs,
+            @NonNull IExternalVibrationController controller, @NonNull IBinder token) {
         mUid = uid;
         mPkg = Preconditions.checkNotNull(pkg);
         mAttrs = Preconditions.checkNotNull(attrs);
         mController = Preconditions.checkNotNull(controller);
-        mToken = new Binder();
+        mToken = Preconditions.checkNotNull(token);
+
+        // IExternalVibrationController is a hidden AIDL interface with implementation provided by
+        // the audio framework to allow mute/unmute control over the external vibration.
+        //
+        // Transactions are locked in audioflinger, and should be blocking to avoid racing
+        // conditions on multiple audio playback.
+        //
+        // They can also be triggered before starting a new external vibration in
+        // IExternalVibratorService, as the ongoing external vibration needs to be muted before the
+        // new one can start, which also requires blocking calls to mute.
+        Binder.allowBlocking(mController.asBinder());
     }
 
     private ExternalVibration(Parcel in) {
-        mUid = in.readInt();
-        mPkg = in.readString();
-        mAttrs = readAudioAttributes(in);
-        mController = IExternalVibrationController.Stub.asInterface(in.readStrongBinder());
-        mToken = in.readStrongBinder();
+        this(in.readInt(), in.readString(), readAudioAttributes(in),
+                IExternalVibrationController.Stub.asInterface(in.readStrongBinder()),
+                in.readStrongBinder());
     }
 
-    private AudioAttributes readAudioAttributes(Parcel in) {
+    private static AudioAttributes readAudioAttributes(Parcel in) {
         int usage = in.readInt();
         int contentType = in.readInt();
         int capturePreset = in.readInt();
diff --git a/services/core/java/com/android/server/vibrator/VibratorManagerService.java b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
index 2a47512bb1471..e5aa72025f6e3 100644
--- a/services/core/java/com/android/server/vibrator/VibratorManagerService.java
+++ b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
@@ -1355,8 +1355,8 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
                     // 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.
+                    // a scale from this function for the new one, so we can be assured that the
+                    // ongoing 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.
diff --git a/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java
index 8f9eb22f0d01d..2c5159aef1e25 100644
--- a/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/vibrator/VibratorManagerServiceTest.java
@@ -35,7 +35,6 @@ 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;
@@ -1016,7 +1015,7 @@ public class VibratorManagerServiceTest {
         assertEquals(IExternalVibratorService.SCALE_NONE, firstScale);
         assertEquals(IExternalVibratorService.SCALE_NONE, secondScale);
         verify(firstController).mute();
-        verifyNoMoreInteractions(secondController);
+        verify(secondController, never()).mute();
         // Set external control called only once.
         assertEquals(Arrays.asList(true), mVibratorProviders.get(1).getExternalControlStates());
     }