From e6bbe4660e9db9790bae6ebd08cba2767f86ed5a Mon Sep 17 00:00:00 2001 From: Peter Li Date: Thu, 5 May 2022 18:43:35 +0800 Subject: [PATCH] Inform Assistant when security exception occurs during onDetected By the behavior, the interactor of assistant needs to call the startRecognition function again after hotword event is triggered. Otherwise it will not receive the result of detection. Currently when security exception occurs during onDetected, we won't inform the interactor of assistant about the hotword event. It will cause the interactor still to wait the hotword event and doesn't call startRecognition function again. After syncing with assistant, we should use onError() callback to inform interactor. The behavior of onError() in the interactor: For both of DSP and Software, assistant will call startRecognition function. The behavior of onRejected() in the interactor: Only for DSP, assistant will call startRecognition function. For Software, assistant doesn't expect to get onRejected() Bug: 230738063 Test: atest HotwordDetectionServiceBasicTest Change-Id: I3083fd5a4a78ad9d2f35acd7fa1a4f2036cf64de Merged-In: Ideb210d4364d4c48c747d66268d77a2f5b017186 --- .../voice/AbstractHotwordDetector.java | 9 +++ ...wordDetectionVoiceInteractionCallback.aidl | 5 ++ .../voice/SoftwareHotwordDetector.java | 9 +++ .../HotwordDetectionConnection.java | 75 ++++++++++++------- 4 files changed, 71 insertions(+), 27 deletions(-) diff --git a/core/java/android/service/voice/AbstractHotwordDetector.java b/core/java/android/service/voice/AbstractHotwordDetector.java index 01d5638461af8..b2bf9bc2ddd4d 100644 --- a/core/java/android/service/voice/AbstractHotwordDetector.java +++ b/core/java/android/service/voice/AbstractHotwordDetector.java @@ -189,5 +189,14 @@ abstract class AbstractHotwordDetector implements HotwordDetector { .setHotwordDetectedResult(hotwordDetectedResult) .build())); } + + /** Called when the detection fails due to an error. */ + @Override + public void onError() { + Slog.v(TAG, "BinderCallback#onError"); + mHandler.sendMessage(obtainMessage( + HotwordDetector.Callback::onError, + mCallback)); + } } } diff --git a/core/java/android/service/voice/IMicrophoneHotwordDetectionVoiceInteractionCallback.aidl b/core/java/android/service/voice/IMicrophoneHotwordDetectionVoiceInteractionCallback.aidl index 80f20fe405b12..e8650894ac14b 100644 --- a/core/java/android/service/voice/IMicrophoneHotwordDetectionVoiceInteractionCallback.aidl +++ b/core/java/android/service/voice/IMicrophoneHotwordDetectionVoiceInteractionCallback.aidl @@ -33,4 +33,9 @@ oneway interface IMicrophoneHotwordDetectionVoiceInteractionCallback { in HotwordDetectedResult hotwordDetectedResult, in AudioFormat audioFormat, in ParcelFileDescriptor audioStream); + + /** + * Called when the detection fails due to an error. + */ + void onError(); } diff --git a/core/java/android/service/voice/SoftwareHotwordDetector.java b/core/java/android/service/voice/SoftwareHotwordDetector.java index 2d662eaf0a4f0..f5a0c66f7b1b8 100644 --- a/core/java/android/service/voice/SoftwareHotwordDetector.java +++ b/core/java/android/service/voice/SoftwareHotwordDetector.java @@ -155,6 +155,15 @@ class SoftwareHotwordDetector extends AbstractHotwordDetector { .setHotwordDetectedResult(hotwordDetectedResult) .build())); } + + /** Called when the detection fails due to an error. */ + @Override + public void onError() { + Slog.v(TAG, "BinderCallback#onError"); + mHandler.sendMessage(obtainMessage( + HotwordDetector.Callback::onError, + mCallback)); + } } private static class InitializationStateListener diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java index 2cb3982047435..22e0d0831dc75 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java @@ -124,6 +124,10 @@ final class HotwordDetectionConnection { private static final long RESET_DEBUG_HOTWORD_LOGGING_TIMEOUT_MILLIS = 60 * 60 * 1000; // 1 hour private static final int MAX_ISOLATED_PROCESS_NUMBER = 10; + // The error codes are used for onError callback + private static final int HOTWORD_DETECTION_SERVICE_DIED = -1; + private static final int CALLBACK_ONDETECTED_GOT_SECURITY_EXCEPTION = -2; + // Hotword metrics private static final int METRICS_INIT_UNKNOWN_TIMEOUT = HOTWORD_DETECTION_SERVICE_INIT_RESULT_REPORTED__RESULT__CALLBACK_INIT_STATE_UNKNOWN_TIMEOUT; @@ -420,19 +424,24 @@ final class HotwordDetectionConnection { Slog.d(TAG, "onDetected"); } synchronized (mLock) { - if (mPerformingSoftwareHotwordDetection) { - enforcePermissionsForDataDelivery(); - mSoftwareCallback.onDetected(result, null, null); - mPerformingSoftwareHotwordDetection = false; - if (result != null) { - Slog.i(TAG, "Egressed " + HotwordDetectedResult.getUsageSize(result) - + " bits from hotword trusted process"); - if (mDebugHotwordLogging) { - Slog.i(TAG, "Egressed detected result: " + result); - } - } - } else { + if (!mPerformingSoftwareHotwordDetection) { Slog.i(TAG, "Hotword detection has already completed"); + return; + } + mPerformingSoftwareHotwordDetection = false; + try { + enforcePermissionsForDataDelivery(); + } catch (SecurityException e) { + mSoftwareCallback.onError(); + return; + } + mSoftwareCallback.onDetected(result, null, null); + if (result != null) { + Slog.i(TAG, "Egressed " + HotwordDetectedResult.getUsageSize(result) + + " bits from hotword trusted process"); + if (mDebugHotwordLogging) { + Slog.i(TAG, "Egressed detected result: " + result); + } } } } @@ -513,19 +522,24 @@ final class HotwordDetectionConnection { public void onDetected(HotwordDetectedResult result) throws RemoteException { Slog.v(TAG, "onDetected"); synchronized (mLock) { - if (mValidatingDspTrigger) { - mValidatingDspTrigger = false; - enforcePermissionsForDataDelivery(); - externalCallback.onKeyphraseDetected(recognitionEvent, result); - if (result != null) { - Slog.i(TAG, "Egressed " + HotwordDetectedResult.getUsageSize(result) - + " bits from hotword trusted process"); - if (mDebugHotwordLogging) { - Slog.i(TAG, "Egressed detected result: " + result); - } - } - } else { + if (!mValidatingDspTrigger) { Slog.i(TAG, "Ignored hotword detected since trigger has been handled"); + return; + } + mValidatingDspTrigger = false; + try { + enforcePermissionsForDataDelivery(); + } catch (SecurityException e) { + externalCallback.onError(CALLBACK_ONDETECTED_GOT_SECURITY_EXCEPTION); + return; + } + externalCallback.onKeyphraseDetected(recognitionEvent, result); + if (result != null) { + Slog.i(TAG, "Egressed " + HotwordDetectedResult.getUsageSize(result) + + " bits from hotword trusted process"); + if (mDebugHotwordLogging) { + Slog.i(TAG, "Egressed detected result: " + result); + } } } } @@ -595,7 +609,8 @@ final class HotwordDetectionConnection { HotwordMetricsLogger.writeKeyphraseTriggerEvent( mDetectorType, METRICS_KEYPHRASE_TRIGGERED_DETECT_SECURITY_EXCEPTION); - throw e; + externalCallback.onError(CALLBACK_ONDETECTED_GOT_SECURITY_EXCEPTION); + return; } externalCallback.onKeyphraseDetected(recognitionEvent, result); if (result != null) { @@ -885,7 +900,13 @@ final class HotwordDetectionConnection { throws RemoteException { bestEffortClose(serviceAudioSink); bestEffortClose(serviceAudioSource); - enforcePermissionsForDataDelivery(); + try { + enforcePermissionsForDataDelivery(); + } catch (SecurityException e) { + bestEffortClose(audioSource); + callback.onError(); + return; + } callback.onDetected(triggerResult, null /* audioFormat */, null /* audioStream */); if (triggerResult != null) { @@ -984,7 +1005,7 @@ final class HotwordDetectionConnection { Slog.w(TAG, "binderDied"); try { - mCallback.onError(-1); + mCallback.onError(HOTWORD_DETECTION_SERVICE_DIED); } catch (RemoteException e) { Slog.w(TAG, "Failed to report onError status: " + e); }