Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6bdc56c6 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Simplify IMMS.MyPackageMonitor

With my previous changes [1][2], long and/or frequent operations in

 MyPackageMonitor#onFinishPackageChangesInternal()

are expected to be no longer contributing to UI janks. Now it is time
to focus more on the correctness rather than trying to optimize things
in a complicated way. Complicated optimizations make it difficult for
us to diagnose issues like Bug 335281466.

This CL basically replaces my past optimizations [3][4] in a more
strict and straightforward way. We first create a new InputMethodMap
based on the latest results returned from the PackageManagerService,
then see if there is any actual data change from the current
InputMethodMap. If those two InputMethodMap instances are exactly the
same, it means that no further update is necessary.

There should be no semantic observable change.

 [1]: I263cd49dd4d64b64136acc3dad469f83a862ce97
      db19711e
 [2]: Icc84fe9d0d3b5842b4d5ee51b12b0d3cd10a1d24
      81a9a1ba
 [3]: I7b69c349318ce06a48d03a4468cf2c45bfb73dc2
      c4e44917
 [4]: I063688297156188f68fe0b55a46d72f2e811dc88
      5e3e8a52

Bug: 329703038
Test: presubmit
Change-Id: I6e9de6ff5c6dd55e3c40f23586110cb713c62851
parent 825a504a
Loading
Loading
Loading
Loading
+7 −136
Original line number Diff line number Diff line
@@ -839,37 +839,6 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.
    }

    final class MyPackageMonitor extends PackageMonitor {
        /**
         * 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
         * dynamically unless the entire package is updated, which also always triggers package
         * rescanning.</p>
         */
        @GuardedBy("ImfLock.class")
        private final 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;

        /**
         * Remembers package names passed to {@link #onPackageDataCleared(String, int)}.
         *
@@ -882,16 +851,6 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.
            super(true);
        }

        @GuardedBy("ImfLock.class")
        void clearKnownImePackageNamesLocked() {
            mKnownImePackageNames.clear();
        }

        @GuardedBy("ImfLock.class")
        void addKnownImePackageNameLocked(@NonNull String packageName) {
            mKnownImePackageNames.add(packageName);
        }

        @GuardedBy("ImfLock.class")
        private boolean isChangingPackagesOfCurrentUserLocked() {
            final int userId = getChangingUserId();
@@ -941,53 +900,8 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.
            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 onPackageDataCleared(String packageName, int uid) {
            mChangedPackages.add(packageName);
            mDataClearedPackages.add(packageName);
        }

@@ -999,32 +913,7 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.

        private void clearPackageChangeState() {
            // No need to lock them because we access these fields only on getRegisteredHandler().
            mChangedPackages.clear();
            mDataClearedPackages.clear();
            mImePackageAppeared = false;
        }

        @GuardedBy("ImfLock.class")
        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 numPackages = mChangedPackages.size();
            for (int i = 0; i < numPackages; ++i) {
                final String packageName = mChangedPackages.get(i);
                if (mKnownImePackageNames.contains(packageName)) {
                    return true;
                }
            }
            return false;
        }

        private void onFinishPackageChangesInternal() {
@@ -1085,12 +974,15 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.
                    AdditionalSubtypeMapRepository.putAndSave(userId, newAdditionalSubtypeMap,
                            settings.getMethodMap());
                }
                if (isCurrentUser
                        && !(additionalSubtypeChanged || shouldRebuildInputMethodListLocked())) {
                    return;
                }

                final var newMethodMap = newMethodMapWithoutAdditionalSubtypes
                        .applyAdditionalSubtypes(newAdditionalSubtypeMap);

                if (InputMethodMap.areSame(settings.getMethodMap(), newMethodMap)) {
                    // No update in the actual IME map.
                    return;
                }

                final InputMethodSettings newSettings =
                        InputMethodSettings.create(newMethodMap, userId);
                InputMethodSettingsRepository.put(userId, newSettings);
@@ -5044,30 +4936,9 @@ public final class InputMethodManagerService implements IInputMethodManagerImpl.
            return;
        }
        mMethodMapUpdateCount++;
        mMyPackageMonitor.clearKnownImePackageNamesLocked();

        final InputMethodSettings settings = InputMethodSettingsRepository.get(mCurrentUserId);

        // 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 =
                    mContext.getPackageManager().queryIntentServicesAsUser(
                            new Intent(InputMethod.SERVICE_INTERFACE),
                            PackageManager.MATCH_DISABLED_COMPONENTS, settings.getUserId());
            final int numImes = allInputMethodServices.size();
            for (int i = 0; i < numImes; ++i) {
                final ServiceInfo si = allInputMethodServices.get(i).serviceInfo;
                if (android.Manifest.permission.BIND_INPUT_METHOD.equals(si.permission)) {
                    mMyPackageMonitor.addKnownImePackageNameLocked(si.packageName);
                }
            }
        }

        boolean reenableMinimumNonAuxSystemImes = false;
        // TODO: The following code should find better place to live.
        if (!resetDefaultEnabledIme) {