Merge "Cancel current vibration asynchronously in VibratorService" into sc-dev

services/core/java/com/android/server/VibratorService.java

@@ -102,7 +102,10 @@ public class VibratorService extends IVibratorService.Stub {
     private VibrationScaler mVibrationScaler;
     private InputDeviceDelegate mInputDeviceDelegate;
 
-    private volatile VibrationThread mThread;
+    @GuardedBy("mLock")
+    private VibrationThread mThread;
+    @GuardedBy("mLock")
+    private VibrationThread mNextVibrationThread;
 
     @GuardedBy("mLock")
     private Vibration mCurrentVibration;
@@ -132,6 +135,10 @@ public class VibratorService extends IVibratorService.Stub {
                 if (mCurrentVibration != null && mCurrentVibration.id == vibrationId) {
                     mThread = null;
                     reportFinishVibrationLocked(status);
+                    if (mNextVibrationThread != null) {
+                        startVibrationThreadLocked(mNextVibrationThread);
+                        mNextVibrationThread = null;
+                    }
                 }
             }
         }
@@ -258,18 +265,14 @@ public class VibratorService extends IVibratorService.Stub {
     @VisibleForTesting
     public void onVibrationComplete(int vibratorId, long vibrationId) {
         synchronized (mLock) {
-            if (mCurrentVibration != null && mCurrentVibration.id == vibrationId) {
+            if (mCurrentVibration != null && mCurrentVibration.id == vibrationId
+                    && mThread != null) {
                 if (DEBUG) {
                     Slog.d(TAG, "Vibration onComplete callback, notifying VibrationThread");
                 }
-                if (mThread != null) {
+                // Let the thread playing the vibration handle the callback, since it might be
-                    // Let the thread playing the vibration handle the callback, since it might be
+                // expecting the vibrator to turn off multiple times during a single vibration.
-                    // expecting the vibrator to turn off multiple times during a single vibration.
+                mThread.vibratorComplete(vibratorId);
-                    mThread.vibratorComplete(vibratorId);
-                } else {
-                    // No vibration is playing in the thread, but clean up service just in case.
-                    doCancelVibrateLocked(Vibration.Status.FINISHED);
-                }
             }
         }
     }
@@ -462,8 +465,10 @@ public class VibratorService extends IVibratorService.Stub {
                 try {
                     doCancelVibrateLocked(Vibration.Status.CANCELLED);
                     startVibrationLocked(vib);
+                    boolean isNextVibration = mNextVibrationThread != null
+                            && vib.equals(mNextVibrationThread.getVibration());
 
-                    if (!vib.hasEnded() && mCurrentVibration.id != vib.id) {
+                    if (!vib.hasEnded() && !vib.equals(mCurrentVibration) && !isNextVibration) {
                         // Vibration was unexpectedly ignored: add to list for debugging
                         endVibrationLocked(vib, Vibration.Status.IGNORED);
                     }
@@ -532,6 +537,7 @@ public class VibratorService extends IVibratorService.Stub {
                 }
                 final long ident = Binder.clearCallingIdentity();
                 try {
+                    mNextVibrationThread = null;
                     doCancelVibrateLocked(Vibration.Status.CANCELLED);
                 } finally {
                     Binder.restoreCallingIdentity(ident);
@@ -546,16 +552,14 @@ public class VibratorService extends IVibratorService.Stub {
         try {
             if (mThread != null) {
                 mThread.cancel();
-                mThread = null;
             }
+            mInputDeviceDelegate.cancelVibrateIfAvailable();
             if (mCurrentExternalVibration != null) {
                 endVibrationLocked(mCurrentExternalVibration, status);
                 mCurrentExternalVibration.externalVibration.mute();
                 mCurrentExternalVibration = null;
                 mVibratorController.setExternalControl(false);
             }
-            doVibratorOff();
-            reportFinishVibrationLocked(status);
         } finally {
             Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR);
         }
@@ -579,28 +583,30 @@ public class VibratorService extends IVibratorService.Stub {
     private void startVibrationInnerLocked(Vibration vib) {
         Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "startVibrationInnerLocked");
         try {
-            // Set current vibration before starting it, so callback will work.
-            mCurrentVibration = vib;
-            VibrationEffect effect = getEffect(vib);
-            Trace.asyncTraceBegin(Trace.TRACE_TAG_VIBRATOR, "vibration", 0);
             boolean inputDevicesAvailable = mInputDeviceDelegate.vibrateIfAvailable(
                     vib.uid, vib.opPkg, vib.getEffect(), vib.reason, vib.attrs);
             if (inputDevicesAvailable) {
-                // The set current vibration is no longer being played by this service, so drop it.
-                mCurrentVibration = null;
                 endVibrationLocked(vib, Vibration.Status.FORWARDED_TO_INPUT_DEVICES);
+            } else if (mThread == null) {
+                startVibrationThreadLocked(new VibrationThread(vib, mVibratorController, mWakeLock,
+                        mBatteryStatsService, mVibrationCallbacks));
             } else {
-                // mThread better be null here. doCancelVibrate should always be
+                mNextVibrationThread = new VibrationThread(vib, mVibratorController, mWakeLock,
-                // called before startVibrationInnerLocked
-                mThread = new VibrationThread(vib, mVibratorController, mWakeLock,
                         mBatteryStatsService, mVibrationCallbacks);
-                mThread.start();
             }
         } finally {
             Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR);
         }
     }
 
+    @GuardedBy("mLock")
+    private void startVibrationThreadLocked(VibrationThread thread) {
+        Trace.asyncTraceBegin(Trace.TRACE_TAG_VIBRATOR, "vibration", 0);
+        mCurrentVibration = thread.getVibration();
+        mThread = thread;
+        mThread.start();
+    }
+
     /** Scale the vibration effect by the intensity as appropriate based its intent. */
     private void applyVibrationIntensityScalingLocked(Vibration vib) {
         vib.updateEffect(mVibrationScaler.scale(vib.getEffect(), vib.attrs.getUsage()));
@@ -665,13 +671,14 @@ public class VibratorService extends IVibratorService.Stub {
 
     @GuardedBy("mLock")
     private void reportFinishVibrationLocked(Vibration.Status status) {
-        Trace.asyncTraceEnd(Trace.TRACE_TAG_VIBRATOR, "vibration", 0);
         Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "reportFinishVibrationLocked");
         try {
             if (mCurrentVibration != null) {
                 endVibrationLocked(mCurrentVibration, status);
                 mAppOps.finishOp(AppOpsManager.OP_VIBRATE, mCurrentVibration.uid,
                         mCurrentVibration.opPkg);
+
+                Trace.asyncTraceEnd(Trace.TRACE_TAG_VIBRATOR, "vibration", 0);
                 mCurrentVibration = null;
             }
         } finally {
@@ -697,21 +704,6 @@ public class VibratorService extends IVibratorService.Stub {
         }
     }
 
-    private void doVibratorOff() {
-        Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "doVibratorOff");
-        try {
-            if (DEBUG) {
-                Slog.d(TAG, "Turning vibrator off.");
-            }
-            boolean inputDevicesAvailable = mInputDeviceDelegate.cancelVibrateIfAvailable();
-            if (!inputDevicesAvailable) {
-                mVibratorController.off();
-            }
-        } finally {
-            Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR);
-        }
-    }
-
     private boolean isSystemHapticFeedback(Vibration vib) {
         if (vib.attrs.getUsage() != VibrationAttributes.USAGE_TOUCH) {
             return false;
@@ -844,6 +836,7 @@ public class VibratorService extends IVibratorService.Stub {
                     // haptic feedback as part of the transition.  So we don't cancel
                     // system vibrations.
                     if (mCurrentVibration != null && !isSystemHapticFeedback(mCurrentVibration)) {
+                        mNextVibrationThread = null;
                         doCancelVibrateLocked(Vibration.Status.CANCELLED);
                     }
                 }
@@ -910,6 +903,8 @@ public class VibratorService extends IVibratorService.Stub {
                 return IExternalVibratorService.SCALE_MUTE;
             }
 
+            VibrationThread cancelingVibration = null;
+            int scale;
             synchronized (mLock) {
                 if (mCurrentExternalVibration != null
                         && mCurrentExternalVibration.externalVibration.equals(vib)) {
@@ -920,11 +915,9 @@ public class VibratorService extends IVibratorService.Stub {
                 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 (DEBUG) {
+                    mNextVibrationThread = null;
-                        Slog.d(TAG, "Vibrator going under external control.");
-                    }
                     doCancelVibrateLocked(Vibration.Status.CANCELLED);
-                    mVibratorController.setExternalControl(true);
+                    cancelingVibration = mThread;
                 } else {
                     endVibrationLocked(mCurrentExternalVibration, Vibration.Status.CANCELLED);
                 }
@@ -941,11 +934,24 @@ public class VibratorService extends IVibratorService.Stub {
                 vib.linkToDeath(mCurrentExternalDeathRecipient);
                 mCurrentExternalVibration.scale = mVibrationScaler.getExternalVibrationScale(
                         vib.getVibrationAttributes().getUsage());
-                if (DEBUG) {
+                scale = mCurrentExternalVibration.scale;
-                    Slog.e(TAG, "Playing external vibration: " + vib);
-                }
-                return mCurrentExternalVibration.scale;
             }
+            if (cancelingVibration != null) {
+                try {
+                    cancelingVibration.join();
+                } catch (InterruptedException e) {
+                    Slog.w("Interrupted while waiting current vibration to be cancelled before "
+                            + "starting external vibration", e);
+                }
+            }
+            if (DEBUG) {
+                Slog.d(TAG, "Vibrator going under external control.");
+            }
+            mVibratorController.setExternalControl(true);
+            if (DEBUG) {
+                Slog.e(TAG, "Playing external vibration: " + vib);
+            }
+            return scale;
         }
 
         @Override

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

@@ -74,6 +74,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
         void onVibrationEnded(long vibrationId, Vibration.Status status);
     }
 
+    private final Object mLock = new Object();
     private final WorkSource mWorkSource = new WorkSource();
     private final PowerManager.WakeLock mWakeLock;
     private final IBatteryStats mBatteryStatsService;
@@ -81,10 +82,10 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
     private final VibrationCallbacks mCallbacks;
     private final SparseArray<VibratorController> mVibrators;
 
-    @GuardedBy("this")
+    @GuardedBy("mLock")
     @Nullable
     private VibrateStep mCurrentVibrateStep;
-    @GuardedBy("this")
+    @GuardedBy("mLock")
     private boolean mForceStop;
 
     // TODO(b/159207608): Remove this constructor once VibratorService is removed
@@ -113,6 +114,10 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
         }
     }
 
+    public Vibration getVibration() {
+        return mVibration;
+    }
+
     @Override
     public void binderDied() {
         cancel();
@@ -136,15 +141,15 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
 
     /** Cancel current vibration and shuts down the thread gracefully. */
     public void cancel() {
-        synchronized (this) {
+        synchronized (mLock) {
             mForceStop = true;
-            notify();
+            mLock.notify();
         }
     }
 
     /** Notify current vibration that a step has completed on given vibrator. */
     public void vibratorComplete(int vibratorId) {
-        synchronized (this) {
+        synchronized (mLock) {
             if (mCurrentVibrateStep != null) {
                 mCurrentVibrateStep.vibratorComplete(vibratorId);
             }
@@ -168,7 +173,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
             final int stepCount = steps.size();
             for (int i = 0; i < stepCount; i++) {
                 Step step = steps.get(i);
-                synchronized (this) {
+                synchronized (mLock) {
                     if (step instanceof VibrateStep) {
                         mCurrentVibrateStep = (VibrateStep) step;
                     } else {
@@ -295,21 +300,48 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
      * Sleeps until given {@code wakeUpTime}.
      *
      * <p>This stops immediately when {@link #cancel()} is called.
+     *
+     * @return true if waited until wake-up time, false if it was cancelled.
      */
-    private void waitUntil(long wakeUpTime) {
+    private boolean waitUntil(long wakeUpTime) {
-        synchronized (this) {
+        synchronized (mLock) {
             long durationRemaining = wakeUpTime - SystemClock.uptimeMillis();
             while (durationRemaining > 0) {
                 try {
-                    VibrationThread.this.wait(durationRemaining);
+                    mLock.wait(durationRemaining);
                 } catch (InterruptedException e) {
                 }
                 if (mForceStop) {
-                    break;
+                    return false;
                 }
                 durationRemaining = wakeUpTime - SystemClock.uptimeMillis();
             }
         }
+        return true;
+    }
+
+    /**
+     * Sleeps until given {@link VibrateStep#isVibrationComplete()}, or until {@code wakeUpTime}.
+     *
+     * <p>This stops immediately when {@link #cancel()} is called.
+     *
+     * @return true if finished on vibration complete, false if it was cancelled or timed out.
+     */
+    private boolean waitForVibrationComplete(VibrateStep step, long wakeUpTime) {
+        synchronized (mLock) {
+            long durationRemaining = wakeUpTime - SystemClock.uptimeMillis();
+            while (!step.isVibrationComplete() && durationRemaining > 0) {
+                try {
+                    mLock.wait(durationRemaining);
+                } catch (InterruptedException e) {
+                }
+                if (mForceStop) {
+                    return false;
+                }
+                durationRemaining = wakeUpTime - SystemClock.uptimeMillis();
+            }
+        }
+        return step.isVibrationComplete();
     }
 
     private void noteVibratorOn(long duration) {
@@ -341,6 +373,9 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
     private interface VibrateStep extends Step {
         /** Callback to notify a vibrator has finished playing a effect. */
         void vibratorComplete(int vibratorId);
+
+        /** Returns true if the vibration played by this step is complete. */
+        boolean isVibrationComplete();
     }
 
     /** Represent a vibration on a single vibrator. */
@@ -348,11 +383,20 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
         private final VibratorController mVibrator;
         private final VibrationEffect mEffect;
 
+        @GuardedBy("mLock")
+        private boolean mVibrationComplete;
+
         SingleVibrateStep(VibratorController vibrator, VibrationEffect effect) {
             mVibrator = vibrator;
             mEffect = effect;
         }
 
+        @GuardedBy("mLock")
+        @Override
+        public boolean isVibrationComplete() {
+            return mVibrationComplete;
+        }
+
         @Override
         public void vibratorComplete(int vibratorId) {
             if (mVibrator.getVibratorInfo().getId() != vibratorId) {
@@ -364,8 +408,9 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
                 return;
             }
             mVibrator.off();
-            synchronized (VibrationThread.this) {
+            synchronized (mLock) {
-                VibrationThread.this.notify();
+                mVibrationComplete = true;
+                mLock.notify();
             }
         }
 
@@ -384,12 +429,13 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
                     noteVibratorOn(duration);
                     // Vibration is playing with no need to control amplitudes, just wait for native
                     // callback or timeout.
-                    waitUntil(startTime + duration + CALLBACKS_EXTRA_TIMEOUT);
+                    if (waitForVibrationComplete(this,
-                    if (mForceStop) {
+                            startTime + duration + CALLBACKS_EXTRA_TIMEOUT)) {
-                        mVibrator.off();
+                        return Vibration.Status.FINISHED;
-                        return Vibration.Status.CANCELLED;
                     }
-                    return Vibration.Status.FINISHED;
+                    // Timed out or vibration cancelled. Stop vibrator anyway.
+                    mVibrator.off();
+                    return mForceStop ? Vibration.Status.CANCELLED : Vibration.Status.FINISHED;
                 }
 
                 startTime = SystemClock.uptimeMillis();
@@ -407,8 +453,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
                     noteVibratorOn(duration);
                 }
                 while (amplitudeStep != null) {
-                    waitUntil(amplitudeStep.startTime);
+                    if (!waitUntil(amplitudeStep.startTime)) {
-                    if (mForceStop) {
                         mVibrator.off();
                         return Vibration.Status.CANCELLED;
                     }
@@ -482,7 +527,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
         private final int mRequiredCapabilities;
         private final int[] mVibratorIds;
 
-        @GuardedBy("VibrationThread.this")
+        @GuardedBy("mLock")
         private int mActiveVibratorCounter;
 
         SyncedVibrateStep(SparseArray<VibrationEffect> effects) {
@@ -496,6 +541,12 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
             }
         }
 
+        @GuardedBy("mLock")
+        @Override
+        public boolean isVibrationComplete() {
+            return mActiveVibratorCounter <= 0;
+        }
+
         @Override
         public void vibratorComplete(int vibratorId) {
             VibrationEffect effect = mEffects.get(vibratorId);
@@ -508,10 +559,9 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
                 return;
             }
             mVibrators.get(vibratorId).off();
-            synchronized (VibrationThread.this) {
+            synchronized (mLock) {
-                if (--mActiveVibratorCounter <= 0) {
+                --mActiveVibratorCounter;
-                    VibrationThread.this.notify();
+                mLock.notify();
-                }
             }
         }
 
@@ -532,8 +582,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
 
                 while (!nextSteps.isEmpty()) {
                     AmplitudeStep step = nextSteps.poll();
-                    waitUntil(step.startTime);
+                    if (!waitUntil(step.startTime)) {
-                    if (mForceStop) {
                         stopAllVibrators();
                         return Vibration.Status.CANCELLED;
                     }
@@ -541,7 +590,7 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
                     AmplitudeStep nextStep = step.nextStep();
                     if (nextStep == null) {
                         // This vibrator has finished playing the effect for this step.
-                        synchronized (VibrationThread.this) {
+                        synchronized (mLock) {
                             mActiveVibratorCounter--;
                         }
                     } else {
@@ -549,19 +598,18 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
                     }
                 }
 
-                // All OneShot and Waveform effects have finished. Just wait for the other effects
+                synchronized (mLock) {
-                // to end via native callbacks before finishing this synced step.
+                    // All OneShot and Waveform effects have finished. Just wait for the other
-                synchronized (VibrationThread.this) {
+                    // effects to end via native callbacks before finishing this synced step.
-                    if (mActiveVibratorCounter > 0) {
+                    final long wakeUpTime = startTime + timeout + CALLBACKS_EXTRA_TIMEOUT;
-                        waitUntil(startTime + timeout + CALLBACKS_EXTRA_TIMEOUT);
+                    if (mActiveVibratorCounter <= 0 || waitForVibrationComplete(this, wakeUpTime)) {
+                        return Vibration.Status.FINISHED;
                     }
-                }
-                if (mForceStop) {
-                    stopAllVibrators();
-                    return Vibration.Status.CANCELLED;
-                }
 
-                return Vibration.Status.FINISHED;
+                    // Timed out or vibration cancelled. Stop all vibrators anyway.
+                    stopAllVibrators();
+                    return mForceStop ? Vibration.Status.CANCELLED : Vibration.Status.FINISHED;
+                }
             } finally {
                 if (timeout > 0) {
                     noteVibratorOff();
@@ -774,8 +822,10 @@ public final class VibrationThread extends Thread implements IBinder.DeathRecipi
                 if (DEBUG) {
                     Slog.d(TAG, "DelayStep of " + mDelay + "ms starting...");
                 }
-                waitUntil(SystemClock.uptimeMillis() + mDelay);
+                if (waitUntil(SystemClock.uptimeMillis() + mDelay)) {
-                return mForceStop ? Vibration.Status.CANCELLED : Vibration.Status.FINISHED;
+                    return Vibration.Status.FINISHED;
+                }
+                return Vibration.Status.CANCELLED;
             } finally {
                 if (DEBUG) {
                     Slog.d(TAG, "DelayStep done.");