Skip to content

Commit 57d3a92

Browse files
pinyaotingAndroid Build Coastguard Worker
authored andcommitted
Limit the number of shortcuts per app that can be retained by system
This is a second attempt at fixing the issue, the previous CL ag/20642213 was reverted because it simply throws an exception when the limit is reached, which causes apps to crash since chat apps tends to be sending large amount of conversation shortcuts and they have no way to know how many of these shortcuts are still cached by the system. Instead of throwing an exception, this CL simply removes excessive shortcuts to avoid crashes. Currently there is a limit on the number of shortcuts an app can publish in respect to each launcher activity. This CL further implements a global maximum of total number of shortcuts that can be retained for an app to mitigate from any potential system health issue. When the global maximum is reached, ShortcutService will proactively removes shortcuts from system memory. Cached shortcuts are removed first, followed by dynamic shortcuts, using last updated time as tie-breaker. This CL additionally addresses an unexpected flow where re-publishing previously removed shortcuts that are still retained by the system could cause the total number of shortcuts to exceed previously set limit. Bug: 250576066 233155034 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a6e7958ab84edbd9e5f4653d4d1f56a7438cd7dc) Merged-In: I001c7a87b62aefa9487bf8efaf3cd02d7cb21521 Change-Id: I001c7a87b62aefa9487bf8efaf3cd02d7cb21521
1 parent 65c358f commit 57d3a92

2 files changed

Lines changed: 112 additions & 12 deletions

File tree

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

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -430,14 +430,15 @@ public boolean pushDynamicShortcut(@NonNull ShortcutInfo newShortcut,
430430
@NonNull List<ShortcutInfo> changedShortcuts) {
431431
Preconditions.checkArgument(newShortcut.isEnabled(),
432432
"pushDynamicShortcuts() cannot publish disabled shortcuts");
433+
ensureShortcutCountBeforePush();
433434

434435
newShortcut.addFlags(ShortcutInfo.FLAG_DYNAMIC);
435436

436437
changedShortcuts.clear();
437438
final ShortcutInfo oldShortcut = findShortcutById(newShortcut.getId());
438439
boolean deleted = false;
439440

440-
if (oldShortcut == null) {
441+
if (oldShortcut == null || !oldShortcut.isDynamic()) {
441442
final ShortcutService service = mShortcutUser.mService;
442443
final int maxShortcuts = service.getMaxActivityShortcuts();
443444

@@ -446,18 +447,12 @@ public boolean pushDynamicShortcut(@NonNull ShortcutInfo newShortcut,
446447
final ArrayList<ShortcutInfo> activityShortcuts = all.get(newShortcut.getActivity());
447448

448449
if (activityShortcuts != null && activityShortcuts.size() > maxShortcuts) {
449-
Slog.e(TAG, "Error pushing shortcut. There are already "
450-
+ activityShortcuts.size() + " shortcuts, exceeding the " + maxShortcuts
451-
+ " shortcuts limit when pushing the new shortcut " + newShortcut
452-
+ ". Id of shortcuts currently available in system memory are "
453-
+ activityShortcuts.stream().map(ShortcutInfo::getId)
454-
.collect(Collectors.joining(",", "[", "]")));
455-
// TODO: This should not have happened. If it does, identify the root cause where
456-
// possible, otherwise bail-out early to prevent memory issue.
450+
// Root cause was discovered in b/233155034, so this should not be happening.
451+
service.wtf("Error pushing shortcut. There are already "
452+
+ activityShortcuts.size() + " shortcuts.");
457453
}
458454
if (activityShortcuts != null && activityShortcuts.size() == maxShortcuts) {
459455
// Max has reached. Delete the shortcut with lowest rank.
460-
461456
// Sort by isManifestShortcut() and getRank().
462457
Collections.sort(activityShortcuts, mShortcutTypeAndRankComparator);
463458

@@ -473,7 +468,8 @@ public boolean pushDynamicShortcut(@NonNull ShortcutInfo newShortcut,
473468
deleted = deleteDynamicWithId(shortcut.getId(), /* ignoreInvisible =*/ true,
474469
/*ignorePersistedShortcuts=*/ true) != null;
475470
}
476-
} else {
471+
}
472+
if (oldShortcut != null) {
477473
// It's an update case.
478474
// Make sure the target is updatable. (i.e. should be mutable.)
479475
oldShortcut.ensureUpdatableWith(newShortcut, /*isUpdating=*/ false);
@@ -505,6 +501,32 @@ public boolean pushDynamicShortcut(@NonNull ShortcutInfo newShortcut,
505501
return deleted;
506502
}
507503

504+
private void ensureShortcutCountBeforePush() {
505+
final ShortcutService service = mShortcutUser.mService;
506+
// Ensure the total number of shortcuts doesn't exceed the hard limit per app.
507+
final int maxShortcutPerApp = service.getMaxAppShortcuts();
508+
synchronized (mLock) {
509+
final List<ShortcutInfo> appShortcuts = mShortcuts.values().stream().filter(si ->
510+
!si.isPinned()).collect(Collectors.toList());
511+
if (appShortcuts.size() >= maxShortcutPerApp) {
512+
// Max has reached. Removes shortcuts until they fall within the hard cap.
513+
// Sort by isManifestShortcut(), isDynamic() and getLastChangedTimestamp().
514+
Collections.sort(appShortcuts, mShortcutTypeRankAndTimeComparator);
515+
516+
while (appShortcuts.size() >= maxShortcutPerApp) {
517+
final ShortcutInfo shortcut = appShortcuts.remove(appShortcuts.size() - 1);
518+
if (shortcut.isDeclaredInManifest()) {
519+
// All shortcuts are manifest shortcuts and cannot be removed.
520+
throw new IllegalArgumentException(getPackageName() + " has published "
521+
+ appShortcuts.size() + " manifest shortcuts across different"
522+
+ " activities.");
523+
}
524+
forceDeleteShortcutInner(shortcut.getId());
525+
}
526+
}
527+
}
528+
}
529+
508530
/**
509531
* Remove all shortcuts that aren't pinned, cached nor dynamic.
510532
*
@@ -1366,6 +1388,61 @@ private boolean pushOutExcessShortcuts() {
13661388
return Integer.compare(a.getRank(), b.getRank());
13671389
};
13681390

1391+
/**
1392+
* To sort by isManifestShortcut(), isDynamic(), getRank() and
1393+
* getLastChangedTimestamp(). i.e. manifest shortcuts come before non-manifest shortcuts,
1394+
* dynamic shortcuts come before floating shortcuts, then sort by last changed timestamp.
1395+
*
1396+
* This is used to decide which shortcuts to remove when the total number of shortcuts retained
1397+
* for the app exceeds the limit defined in {@link ShortcutService#getMaxAppShortcuts()}.
1398+
*
1399+
* (Note the number of manifest shortcuts is always <= the max number, because if there are
1400+
* more, ShortcutParser would ignore the rest.)
1401+
*/
1402+
final Comparator<ShortcutInfo> mShortcutTypeRankAndTimeComparator = (ShortcutInfo a,
1403+
ShortcutInfo b) -> {
1404+
if (a.isDeclaredInManifest() && !b.isDeclaredInManifest()) {
1405+
return -1;
1406+
}
1407+
if (!a.isDeclaredInManifest() && b.isDeclaredInManifest()) {
1408+
return 1;
1409+
}
1410+
if (a.isDynamic() && b.isDynamic()) {
1411+
return Integer.compare(a.getRank(), b.getRank());
1412+
}
1413+
if (a.isDynamic()) {
1414+
return -1;
1415+
}
1416+
if (b.isDynamic()) {
1417+
return 1;
1418+
}
1419+
if (a.isCached() && b.isCached()) {
1420+
// if both shortcuts are cached, prioritize shortcuts cached by people tile,
1421+
if (a.hasFlags(ShortcutInfo.FLAG_CACHED_PEOPLE_TILE)
1422+
&& !b.hasFlags(ShortcutInfo.FLAG_CACHED_PEOPLE_TILE)) {
1423+
return -1;
1424+
} else if (!a.hasFlags(ShortcutInfo.FLAG_CACHED_PEOPLE_TILE)
1425+
&& b.hasFlags(ShortcutInfo.FLAG_CACHED_PEOPLE_TILE)) {
1426+
return 1;
1427+
}
1428+
// followed by bubbles.
1429+
if (a.hasFlags(ShortcutInfo.FLAG_CACHED_BUBBLES)
1430+
&& !b.hasFlags(ShortcutInfo.FLAG_CACHED_BUBBLES)) {
1431+
return -1;
1432+
} else if (!a.hasFlags(ShortcutInfo.FLAG_CACHED_BUBBLES)
1433+
&& b.hasFlags(ShortcutInfo.FLAG_CACHED_BUBBLES)) {
1434+
return 1;
1435+
}
1436+
}
1437+
if (a.isCached()) {
1438+
return -1;
1439+
}
1440+
if (b.isCached()) {
1441+
return 1;
1442+
}
1443+
return Long.compare(b.getLastChangedTimestamp(), a.getLastChangedTimestamp());
1444+
};
1445+
13691446
/**
13701447
* Build a list of shortcuts for each target activity and return as a map. The result won't
13711448
* contain "floating" shortcuts because they don't belong on any activities.

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ public class ShortcutService extends IShortcutService.Stub {
180180
@VisibleForTesting
181181
static final int DEFAULT_MAX_SHORTCUTS_PER_ACTIVITY = 15;
182182

183+
@VisibleForTesting
184+
static final int DEFAULT_MAX_SHORTCUTS_PER_APP = 100;
185+
183186
@VisibleForTesting
184187
static final int DEFAULT_MAX_ICON_DIMENSION_DP = 96;
185188

@@ -256,6 +259,11 @@ interface ConfigConstants {
256259
*/
257260
String KEY_MAX_SHORTCUTS = "max_shortcuts";
258261

262+
/**
263+
* Key name for the max shortcuts can be retained in system ram per app. (int)
264+
*/
265+
String KEY_MAX_SHORTCUTS_PER_APP = "max_shortcuts_per_app";
266+
259267
/**
260268
* Key name for icon compression quality, 0-100.
261269
*/
@@ -329,10 +337,15 @@ public boolean test(PackageInfo pi) {
329337
new SparseArray<>();
330338

331339
/**
332-
* Max number of dynamic + manifest shortcuts that each application can have at a time.
340+
* Max number of dynamic + manifest shortcuts that each activity can have at a time.
333341
*/
334342
private int mMaxShortcuts;
335343

344+
/**
345+
* Max number of shortcuts that can exists in system ram for each application.
346+
*/
347+
private int mMaxShortcutsPerApp;
348+
336349
/**
337350
* Max number of updating API calls that each application can make during the interval.
338351
*/
@@ -807,6 +820,9 @@ boolean updateConfigurationLocked(String config) {
807820
mMaxShortcuts = Math.max(0, (int) parser.getLong(
808821
ConfigConstants.KEY_MAX_SHORTCUTS, DEFAULT_MAX_SHORTCUTS_PER_ACTIVITY));
809822

823+
mMaxShortcutsPerApp = Math.max(0, (int) parser.getLong(
824+
ConfigConstants.KEY_MAX_SHORTCUTS_PER_APP, DEFAULT_MAX_SHORTCUTS_PER_APP));
825+
810826
final int iconDimensionDp = Math.max(1, injectIsLowRamDevice()
811827
? (int) parser.getLong(
812828
ConfigConstants.KEY_MAX_ICON_DIMENSION_DP_LOWRAM,
@@ -1758,6 +1774,13 @@ int getMaxActivityShortcuts() {
17581774
return mMaxShortcuts;
17591775
}
17601776

1777+
/**
1778+
* Return the max number of shortcuts can be retaiend in system ram for each application.
1779+
*/
1780+
int getMaxAppShortcuts() {
1781+
return mMaxShortcutsPerApp;
1782+
}
1783+
17611784
/**
17621785
* - Sends a notification to LauncherApps
17631786
* - Write to file

0 commit comments

Comments
 (0)