From 9544903ba880ae7acd8096d1a9c3b48965429339 Mon Sep 17 00:00:00 2001 From: Felix Oghina Date: Thu, 27 Apr 2023 13:30:34 +0000 Subject: [vims] better handle assistant force stop Currently, force stopping a 3p assistant app resets it to the default a ssistant. This is not a desirable user experience. This seems to be accidental, some history that I was able to dig up: * b/20882522 - "clearing data in assistant causes it to die and not respond to queries" - this will happen today anyway, because the in-app setting for it gets turned off * the fix for the above ended up resetting the assistant setting when the assistant is force stopped (as a proxy for data cleared) * this caused b/121104681 - force stopping assistant resets the default assistant setting * also b/124450140 - clearing assistant resets default to none * the fixes for these included clearing the assistant role profile, which ends up setting it to the "fallback" app (set in roles.xml). This fix removes any role and setting clearing / resetting when force stopping or clearing the active assistant app. It keeps the part that ensures the service is restarted, i.e. the original bug is still fixed, i.e. assistant responds to queries after being force stopped (but not cleared, which has never worked anyway). Fixes: 191743558 Test: atest CtsVoiceInteractionTestCases Change-Id: I180e699bd8d3fb5ea6aa4807f435060999436416 (cherry picked from commit ba9625e664c76943448ce5b7d97e3b381e71710d) --- .../VoiceInteractionManagerService.java | 24 +--------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index 423a81ac0523..605af03f112f 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -2343,8 +2343,7 @@ public class VoiceInteractionManagerService extends SystemService { } } if (hitInt && doit) { - // The user is force stopping our current interactor. - // Clear the current settings and restore default state. + // The user is force stopping our current interactor, restart the service. synchronized (VoiceInteractionManagerServiceStub.this) { Slog.i(TAG, "Force stopping current voice interactor: " + getCurInteractor(userHandle)); @@ -2353,28 +2352,7 @@ public class VoiceInteractionManagerService extends SystemService { mImpl.shutdownLocked(); setImplLocked(null); } - - setCurInteractor(null, userHandle); - // TODO: should not reset null here. But even remove this line, the - // initForUser() still reset it because the interactor will be null. Keep - // it now but we should still need to fix it. - setCurRecognizer(null, userHandle); - resetCurAssistant(userHandle); - initForUser(userHandle); switchImplementationIfNeededLocked(true); - - // When resetting the interactor, the recognizer and the assistant settings - // value, we also need to reset the assistant role to keep the values - // consistent. Clear the assistant role will reset to the default value. - Context context = getContext(); - context.getSystemService(RoleManager.class).clearRoleHoldersAsUser( - RoleManager.ROLE_ASSISTANT, 0, UserHandle.of(userHandle), - context.getMainExecutor(), successful -> { - if (!successful) { - Slog.e(TAG, - "Failed to clear default assistant for force stop"); - } - }); } } else if (hitRec && doit) { // We are just force-stopping the current recognizer, which is not -- cgit v1.2.3-59-g8ed1b