Merge "Revert "[People Service] Add throttling to DataManager add / change listener"" into tm-qpr-dev

5 files changed, 36 insertions, 214 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 37f0624464e5..eff9e8da9a76 100644
--- a/services/people/java/com/android/server/people/data/DataManager.java
+++ b/services/people/java/com/android/server/people/data/DataManager.java
@@ -113,7 +113,6 @@ 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;
@@ -130,7 +129,6 @@ 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;
@@ -142,17 +140,14 @@ public class DataManager {
     private ConversationStatusExpirationBroadcastReceiver mStatusExpReceiver;
 
     public DataManager(Context context) {
-        this(context, new Injector(), BackgroundThread.get().getLooper(),
-                new PerPackageThrottlerImpl(BackgroundThread.getHandler(), DEBOUNCE_LENGTH_MS));
+        this(context, new Injector(), BackgroundThread.get().getLooper());
     }
 
-    DataManager(Context context, Injector injector, Looper looper,
-            PerPackageThrottler shortcutsThrottler) {
+    DataManager(Context context, Injector injector, Looper looper) {
         mContext = context;
         mInjector = injector;
         mScheduledExecutor = mInjector.createScheduledExecutor();
         mHandler = new Handler(looper);
-        mShortcutsThrottler = shortcutsThrottler;
     }
 
     /** Initialization. Called when the system services are up running. */
@@ -856,12 +851,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;
@@ -872,8 +867,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;
@@ -1109,35 +1104,26 @@ public class DataManager {
         @Override
         public void onShortcutsAddedOrUpdated(@NonNull String packageName,
                 @NonNull List<ShortcutInfo> shortcuts, @NonNull UserHandle user) {
-            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);
+            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);
                             }
                         }
-                        // 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);
-                        }
-                    });
+                        addOrUpdateConversationInfo(shortcut);
+                    }
+                }
+            });
         }
 
         @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
deleted file mode 100644
index 3d6cd84d326a..000000000000
--- a/services/people/java/com/android/server/people/data/PerPackageThrottler.java
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * 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
deleted file mode 100644
index fa5a67b328b2..000000000000
--- a/services/people/java/com/android/server/people/data/PerPackageThrottlerImpl.java
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * 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 454a23b70cb5..a27602d65118 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,7 +86,6 @@ 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;
@@ -187,7 +186,6 @@ public final class DataManagerTest {
     private ShortcutInfo mShortcutInfo;
     private TestInjector mInjector;
     private TestLooper mLooper;
-    private TestPerPackageThrottler mShortcutThrottler;
 
     @Before
     public void setUp() throws PackageManager.NameNotFoundException {
@@ -277,9 +275,7 @@ public final class DataManagerTest {
 
         mInjector = new TestInjector();
         mLooper = new TestLooper();
-        mShortcutThrottler = new TestPerPackageThrottler();
-        mDataManager = new DataManager(mContext, mInjector, mLooper.getLooper(),
-                mShortcutThrottler);
+        mDataManager = new DataManager(mContext, mInjector, mLooper.getLooper());
         mDataManager.initialize();
 
         when(mShortcutServiceInternal.isSharingShortcut(anyInt(), anyString(), anyString(),
@@ -287,7 +283,10 @@ public final class DataManagerTest {
 
         mShortcutInfo = buildShortcutInfo(TEST_PKG_NAME, USER_ID_PRIMARY, TEST_SHORTCUT_ID,
                 buildPerson());
-        mockGetShortcuts(Collections.singletonList(mShortcutInfo));
+        when(mShortcutServiceInternal.getShortcuts(
+                anyInt(), anyString(), anyLong(), anyString(), anyList(), any(), any(),
+                anyInt(), anyInt(), anyInt(), anyInt()))
+                .thenReturn(Collections.singletonList(mShortcutInfo));
         verify(mShortcutServiceInternal).addShortcutChangeCallback(
                 mShortcutChangeCallbackCaptor.capture());
         mShortcutChangeCallback = mShortcutChangeCallbackCaptor.getValue();
@@ -973,7 +972,6 @@ 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,
@@ -1225,6 +1223,7 @@ public final class DataManagerTest {
                     eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS));
         }
     }
+
     @Test
     public void testUncacheOldestCachedShortcut_missingNotificationEvents() {
         mDataManager.onUserUnlocked(USER_ID_PRIMARY);
@@ -1234,7 +1233,6 @@ 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),
@@ -1254,6 +1252,7 @@ public final class DataManagerTest {
                     eq(ShortcutInfo.FLAG_CACHED_NOTIFICATIONS));
         }
     }
+
     @Test
     public void testUncacheOldestCachedShortcut_legacyConversation() {
         mDataManager.onUserUnlocked(USER_ID_PRIMARY);
@@ -1275,7 +1274,6 @@ 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),
@@ -1313,8 +1311,7 @@ public final class DataManagerTest {
         mDataManager.reportShareTargetEvent(appTargetEvent, intentFilter);
         byte[] payload = mDataManager.getBackupPayload(USER_ID_PRIMARY);
 
-        DataManager dataManager = new DataManager(
-                mContext, mInjector, mLooper.getLooper(), mShortcutThrottler);
+        DataManager dataManager = new DataManager(mContext, mInjector, mLooper.getLooper());
         dataManager.onUserUnlocked(USER_ID_PRIMARY);
         dataManager.restore(USER_ID_PRIMARY, payload);
         ConversationInfo conversationInfo = dataManager.getPackage(TEST_PKG_NAME, USER_ID_PRIMARY)
@@ -1726,13 +1723,6 @@ 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() {
@@ -1958,11 +1948,4 @@ 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
deleted file mode 100644
index 672cbb94e074..000000000000
--- a/services/tests/servicestests/src/com/android/server/people/data/PerPackageThrottlerImplTest.java
+++ /dev/null
@@ -1,67 +0,0 @@
-/*
- * 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());
-    }
-}