From 008f1d29146728c1b3cc30f6d7123c5e0585fc9a Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Sun, 17 Jan 2021 16:10:14 -0800 Subject: [PATCH 1/5] VoiceInteractionServiceInfo cleanup: Nullability, javadoc, try-with, unused method. Bug: 178410946 Test: atest CtsVoiceInteractionTestCases Change-Id: I62a9480f9046e328f16c6f7ecff77570da8d0154 --- .../voice/VoiceInteractionServiceInfo.java | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/core/java/android/service/voice/VoiceInteractionServiceInfo.java b/core/java/android/service/voice/VoiceInteractionServiceInfo.java index e1a9a050212c0..7203f2d86f5f0 100644 --- a/core/java/android/service/voice/VoiceInteractionServiceInfo.java +++ b/core/java/android/service/voice/VoiceInteractionServiceInfo.java @@ -17,6 +17,7 @@ package android.service.voice; import android.Manifest; +import android.annotation.NonNull; import android.app.AppGlobals; import android.content.ComponentName; import android.content.pm.PackageManager; @@ -28,6 +29,7 @@ import android.os.RemoteException; import android.util.AttributeSet; import android.util.Log; import android.util.Xml; + import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -47,17 +49,21 @@ public class VoiceInteractionServiceInfo { private boolean mSupportsLaunchFromKeyguard; private boolean mSupportsLocalInteraction; - public VoiceInteractionServiceInfo(PackageManager pm, ComponentName comp) - throws PackageManager.NameNotFoundException { - this(pm, pm.getServiceInfo(comp, PackageManager.GET_META_DATA)); - } - - public VoiceInteractionServiceInfo(PackageManager pm, ComponentName comp, int userHandle) + /** + * Loads the service metadata published by the component. Success is indicated by + * {@link #getParseError()}. + * + * @param pm A PackageManager from which the XML can be loaded. + * @param comp The {@link VoiceInteractionService} component. + */ + public VoiceInteractionServiceInfo( + @NonNull PackageManager pm, @NonNull ComponentName comp, int userHandle) throws PackageManager.NameNotFoundException { this(pm, getServiceInfoOrThrow(comp, userHandle)); } - static ServiceInfo getServiceInfoOrThrow(ComponentName comp, int userHandle) + @NonNull + private static ServiceInfo getServiceInfoOrThrow(@NonNull ComponentName comp, int userHandle) throws PackageManager.NameNotFoundException { try { ServiceInfo si = AppGlobals.getPackageManager().getServiceInfo(comp, @@ -74,20 +80,23 @@ public class VoiceInteractionServiceInfo { throw new PackageManager.NameNotFoundException(comp.toString()); } - public VoiceInteractionServiceInfo(PackageManager pm, ServiceInfo si) { - if (si == null) { - mParseError = "Service not available"; - return; - } + /** + * Loads the service metadata published by the component. Success is indicated by + * {@link #getParseError()}. + * + * @param pm A PackageManager from which the XML can be loaded; usually the PackageManager + * from which {@code si} was originally retrieved. + * @param si The {@link VoiceInteractionService} info. + */ + public VoiceInteractionServiceInfo(@NonNull PackageManager pm, @NonNull ServiceInfo si) { if (!Manifest.permission.BIND_VOICE_INTERACTION.equals(si.permission)) { mParseError = "Service does not require permission " + Manifest.permission.BIND_VOICE_INTERACTION; return; } - XmlResourceParser parser = null; - try { - parser = si.loadXmlMetaData(pm, VoiceInteractionService.SERVICE_META_DATA); + try (XmlResourceParser parser = si.loadXmlMetaData(pm, + VoiceInteractionService.SERVICE_META_DATA)) { if (parser == null) { mParseError = "No " + VoiceInteractionService.SERVICE_META_DATA + " meta-data for " + si.packageName; @@ -134,20 +143,10 @@ public class VoiceInteractionServiceInfo { mParseError = "No recognitionService specified"; return; } - } catch (XmlPullParserException e) { + } catch (XmlPullParserException | IOException | PackageManager.NameNotFoundException e) { mParseError = "Error parsing voice interation service meta-data: " + e; Log.w(TAG, "error parsing voice interaction service meta-data", e); return; - } catch (IOException e) { - mParseError = "Error parsing voice interation service meta-data: " + e; - Log.w(TAG, "error parsing voice interaction service meta-data", e); - return; - } catch (PackageManager.NameNotFoundException e) { - mParseError = "Error parsing voice interation service meta-data: " + e; - Log.w(TAG, "error parsing voice interaction service meta-data", e); - return; - } finally { - if (parser != null) parser.close(); } mServiceInfo = si; } From cc6f2a78e4cf2082b1168e4f83ec757bf1869433 Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Sun, 17 Jan 2021 17:05:24 -0800 Subject: [PATCH 2/5] VIMS: Remove useless field mCurUserUnlocked. It's never set to true and is only used for logging anyway. Bug: 178410946 Test: atest CtsVoiceInteractionTestCases Change-Id: Ie4370c2e9ddd305daad90f1393897e30a2dee8ac --- .../voiceinteraction/VoiceInteractionManagerService.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index 9f203e1090bd3..7026de7a8462b 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -238,7 +238,6 @@ public class VoiceInteractionManagerService extends SystemService { private boolean mSafeMode; private int mCurUser; - private boolean mCurUserUnlocked; private boolean mCurUserSupported; @GuardedBy("this") @@ -494,7 +493,6 @@ public class VoiceInteractionManagerService extends SystemService { FgThread.getHandler().post(() -> { synchronized (this) { setCurrentUserLocked(userHandle); - mCurUserUnlocked = false; switchImplementationIfNeededLocked(false); } }); @@ -1458,7 +1456,6 @@ public class VoiceInteractionManagerService extends SystemService { pw.println(" mEnableService: " + mEnableService); pw.println(" mTemporarilyDisabled: " + mTemporarilyDisabled); pw.println(" mCurUser: " + mCurUser); - pw.println(" mCurUserUnlocked: " + mCurUserUnlocked); pw.println(" mCurUserSupported: " + mCurUserSupported); dumpSupportedUsers(pw, " "); mDbHelper.dump(pw); From 54a505d46f02f12f0cb3e49bc5bfb976b1efd6f8 Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Sun, 17 Jan 2021 17:18:17 -0800 Subject: [PATCH 3/5] VoiceInteraction: Remove MATCH_DEBUG_TRIAGED_MISSING from PM queries. MATCH_DEBUG_TRIAGED_MISSING is the deprecated equivalent to MATCH_DIRECT_BOOT_AUTO. Not needed since we use MATCH_DIRECT_BOOT_AWARE and MATCH_DIRECT_BOOT_UNAWARE (see comments on ag/1101955). Bug: 178410946 Test: atest CtsVoiceInteractionTestCases Change-Id: I2093b163bb18e43c4f868715d60bc4b637fced6c --- .../android/service/voice/VoiceInteractionServiceInfo.java | 3 +-- .../voiceinteraction/VoiceInteractionManagerService.java | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/core/java/android/service/voice/VoiceInteractionServiceInfo.java b/core/java/android/service/voice/VoiceInteractionServiceInfo.java index 7203f2d86f5f0..f7710e6a82ce8 100644 --- a/core/java/android/service/voice/VoiceInteractionServiceInfo.java +++ b/core/java/android/service/voice/VoiceInteractionServiceInfo.java @@ -69,8 +69,7 @@ public class VoiceInteractionServiceInfo { ServiceInfo si = AppGlobals.getPackageManager().getServiceInfo(comp, PackageManager.GET_META_DATA | PackageManager.MATCH_DIRECT_BOOT_AWARE - | PackageManager.MATCH_DIRECT_BOOT_UNAWARE - | PackageManager.MATCH_DEBUG_TRIAGED_MISSING, + | PackageManager.MATCH_DIRECT_BOOT_UNAWARE, userHandle); if (si != null) { return si; diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index 7026de7a8462b..81d511f42415a 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -585,8 +585,7 @@ public class VoiceInteractionManagerService extends SystemService { mContext.getPackageManager().queryIntentServicesAsUser( new Intent(VoiceInteractionService.SERVICE_INTERFACE), PackageManager.MATCH_DIRECT_BOOT_AWARE - | PackageManager.MATCH_DIRECT_BOOT_UNAWARE - | PackageManager.MATCH_DEBUG_TRIAGED_MISSING, userHandle); + | PackageManager.MATCH_DIRECT_BOOT_UNAWARE, userHandle); int numAvailable = available.size(); if (numAvailable == 0) { From ba37dbb352d9cfaf25f607584c8784010a4a2c97 Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Sun, 17 Jan 2021 19:18:14 -0800 Subject: [PATCH 4/5] VIMS cleanup: set package on pm query instead of filtering results. Bug: 178410946 Test: atest CtsVoiceInteractionTestCases Change-Id: I8ab1824aba534857e2978861d001a74837de281d --- .../VoiceInteractionManagerService.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index 81d511f42415a..e5f194877cf24 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -583,7 +583,8 @@ public class VoiceInteractionManagerService extends SystemService { VoiceInteractionServiceInfo findAvailInteractor(int userHandle, String packageName) { List available = mContext.getPackageManager().queryIntentServicesAsUser( - new Intent(VoiceInteractionService.SERVICE_INTERFACE), + new Intent(VoiceInteractionService.SERVICE_INTERFACE) + .setPackage(packageName), PackageManager.MATCH_DIRECT_BOOT_AWARE | PackageManager.MATCH_DIRECT_BOOT_UNAWARE, userHandle); int numAvailable = available.size(); @@ -603,19 +604,16 @@ public class VoiceInteractionManagerService extends SystemService { VoiceInteractionServiceInfo info = new VoiceInteractionServiceInfo( mContext.getPackageManager(), comp, userHandle); if (info.getParseError() == null) { - if (packageName == null || info.getServiceInfo().packageName.equals( - packageName)) { - if (foundInfo == null) { - foundInfo = info; - } else { - Slog.w(TAG, "More than one voice interaction service, " - + "picking first " - + new ComponentName( - foundInfo.getServiceInfo().packageName, - foundInfo.getServiceInfo().name) - + " over " - + new ComponentName(cur.packageName, cur.name)); - } + if (foundInfo == null) { + foundInfo = info; + } else { + Slog.w(TAG, "More than one voice interaction service, " + + "picking first " + + new ComponentName( + foundInfo.getServiceInfo().packageName, + foundInfo.getServiceInfo().name) + + " over " + + new ComponentName(cur.packageName, cur.name)); } } else { Slog.w(TAG, "Bad interaction service " + comp + ": " From 61732f5f6901ce66cae605b880298c00642d55f4 Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Sun, 17 Jan 2021 19:32:23 -0800 Subject: [PATCH 5/5] VIMS: Simplify findAvailInteractor() by getting metadata in initial pm query. Also reorders the conditional blocks for readability. Currently, available interactors are queried and one of them is selected, then the metadata for it is retrieved through the VoiceInteractionServiceInfo constructor. With this change, both things are done in a single step. That might mean we retrieve some unneeded data (the metadata for the packages that aren't selected), but optimizing it isn't important since there's usually just one interactor. Bug: 178410946 Test: atest CtsVoiceInteractionTestCases Change-Id: I9a0d859f12d043f47f33a307609dbcf9de759e7e --- .../VoiceInteractionManagerService.java | 66 ++++++++----------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index e5f194877cf24..a5fea344f9810 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -585,48 +585,41 @@ public class VoiceInteractionManagerService extends SystemService { mContext.getPackageManager().queryIntentServicesAsUser( new Intent(VoiceInteractionService.SERVICE_INTERFACE) .setPackage(packageName), - PackageManager.MATCH_DIRECT_BOOT_AWARE + PackageManager.GET_META_DATA + | PackageManager.MATCH_DIRECT_BOOT_AWARE | PackageManager.MATCH_DIRECT_BOOT_UNAWARE, userHandle); int numAvailable = available.size(); - if (numAvailable == 0) { Slog.w(TAG, "no available voice interaction services found for user " + userHandle); return null; - } else { - // Find first system package. We never want to allow third party services to - // be automatically selected, because those require approval of the user. - VoiceInteractionServiceInfo foundInfo = null; - for (int i=0; i