From dde8e3e9db9a075af3006d3c040f58f7b0424a92 Mon Sep 17 00:00:00 2001 From: Peter Li Date: Mon, 9 May 2022 12:50:21 +0000 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 Change-Id: Ideb210d4364d4c48c747d66268d77a2f5b017186 Test: atest HotwordDetectionServiceBasicTest --- .../voice/AbstractHotwordDetector.java | 9 +++ ...wordDetectionVoiceInteractionCallback.aidl | 5 ++ .../voice/SoftwareHotwordDetector.java | 9 +++ .../HotwordDetectionConnection.java | 73 ++++++++++++------- 4 files changed, 70 insertions(+), 26 deletions(-) diff --git a/core/java/android/service/voice/AbstractHotwordDetector.java b/core/java/android/service/voice/AbstractHotwordDetector.java index 5b3b78b65786e..84d67116db3a8 100644 --- a/core/java/android/service/voice/AbstractHotwordDetector.java +++ b/core/java/android/service/voice/AbstractHotwordDetector.java @@ -204,5 +204,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 36ea91e4edb57..6600fb7bcdf69 100644 --- a/core/java/android/service/voice/SoftwareHotwordDetector.java +++ b/core/java/android/service/voice/SoftwareHotwordDetector.java @@ -166,6 +166,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 99fcbc7dc87a0..f5b5e1aadcf88 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java @@ -125,6 +125,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; @@ -421,19 +425,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); + } } } } @@ -514,20 +523,25 @@ final class HotwordDetectionConnection { public void onDetected(HotwordDetectedResult result) throws RemoteException { Slog.v(TAG, "onDetected"); synchronized (mLock) { - if (mValidatingDspTrigger) { - mValidatingDspTrigger = false; + if (!mValidatingDspTrigger) { + Slog.i(TAG, "Ignored hotword detected since trigger has been handled"); + return; + } + mValidatingDspTrigger = false; + try { enforcePermissionsForDataDelivery(); enforceExtraKeyphraseIdNotLeaked(result, recognitionEvent); - 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); - } + } 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); } - } else { - Slog.i(TAG, "Ignored hotword detected since trigger has been handled"); } } } @@ -598,7 +612,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) { @@ -888,7 +903,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) { @@ -988,7 +1009,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); }