OSDN Git Service

Optimize IMMS.MyPackageMonitor more
authorYohei Yukawa <yukawa@google.com>
Wed, 15 Feb 2017 04:05:17 +0000 (20:05 -0800)
committerYohei Yukawa <yukawa@google.com>
Thu, 23 Feb 2017 01:58:28 +0000 (01:58 +0000)
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

services/core/java/com/android/server/InputMethodManagerService.java

index 101b738..27c4952 100644 (file)
@@ -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}.
          *
          * <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
@@ -715,16 +715,36 @@ public class InputMethodManagerService extends IInputMethodManager.Stub
          * rescanning.</p>
          */
         @GuardedBy("mMethodMap")
-        private ArraySet<String> mPackagesToMonitorComponentChange = new ArraySet<>();
+        final private ArraySet<String> mKnownImePackageNames = new ArraySet<>();
+
+        /**
+         * Packages that are appeared, disappeared, or modified for whatever reason.
+         *
+         * <p>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.</p>
+         *
+         * <p>This object must be accessed only from callback methods in {@link PackageMonitor},
+         * which should be bound to {@link #getRegisteredHandler()}.</p>
+         */
+        private final ArrayList<String> mChangedPackages = new ArrayList<>();
+
+        /**
+         * {@code true} if one or more packages that contain {@link InputMethodService} appeared.
+         *
+         * <p>This field must be accessed only from callback methods in {@link PackageMonitor},
+         * which should be bound to {@link #getRegisteredHandler()}.</p>
+         */
+        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<ResolveInfo> 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);
             }
         }