From c4e4491735ad5614ec4592fae98f05c455f5944d Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Thu, 9 Feb 2017 19:30:22 -0800 Subject: [PATCH] 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 --- .../server/InputMethodManagerService.java | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java index ca708b6262e99..f0f50f07fccab 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; 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 @@ public class InputMethodManagerService extends IInputMethodManager.Stub Settings.Secure.ENABLED_INPUT_METHODS, mergedImesAndSubtypesString); } - class MyPackageMonitor extends PackageMonitor { + final class MyPackageMonitor extends PackageMonitor { + /** + * Set of packages to be monitored. + * + *

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.

+ */ + @GuardedBy("mMethodMap") + private ArraySet 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(); @@ -681,6 +703,14 @@ public class InputMethodManagerService extends IInputMethodManager.Stub return false; } + @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()) { @@ -3039,6 +3069,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } mMethodList.clear(); mMethodMap.clear(); + mMyPackageMonitor.clearPackagesToMonitorComponentChangeLocked(); // Use for queryIntentServicesAsUser final PackageManager pm = mContext.getPackageManager(); @@ -3081,6 +3112,26 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } } + // 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 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;