Improve ACTION_PACKAGE_CHANGED handling in IMMS
This CL fixes a false negative case when handling
ACTION_PACKAGE_CHANGED in IMMS#MyPackageMonitor with greatly reducing
unnecessary false positives cases as well.
PackageMonitor#onPackageChanged(), which is the default handler of
ACTION_PACKAGE_CHANGED, returns true when and only when the entire
package state is changed to let InputMethodManagerService (IMMS)
rebuild the list of available IMEs when it returns true. Here we have
a false negative case and false positive cases.
Case 1 - false negative (Bug 28181208)
If ACTION_PACKAGE_CHANGED was about some components not the
entire package itself, currently MyPackageMonitor#onPackageChanged()
returns false and IMMS fails to rebuild the list of available IMEs.
Case 2 - false positive (contributing to Bug 32343335)
Even if ACTION_PACKAGE_CHANGED was about a package that implements
no IME service at all, currently MyPackageMonitor#onPackageChanged()
returns true and IMMS ends up with rebuilding the list of avilable
IMEs unnecessarily. Note that package replacement is a different
story that should be dealt with ACTION_PACKAGE_{ADDED, REMOVED}.
For both cases, luckily we can easily ask PackageManager to give the
list of relevant package names that might contain IMEs regardless of
enabled/disabled state, which is exactly what we want to use the watch
list for ACTION_PACKAGE_CHANGED events.
For the case 3, we can just check the current user ID.
Test: Manually verified as follows.
1. adb root
2. adb install -r LatinIME.apk
3. adb shell dumpsys input_method
Make sure that com.android.inputmethod.latin/.LatinIME is
recognized by IMMS.
4. adb shell pm disable com.android.inputmethod.latin/.LatinIME
5. adb shell dumpsys input_method
Make sure that com.android.inputmethod.latin/.LatinIME is
no longer recognized by IMMS.
6. adb shell pm enable com.android.inputmethod.latin/.LatinIME
7. adb shell dumpsys input_method
Make sure that com.android.inputmethod.latin/.LatinIME is
recognized by IMMS again.
Test: Manually verified as follows.
1. Build a custom APK LatinIME_no_ime.apk that has no input
method service.
2. adb install -r LatinIME_no_ime.apk
3. adb shell dumpsys input_method
Make sure that com.android.inputmethod.latin/.LatinIME is
not recognized by IMMS.
4. adb install -r LatinIME.apk
5. adb shell dumpsys input_method
Make sure that com.android.inputmethod.latin/.LatinIME is
recognized by IMMS.
6. adb install -r LatinIME_no_ime.apk
7. adb shell dumpsys input_method
Make sure that com.android.inputmethod.latin/.LatinIME is
no longer recognized by IMMS.
Bug: 32343335
Fixes: 28181208
Change-Id: I7b69c349318ce06a48d03a4468cf2c45bfb73dc2
diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java
index ca708b6..f0f50f0 100644
--- a/services/core/java/com/android/server/InputMethodManagerService.java
+++ b/services/core/java/com/android/server/InputMethodManagerService.java
@@ -21,6 +21,7 @@
import static android.view.WindowManager.LayoutParams.TYPE_INPUT_METHOD_DIALOG;
import static java.lang.annotation.RetentionPolicy.SOURCE;
+import com.android.internal.annotations.GuardedBy;
import com.android.internal.content.PackageMonitor;
import com.android.internal.inputmethod.IInputContentUriToken;
import com.android.internal.inputmethod.InputMethodSubtypeSwitchingController;
@@ -640,7 +641,28 @@
Settings.Secure.ENABLED_INPUT_METHODS, mergedImesAndSubtypesString);
}
- class MyPackageMonitor extends PackageMonitor {
+ final class MyPackageMonitor extends PackageMonitor {
+ /**
+ * Set of packages to be monitored.
+ *
+ * <p>No need to include packages because of direct-boot unaware IMEs since we always rescan
+ * all the packages when the user is unlocked, and direct-boot awareness will not be changed
+ * dynamically unless the entire package is updated, which also always triggers package
+ * rescanning.</p>
+ */
+ @GuardedBy("mMethodMap")
+ private ArraySet<String> mPackagesToMonitorComponentChange = new ArraySet<>();
+
+ @GuardedBy("mMethodMap")
+ void clearPackagesToMonitorComponentChangeLocked() {
+ mPackagesToMonitorComponentChange.clear();
+ }
+
+ @GuardedBy("mMethodMap")
+ final void addPackageToMonitorComponentChangeLocked(@NonNull String packageName) {
+ mPackagesToMonitorComponentChange.add(packageName);
+ }
+
private boolean isChangingPackagesOfCurrentUser() {
final int userId = getChangingUserId();
final boolean retval = userId == mSettings.getCurrentUserId();
@@ -682,6 +704,14 @@
}
@Override
+ public boolean onPackageChanged(String packageName, int uid, String[] components) {
+ // If this package is in the watch list, we want to check it.
+ synchronized (mMethodMap) {
+ return mPackagesToMonitorComponentChange.contains(packageName);
+ }
+ }
+
+ @Override
public void onSomePackagesChanged() {
if (!isChangingPackagesOfCurrentUser()) {
return;
@@ -3039,6 +3069,7 @@
}
mMethodList.clear();
mMethodMap.clear();
+ mMyPackageMonitor.clearPackagesToMonitorComponentChangeLocked();
// Use for queryIntentServicesAsUser
final PackageManager pm = mContext.getPackageManager();
@@ -3081,6 +3112,26 @@
}
}
+ // Construct the set of possible IME packages for onPackageChanged() to avoid false
+ // negatives when the package state remains to be the same but only the component state is
+ // changed.
+ {
+ // Here we intentionally use PackageManager.MATCH_DISABLED_COMPONENTS since the purpose
+ // of this query is to avoid false negatives. PackageManager.MATCH_ALL could be more
+ // conservative, but it seems we cannot use it for now (Issue 35176630).
+ final List<ResolveInfo> allInputMethodServices = pm.queryIntentServicesAsUser(
+ new Intent(InputMethod.SERVICE_INTERFACE),
+ PackageManager.MATCH_DISABLED_COMPONENTS, mSettings.getCurrentUserId());
+ final int N = allInputMethodServices.size();
+ for (int i = 0; i < N; ++i) {
+ final ServiceInfo si = allInputMethodServices.get(i).serviceInfo;
+ if (!android.Manifest.permission.BIND_INPUT_METHOD.equals(si.permission)) {
+ continue;
+ }
+ mMyPackageMonitor.addPackageToMonitorComponentChangeLocked(si.packageName);
+ }
+ }
+
// TODO: The following code should find better place to live.
if (!resetDefaultEnabledIme) {
boolean enabledImeFound = false;