Merge "Unify current session in vibrator service" into main
diff --git a/core/proto/android/server/vibrator/vibratormanagerservice.proto b/core/proto/android/server/vibrator/vibratormanagerservice.proto
index 258832e..8a9f5f3 100644
--- a/core/proto/android/server/vibrator/vibratormanagerservice.proto
+++ b/core/proto/android/server/vibrator/vibratormanagerservice.proto
@@ -158,7 +158,6 @@
     repeated int32 vibrator_ids = 1;
     optional VibrationProto current_vibration = 2;
     optional int32 is_vibrator_controller_registered = 27;
-    optional VibrationProto current_external_vibration = 4;
     optional bool low_power_mode = 6;
     optional bool vibrate_on = 24;
     reserved 25; // prev keyboard_vibration_on
@@ -183,4 +182,5 @@
     reserved 17; // prev previous_external_vibrations
     reserved 3; // prev is_vibrating, check current_vibration instead
     reserved 5; // prev vibrator_under_external_control, check current_external_vibration instead
+    reserved 4; // prev current_external_vibration, check current_vibration instead
 }
\ No newline at end of file
diff --git a/services/core/java/com/android/server/vibrator/ExternalVibrationSession.java b/services/core/java/com/android/server/vibrator/ExternalVibrationSession.java
index e650c52..df44e50 100644
--- a/services/core/java/com/android/server/vibrator/ExternalVibrationSession.java
+++ b/services/core/java/com/android/server/vibrator/ExternalVibrationSession.java
@@ -24,8 +24,8 @@
 import android.os.IBinder;
 import android.os.VibrationAttributes;
 import android.os.vibrator.Flags;
+import android.util.Slog;
 
-import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.FrameworkStatsLog;
 
 /**
@@ -33,22 +33,31 @@
  */
 final class ExternalVibrationSession extends Vibration
         implements VibrationSession, IBinder.DeathRecipient {
+    private static final String TAG = "ExternalVibrationSession";
 
-    private final Object mLock = new Object();
+    /** Calls into VibratorManager functionality needed for playing an {@link ExternalVibration}. */
+    interface VibratorManagerHooks {
+
+        /**
+         * Tells the manager that the external vibration is finished and the vibrators can now be
+         * used for another vibration.
+         */
+        void onExternalVibrationReleased(long vibrationId);
+    }
+
     private final ExternalVibration mExternalVibration;
     private final ExternalVibrationScale mScale = new ExternalVibrationScale();
+    private final VibratorManagerHooks mManagerHooks;
 
-    @GuardedBy("mLock")
-    @Nullable
-    private Runnable mBinderDeathCallback;
-
-    ExternalVibrationSession(ExternalVibration externalVibration) {
+    ExternalVibrationSession(ExternalVibration externalVibration,
+            VibratorManagerHooks managerHooks) {
         super(new CallerInfo(
                 externalVibration.getVibrationAttributes(), externalVibration.getUid(),
                 // TODO(b/249785241): Find a way to link ExternalVibration to a VirtualDevice
                 // instead of using DEVICE_ID_INVALID here and relying on the UID checks.
                 Context.DEVICE_ID_INVALID, externalVibration.getPackage(), null));
         mExternalVibration = externalVibration;
+        mManagerHooks = managerHooks;
     }
 
     public ExternalVibrationScale getScale() {
@@ -94,10 +103,7 @@
     }
 
     @Override
-    public boolean linkToDeath(Runnable callback) {
-        synchronized (mLock) {
-            mBinderDeathCallback = callback;
-        }
+    public boolean linkToDeath() {
         mExternalVibration.linkToDeath(this);
         return true;
     }
@@ -105,39 +111,33 @@
     @Override
     public void unlinkToDeath() {
         mExternalVibration.unlinkToDeath(this);
-        synchronized (mLock) {
-            mBinderDeathCallback = null;
-        }
     }
 
     @Override
     public void binderDied() {
-        Runnable callback;
-        synchronized (mLock) {
-            callback = mBinderDeathCallback;
-        }
-        if (callback != null) {
-            callback.run();
-        }
+        Slog.d(TAG, "Binder died, cancelling external vibration...");
+        requestEnd(Status.CANCELLED_BINDER_DIED);
     }
 
     @Override
     void end(EndInfo endInfo) {
         super.end(endInfo);
         if (stats.hasStarted()) {
+            // Notify external client that this vibration should stop sending data to the vibrator.
+            mExternalVibration.mute();
             // External vibration doesn't have feedback from total time the vibrator was playing
             // with non-zero amplitude, so we use the duration between start and end times of
             // the vibration as the time the vibrator was ON, since the haptic channels are
             // open for this duration and can receive vibration waveform data.
             stats.reportVibratorOn(stats.getEndUptimeMillis() - stats.getStartUptimeMillis());
+            // Notify the manager that external client has released the vibrator control.
+            mManagerHooks.onExternalVibrationReleased(id);
         }
     }
 
     @Override
     public void requestEnd(@NonNull Status status, @Nullable CallerInfo endedBy,
             boolean immediate) {
-        // Notify external client that this vibration should stop sending data to the vibrator.
-        mExternalVibration.mute();
         end(new EndInfo(status, endedBy));
     }
 
@@ -170,4 +170,14 @@
         mScale.adaptiveHapticsScale = scaler.getAdaptiveHapticsScale(usage);
         stats.reportAdaptiveScale(mScale.adaptiveHapticsScale);
     }
+
+    @Override
+    public String toString() {
+        return "ExternalVibrationSession{"
+                + "id=" + id
+                + ", callerInfo=" + callerInfo
+                + ", externalVibration=" + mExternalVibration
+                + ", scale=" + mScale
+                + '}';
+    }
 }
diff --git a/services/core/java/com/android/server/vibrator/SingleVibrationSession.java b/services/core/java/com/android/server/vibrator/SingleVibrationSession.java
index f80407d..67ba25f 100644
--- a/services/core/java/com/android/server/vibrator/SingleVibrationSession.java
+++ b/services/core/java/com/android/server/vibrator/SingleVibrationSession.java
@@ -41,10 +41,6 @@
     @GuardedBy("mLock")
     private VibrationStepConductor mConductor;
 
-    @GuardedBy("mLock")
-    @Nullable
-    private Runnable mBinderDeathCallback;
-
     SingleVibrationSession(@NonNull IBinder callerToken, @NonNull CallerInfo callerInfo,
             @NonNull CombinedVibration vibration) {
         mCallerToken = callerToken;
@@ -100,20 +96,10 @@
     public void binderDied() {
         Slog.d(TAG, "Binder died, cancelling vibration...");
         requestEnd(Status.CANCELLED_BINDER_DIED, /* endedBy= */ null, /* immediate= */ false);
-        Runnable callback;
-        synchronized (mLock) {
-            callback = mBinderDeathCallback;
-        }
-        if (callback != null) {
-            callback.run();
-        }
     }
 
     @Override
-    public boolean linkToDeath(@Nullable Runnable callback) {
-        synchronized (mLock) {
-            mBinderDeathCallback = callback;
-        }
+    public boolean linkToDeath() {
         try {
             mCallerToken.linkToDeath(this, 0);
         } catch (RemoteException e) {
@@ -130,9 +116,6 @@
         } catch (NoSuchElementException e) {
             Slog.wtf(TAG, "Failed to unlink vibration to token death", e);
         }
-        synchronized (mLock) {
-            mBinderDeathCallback = null;
-        }
     }
 
     @Override
@@ -170,4 +153,12 @@
             }
         }
     }
+
+    @Override
+    public String toString() {
+        return "SingleVibrationSession{"
+                + "callerToken= " + mCallerToken
+                + ", vibration=" + mVibration
+                + '}';
+    }
 }
diff --git a/services/core/java/com/android/server/vibrator/VibrationSession.java b/services/core/java/com/android/server/vibrator/VibrationSession.java
index 4de8f78..b511ba8 100644
--- a/services/core/java/com/android/server/vibrator/VibrationSession.java
+++ b/services/core/java/com/android/server/vibrator/VibrationSession.java
@@ -54,15 +54,8 @@
     /** Returns debug data for logging and metric reports. */
     DebugInfo getDebugInfo();
 
-    /**
-     * Links this session to the app process death with given callback to handle it.
-     *
-     * <p>This can be used by the service to end the vibration session when the app process dies.
-     *
-     * @param callback The service callback to be triggered when the binder dies
-     * @return true if the link was successful, false otherwise
-     */
-    boolean linkToDeath(@Nullable Runnable callback);
+    /** Links this session to the app process death, returning false if link failed. */
+    boolean linkToDeath();
 
     /** Removes link to the app process death. */
     void unlinkToDeath();
diff --git a/services/core/java/com/android/server/vibrator/VibratorManagerService.java b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
index 7d5d34d..ff34911 100644
--- a/services/core/java/com/android/server/vibrator/VibratorManagerService.java
+++ b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
@@ -51,6 +51,7 @@
 import android.os.ServiceManager;
 import android.os.ShellCallback;
 import android.os.ShellCommand;
+import android.os.SystemClock;
 import android.os.Trace;
 import android.os.VibrationAttributes;
 import android.os.VibrationEffect;
@@ -94,6 +95,7 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.function.Predicate;
 
 /** System implementation of {@link IVibratorManagerService}. */
 public class VibratorManagerService extends IVibratorManagerService.Stub {
@@ -155,14 +157,14 @@
     private final SparseArray<VibratorController> mVibrators;
     private final VibrationThreadCallbacks mVibrationThreadCallbacks =
             new VibrationThreadCallbacks();
+    private final ExternalVibrationCallbacks mExternalVibrationCallbacks =
+            new ExternalVibrationCallbacks();
     @GuardedBy("mLock")
     private final SparseArray<AlwaysOnVibration> mAlwaysOnEffects = new SparseArray<>();
     @GuardedBy("mLock")
-    private SingleVibrationSession mCurrentVibration;
+    private VibrationSession mCurrentVibration;
     @GuardedBy("mLock")
-    private SingleVibrationSession mNextVibration;
-    @GuardedBy("mLock")
-    private ExternalVibrationSession mCurrentExternalVibration;
+    private VibrationSession mNextVibration;
     @GuardedBy("mLock")
     private boolean mServiceReady;
 
@@ -186,25 +188,19 @@
         @Override
         public void onReceive(Context context, Intent intent) {
             if (intent.getAction().equals(Intent.ACTION_SCREEN_OFF)) {
+                // When the system is entering a non-interactive state, we want to cancel
+                // vibrations in case a misbehaving app has abandoned them.
                 synchronized (mLock) {
-                    // When the system is entering a non-interactive state, we want to cancel
-                    // vibrations in case a misbehaving app has abandoned them.
-                    if (shouldCancelOnScreenOffLocked(mNextVibration)) {
-                        clearNextVibrationLocked(Status.CANCELLED_BY_SCREEN_OFF);
-                    }
-                    if (shouldCancelOnScreenOffLocked(mCurrentVibration)) {
-                        mCurrentVibration.requestEnd(Status.CANCELLED_BY_SCREEN_OFF);
-                    }
+                    maybeClearCurrentAndNextVibrationsLocked(
+                            VibratorManagerService.this::shouldCancelOnScreenOffLocked,
+                            Status.CANCELLED_BY_SCREEN_OFF);
                 }
             } else if (android.multiuser.Flags.addUiForSoundsFromBackgroundUsers()
                     && intent.getAction().equals(BackgroundUserSoundNotifier.ACTION_MUTE_SOUND)) {
                 synchronized (mLock) {
-                    if (shouldCancelOnFgUserRequest(mNextVibration)) {
-                        clearNextVibrationLocked(Status.CANCELLED_BY_FOREGROUND_USER);
-                    }
-                    if (shouldCancelOnFgUserRequest(mCurrentVibration)) {
-                        mCurrentVibration.requestEnd(Status.CANCELLED_BY_FOREGROUND_USER);
-                    }
+                    maybeClearCurrentAndNextVibrationsLocked(
+                            VibratorManagerService.this::shouldCancelOnFgUserRequest,
+                            Status.CANCELLED_BY_FOREGROUND_USER);
                 }
             }
         }
@@ -219,12 +215,9 @@
                         return;
                     }
                     synchronized (mLock) {
-                        if (shouldCancelAppOpModeChangedLocked(mNextVibration)) {
-                            clearNextVibrationLocked(Status.CANCELLED_BY_APP_OPS);
-                        }
-                        if (shouldCancelAppOpModeChangedLocked(mCurrentVibration)) {
-                            mCurrentVibration.requestEnd(Status.CANCELLED_BY_APP_OPS);
-                        }
+                        maybeClearCurrentAndNextVibrationsLocked(
+                                VibratorManagerService.this::shouldCancelAppOpModeChangedLocked,
+                                Status.CANCELLED_BY_APP_OPS);
                     }
                 }
             };
@@ -641,14 +634,8 @@
             if (ignoreStatus == null) {
                 final long ident = Binder.clearCallingIdentity();
                 try {
-                    if (mCurrentExternalVibration != null) {
-                        vib.stats.reportInterruptedAnotherVibration(
-                                mCurrentExternalVibration.getCallerInfo());
-                        endExternalVibrateLocked(Status.CANCELLED_SUPERSEDED, callerInfo,
-                                /* continueExternalControl= */ false);
-                    } else if (mCurrentVibration != null) {
-                        if (mCurrentVibration.getVibration().canPipelineWith(vib, mVibratorInfos,
-                                mVibrationConfig.getVibrationPipelineMaxDurationMs())) {
+                    if (mCurrentVibration != null) {
+                        if (shouldPipelineVibrationLocked(mCurrentVibration, vib)) {
                             // Don't cancel the current vibration if it's pipeline-able.
                             // Note that if there is a pending next vibration that can't be
                             // pipelined, it will have already cancelled the current one, so we
@@ -658,11 +645,12 @@
                             }
                         } else {
                             vib.stats.reportInterruptedAnotherVibration(
-                                    mCurrentVibration.getVibration().callerInfo);
+                                    mCurrentVibration.getCallerInfo());
                             mCurrentVibration.requestEnd(Status.CANCELLED_SUPERSEDED, callerInfo,
                                     /* immediate= */ false);
                         }
                     }
+                    clearNextVibrationLocked(Status.CANCELLED_SUPERSEDED, callerInfo);
                     ignoreStatus = startVibrationLocked(session);
                 } finally {
                     Binder.restoreCallingIdentity(ident);
@@ -691,17 +679,17 @@
                 }
                 final long ident = Binder.clearCallingIdentity();
                 try {
-                    if (shouldCancelVibration(mNextVibration, usageFilter, token)) {
+                    // TODO(b/370948466): investigate why token not checked on external vibrations.
+                    IBinder cancelToken =
+                            (mNextVibration instanceof ExternalVibrationSession) ? null : token;
+                    if (shouldCancelVibration(mNextVibration, usageFilter, cancelToken)) {
                         clearNextVibrationLocked(Status.CANCELLED_BY_USER);
                     }
-                    if (shouldCancelVibration(mCurrentVibration, usageFilter, token)) {
+                    cancelToken =
+                            (mCurrentVibration instanceof ExternalVibrationSession) ? null : token;
+                    if (shouldCancelVibration(mCurrentVibration, usageFilter, cancelToken)) {
                         mCurrentVibration.requestEnd(Status.CANCELLED_BY_USER);
                     }
-                    // TODO(b/370948466): investigate why token is not checked here and fix it.
-                    if (shouldCancelVibration(mCurrentExternalVibration, usageFilter, null)) {
-                        endExternalVibrateLocked(Status.CANCELLED_BY_USER,
-                                /* endedBy= */ null, /* continueExternalControl= */ false);
-                    }
                 } finally {
                     Binder.restoreCallingIdentity(ident);
                 }
@@ -760,7 +748,7 @@
             pw.println("CurrentVibration:");
             pw.increaseIndent();
             if (mCurrentVibration != null) {
-                mCurrentVibration.getVibration().getDebugInfo().dump(pw);
+                mCurrentVibration.getDebugInfo().dump(pw);
             } else {
                 pw.println("null");
             }
@@ -770,17 +758,7 @@
             pw.println("NextVibration:");
             pw.increaseIndent();
             if (mNextVibration != null) {
-                mNextVibration.getVibration().getDebugInfo().dump(pw);
-            } else {
-                pw.println("null");
-            }
-            pw.decreaseIndent();
-            pw.println();
-
-            pw.println("CurrentExternalVibration:");
-            pw.increaseIndent();
-            if (mCurrentExternalVibration != null) {
-                mCurrentExternalVibration.getDebugInfo().dump(pw);
+                mNextVibration.getDebugInfo().dump(pw);
             } else {
                 pw.println("null");
             }
@@ -805,13 +783,9 @@
             mVibrationSettings.dump(proto);
             mVibrationScaler.dump(proto);
             if (mCurrentVibration != null) {
-                mCurrentVibration.getVibration().getDebugInfo().dump(proto,
+                mCurrentVibration.getDebugInfo().dump(proto,
                         VibratorManagerServiceDumpProto.CURRENT_VIBRATION);
             }
-            if (mCurrentExternalVibration != null) {
-                mCurrentExternalVibration.getDebugInfo().dump(proto,
-                        VibratorManagerServiceDumpProto.CURRENT_EXTERNAL_VIBRATION);
-            }
             for (int i = 0; i < mVibrators.size(); i++) {
                 proto.write(VibratorManagerServiceDumpProto.VIBRATOR_IDS, mVibrators.keyAt(i));
             }
@@ -841,7 +815,9 @@
                 updateAlwaysOnLocked(mAlwaysOnEffects.valueAt(i));
             }
 
-            if (mCurrentVibration == null) {
+            // TODO(b/372241975): investigate why external vibrations were not handled here before
+            if (mCurrentVibration == null
+                    || (mCurrentVibration instanceof ExternalVibrationSession)) {
                 return;
             }
 
@@ -916,14 +892,14 @@
                 Trace.asyncTraceBegin(TRACE_TAG_VIBRATOR, "vibration", 0);
                 // Make sure mCurrentVibration is set while triggering the VibrationThread.
                 mCurrentVibration = session;
-                if (!mCurrentVibration.linkToDeath(null)) {
-                    // Shouldn't happen. The method call already logs a wtf.
+                if (!mCurrentVibration.linkToDeath()) {
+                    // Shouldn't happen. The method call already logs.
                     mCurrentVibration = null;  // Aborted.
                     return Status.IGNORED_ERROR_TOKEN;
                 }
                 if (!mVibrationThread.runVibrationOnVibrationThread(conductor)) {
-                    // Shouldn't happen. The method call already logs a wtf.
-                    mCurrentVibration.setVibrationConductor(null);
+                    // Shouldn't happen. The method call already logs.
+                    session.setVibrationConductor(null); // Rejected by thread, clear it in session.
                     mCurrentVibration = null;  // Aborted.
                     return Status.IGNORED_ERROR_SCHEDULING;
                 }
@@ -938,6 +914,17 @@
     }
 
     @GuardedBy("mLock")
+    private void maybeStartNextSingleVibrationLocked() {
+        if (mNextVibration instanceof SingleVibrationSession session) {
+            mNextVibration = null;
+            Status errorStatus = startVibrationOnThreadLocked(session);
+            if (errorStatus != null) {
+                endVibrationLocked(session, errorStatus);
+            }
+        }
+    }
+
+    @GuardedBy("mLock")
     private void endVibrationLocked(VibrationSession session, Status status) {
         endVibrationLocked(session, status, /* endedBy= */ null);
     }
@@ -1070,10 +1057,6 @@
     @GuardedBy("mLock")
     @Nullable
     private Vibration.EndInfo shouldIgnoreVibrationForOngoingLocked(VibrationSession session) {
-        if (mCurrentExternalVibration != null) {
-            return shouldIgnoreVibrationForOngoing(session, mCurrentExternalVibration);
-        }
-
         if (mNextVibration != null) {
             Vibration.EndInfo vibrationEndInfo = shouldIgnoreVibrationForOngoing(session,
                     mNextVibration);
@@ -1168,6 +1151,18 @@
         }
     }
 
+    /** Returns true if ongoing session should pipeline with the next vibration requested. */
+    @GuardedBy("mLock")
+    private boolean shouldPipelineVibrationLocked(VibrationSession currentSession,
+            HalVibration nextVibration) {
+        if (!(currentSession instanceof SingleVibrationSession currentVibration)) {
+            // Only single vibration session can be pipelined.
+            return false;
+        }
+        return currentVibration.getVibration().canPipelineWith(nextVibration, mVibratorInfos,
+                mVibrationConfig.getVibrationPipelineMaxDurationMs());
+    }
+
     /**
      * Check if given vibration should be ignored by this service.
      *
@@ -1304,7 +1299,7 @@
         try {
             effect.validate();
         } catch (Exception e) {
-            Slog.wtf(TAG, "Encountered issue when verifying CombinedVibrationEffect.", e);
+            Slog.wtf(TAG, "Encountered issue when verifying vibration: " + effect, e);
             return false;
         }
         return true;
@@ -1423,7 +1418,6 @@
         return checkAppOpModeLocked(session.getCallerInfo()) != AppOpsManager.MODE_ALLOWED;
     }
 
-    @GuardedBy("mLock")
     private boolean shouldCancelOnFgUserRequest(@Nullable VibrationSession session) {
         if (session == null) {
             return false;
@@ -1573,40 +1567,70 @@
         @Override
         public void onVibrationThreadReleased(long vibrationId) {
             if (DEBUG) {
-                Slog.d(TAG, "VibrationThread released after finished vibration");
+                Slog.d(TAG, "VibrationThread released vibration " + vibrationId);
             }
             Trace.traceBegin(TRACE_TAG_VIBRATOR, "onVibrationThreadReleased");
-
             try {
                 synchronized (mLock) {
-                    if (DEBUG) {
-                        Slog.d(TAG, "Processing VibrationThread released callback");
+                    if (!(mCurrentVibration instanceof SingleVibrationSession session)) {
+                        if (Build.IS_DEBUGGABLE) {
+                            Slog.wtf(TAG, "VibrationSession invalid on vibration thread release."
+                                    + " currentSession=" + mCurrentVibration);
+                        }
+                        // Only single vibration sessions are ended by thread being released. Abort.
+                        return;
                     }
-                    if (Build.IS_DEBUGGABLE && mCurrentVibration != null
-                            && mCurrentVibration.getVibration().id != vibrationId) {
+                    if (Build.IS_DEBUGGABLE && (session.getVibration().id != vibrationId)) {
                         Slog.wtf(TAG, TextUtils.formatSimple(
-                                "VibrationId mismatch on release. expected=%d, released=%d",
-                                mCurrentVibration.getVibration().id, vibrationId));
+                                "VibrationId mismatch on vibration thread release."
+                                        + " expected=%d, released=%d",
+                                session.getVibration().id, vibrationId));
                     }
-                    if (mCurrentVibration != null) {
-                        // This is when we consider the current vibration complete, report metrics.
-                        if (DEBUG) {
-                            Slog.d(TAG, "Reporting vibration " + vibrationId + " finished.");
+                    finishAppOpModeLocked(mCurrentVibration.getCallerInfo());
+                    clearCurrentVibrationLocked();
+                    Trace.asyncTraceEnd(Trace.TRACE_TAG_VIBRATOR, "vibration", 0);
+                    // Start next vibration if it's a single vibration waiting for the thread.
+                    maybeStartNextSingleVibrationLocked();
+                }
+            } finally {
+                Trace.traceEnd(TRACE_TAG_VIBRATOR);
+            }
+        }
+    }
+
+    /**
+     * Implementation of {@link ExternalVibrationSession.VibratorManagerHooks} that controls
+     * external vibrations and reports them when finished.
+     */
+    private final class ExternalVibrationCallbacks
+            implements ExternalVibrationSession.VibratorManagerHooks {
+
+        @Override
+        public void onExternalVibrationReleased(long vibrationId) {
+            if (DEBUG) {
+                Slog.d(TAG, "External vibration " + vibrationId + " released");
+            }
+            Trace.traceBegin(TRACE_TAG_VIBRATOR, "onExternalVibrationReleased");
+            try {
+                synchronized (mLock) {
+                    if (!(mCurrentVibration instanceof ExternalVibrationSession session)) {
+                        if (Build.IS_DEBUGGABLE) {
+                            Slog.wtf(TAG, "VibrationSession invalid on external vibration release."
+                                    + " currentSession=" + mCurrentVibration);
                         }
-                        mCurrentVibration.unlinkToDeath();
-                        finishAppOpModeLocked(mCurrentVibration.getCallerInfo());
-                        logAndRecordVibration(mCurrentVibration.getDebugInfo());
-                        Trace.asyncTraceEnd(Trace.TRACE_TAG_VIBRATOR, "vibration", 0);
-                        mCurrentVibration = null;
+                        // Only external vibration sessions are ended by this callback. Abort.
+                        return;
                     }
-                    if (mNextVibration != null) {
-                        SingleVibrationSession nextVibration = mNextVibration;
-                        mNextVibration = null;
-                        Status startErrorStatus = startVibrationOnThreadLocked(nextVibration);
-                        if (startErrorStatus != null) {
-                            endVibrationLocked(nextVibration, startErrorStatus);
-                        }
+                    if (Build.IS_DEBUGGABLE && (session.id != vibrationId)) {
+                        Slog.wtf(TAG, TextUtils.formatSimple(
+                                "VibrationId mismatch on external vibration release."
+                                        + " expected=%d, released=%d", session.id, vibrationId));
                     }
+                    setExternalControl(false, session.stats);
+                    clearCurrentVibrationLocked();
+                    // Start next vibration if it's a single vibration waiting for the external
+                    // control to be over.
+                    maybeStartNextSingleVibrationLocked();
                 }
             } finally {
                 Trace.traceEnd(TRACE_TAG_VIBRATOR);
@@ -1826,6 +1850,7 @@
             mInfo.dump(proto, fieldId);
         }
     }
+
     /** Clears mNextVibration if set, ending it cleanly */
     @GuardedBy("mLock")
     private void clearNextVibrationLocked(Status status) {
@@ -1846,28 +1871,56 @@
         }
     }
 
-    /**
-     * Ends the external vibration, and clears related service state.
-     *
-     * @param status                  the status to end the associated Vibration
-     * @param endedBy                 the caller that caused this vibration to end
-     * @param continueExternalControl indicates whether external control will continue. If not, the
-     *                                HAL will have external control turned off.
-     */
+    /** Clears mCurrentVibration if set, reporting metrics */
     @GuardedBy("mLock")
-    private void endExternalVibrateLocked(Status status, CallerInfo endedBy,
-            boolean continueExternalControl) {
-        if (mCurrentExternalVibration == null) {
-            return;
+    private void clearCurrentVibrationLocked() {
+        if (mCurrentVibration != null) {
+            mCurrentVibration.unlinkToDeath();
+            logAndRecordVibration(mCurrentVibration.getDebugInfo());
+            mCurrentVibration = null;
+            mLock.notify(); // Notify if waiting for current vibration to end.
         }
-        mCurrentExternalVibration.requestEnd(status, endedBy, /* immediate= */ true);
-        mCurrentExternalVibration.unlinkToDeath();
-        if (!continueExternalControl) {
-            setExternalControl(false, mCurrentExternalVibration.stats);
+    }
+
+    @GuardedBy("mLock")
+    private void maybeClearCurrentAndNextVibrationsLocked(
+            Predicate<VibrationSession> shouldEndSessionPredicate, Status endStatus) {
+        // TODO(b/372241975): investigate why external vibrations were not handled here before
+        if (!(mNextVibration instanceof ExternalVibrationSession)
+                && shouldEndSessionPredicate.test(mNextVibration)) {
+            clearNextVibrationLocked(endStatus);
         }
-        // The external control was turned off, end it and report metrics right away.
-        logAndRecordVibration(mCurrentExternalVibration.getDebugInfo());
-        mCurrentExternalVibration = null;
+        if (!(mCurrentVibration instanceof ExternalVibrationSession)
+                && shouldEndSessionPredicate.test(mCurrentVibration)) {
+            mCurrentVibration.requestEnd(endStatus);
+        }
+    }
+
+    /**
+     * Waits until the current vibration finished processing, timing out after the given
+     * number of milliseconds.
+     *
+     * @return true if the vibration completed, or false if waiting timed out.
+     */
+    public boolean waitForCurrentVibrationEnd(long maxWaitMillis) {
+        long now = SystemClock.elapsedRealtime();
+        long deadline = now + maxWaitMillis;
+        synchronized (mLock) {
+            while (true) {
+                if (mCurrentVibration == null) {
+                    return true;  // Done
+                }
+                if (now >= deadline) {  // Note that thread.wait(0) waits indefinitely.
+                    return false;  // Timed out.
+                }
+                try {
+                    mLock.wait(deadline - now);
+                } catch (InterruptedException e) {
+                    Slog.w(TAG, "VibratorManagerService interrupted waiting to stop, continuing");
+                }
+                now = SystemClock.elapsedRealtime();
+            }
+        }
     }
 
     private HapticFeedbackVibrationProvider getHapticVibrationProvider() {
@@ -1904,10 +1957,10 @@
             try {
                 // Create Vibration.Stats as close to the received request as possible, for
                 // tracking.
-                ExternalVibrationSession session = new ExternalVibrationSession(vib);
+                ExternalVibrationSession session = new ExternalVibrationSession(vib,
+                        mExternalVibrationCallbacks);
                 // Mute the request until we run all the checks and accept the vibration.
                 session.muteScale();
-                boolean alreadyUnderExternalControl = false;
                 boolean waitForCompletion = false;
 
                 synchronized (mLock) {
@@ -1933,85 +1986,83 @@
                         return session.getScale();
                     }
 
-                    if (mCurrentExternalVibration != null
-                            && mCurrentExternalVibration.isHoldingSameVibration(vib)) {
+                    if ((mCurrentVibration instanceof ExternalVibrationSession evs)
+                            && evs.isHoldingSameVibration(vib)) {
                         // We are already playing this external vibration, so we can return the same
                         // scale calculated in the previous call to this method.
-                        return mCurrentExternalVibration.getScale();
+                        return evs.getScale();
                     }
 
                     // Check if ongoing vibration is more important than this vibration.
-                    Vibration.EndInfo vibrationEndInfo =
-                            shouldIgnoreVibrationForOngoingLocked(session);
-                    if (vibrationEndInfo != null) {
-                        endVibrationLocked(session, vibrationEndInfo.status,
-                                vibrationEndInfo.endedBy);
+                    Vibration.EndInfo ignoreInfo = shouldIgnoreVibrationForOngoingLocked(session);
+                    if (ignoreInfo != null) {
+                        endVibrationLocked(session, ignoreInfo.status, ignoreInfo.endedBy);
                         return session.getScale();
                     }
 
-                    if (mCurrentExternalVibration == null) {
-                        // If we're not under external control right now, then cancel any normal
-                        // vibration that may be playing and ready the vibrator for external
-                        // control.
-                        if (mCurrentVibration != null) {
-                            session.stats.reportInterruptedAnotherVibration(
-                                    mCurrentVibration.getVibration().callerInfo);
-                            clearNextVibrationLocked(Status.IGNORED_FOR_EXTERNAL,
-                                    session.callerInfo);
-                            mCurrentVibration.requestEnd(Status.CANCELLED_SUPERSEDED,
-                                    session.callerInfo, /* immediate= */ true);
-                            waitForCompletion = true;
-                        }
-                    } else {
-                        // At this point we have an externally controlled vibration playing already.
+                    // First clear next request, so it won't start when the current one ends.
+                    clearNextVibrationLocked(Status.IGNORED_FOR_EXTERNAL, session.callerInfo);
+                    mNextVibration = session;
+
+                    if (mCurrentVibration != null) {
+                        // Cancel any vibration that may be playing and ready the vibrator, even if
+                        // 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, so we can be assured that the
-                        // ongoing will be muted in favor of the new vibration.
+                        // 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, 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.
-                        alreadyUnderExternalControl = true;
                         session.stats.reportInterruptedAnotherVibration(
-                                mCurrentExternalVibration.getCallerInfo());
-                        endExternalVibrateLocked(Status.CANCELLED_SUPERSEDED,
-                                session.callerInfo, /* continueExternalControl= */ true);
+                                mCurrentVibration.getCallerInfo());
+                        mCurrentVibration.requestEnd(Status.CANCELLED_SUPERSEDED,
+                                session.callerInfo, /* immediate= */ true);
+                        waitForCompletion = true;
                     }
                 }
                 // Wait for lock and interact with HAL to set external control outside main lock.
                 if (waitForCompletion) {
-                    if (!mVibrationThread.waitForThreadIdle(VIBRATION_CANCEL_WAIT_MILLIS)) {
+                    if (!waitForCurrentVibrationEnd(VIBRATION_CANCEL_WAIT_MILLIS)) {
                         Slog.e(TAG, "Timed out waiting for vibration to cancel");
                         synchronized (mLock) {
+                            if (mNextVibration == session) {
+                                mNextVibration = null;
+                            }
                             endVibrationLocked(session, Status.IGNORED_ERROR_CANCELLING);
                             return session.getScale();
                         }
                     }
                 }
-                if (!alreadyUnderExternalControl) {
+                synchronized (mLock) {
+                    if (mNextVibration == session) {
+                        // This is still the next vibration to be played.
+                        mNextVibration = null;
+                    } else {
+                        // A new request took the place of this one, maybe with higher importance.
+                        // Next vibration already cleared with the right status, just return here.
+                        return session.getScale();
+                    }
+                    if (!session.linkToDeath()) {
+                        endVibrationLocked(session, Status.IGNORED_ERROR_TOKEN);
+                        return session.getScale();
+                    }
                     if (DEBUG) {
                         Slog.d(TAG, "Vibrator going under external control.");
                     }
                     setExternalControl(true, session.stats);
-                }
-                synchronized (mLock) {
                     if (DEBUG) {
                         Slog.d(TAG, "Playing external vibration: " + vib);
                     }
                     VibrationAttributes attrs = fixupVibrationAttributes(
-                            vib.getVibrationAttributes(),
-                            /* effect= */ null);
+                            vib.getVibrationAttributes(), /* effect= */ null);
                     if (attrs.isFlagSet(VibrationAttributes.FLAG_INVALIDATE_SETTINGS_CACHE)) {
                         // Force update of user settings before checking if this vibration effect
                         // should be ignored or scaled.
                         mVibrationSettings.update();
                     }
-
-                    mCurrentExternalVibration = session;
-                    session.linkToDeath(this::onExternalVibrationBinderDied);
+                    mCurrentVibration = session;
                     session.scale(mVibrationScaler, attrs.getUsage());
 
                     // Vibrator will start receiving data from external channels after this point.
@@ -2029,13 +2080,12 @@
             Trace.traceBegin(TRACE_TAG_VIBRATOR, "onExternalVibrationStop");
             try {
                 synchronized (mLock) {
-                    if (mCurrentExternalVibration != null
-                            && mCurrentExternalVibration.isHoldingSameVibration(vib)) {
+                    if ((mCurrentVibration instanceof ExternalVibrationSession evs)
+                            && evs.isHoldingSameVibration(vib)) {
                         if (DEBUG) {
                             Slog.d(TAG, "Stopping external vibration: " + vib);
                         }
-                        endExternalVibrateLocked(Status.FINISHED, /* endedBy= */ null,
-                                /* continueExternalControl= */ false);
+                        mCurrentVibration.requestEnd(Status.FINISHED);
                     }
                 }
             } finally {
@@ -2051,18 +2101,6 @@
             }
             return false;
         }
-
-        private void onExternalVibrationBinderDied() {
-            synchronized (mLock) {
-                if (mCurrentExternalVibration != null) {
-                    if (DEBUG) {
-                        Slog.d(TAG, "External vibration finished because binder died");
-                    }
-                    endExternalVibrateLocked(Status.CANCELLED_BINDER_DIED, /* endedBy= */ null,
-                            /* continueExternalControl= */ false);
-                }
-            }
-        }
     }
 
     /** Provide limited functionality from {@link VibratorManagerService} as shell commands. */
diff --git a/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java b/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java
index 7f5da41..dfdd0cd 100644
--- a/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java
+++ b/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java
@@ -2248,13 +2248,13 @@
         assertNotEquals(ExternalVibrationScale.ScaleLevel.SCALE_MUTE, secondScale.scaleLevel);
         verify(firstController).mute();
         verify(secondController, never()).mute();
-        // Set external control called only once.
-        assertEquals(Arrays.asList(false, true),
+        // Set external control called for each vibration independently.
+        assertEquals(Arrays.asList(false, true, false, true),
                 mVibratorProviders.get(1).getExternalControlStates());
 
         mExternalVibratorService.onExternalVibrationStop(secondVibration);
         mExternalVibratorService.onExternalVibrationStop(firstVibration);
-        assertEquals(Arrays.asList(false, true, false),
+        assertEquals(Arrays.asList(false, true, false, true, false),
                 mVibratorProviders.get(1).getExternalControlStates());
 
         verify(firstToken).linkToDeath(any(), eq(0));