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

Commit 1e4c0ddb authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Avoid redundant AdditionalSubtypeMapRepository#putAndSave()

This is a follow up CL to my previous CLs [1][2][3], which aimed to
fix bugs that additional subtypes were cleared only when the current
user's IME APKs were updated but not for other users'.

What this CL does it to avoid unnecessary file operations by
making sure that

  AdditionalSubtypeMapRepository#putAndSave()

gets called only once from

  MyPackageMonitor#onFinishPackageChangesInternal().

This guarantees that for each package callback transactions additional
subtype persistent file will be saved at most once, compared to
multiple times in the previous implementation. Hopefully this helps
mitivate the potential performance regression discussed in Bug
327861441.

The observable behavior should remain the same semantically.

 [1]: I07e32739f486d960c9dd22476120fa35bf1899e4
      0c5feb22
 [2]: I10aa547f0de607ef3c0ba26764dac0585c40c843
      2501e749
 [3]: If42c518765171ff8cb51af000542671676cd3801
      87218eb2

Bug: 309837937
Bug: 322062773
Bug: 327861441
Bug: 328098968
Test: presubmit
Change-Id: I5b3dae7579feb412517251c6c8ad770b49831600
parent 53ce8719
Loading
Loading
Loading
Loading
+30 −36
Original line number Diff line number Diff line
@@ -1247,6 +1247,14 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
         */
        private boolean mImePackageAppeared = false;

        /**
         * Remembers package names passed to {@link #onPackageDataCleared(String, int)}.
         *
         * <p>This field must be accessed only from callback methods in {@link PackageMonitor},
         * which should be bound to {@link #getRegisteredHandler()}.</p>
         */
        private ArrayList<String> mDataClearedPackages = new ArrayList<>();

        @GuardedBy("ImfLock.class")
        void clearKnownImePackageNamesLocked() {
            mKnownImePackageNames.clear();
@@ -1350,36 +1358,8 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub

        @Override
        public void onPackageDataCleared(String packageName, int uid) {
            final int userId = getChangingUserId();
            synchronized (ImfLock.class) {
                final boolean isCurrentUser = (userId == mSettings.getUserId());
                final AdditionalSubtypeMap additionalSubtypeMap =
                        AdditionalSubtypeMapRepository.get(userId);
                final InputMethodSettings settings;
                if (isCurrentUser) {
                    settings = mSettings;
                } else {
                    settings = queryInputMethodServicesInternal(mContext, userId,
                            additionalSubtypeMap, DirectBootAwareness.AUTO);
                }

                // Note that one package may implement multiple IMEs.
                final ArrayList<String> changedImes = new ArrayList<>();
                for (InputMethodInfo imi : settings.getMethodList()) {
                    if (imi.getPackageName().equals(packageName)) {
                        changedImes.add(imi.getId());
                    }
                }
                final AdditionalSubtypeMap newMap =
                        additionalSubtypeMap.cloneWithRemoveOrSelf(changedImes);
                if (newMap != additionalSubtypeMap) {
                    AdditionalSubtypeMapRepository.putAndSave(userId, newMap,
                            settings.getMethodMap());
                }
                if (!changedImes.isEmpty()) {
            mChangedPackages.add(packageName);
                }
            }
            mDataClearedPackages.add(packageName);
        }

        @Override
@@ -1391,6 +1371,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
        private void clearPackageChangeState() {
            // No need to lock them because we access these fields only on getRegisteredHandler().
            mChangedPackages.clear();
            mDataClearedPackages.clear();
            mImePackageAppeared = false;
        }

@@ -1421,7 +1402,7 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
            synchronized (ImfLock.class) {
                final int userId = getChangingUserId();
                final boolean isCurrentUser = (userId == mSettings.getUserId());
                AdditionalSubtypeMap additionalSubtypeMap =
                final AdditionalSubtypeMap additionalSubtypeMap =
                        AdditionalSubtypeMapRepository.get(userId);
                final InputMethodSettings settings;
                if (isCurrentUser) {
@@ -1434,6 +1415,8 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
                InputMethodInfo curIm = null;
                String curInputMethodId = settings.getSelectedInputMethod();
                final List<InputMethodInfo> methodList = settings.getMethodList();

                final ArrayList<String> imesToClearAdditionalSubtypes = new ArrayList<>();
                final int numImes = methodList.size();
                for (int i = 0; i < numImes; i++) {
                    InputMethodInfo imi = methodList.get(i);
@@ -1441,6 +1424,9 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
                    if (imiId.equals(curInputMethodId)) {
                        curIm = imi;
                    }
                    if (mDataClearedPackages.contains(imi.getPackageName())) {
                        imesToClearAdditionalSubtypes.add(imiId);
                    }
                    int change = isPackageDisappearing(imi.getPackageName());
                    if (change == PACKAGE_TEMPORARY_CHANGE || change == PACKAGE_PERMANENT_CHANGE) {
                        Slog.i(TAG, "Input method uninstalled, disabling: " + imi.getComponent());
@@ -1455,14 +1441,22 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub
                    } else if (change == PACKAGE_UPDATING) {
                        Slog.i(TAG, "Input method reinstalling, clearing additional subtypes: "
                                + imi.getComponent());
                        additionalSubtypeMap =
                                additionalSubtypeMap.cloneWithRemoveOrSelf(imi.getId());
                        AdditionalSubtypeMapRepository.putAndSave(userId,
                                additionalSubtypeMap, settings.getMethodMap());
                        imesToClearAdditionalSubtypes.add(imiId);
                    }
                }

                if (!isCurrentUser || !shouldRebuildInputMethodListLocked()) {
                // Clear additional subtypes as a batch operation.
                final AdditionalSubtypeMap newAdditionalSubtypeMap =
                        additionalSubtypeMap.cloneWithRemoveOrSelf(imesToClearAdditionalSubtypes);
                final boolean additionalSubtypeChanged =
                        (newAdditionalSubtypeMap != additionalSubtypeMap);
                if (additionalSubtypeChanged) {
                    AdditionalSubtypeMapRepository.putAndSave(userId, newAdditionalSubtypeMap,
                            settings.getMethodMap());
                }

                if (!isCurrentUser
                        || !(additionalSubtypeChanged || shouldRebuildInputMethodListLocked())) {
                    return;
                }