From 1af4c0d40e266024e11578d0cbb1a049012c6f48 Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Thu, 8 Jun 2023 12:44:05 -0700 Subject: [PATCH] Relax delivery group policy constraints for resultTo bcasts. For resultTo broadcasts, we can always skip and remove the old broadcast for a deferred receiver as long as it is one of the intended receiver for the new broadcast. Also, limit the application of "merged" delivery group policy to only broadcasts with one receiver for now as it is not trivial to support broadcasts with multiple receivers. Bug: 279696145 Test: atest services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java Test: atest services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java Change-Id: Iaf30a8f6d7d3bb0c77727ea621b22c190ec86f1e --- .../server/am/BroadcastQueueModernImpl.java | 53 +++++++++++++++---- .../am/BroadcastQueueModernImplTest.java | 41 ++++++++++++++ 2 files changed, 85 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index cbc75401f0b86..55f8cef019f84 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -33,6 +33,7 @@ import static com.android.server.am.ActivityManagerDebugConfig.LOG_WRITER_INFO; import static com.android.server.am.BroadcastProcessQueue.insertIntoRunnableList; import static com.android.server.am.BroadcastProcessQueue.reasonToString; import static com.android.server.am.BroadcastProcessQueue.removeFromRunnableList; +import static com.android.server.am.BroadcastRecord.DELIVERY_DEFERRED; import static com.android.server.am.BroadcastRecord.deliveryStateToString; import static com.android.server.am.BroadcastRecord.getReceiverClassName; import static com.android.server.am.BroadcastRecord.getReceiverPackageName; @@ -68,6 +69,7 @@ import android.os.RemoteException; import android.os.SystemClock; import android.os.UserHandle; import android.text.format.DateUtils; +import android.util.ArrayMap; import android.util.ArraySet; import android.util.IndentingPrintWriter; import android.util.MathUtils; @@ -212,6 +214,13 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private final AtomicReference> mReplacedBroadcastsCache = new AtomicReference<>(); + /** + * Container for holding the set of broadcast records that satisfied a certain criteria. + */ + @GuardedBy("mService") + private final AtomicReference> mRecordsLookupCache = + new AtomicReference<>(); + /** * Map from UID to its last known "foreground" state. A UID is considered to be in * "foreground" state when it's procState is {@link ActivityManager#PROCESS_STATE_TOP}. @@ -741,13 +750,16 @@ class BroadcastQueueModernImpl extends BroadcastQueue { broadcastConsumer = mBroadcastConsumerSkipAndCanceled; break; case BroadcastOptions.DELIVERY_GROUP_POLICY_MERGED: + // TODO: Allow applying MERGED policy for broadcasts with more than one receiver. + if (r.receivers.size() > 1) { + return; + } final BundleMerger extrasMerger = r.options.getDeliveryGroupExtrasMerger(); if (extrasMerger == null) { // Extras merger is required to be able to merge the extras. So, if it's not // supplied, then ignore the delivery group policy. return; } - // TODO: Don't merge with the same BroadcastRecord more than once. broadcastConsumer = (record, recordIndex) -> { r.intent.mergeExtras(record.intent, extrasMerger); mBroadcastConsumerSkipAndCanceled.accept(record, recordIndex); @@ -757,6 +769,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { logw("Unknown delivery group policy: " + policy); return; } + final ArrayMap recordsLookupCache = getRecordsLookupCache(); forEachMatchingBroadcast(QUEUE_PREDICATE_ANY, (testRecord, testIndex) -> { // If the receiver is already in a terminal state, then ignore it. if (isDeliveryStateTerminal(testRecord.getDeliveryState(testIndex))) { @@ -768,22 +781,44 @@ class BroadcastQueueModernImpl extends BroadcastQueue { || !r.matchesDeliveryGroup(testRecord)) { return false; } - // TODO: If a process is in a deferred state, we can always apply the policy as long - // as it is one of the receivers for the new broadcast. // For ordered broadcast, check if the receivers for the new broadcast is a superset // of those for the previous one as skipping and removing only one of them could result // in an inconsistent state. - if (testRecord.ordered || testRecord.resultTo != null) { - // TODO: Cache this result in some way so that we don't have to perform the - // same check for all the broadcast receivers. - return r.containsAllReceivers(testRecord.receivers); - } else if (testRecord.prioritized) { - return r.containsAllReceivers(testRecord.receivers); + if (testRecord.ordered || testRecord.prioritized) { + return containsAllReceivers(r, testRecord, recordsLookupCache); + } else if (testRecord.resultTo != null) { + return testRecord.getDeliveryState(testIndex) == DELIVERY_DEFERRED + ? r.containsReceiver(testRecord.receivers.get(testIndex)) + : containsAllReceivers(r, testRecord, recordsLookupCache); } else { return r.containsReceiver(testRecord.receivers.get(testIndex)); } }, broadcastConsumer, true); + recordsLookupCache.clear(); + mRecordsLookupCache.compareAndSet(null, recordsLookupCache); + } + + @NonNull + private ArrayMap getRecordsLookupCache() { + ArrayMap recordsLookupCache = + mRecordsLookupCache.getAndSet(null); + if (recordsLookupCache == null) { + recordsLookupCache = new ArrayMap<>(); + } + return recordsLookupCache; + } + + private boolean containsAllReceivers(@NonNull BroadcastRecord record, + @NonNull BroadcastRecord testRecord, + @NonNull ArrayMap recordsLookupCache) { + final int idx = recordsLookupCache.indexOfKey(testRecord); + if (idx > 0) { + return recordsLookupCache.valueAt(idx); + } + final boolean containsAll = record.containsAllReceivers(testRecord.receivers); + recordsLookupCache.put(testRecord, containsAll); + return containsAll; } /** diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java index 948687ae16453..e6eeaaa65e029 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java @@ -1092,6 +1092,17 @@ public final class BroadcastQueueModernImplTest { verifyPendingRecords(greenQueue, List.of(screenOff, screenOn)); verifyPendingRecords(redQueue, List.of(screenOff)); verifyPendingRecords(blueQueue, List.of(screenOff, screenOn)); + + final BroadcastRecord screenOffRecord = makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), resultTo, false); + screenOffRecord.setDeliveryState(2, BroadcastRecord.DELIVERY_DEFERRED, + "testDeliveryGroupPolicy_resultTo_diffReceivers"); + mImpl.enqueueBroadcastLocked(screenOffRecord); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), resultTo, false)); + verifyPendingRecords(greenQueue, List.of(screenOff, screenOn)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOn)); } @Test @@ -1276,6 +1287,36 @@ public final class BroadcastQueueModernImplTest { dropboxEntryBroadcast2.first, expectedMergedBroadcast.first)); } + @Test + public void testDeliveryGroupPolicy_merged_multipleReceivers() { + final long now = SystemClock.elapsedRealtime(); + final Pair dropboxEntryBroadcast1 = createDropboxBroadcast( + "TAG_A", now, 2); + final Pair dropboxEntryBroadcast2 = createDropboxBroadcast( + "TAG_A", now + 1000, 4); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(dropboxEntryBroadcast1.first, + dropboxEntryBroadcast1.second, + List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN), + makeManifestReceiver(PACKAGE_RED, CLASS_RED)), + false)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(dropboxEntryBroadcast2.first, + dropboxEntryBroadcast2.second, + List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN), + makeManifestReceiver(PACKAGE_RED, CLASS_RED)), + false)); + + final BroadcastProcessQueue greenQueue = mImpl.getProcessQueue(PACKAGE_GREEN, + getUidForPackage(PACKAGE_GREEN)); + final BroadcastProcessQueue redQueue = mImpl.getProcessQueue(PACKAGE_RED, + getUidForPackage(PACKAGE_RED)); + + verifyPendingRecords(greenQueue, + List.of(dropboxEntryBroadcast1.first, dropboxEntryBroadcast2.first)); + verifyPendingRecords(redQueue, + List.of(dropboxEntryBroadcast1.first, dropboxEntryBroadcast2.first)); + } + @Test public void testDeliveryGroupPolicy_sameAction_differentMatchingCriteria() { final Intent closeSystemDialogs1 = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS);