diff options
5 files changed, 214 insertions, 36 deletions
diff --git a/services/people/java/com/android/server/people/data/DataManager.java b/services/people/java/com/android/server/people/data/DataManager.java index eff9e8da9a76..37f0624464e5 100644 --- a/services/people/java/com/android/server/people/data/DataManager.java +++ b/services/people/java/com/android/server/people/data/DataManager.java @@ -113,6 +113,7 @@ public class DataManager { private static final long USAGE_STATS_QUERY_INTERVAL_SEC = 120L; @VisibleForTesting static final int MAX_CACHED_RECENT_SHORTCUTS = 30; + private static final int DEBOUNCE_LENGTH_MS = 500; private final Context mContext; private final Injector mInjector; @@ -129,6 +130,7 @@ public class DataManager { private final List<PeopleService.ConversationsListener> mConversationsListeners = new ArrayList<>(1); private final Handler mHandler; + private final PerPackageThrottler mShortcutsThrottler; private ContentObserver mCallLogContentObserver; private ContentObserver mMmsSmsContentObserver; @@ -140,14 +142,17 @@ public class DataManager { private ConversationStatusExpirationBroadcastReceiver mStatusExpReceiver; public DataManager(Context context) { - this(context, new Injector(), BackgroundThread.get().getLooper()); + this(context, new Injector(), BackgroundThread.get().getLooper(), + new PerPackageThrottlerImpl(BackgroundThread.getHandler(), DEBOUNCE_LENGTH_MS)); } - DataManager(Context context, Injector injector, Looper looper) { + DataManager(Context context, Injector injector, Looper looper, + PerPackageThrottler shortcutsThrottler) { mContext = context; mInjector = injector; mScheduledExecutor = mInjector.createScheduledExecutor(); mHandler = new Handler(looper); + mShortcutsThrottler = shortcutsThrottler; } /** Initialization. Called when the system services are up running. */ @@ -851,12 +856,12 @@ public class DataManager { // pair of <package name, conversation info> List<Pair<String, ConversationInfo>> cachedConvos = new ArrayList<>(); userData.forAllPackages(packageData -> { - packageData.forAllConversations(conversationInfo -> { - if (isEligibleForCleanUp(conversationInfo)) { - cachedConvos.add( - Pair.create(packageData.getPackageName(), conversationInfo)); - } - }); + packageData.forAllConversations(conversationInfo -> { + if (isEligibleForCleanUp(conversationInfo)) { + cachedConvos.add( + Pair.create(packageData.getPackageName(), conversationInfo)); + } + }); }); if (cachedConvos.size() <= targetCachedCount) { return; @@ -867,8 +872,8 @@ public class DataManager { numToUncache + 1, Comparator.comparingLong((Pair<String, ConversationInfo> pair) -> Math.max( - pair.second.getLastEventTimestamp(), - pair.second.getCreationTimestamp())).reversed()); + pair.second.getLastEventTimestamp(), + pair.second.getCreationTimestamp())).reversed()); for (Pair<String, ConversationInfo> cached : cachedConvos) { if (hasActiveNotifications(cached.first, userId, cached.second.getShortcutId())) { continue; @@ -1104,26 +1109,35 @@ public class DataManager { @Override public void onShortcutsAddedOrUpdated(@NonNull String packageName, @NonNull List<ShortcutInfo> shortcuts, @NonNull UserHandle user) { - mInjector.getBackgroundExecutor().execute(() -> { - PackageData packageData = getPackage(packageName, user.getIdentifier()); - for (ShortcutInfo shortcut : shortcuts) { - if (ShortcutHelper.isConversationShortcut( - shortcut, mShortcutServiceInternal, user.getIdentifier())) { - if (shortcut.isCached()) { - ConversationInfo conversationInfo = packageData != null - ? packageData.getConversationInfo(shortcut.getId()) : null; - if (conversationInfo == null - || !conversationInfo.isShortcutCachedForNotification()) { - // This is a newly cached shortcut. Clean up the existing cached - // shortcuts to ensure the cache size is under the limit. - cleanupCachedShortcuts(user.getIdentifier(), - MAX_CACHED_RECENT_SHORTCUTS - 1); + mShortcutsThrottler.scheduleDebounced( + new Pair<>(packageName, user.getIdentifier()), + () -> { + PackageData packageData = getPackage(packageName, user.getIdentifier()); + List<ShortcutInfo> queriedShortcuts = getShortcuts(packageName, + user.getIdentifier(), null); + boolean hasCachedShortcut = false; + for (ShortcutInfo shortcut : queriedShortcuts) { + if (ShortcutHelper.isConversationShortcut( + shortcut, mShortcutServiceInternal, user.getIdentifier())) { + if (shortcut.isCached()) { + ConversationInfo info = packageData != null + ? packageData.getConversationInfo(shortcut.getId()) + : null; + if (info == null + || !info.isShortcutCachedForNotification()) { + hasCachedShortcut = true; + } + } + addOrUpdateConversationInfo(shortcut); } } - addOrUpdateConversationInfo(shortcut); - } - } - }); + // Added at least one new conversation. Uncache older existing cached + // shortcuts to ensure the cache size is under the limit. + if (hasCachedShortcut) { + cleanupCachedShortcuts(user.getIdentifier(), + MAX_CACHED_RECENT_SHORTCUTS); + } + }); } @Override diff --git a/services/people/java/com/android/server/people/data/PerPackageThrottler.java b/services/people/java/com/android/server/people/data/PerPackageThrottler.java new file mode 100644 index 000000000000..3d6cd84d326a --- /dev/null +++ b/services/people/java/com/android/server/people/data/PerPackageThrottler.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.people.data; + +import android.util.Pair; + +/** The interface for throttling expensive runnables per package. */ +interface PerPackageThrottler { + /** + * Schedule a runnable to run in the future, and debounce runnables for same {@code pkgUserId} + * that occur until that future has run. + */ + void scheduleDebounced(Pair<String, Integer> pkgUserId, Runnable runnable); +} diff --git a/services/people/java/com/android/server/people/data/PerPackageThrottlerImpl.java b/services/people/java/com/android/server/people/data/PerPackageThrottlerImpl.java new file mode 100644 index 000000000000..fa5a67b328b2 --- /dev/null +++ b/services/people/java/com/android/server/people/data/PerPackageThrottlerImpl.java @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.people.data; + +import android.os.Handler; +import android.util.Pair; + +import java.util.HashSet; + +/** + * A class that implements a per-package throttler that prevents a runnable from executing more than + * once every {@code debounceTime}. + */ +public class PerPackageThrottlerImpl implements PerPackageThrottler { + private final Handler mBackgroundHandler; + private final int mDebounceTime; + private final HashSet<Pair<String, Integer>> mPkgScheduledTasks = new HashSet<>(); + + PerPackageThrottlerImpl(Handler backgroundHandler, int debounceTime) { + mBackgroundHandler = backgroundHandler; + mDebounceTime = debounceTime; + } + + @Override + public synchronized void scheduleDebounced( + Pair<String, Integer> pkgUserId, Runnable runnable) { + if (mPkgScheduledTasks.contains(pkgUserId)) { + return; + } + mPkgScheduledTasks.add(pkgUserId); + mBackgroundHandler.postDelayed(() -> { + synchronized (this) { + mPkgScheduledTasks.remove(pkgUserId); + runnable.run(); + } + }, mDebounceTime); + } +} diff --git a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java b/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java index a27602d65118..454a23b70cb5 100644 --- a/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/people/data/DataManagerTest.java @@ -86,6 +86,7 @@ import android.service.notification.StatusBarNotification; import android.telecom.TelecomManager; import android.telephony.TelephonyManager; import android.text.format.DateUtils; +import android.util.Pair; import android.util.Range; import com.android.internal.app.ChooserActivity; @@ -186,6 +187,7 @@ public final class DataManagerTest { private ShortcutInfo mShortcutInfo; private TestInjector mInjector; private TestLooper mLooper; + private TestPerPackageThrottler mShortcutThrottler; @Before public void setUp() throws PackageManager.NameNotFoundException { @@ -275,7 +277,9 @@ public final class DataManagerTest { mInjector = new TestInjector(); mLooper = new TestLooper(); - mDataManager = new DataManager(mContext, mInjector, mLooper.getLooper()); + mShortcutThrottler = new TestPerPackageThrottler(); + mDataManager = new DataManager(mContext, mInjector, mLooper.getLooper(), + mShortcutThrottler); mDataManager.initialize(); when(mShortcutServiceInternal.isSharingShortcut(anyInt(), anyString(), anyString(), @@ -283,10 +287,7 @@ public final class DataManagerTest { mShortcutInfo = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID, buildPerson()); - when(mShortcutServiceInternal.getShortcuts( - anyInt(), anyString(), anyLong(), anyString(), anyList(), any(), any(), - anyInt(), anyInt(), anyInt(), anyInt())) - .thenReturn(Collections.singletonList(mShortcutInfo)); + mockGetShortcuts(Collections.singletonList(mShortcutInfo)); verify(mShortcutServiceInternal).addShortcutChangeCallback( mShortcutChangeCallbackCaptor.capture()); mShortcutChangeCallback = mShortcutChangeCallbackCaptor.getValue(); @@ -972,6 +973,7 @@ public final class DataManagerTest { buildPerson()); ShortcutInfo shortcut3 = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, "sc3", buildPerson()); + mockGetShortcuts(List.of(shortcut1, shortcut2, shortcut3)); mShortcutChangeCallback.onShortcutsAddedOrUpdated(TEST_PKG_NAME, Arrays.asList(shortcut1, shortcut2, shortcut3), UserHandle.of(USER_ID_PRIMARY)); mShortcutChangeCallback.onShortcutsRemoved(TEST_PKG_NAME, @@ -1223,7 +1225,6 @@ public final class DataManagerTest { eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS)); } } - @Test public void testUncacheOldestCachedShortcut_missingNotificationEvents() { mDataManager.onUserUnlocked(USER_ID_PRIMARY); @@ -1233,6 +1234,7 @@ public final class DataManagerTest { ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, shortcutId, buildPerson()); shortcut.setCached(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS); + mockGetShortcuts(Collections.singletonList(shortcut)); mShortcutChangeCallback.onShortcutsAddedOrUpdated( TEST_PKG_NAME, Collections.singletonList(shortcut), @@ -1252,7 +1254,6 @@ public final class DataManagerTest { eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS)); } } - @Test public void testUncacheOldestCachedShortcut_legacyConversation() { mDataManager.onUserUnlocked(USER_ID_PRIMARY); @@ -1274,6 +1275,7 @@ public final class DataManagerTest { ShortcutInfo shortcut = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, shortcutId, buildPerson()); shortcut.setCached(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS); + mockGetShortcuts(Collections.singletonList(shortcut)); mShortcutChangeCallback.onShortcutsAddedOrUpdated( TEST_PKG_NAME, Collections.singletonList(shortcut), @@ -1311,7 +1313,8 @@ public final class DataManagerTest { mDataManager.reportShareTargetEvent(appTargetEvent, intentFilter); byte[] payload = mDataManager.getBackupPayload(USER_ID_PRIMARY); - DataManager dataManager = new DataManager(mContext, mInjector, mLooper.getLooper()); + DataManager dataManager = new DataManager( + mContext, mInjector, mLooper.getLooper(), mShortcutThrottler); dataManager.onUserUnlocked(USER_ID_PRIMARY); dataManager.restore(USER_ID_PRIMARY, payload); ConversationInfo conversationInfo = dataManager.getPackage(TEST_PKG_NAME, USER_ID_PRIMARY) @@ -1723,6 +1726,13 @@ public final class DataManagerTest { return (queryFlags & flag) != 0; } + private void mockGetShortcuts(List<ShortcutInfo> shortcutInfoList) { + when(mShortcutServiceInternal.getShortcuts( + anyInt(), anyString(), anyLong(), anyString(), any(), any(), any(), + anyInt(), anyInt(), anyInt(), anyInt())) + .thenReturn(shortcutInfoList); + } + // "Sends" a notification to a non-customized notification channel - the notification channel // is something generic like "messages" and the notification has a shortcut id private void sendGenericNotification() { @@ -1948,4 +1958,11 @@ public final class DataManagerTest { return mUsageStatsQueryHelper; } } + + private static class TestPerPackageThrottler implements PerPackageThrottler { + @Override + public void scheduleDebounced(Pair<String, Integer> pkgUserId, Runnable runnable) { + runnable.run(); + } + } } diff --git a/services/tests/servicestests/src/com/android/server/people/data/PerPackageThrottlerImplTest.java b/services/tests/servicestests/src/com/android/server/people/data/PerPackageThrottlerImplTest.java new file mode 100644 index 000000000000..672cbb94e074 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/people/data/PerPackageThrottlerImplTest.java @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.people.data; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import android.util.Pair; + +import com.android.server.testutils.OffsettableClock; +import com.android.server.testutils.TestHandler; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +import java.util.concurrent.atomic.AtomicBoolean; + +@RunWith(JUnit4.class) +public class PerPackageThrottlerImplTest { + private static final int DEBOUNCE_INTERVAL = 500; + private static final String PKG_ONE = "pkg_one"; + private static final String PKG_TWO = "pkg_two"; + private static final int USER_ID = 10; + + private final OffsettableClock mClock = new OffsettableClock.Stopped(); + private final TestHandler mTestHandler = new TestHandler(null, mClock); + private PerPackageThrottlerImpl mThrottler; + + @Before + public void setUp() { + mThrottler = new PerPackageThrottlerImpl(mTestHandler, DEBOUNCE_INTERVAL); + } + + @Test + public void scheduleDebounced() { + AtomicBoolean pkgOneRan = new AtomicBoolean(); + AtomicBoolean pkgTwoRan = new AtomicBoolean(); + + mThrottler.scheduleDebounced(new Pair<>(PKG_ONE, USER_ID), () -> pkgOneRan.set(true)); + mThrottler.scheduleDebounced(new Pair<>(PKG_ONE, USER_ID), () -> pkgOneRan.set(true)); + mThrottler.scheduleDebounced(new Pair<>(PKG_TWO, USER_ID), () -> pkgTwoRan.set(true)); + mThrottler.scheduleDebounced(new Pair<>(PKG_TWO, USER_ID), () -> pkgTwoRan.set(true)); + + assertFalse(pkgOneRan.get()); + assertFalse(pkgTwoRan.get()); + mClock.fastForward(DEBOUNCE_INTERVAL); + mTestHandler.timeAdvance(); + assertTrue(pkgOneRan.get()); + assertTrue(pkgTwoRan.get()); + } +} |