Tighten the checks around a detector being invalidated

Don't call back for a detector being marked invalid because
that happens when someone else obtains a detector or VIS shuts down,
in either case we don't want a loop where two entities keep creating new detectors
and being invalidated.

Don't call back on an invalid detector for availability change/detected/started and stopped
only propagate errors.

This helps us with cases where a callback for the previous VIS may get called and then crash because it
tries to make calls without being the current VIS.

In the new scheme of things, if the VIS changes, or the current VIS obtains a new AlwaysOnHotwordDetector,
the previous one is shutdown and internally marked as invalid and all calls to it fail with an IllegalStateException.

Bug: 16629417
Change-Id: I74417bf76ba80916ebc21b042c18b3467857733e

3 files changed, 60 insertions, 12 deletions

diff --git a/api/current.txt b/api/current.txt
index c5027ae..28cc5d9 100644
--- a/api/current.txt
+++ b/api/current.txt
@@ -27502,7 +27502,6 @@ package android.service.voice {
     field public static final int RECOGNITION_MODE_USER_IDENTIFICATION = 2; // 0x2
     field public static final int RECOGNITION_MODE_VOICE_TRIGGER = 1; // 0x1
     field public static final int STATE_HARDWARE_UNAVAILABLE = -2; // 0xfffffffe
-    field public static final int STATE_INVALID = -3; // 0xfffffffd
     field public static final int STATE_KEYPHRASE_ENROLLED = 2; // 0x2
     field public static final int STATE_KEYPHRASE_UNENROLLED = 1; // 0x1
     field public static final int STATE_KEYPHRASE_UNSUPPORTED = -1; // 0xffffffff
diff --git a/core/java/android/service/voice/AlwaysOnHotwordDetector.java b/core/java/android/service/voice/AlwaysOnHotwordDetector.java
index 3f53ad4..6278e69 100644
--- a/core/java/android/service/voice/AlwaysOnHotwordDetector.java
+++ b/core/java/android/service/voice/AlwaysOnHotwordDetector.java
@@ -45,7 +45,8 @@ public class AlwaysOnHotwordDetector {
      * Indicates that this hotword detector is no longer valid for any recognition
      * and should not be used anymore.
      */
-    public static final int STATE_INVALID = -3;
+    private static final int STATE_INVALID = -3;
+
     /**
      * Indicates that recognition for the given keyphrase is not available on the system
      * because of the hardware configuration.
@@ -156,9 +157,6 @@ public class AlwaysOnHotwordDetector {
          * If it is {@link #STATE_KEYPHRASE_UNENROLLED} the caller may choose to begin
          * an enrollment flow for the keyphrase. <br/>
          * and for {@link #STATE_KEYPHRASE_ENROLLED} a recognition can be started as desired. <p/>
-         *
-         * If the return code is {@link #STATE_INVALID}, this detector is stale.
-         * A new detector should be obtained for use in the future.
          */
         void onAvailabilityChanged(int status);
         /**
@@ -220,6 +218,9 @@ public class AlwaysOnHotwordDetector {
      * @throws UnsupportedOperationException if the keyphrase itself isn't supported.
      *         Callers should only call this method after a supported state callback on
      *         {@link Callback#onAvailabilityChanged(int)} to avoid this exception.
+     * @throws IllegalStateException if the detector is in an invalid state.
+     *         This may happen if another detector has been instantiated or the
+     *         {@link VoiceInteractionService} hosting this detector has been shut down.
      */
     public int getSupportedRecognitionModes() {
         synchronized (mLock) {
@@ -228,6 +229,11 @@ public class AlwaysOnHotwordDetector {
     }
 
     private int getSupportedRecognitionModesLocked() {
+        if (mAvailability == STATE_INVALID) {
+            throw new IllegalStateException(
+                    "getSupportedRecognitionModes called on an invalid detector");
+        }
+
         // This method only makes sense if we can actually support a recognition.
         if (mAvailability != STATE_KEYPHRASE_ENROLLED
                 && mAvailability != STATE_KEYPHRASE_UNENROLLED) {
@@ -247,9 +253,16 @@ public class AlwaysOnHotwordDetector {
      * @throws UnsupportedOperationException if the recognition isn't supported.
      *         Callers should only call this method after a supported state callback on
      *         {@link Callback#onAvailabilityChanged(int)} to avoid this exception.
+     * @throws IllegalStateException if the detector is in an invalid state.
+     *         This may happen if another detector has been instantiated or the
+     *         {@link VoiceInteractionService} hosting this detector has been shut down.
      */
     public void startRecognition(int recognitionFlags) {
         synchronized (mLock) {
+            if (mAvailability == STATE_INVALID) {
+                throw new IllegalStateException("startRecognition called on an invalid detector");
+            }
+
             // Check if we can start/stop a recognition.
             if (mAvailability != STATE_KEYPHRASE_ENROLLED) {
                 throw new UnsupportedOperationException(
@@ -268,9 +281,16 @@ public class AlwaysOnHotwordDetector {
      * @throws UnsupportedOperationException if the recognition isn't supported.
      *         Callers should only call this method after a supported state callback on
      *         {@link Callback#onAvailabilityChanged(int)} to avoid this exception.
+     * @throws IllegalStateException if the detector is in an invalid state.
+     *         This may happen if another detector has been instantiated or the
+     *         {@link VoiceInteractionService} hosting this detector has been shut down.
      */
     public void stopRecognition() {
         synchronized (mLock) {
+            if (mAvailability == STATE_INVALID) {
+                throw new IllegalStateException("stopRecognition called on an invalid detector");
+            }
+
             // Check if we can start/stop a recognition.
             if (mAvailability != STATE_KEYPHRASE_ENROLLED) {
                 throw new UnsupportedOperationException(
@@ -293,14 +313,28 @@ public class AlwaysOnHotwordDetector {
      * @throws UnsupportedOperationException if managing they keyphrase isn't supported.
      *         Callers should only call this method after a supported state callback on
      *         {@link Callback#onAvailabilityChanged(int)} to avoid this exception.
+     * @throws IllegalStateException if the detector is in an invalid state.
+     *         This may happen if another detector has been instantiated or the
+     *         {@link VoiceInteractionService} hosting this detector has been shut down.
      */
     public Intent getManageIntent(int action) {
+        synchronized (mLock) {
+            return getManageIntentLocked(action);
+        }
+    }
+
+    private Intent getManageIntentLocked(int action) {
+        if (mAvailability == STATE_INVALID) {
+            throw new IllegalStateException("getManageIntent called on an invalid detector");
+        }
+
         // This method only makes sense if we can actually support a recognition.
         if (mAvailability != STATE_KEYPHRASE_ENROLLED
                 && mAvailability != STATE_KEYPHRASE_UNENROLLED) {
             throw new UnsupportedOperationException(
                     "Managing the given keyphrase is not supported");
         }
+
         if (action != MANAGE_ACTION_ENROLL
                 && action != MANAGE_ACTION_RE_ENROLL
                 && action != MANAGE_ACTION_UN_ENROLL) {
@@ -387,7 +421,6 @@ public class AlwaysOnHotwordDetector {
             if (DBG) Slog.d(TAG, "starting recognition...");
             int status = startRecognitionLocked();
             if (status == STATUS_OK) {
-                mInternalState |= FLAG_STARTED;
                 mHandler.sendEmptyMessage(MSG_DETECTION_STARTED);
             } else {
                 if (DBG) Slog.d(TAG, "failed to start recognition: " + status);
@@ -404,7 +437,6 @@ public class AlwaysOnHotwordDetector {
             if (DBG) Slog.d(TAG, "stopping recognition...");
             int status = stopRecognitionLocked();
             if (status == STATUS_OK) {
-                mInternalState &= ~FLAG_STARTED;
                 if (!requested) mHandler.sendEmptyMessage(MSG_DETECTION_STOPPED);
             } else {
                 if (!requested) mHandler.sendEmptyMessage(MSG_DETECTION_ERROR);
@@ -483,20 +515,42 @@ public class AlwaysOnHotwordDetector {
     class MyHandler extends Handler {
         @Override
         public void handleMessage(Message msg) {
+            synchronized (mLock) {
+                if (mAvailability == STATE_INVALID) {
+                    Slog.w(TAG, "Received message: " + msg.what + " for an invalid detector");
+                    return;
+                }
+            }
+
             switch (msg.what) {
                 case MSG_STATE_CHANGED:
                     mExternalCallback.onAvailabilityChanged(msg.arg1);
                     break;
                 case MSG_HOTWORD_DETECTED:
+                    synchronized (mLock) {
+                        mInternalState &= ~FLAG_REQUESTED;
+                        mInternalState &= ~FLAG_STARTED;
+                    }
                     mExternalCallback.onDetected((byte[]) msg.obj);
                     break;
                 case MSG_DETECTION_STARTED:
+                    synchronized (mLock) {
+                        mInternalState |= FLAG_STARTED;
+                    }
                     mExternalCallback.onDetectionStarted();
                     break;
                 case MSG_DETECTION_STOPPED:
+                    synchronized (mLock) {
+                        mInternalState &= ~FLAG_REQUESTED;
+                        mInternalState &= ~FLAG_STARTED;
+                    }
                     mExternalCallback.onDetectionStopped();
                     break;
                 case MSG_DETECTION_ERROR:
+                    synchronized (mLock) {
+                        mInternalState &= ~FLAG_REQUESTED;
+                        mInternalState &= ~FLAG_STARTED;
+                    }
                     mExternalCallback.onError();
                     break;
                 default:
diff --git a/tests/VoiceInteraction/src/com/android/test/voiceinteraction/MainInteractionService.java b/tests/VoiceInteraction/src/com/android/test/voiceinteraction/MainInteractionService.java
index 49c3d0a..b43ad6f 100644
--- a/tests/VoiceInteraction/src/com/android/test/voiceinteraction/MainInteractionService.java
+++ b/tests/VoiceInteraction/src/com/android/test/voiceinteraction/MainInteractionService.java
@@ -81,11 +81,6 @@ public class MainInteractionService extends VoiceInteractionService {
     private void hotwordAvailabilityChangeHelper(int availability) {
         Log.i(TAG, "Hotword availability = " + availability);
         switch (availability) {
-            case AlwaysOnHotwordDetector.STATE_INVALID:
-                Log.i(TAG, "STATE_INVALID");
-                mHotwordDetector =
-                        createAlwaysOnHotwordDetector("Hello There", "en-US", mHotwordCallback);
-                break;
             case AlwaysOnHotwordDetector.STATE_HARDWARE_UNAVAILABLE:
                 Log.i(TAG, "STATE_HARDWARE_UNAVAILABLE");
                 break;