From 5e3e8a5205f5ed584ad9466746f6fa397f293476 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Tue, 14 Feb 2017 20:05:17 -0800 Subject: [PATCH] Optimize IMMS.MyPackageMonitor more This CL is a follow up CL to my previous CL [1] that aimed to reduce false positives in InputMethodManagerService.MyPackageMonitor when deciding if InputMethodManagerService (IMMS) needs to rebuild the list of enabled IMEs or not. Currently IMMS.MyPackageMonitor#onSomePackagesChanged() gets called back to trigger IMMS#buildInputMethodListLocked() when either the following rule A or B is fulfiled. A. Intent with one of the following actions for any package name: - ACTION_PACKAGE_ADDED - ACTION_PACKAGE_REMOVED - ACTION_EXTERNAL_APPLICATIONS_AVAILABLE - ACTION_EXTERNAL_APPLICATIONS_UNAVAILABLE - ACTION_PACKAGES_SUSPENDED - ACTION_PACKAGES_UNSUSPENDED B. ACTION_PACKAGE_CHANGED with a package that is included in the known IME package list, which can be obtained from PackageManager. The previous CL [1] addressed Bug 28181208 by introducing the rule B, but we can actually apply the same optimization for A, except for one false negative case where an appearing package that is not in the known IME package list actually contains one or more InputMethodService. In short, we can reduce false positives by replacing the above two rules with the following two rules. A. Intent with one of the following actions for any package name that is in the known IME package list: - ACTION_PACKAGE_ADDED - ACTION_PACKAGE_CHANGED - ACTION_PACKAGE_REMOVED - ACTION_EXTERNAL_APPLICATIONS_AVAILABLE - ACTION_EXTERNAL_APPLICATIONS_UNAVAILABLE - ACTION_PACKAGES_SUSPENDED - ACTION_PACKAGES_UNSUSPENDED B. Intent with one of the following actions for any package that implements at least one InputMethodServivce. - ACTION_PACKAGE_ADDED - ACTION_EXTERNAL_APPLICATIONS_AVAILABLE Basically in the rule A PackageManager gives us the list of relevant package names that might contain IMEs regardless enabled/disabled state, and such a list works well to filter out irrelevant notifications except for one case where a new package is adding one or new IMEs that we did not know. This is why we also need the rule B. Even though the rule B requires a secondary query to PackageManager, it can be done outside of the state lock of IMMS. [1]: I7b69c349318ce06a48d03a4468cf2c45bfb73dc2 c4e4491735ad5614ec4592fae98f05c455f5944d Test: Manually verified as follows. 1. tapas ShortcutDemo && make -j 2. Copy ShortcutDemo.apk to the current directory. 3. adb shell dumpsys input_method | grep mMethodMapUpdateCount= to check the "mMethodMapUpdateCount". 4. adb install -r ShortcutDemo.apk 5. adb shell dumpsys input_method | grep mMethodMapUpdateCount= to make sure "mMethodMapUpdateCount" remains unchanged. 6. adb install -r ShortcutDemo.apk 7. adb shell dumpsys input_method | grep mMethodMapUpdateCount= to make sure "mMethodMapUpdateCount" remains unchanged. 8. adb uninstall com.example.android.pm.shortcutdemo 9. adb shell dumpsys input_method | grep mMethodMapUpdateCount= to make sure "mMethodMapUpdateCount" remains unchanged. Test: Manually verified as follows. 1. tapas SoftKeyboard && make -j 2. Copy SoftKeyboard.apk to the current directory. 3. adb root 4. adb install -r SoftKeyboard.apk 5. adb shell dumpsys input_method Make sure that com.example.android.softkeyboard/.SoftKeyboard is recognized by IMMS. 6. adb shell pm disable com.example.android.softkeyboard/.SoftKeyboard 7. adb shell dumpsys input_method Make sure that com.example.android.softkeyboard/.SoftKeyboard is no longer recognized by IMMS. 8. adb shell pm enable com.example.android.softkeyboard/.SoftKeyboard 9. adb shell dumpsys input_method Make sure that com.example.android.softkeyboard/.SoftKeyboard is recognized by IMMS again. Fixes: 35361128 Change-Id: I063688297156188f68fe0b55a46d72f2e811dc88 --- .../android/server/InputMethodManagerService.java | 127 ++++++++++++++++++--- 1 file changed, 112 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java index 101b73834cb6..27c49523a659 100644 --- a/services/core/java/com/android/server/InputMethodManagerService.java +++ b/services/core/java/com/android/server/InputMethodManagerService.java @@ -707,7 +707,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub final class MyPackageMonitor extends PackageMonitor { /** - * Set of packages to be monitored. + * Package names that are known to contain {@link InputMethodService}. * *

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 @@ -715,16 +715,36 @@ public class InputMethodManagerService extends IInputMethodManager.Stub * rescanning.

*/ @GuardedBy("mMethodMap") - private ArraySet mPackagesToMonitorComponentChange = new ArraySet<>(); + final private ArraySet mKnownImePackageNames = new ArraySet<>(); + + /** + * Packages that are appeared, disappeared, or modified for whatever reason. + * + *

Note: For now we intentionally use {@link ArrayList} instead of {@link ArraySet} + * because 1) the number of elements is almost always 1 or so, and 2) we do not care + * duplicate elements for our use case.

+ * + *

This object must be accessed only from callback methods in {@link PackageMonitor}, + * which should be bound to {@link #getRegisteredHandler()}.

+ */ + private final ArrayList mChangedPackages = new ArrayList<>(); + + /** + * {@code true} if one or more packages that contain {@link InputMethodService} appeared. + * + *

This field must be accessed only from callback methods in {@link PackageMonitor}, + * which should be bound to {@link #getRegisteredHandler()}.

+ */ + private boolean mImePackageAppeared = false; @GuardedBy("mMethodMap") - void clearPackagesToMonitorComponentChangeLocked() { - mPackagesToMonitorComponentChange.clear(); + void clearKnownImePackageNamesLocked() { + mKnownImePackageNames.clear(); } @GuardedBy("mMethodMap") - final void addPackageToMonitorComponentChangeLocked(@NonNull String packageName) { - mPackagesToMonitorComponentChange.add(packageName); + final void addKnownImePackageNameLocked(@NonNull String packageName) { + mKnownImePackageNames.add(packageName); } @GuardedBy("mMethodMap") @@ -769,19 +789,97 @@ public class InputMethodManagerService extends IInputMethodManager.Stub } @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); + public void onBeginPackageChanges() { + clearPackageChangeState(); + } + + @Override + public void onPackageAppeared(String packageName, int reason) { + if (!mImePackageAppeared) { + final PackageManager pm = mContext.getPackageManager(); + final List services = pm.queryIntentServicesAsUser( + new Intent(InputMethod.SERVICE_INTERFACE).setPackage(packageName), + PackageManager.MATCH_DISABLED_COMPONENTS, getChangingUserId()); + // No need to lock this because we access it only on getRegisteredHandler(). + if (!services.isEmpty()) { + mImePackageAppeared = true; + } + } + // No need to lock this because we access it only on getRegisteredHandler(). + mChangedPackages.add(packageName); + } + + @Override + public void onPackageDisappeared(String packageName, int reason) { + // No need to lock this because we access it only on getRegisteredHandler(). + mChangedPackages.add(packageName); + } + + @Override + public void onPackageModified(String packageName) { + // No need to lock this because we access it only on getRegisteredHandler(). + mChangedPackages.add(packageName); + } + + @Override + public void onPackagesSuspended(String[] packages) { + // No need to lock this because we access it only on getRegisteredHandler(). + for (String packageName : packages) { + mChangedPackages.add(packageName); + } + } + + @Override + public void onPackagesUnsuspended(String[] packages) { + // No need to lock this because we access it only on getRegisteredHandler(). + for (String packageName : packages) { + mChangedPackages.add(packageName); } } @Override - public void onSomePackagesChanged() { + public void onFinishPackageChanges() { + onFinishPackageChangesInternal(); + clearPackageChangeState(); + } + + private void clearPackageChangeState() { + // No need to lock them because we access these fields only on getRegisteredHandler(). + mChangedPackages.clear(); + mImePackageAppeared = false; + } + + private boolean shouldRebuildInputMethodListLocked() { + // This method is guaranteed to be called only by getRegisteredHandler(). + + // If there is any new package that contains at least one IME, then rebuilt the list + // of IMEs. + if (mImePackageAppeared) { + return true; + } + + // Otherwise, check if mKnownImePackageNames and mChangedPackages have any intersection. + // TODO: Consider to create a utility method to do the following test. List.retainAll() + // is an option, but it may still do some extra operations that we do not need here. + final int N = mChangedPackages.size(); + for (int i = 0; i < N; ++i) { + final String packageName = mChangedPackages.get(i); + if (mKnownImePackageNames.contains(packageName)) { + return true; + } + } + return false; + } + + private void onFinishPackageChangesInternal() { synchronized (mMethodMap) { if (!isChangingPackagesOfCurrentUserLocked()) { return; } + if (!shouldRebuildInputMethodListLocked()) { + return; + } + InputMethodInfo curIm = null; String curInputMethodId = mSettings.getSelectedInputMethod(); final int N = mMethodList.size(); @@ -3112,7 +3210,7 @@ public class InputMethodManagerService extends IInputMethodManager.Stub mMethodList.clear(); mMethodMap.clear(); mMethodMapUpdateCount++; - mMyPackageMonitor.clearPackagesToMonitorComponentChangeLocked(); + mMyPackageMonitor.clearKnownImePackageNamesLocked(); // Use for queryIntentServicesAsUser final PackageManager pm = mContext.getPackageManager(); @@ -3168,10 +3266,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub 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; + if (android.Manifest.permission.BIND_INPUT_METHOD.equals(si.permission)) { + mMyPackageMonitor.addKnownImePackageNameLocked(si.packageName); } - mMyPackageMonitor.addPackageToMonitorComponentChangeLocked(si.packageName); } } -- 2.11.0