From d3bee4f42dcb1524539d64ee475e0bc9085f3497 Mon Sep 17 00:00:00 2001 From: Ytai Ben-Tsvi Date: Mon, 19 Jul 2021 05:43:56 -0700 Subject: [PATCH] Restart recognition when failing to deliver event If the client does not know that recognition is not running due to a recognition event not having been delivered, we should attempt to restart it. Bug: 193579626 Test: Manual verification of the scenario described in the bug. Change-Id: I4fc3b3e8defed59a900fd156273e9e695a322b0c --- .../SoundTriggerMiddlewarePermission.java | 18 +++------- .../SoundTriggerMiddlewareValidation.java | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java index 32ba852634bea..76927e15a0f7f 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java @@ -121,13 +121,8 @@ public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddleware * Throws a {@link SecurityException} iff the originator has permission to receive data. */ void enforcePermissionsForDataDelivery(@NonNull Identity identity, @NonNull String reason) { - // TODO(b/186164881): remove - // START TEMP HACK - enforcePermissionForPreflight(mContext, identity, RECORD_AUDIO); - int hotwordOp = AppOpsManager.strOpToOp(AppOpsManager.OPSTR_RECORD_AUDIO_HOTWORD); - mContext.getSystemService(AppOpsManager.class).noteOpNoThrow(hotwordOp, identity.uid, - identity.packageName, identity.attributionTag, reason); - // END TEMP HACK + enforcePermissionForDataDelivery(mContext, identity, RECORD_AUDIO, + reason); enforcePermissionForDataDelivery(mContext, identity, CAPTURE_AUDIO_HOTWORD, reason); } @@ -155,8 +150,8 @@ public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddleware /** * Throws a {@link SecurityException} if originator permanently doesn't have the given - * permission, or a {@link ServiceSpecificException} with a {@link - * Status#TEMPORARY_PERMISSION_DENIED} if caller originator doesn't have the given permission. + * permission. + * Soft (temporary) denials are considered OK for preflight purposes. * * @param context A {@link Context}, used for permission checks. * @param identity The identity to check. @@ -168,15 +163,12 @@ public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddleware permission); switch (status) { case PermissionChecker.PERMISSION_GRANTED: + case PermissionChecker.PERMISSION_SOFT_DENIED: return; case PermissionChecker.PERMISSION_HARD_DENIED: throw new SecurityException( String.format("Failed to obtain permission %s for identity %s", permission, ObjectPrinter.print(identity, true, 16))); - case PermissionChecker.PERMISSION_SOFT_DENIED: - throw new ServiceSpecificException(Status.TEMPORARY_PERMISSION_DENIED, - String.format("Failed to obtain permission %s for identity %s", permission, - ObjectPrinter.print(identity, true, 16))); default: throw new RuntimeException("Unexpected perimission check result."); } diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java index d9fa79211a894..7b31946e0c9dc 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java @@ -279,6 +279,9 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware /** Activity state. */ Activity activityState = Activity.LOADED; + /** Recognition config, used to start the model. */ + RecognitionConfig config; + /** Human-readable description of the model. */ final String description; @@ -455,6 +458,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware // From here on, every exception isn't client's fault. try { mDelegate.startRecognition(modelHandle, config); + modelState.config = config; modelState.activityState = ModelState.Activity.ACTIVE; } catch (Exception e) { throw handleException(e); @@ -512,6 +516,27 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware } } + private void restartIfIntercepted(int modelHandle) { + synchronized (SoundTriggerMiddlewareValidation.this) { + // State validation. + if (mState == ModuleStatus.DETACHED) { + return; + } + ModelState modelState = mLoadedModels.get(modelHandle); + if (modelState == null + || modelState.activityState != ModelState.Activity.INTERCEPTED) { + return; + } + try { + mDelegate.startRecognition(modelHandle, modelState.config); + modelState.activityState = ModelState.Activity.ACTIVE; + Log.i(TAG, "Restarted intercepted model " + modelHandle); + } catch (Exception e) { + Log.i(TAG, "Failed to restart intercepted model " + modelHandle, e); + } + } + } + @Override public void forceRecognitionEvent(int modelHandle) { // Input validation (always valid). @@ -720,6 +745,11 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware ModelState modelState = mLoadedModels.get(modelHandle); if (event.status != RecognitionStatus.FORCED) { modelState.activityState = ModelState.Activity.INTERCEPTED; + // If we failed to deliver an actual event to the client, they would + // never know to restart it whenever circumstances change. Thus, we + // restart it here. We do this from a separate thread to avoid any + // race conditions. + new Thread(() -> restartIfIntercepted(modelHandle)).start(); } } } @@ -744,6 +774,11 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware ModelState modelState = mLoadedModels.get(modelHandle); if (event.common.status != RecognitionStatus.FORCED) { modelState.activityState = ModelState.Activity.INTERCEPTED; + // If we failed to deliver an actual event to the client, they would + // never know to restart it whenever circumstances change. Thus, we + // restart it here. We do this from a separate thread to avoid any + // race conditions. + new Thread(() -> restartIfIntercepted(modelHandle)).start(); } } }