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

Commit 8880128e authored by Pinyao Ting's avatar Pinyao Ting
Browse files

Refrain from choking sysram due to excessive onShortcutChanged callbacks 

When developers calls pushDynamicShortcut, LauncherAppService captures a
snapshot (i.e. cloned) of all of the existing shortcuts and passes them
through the callback. We observed an issue where, if the publishing app
decided to push a large quantity (say, 100+) of shortcuts in a for-loop,
we ended up generating a massive amount of shortcut instances (since the
shortcuts passes to the callback is a cloned instance) which leads to
a short burst in memory usage.

This CL debounces the callback by 100ms so that when the developers are
calling pushDynamicShortcut in a for-loop, callback will only be fired
for the last invokation.

Bug: 218545269
Test: manual
Change-Id: I9deca76d15fe9a71574c53031bd7aef7f7740740
parent 8b50a568

services/core/java/com/android/server/pm/ShortcutPackage.java

+2 −2
Original line number Diff line number Diff line
@@ -1172,7 +1172,7 @@ class ShortcutPackage extends ShortcutPackageItem { 


        // This will send a notification to the launcher, and also save .

        // TODO: List changed and removed manifest shortcuts and pass to packageShortcutsChanged()

        s.packageShortcutsChanged(getPackageName(), getPackageUserId(), null, null);

        s.packageShortcutsChanged(this, null, null);



        return true;

    }
@@ -1449,7 +1449,7 @@ class ShortcutPackage extends ShortcutPackageItem { 
            });

        }

        if (!CollectionUtils.isEmpty(changedShortcuts)) {

            s.packageShortcutsChanged(getPackageName(), getPackageUserId(), changedShortcuts, null);

            s.packageShortcutsChanged(this, changedShortcuts, null);

        }

    }

services/core/java/com/android/server/pm/ShortcutRequestPinProcessor.java

+3 −3
Original line number Diff line number Diff line
@@ -454,6 +454,7 @@ class ShortcutRequestPinProcessor { 
        final String shortcutId = original.getId();



        List<ShortcutInfo> changedShortcuts = null;

        final ShortcutPackage ps;



        synchronized (mLock) {

            if (!(mService.isUserUnlockedL(appUserId)
@@ -472,8 +473,7 @@ class ShortcutRequestPinProcessor { 
                return true;

            }



            final ShortcutPackage ps = mService.getPackageShortcutsForPublisherLocked(

                    appPackageName, appUserId);

            ps = mService.getPackageShortcutsForPublisherLocked(appPackageName, appUserId);

            final ShortcutInfo current = ps.findShortcutById(shortcutId);



            // The shortcut might have been changed, so we need to do the same validation again.
@@ -527,7 +527,7 @@ class ShortcutRequestPinProcessor { 
        }



        mService.verifyStates();

        mService.packageShortcutsChanged(appPackageName, appUserId, changedShortcuts, null);

        mService.packageShortcutsChanged(ps, changedShortcuts, null);



        return true;

    }

services/core/java/com/android/server/pm/ShortcutService.java

+63 −44
Original line number Diff line number Diff line
@@ -221,6 +221,8 @@ public class ShortcutService extends IShortcutService.Stub { 


    private static final String DUMMY_MAIN_ACTIVITY = "android.__dummy__";



    private static final long CALLBACK_DELAY = 100L;



    @VisibleForTesting

    interface ConfigConstants {

        /**
@@ -1745,9 +1747,13 @@ public class ShortcutService extends IShortcutService.Stub { 
        new Thread(r).start();

    }



    void injectPostToHandlerIfAppSearch(Runnable r) {

        // TODO: move to background thread when app search is enabled.

        r.run();

    void injectPostToHandlerDebounced(@NonNull final Object token, @NonNull final Runnable r) {

        Objects.requireNonNull(token);

        Objects.requireNonNull(r);

        synchronized (mLock) {

            mHandler.removeCallbacksAndMessages(token);

            mHandler.postDelayed(r, token, CALLBACK_DELAY);

        }

    }



    /**
@@ -1771,20 +1777,33 @@ public class ShortcutService extends IShortcutService.Stub { 
     * - Sends a notification to LauncherApps

     * - Write to file

     */

    void packageShortcutsChanged(@NonNull String packageName, @UserIdInt int userId,

    void packageShortcutsChanged(

            @NonNull final ShortcutPackage sp,

            @Nullable final List<ShortcutInfo> changedShortcuts,

            @Nullable final List<ShortcutInfo> removedShortcuts) {

        notifyListeners(packageName, userId);

        Objects.requireNonNull(sp);

        final String packageName = sp.getPackageName();

        final int userId = sp.getPackageUserId();

        if (DEBUG) {

            Slog.d(TAG, String.format(

                    "Shortcut changes: package=%s, user=%d", packageName, userId));

        }

        injectPostToHandlerDebounced(sp, notifyListenerRunnable(packageName, userId));

        notifyShortcutChangeCallbacks(packageName, userId, changedShortcuts, removedShortcuts);

        scheduleSaveUser(userId);

    }



    private void notifyListeners(@NonNull String packageName, @UserIdInt int userId) {

    private void notifyListeners(@NonNull final String packageName, @UserIdInt final int userId) {

        if (DEBUG) {

            Slog.d(TAG, String.format(

                    "Shortcut changes: package=%s, user=%d", packageName, userId));

        }

        injectPostToHandler(() -> {

        injectPostToHandler(notifyListenerRunnable(packageName, userId));

    }



    private Runnable notifyListenerRunnable(@NonNull final String packageName,

            @UserIdInt final int userId) {

        return () -> {

            try {

                final ArrayList<ShortcutChangeListener> copy;

                synchronized (mLock) {
@@ -1800,7 +1819,7 @@ public class ShortcutService extends IShortcutService.Stub { 
                }

            } catch (Exception ignore) {

            }

        });

        };

    }



    private void notifyShortcutChangeCallbacks(@NonNull String packageName, @UserIdInt int userId,
@@ -1948,12 +1967,12 @@ public class ShortcutService extends IShortcutService.Stub { 


        List<ShortcutInfo> changedShortcuts = null;

        List<ShortcutInfo> removedShortcuts = null;

        final ShortcutPackage ps;



        synchronized (mLock) {

            throwIfUserLockedL(userId);



            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName,

                    userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);



            ps.ensureImmutableShortcutsNotIncluded(newShortcuts, /*ignoreInvisible=*/ true);

            ps.ensureNoBitmapIconIfShortcutIsLongLived(newShortcuts);
@@ -1997,7 +2016,7 @@ public class ShortcutService extends IShortcutService.Stub { 
                    cachedOrPinned, newShortcuts, removedShortcuts, ps);

        }



        packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

        packageShortcutsChanged(ps, changedShortcuts, removedShortcuts);



        verifyStates();
@@ -2017,12 +2036,12 @@ public class ShortcutService extends IShortcutService.Stub { 
        final int size = newShortcuts.size();



        final List<ShortcutInfo> changedShortcuts = new ArrayList<>(1);

        final ShortcutPackage ps;



        synchronized (mLock) {

            throwIfUserLockedL(userId);



            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName,

                    userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);



            ps.ensureImmutableShortcutsNotIncluded(newShortcuts, /*ignoreInvisible=*/ true);

            ps.ensureNoBitmapIconIfShortcutIsLongLived(newShortcuts);
@@ -2097,8 +2116,7 @@ public class ShortcutService extends IShortcutService.Stub { 
            // Lastly, adjust the ranks.

            ps.adjustRanks();

        }

        packageShortcutsChanged(packageName, userId,

                changedShortcuts.isEmpty() ? null : changedShortcuts, null);

        packageShortcutsChanged(ps, changedShortcuts.isEmpty() ? null : changedShortcuts, null);



        verifyStates();
@@ -2118,12 +2136,12 @@ public class ShortcutService extends IShortcutService.Stub { 
        final int size = newShortcuts.size();



        List<ShortcutInfo> changedShortcuts = null;

        final ShortcutPackage ps;



        synchronized (mLock) {

            throwIfUserLockedL(userId);



            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName,

                    userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);



            ps.ensureImmutableShortcutsNotIncluded(newShortcuts, /*ignoreInvisible=*/ true);

            ps.ensureNoBitmapIconIfShortcutIsLongLived(newShortcuts);
@@ -2162,7 +2180,7 @@ public class ShortcutService extends IShortcutService.Stub { 
            // Lastly, adjust the ranks.

            ps.adjustRanks();

        }

        packageShortcutsChanged(packageName, userId, changedShortcuts, null);

        packageShortcutsChanged(ps, changedShortcuts, null);

        verifyStates();

        return true;

    }
@@ -2175,12 +2193,12 @@ public class ShortcutService extends IShortcutService.Stub { 


        List<ShortcutInfo> changedShortcuts = new ArrayList<>();

        List<ShortcutInfo> removedShortcuts = null;

        final ShortcutPackage ps;



        synchronized (mLock) {

            throwIfUserLockedL(userId);



            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName,

                    userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);



            ps.ensureNotImmutable(shortcut.getId(), /*ignoreInvisible=*/ true);

            fillInDefaultActivity(Arrays.asList(shortcut));
@@ -2215,7 +2233,7 @@ public class ShortcutService extends IShortcutService.Stub { 
            ps.adjustRanks();

        }



        packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

        packageShortcutsChanged(ps, changedShortcuts, removedShortcuts);



        reportShortcutUsedInternal(packageName, shortcut.getId(), userId);
@@ -2292,8 +2310,7 @@ public class ShortcutService extends IShortcutService.Stub { 


                    ps.updateInvisibleShortcutForPinRequestWith(shortcut);



                    packageShortcutsChanged(shortcutPackage, userId,

                            Collections.singletonList(shortcut), null);

                    packageShortcutsChanged(ps, Collections.singletonList(shortcut), null);

                }

            }
@@ -2314,9 +2331,10 @@ public class ShortcutService extends IShortcutService.Stub { 
        Objects.requireNonNull(shortcutIds, "shortcutIds must be provided");

        List<ShortcutInfo> changedShortcuts = null;

        List<ShortcutInfo> removedShortcuts = null;

        final ShortcutPackage ps;

        synchronized (mLock) {

            throwIfUserLockedL(userId);

            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds,

                    /*ignoreInvisible=*/ true);

            final String disabledMessageString =
@@ -2345,7 +2363,7 @@ public class ShortcutService extends IShortcutService.Stub { 
            // We may have removed dynamic shortcuts which may have left a gap, so adjust the ranks.

            ps.adjustRanks();

        }

        packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

        packageShortcutsChanged(ps, changedShortcuts, removedShortcuts);

        verifyStates();

    }
@@ -2354,9 +2372,10 @@ public class ShortcutService extends IShortcutService.Stub { 
        verifyCaller(packageName, userId);

        Objects.requireNonNull(shortcutIds, "shortcutIds must be provided");

        List<ShortcutInfo> changedShortcuts = null;

        final ShortcutPackage ps;

        synchronized (mLock) {

            throwIfUserLockedL(userId);

            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds,

                    /*ignoreInvisible=*/ true);

            for (int i = shortcutIds.size() - 1; i >= 0; i--) {
@@ -2371,7 +2390,7 @@ public class ShortcutService extends IShortcutService.Stub { 
                changedShortcuts.add(ps.findShortcutById(id));

            }

        }

        packageShortcutsChanged(packageName, userId, changedShortcuts, null);

        packageShortcutsChanged(ps, changedShortcuts, null);

        verifyStates();

    }
@@ -2383,11 +2402,10 @@ public class ShortcutService extends IShortcutService.Stub { 
        Objects.requireNonNull(shortcutIds, "shortcutIds must be provided");

        List<ShortcutInfo> changedShortcuts = null;

        List<ShortcutInfo> removedShortcuts = null;



        final ShortcutPackage ps;

        synchronized (mLock) {

            throwIfUserLockedL(userId);

            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName,

                    userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds,

                    /*ignoreInvisible=*/ true);

            for (int i = shortcutIds.size() - 1; i >= 0; i--) {
@@ -2413,7 +2431,7 @@ public class ShortcutService extends IShortcutService.Stub { 
            // We may have removed dynamic shortcuts which may have left a gap, so adjust the ranks.

            ps.adjustRanks();

        }

        packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

        packageShortcutsChanged(ps, changedShortcuts, removedShortcuts);

        verifyStates();

    }
@@ -2422,10 +2440,10 @@ public class ShortcutService extends IShortcutService.Stub { 
        verifyCaller(packageName, userId);

        List<ShortcutInfo> changedShortcuts = new ArrayList<>();

        List<ShortcutInfo> removedShortcuts = null;

        final ShortcutPackage ps;

        synchronized (mLock) {

            throwIfUserLockedL(userId);

            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName,

                    userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            // Dynamic shortcuts that are either cached or pinned will not get deleted.

            ps.findAll(changedShortcuts,

                    (ShortcutInfo si) -> si.isVisibleToPublisher()
@@ -2435,7 +2453,7 @@ public class ShortcutService extends IShortcutService.Stub { 
            changedShortcuts = prepareChangedShortcuts(

                    changedShortcuts, null, removedShortcuts, ps);

        }

        packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

        packageShortcutsChanged(ps, changedShortcuts, removedShortcuts);

        verifyStates();

    }
@@ -2446,9 +2464,10 @@ public class ShortcutService extends IShortcutService.Stub { 
        Objects.requireNonNull(shortcutIds, "shortcutIds must be provided");

        List<ShortcutInfo> changedShortcuts = null;

        List<ShortcutInfo> removedShortcuts = null;

        final ShortcutPackage ps;

        synchronized (mLock) {

            throwIfUserLockedL(userId);

            final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            ps = getPackageShortcutsForPublisherLocked(packageName, userId);

            ps.ensureImmutableShortcutsNotIncludedWithIds((List<String>) shortcutIds,

                    /*ignoreInvisible=*/ true);

            for (int i = shortcutIds.size() - 1; i >= 0; i--) {
@@ -2472,7 +2491,7 @@ public class ShortcutService extends IShortcutService.Stub { 
            // We may have removed dynamic shortcuts which may have left a gap, so adjust the ranks.

            ps.adjustRanks();

        }

        packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

        packageShortcutsChanged(ps, changedShortcuts, removedShortcuts);

        verifyStates();

    }
@@ -3113,7 +3132,7 @@ public class ShortcutService extends IShortcutService.Stub { 


            List<ShortcutInfo> changedShortcuts = null;

            List<ShortcutInfo> removedShortcuts = null;



            final ShortcutPackage sp;

            synchronized (mLock) {

                throwIfUserLockedL(userId);

                throwIfUserLockedL(launcherUserId);
@@ -3122,8 +3141,7 @@ public class ShortcutService extends IShortcutService.Stub { 
                        getLauncherShortcutsLocked(callingPackage, userId, launcherUserId);

                launcher.attemptToRestoreIfNeededAndSave();



                final ShortcutPackage sp = getUserShortcutsLocked(userId)

                        .getPackageShortcutsIfExists(packageName);

                sp = getUserShortcutsLocked(userId).getPackageShortcutsIfExists(packageName);

                if (sp != null) {

                    // List the shortcuts that are pinned only, these will get removed.

                    removedShortcuts = new ArrayList<>();
@@ -3147,7 +3165,9 @@ public class ShortcutService extends IShortcutService.Stub { 
                        oldPinnedIds, new ArraySet<>(shortcutIds), removedShortcuts, sp);

            }



            packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

            if (sp != null) {

                packageShortcutsChanged(sp, changedShortcuts, removedShortcuts);

            }



            verifyStates();

        }
@@ -3198,14 +3218,13 @@ public class ShortcutService extends IShortcutService.Stub { 


            List<ShortcutInfo> changedShortcuts = null;

            List<ShortcutInfo> removedShortcuts = null;



            final ShortcutPackage sp;

            synchronized (mLock) {

                throwIfUserLockedL(userId);

                throwIfUserLockedL(launcherUserId);



                final int idSize = shortcutIds.size();

                final ShortcutPackage sp = getUserShortcutsLocked(userId)

                        .getPackageShortcutsIfExists(packageName);

                sp = getUserShortcutsLocked(userId).getPackageShortcutsIfExists(packageName);

                if (idSize == 0 || sp == null) {

                    return;

                }
@@ -3248,7 +3267,7 @@ public class ShortcutService extends IShortcutService.Stub { 
                    }

                }

            }

            packageShortcutsChanged(packageName, userId, changedShortcuts, removedShortcuts);

            packageShortcutsChanged(sp, changedShortcuts, removedShortcuts);



            verifyStates();

        }

services/tests/servicestests/src/com/android/server/pm/BaseShortcutManagerTest.java

+5 −0
Original line number Diff line number Diff line
@@ -505,6 +505,11 @@ public abstract class BaseShortcutManagerTest extends InstrumentationTestCase { 
            runOnHandler(r);

        }



        @Override

        void injectPostToHandlerDebounced(@NonNull final Object token, @NonNull final Runnable r) {

            runOnHandler(r);

        }



        @Override

        void injectEnforceCallingPermission(String permission, String message) {

            if (!mCallerPermissions.contains(permission)) {