From e75cadfaf9cec6513170d93c95cbd8ea1df408c5 Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Mon, 22 May 2023 11:37:16 -0700 Subject: [PATCH] Apply delivery policies only to receivers that receive the new bcast. If a receiver is not going to receive the new broadcast, we should not skip and remove the older broadcast to it. 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: I626c67e86712d8e255b2b706d4f10803728e8434 --- .../server/am/BroadcastQueueModernImpl.java | 28 ++- .../android/server/am/BroadcastRecord.java | 18 ++ .../am/BroadcastQueueModernImplTest.java | 160 ++++++++++++++++++ 3 files changed, 203 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index d9b315794ea3f..60401ccc98090 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -675,6 +675,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // 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); @@ -685,10 +686,31 @@ class BroadcastQueueModernImpl extends BroadcastQueue { return; } forEachMatchingBroadcast(QUEUE_PREDICATE_ANY, (testRecord, testIndex) -> { + // If the receiver is already in a terminal state, then ignore it. + if (isDeliveryStateTerminal(testRecord.getDeliveryState(testIndex))) { + return false; + } // We only allow caller to remove broadcasts they enqueued - return (r.callingUid == testRecord.callingUid) - && (r.userId == testRecord.userId) - && r.matchesDeliveryGroup(testRecord); + if ((r.callingUid != testRecord.callingUid) + || (r.userId != testRecord.userId) + || !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); + } else { + return r.containsReceiver(testRecord.receivers.get(testIndex)); + } }, broadcastConsumer, true); } diff --git a/services/core/java/com/android/server/am/BroadcastRecord.java b/services/core/java/com/android/server/am/BroadcastRecord.java index a402db945321d..564b1fe8de34f 100644 --- a/services/core/java/com/android/server/am/BroadcastRecord.java +++ b/services/core/java/com/android/server/am/BroadcastRecord.java @@ -1092,6 +1092,24 @@ final class BroadcastRecord extends Binder { } } + boolean containsReceiver(@NonNull Object receiver) { + for (int i = receivers.size() - 1; i >= 0; --i) { + if (isReceiverEquals(receiver, receivers.get(i))) { + return true; + } + } + return false; + } + + boolean containsAllReceivers(@NonNull List otherReceivers) { + for (int i = otherReceivers.size() - 1; i >= 0; --i) { + if (!containsReceiver(otherReceivers.get(i))) { + return false; + } + } + return true; + } + boolean matchesDeliveryGroup(@NonNull BroadcastRecord other) { return matchesDeliveryGroup(this, other); } 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 f47954baf6494..e53fc17778963 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java @@ -947,6 +947,166 @@ public final class BroadcastQueueModernImplTest { List.of(musicVolumeChanged, alarmVolumeChanged, timeTick)); } + @Test + public void testDeliveryGroupPolicy_diffReceivers() { + final Intent screenOn = new Intent(Intent.ACTION_SCREEN_ON); + final Intent screenOff = new Intent(Intent.ACTION_SCREEN_OFF); + final BroadcastOptions screenOnOffOptions = BroadcastOptions.makeBasic() + .setDeliveryGroupPolicy(BroadcastOptions.DELIVERY_GROUP_POLICY_MOST_RECENT) + .setDeliveryGroupMatchingKey("screenOnOff", Intent.ACTION_SCREEN_ON); + + final Object greenReceiver = makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN); + final Object redReceiver = makeManifestReceiver(PACKAGE_RED, CLASS_RED); + final Object blueReceiver = makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), false)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), false)); + final BroadcastProcessQueue greenQueue = mImpl.getProcessQueue(PACKAGE_GREEN, + getUidForPackage(PACKAGE_GREEN)); + final BroadcastProcessQueue redQueue = mImpl.getProcessQueue(PACKAGE_RED, + getUidForPackage(PACKAGE_RED)); + final BroadcastProcessQueue blueQueue = mImpl.getProcessQueue(PACKAGE_BLUE, + getUidForPackage(PACKAGE_BLUE)); + verifyPendingRecords(greenQueue, List.of(screenOff)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOff)); + + assertTrue(greenQueue.isEmpty()); + assertTrue(redQueue.isEmpty()); + assertTrue(blueQueue.isEmpty()); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), false)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), false)); + verifyPendingRecords(greenQueue, List.of(screenOn)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOn)); + } + + @Test + public void testDeliveryGroupPolicy_ordered_diffReceivers() { + final Intent screenOn = new Intent(Intent.ACTION_SCREEN_ON); + final Intent screenOff = new Intent(Intent.ACTION_SCREEN_OFF); + final BroadcastOptions screenOnOffOptions = BroadcastOptions.makeBasic() + .setDeliveryGroupPolicy(BroadcastOptions.DELIVERY_GROUP_POLICY_MOST_RECENT) + .setDeliveryGroupMatchingKey("screenOnOff", Intent.ACTION_SCREEN_ON); + + final Object greenReceiver = makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN); + final Object redReceiver = makeManifestReceiver(PACKAGE_RED, CLASS_RED); + final Object blueReceiver = makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), true)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), true)); + final BroadcastProcessQueue greenQueue = mImpl.getProcessQueue(PACKAGE_GREEN, + getUidForPackage(PACKAGE_GREEN)); + final BroadcastProcessQueue redQueue = mImpl.getProcessQueue(PACKAGE_RED, + getUidForPackage(PACKAGE_RED)); + final BroadcastProcessQueue blueQueue = mImpl.getProcessQueue(PACKAGE_BLUE, + getUidForPackage(PACKAGE_BLUE)); + verifyPendingRecords(greenQueue, List.of(screenOff)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOff)); + + assertTrue(greenQueue.isEmpty()); + assertTrue(redQueue.isEmpty()); + assertTrue(blueQueue.isEmpty()); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), true)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), true)); + verifyPendingRecords(greenQueue, List.of(screenOff, screenOn)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOff, screenOn)); + } + + @Test + public void testDeliveryGroupPolicy_resultTo_diffReceivers() { + final Intent screenOn = new Intent(Intent.ACTION_SCREEN_ON); + final Intent screenOff = new Intent(Intent.ACTION_SCREEN_OFF); + final BroadcastOptions screenOnOffOptions = BroadcastOptions.makeBasic() + .setDeliveryGroupPolicy(BroadcastOptions.DELIVERY_GROUP_POLICY_MOST_RECENT) + .setDeliveryGroupMatchingKey("screenOnOff", Intent.ACTION_SCREEN_ON); + + final Object greenReceiver = makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN); + final Object redReceiver = makeManifestReceiver(PACKAGE_RED, CLASS_RED); + final Object blueReceiver = makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE); + final IIntentReceiver resultTo = mock(IIntentReceiver.class); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), resultTo, false)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), resultTo, false)); + final BroadcastProcessQueue greenQueue = mImpl.getProcessQueue(PACKAGE_GREEN, + getUidForPackage(PACKAGE_GREEN)); + final BroadcastProcessQueue redQueue = mImpl.getProcessQueue(PACKAGE_RED, + getUidForPackage(PACKAGE_RED)); + final BroadcastProcessQueue blueQueue = mImpl.getProcessQueue(PACKAGE_BLUE, + getUidForPackage(PACKAGE_BLUE)); + verifyPendingRecords(greenQueue, List.of(screenOff)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOff)); + + assertTrue(greenQueue.isEmpty()); + assertTrue(redQueue.isEmpty()); + assertTrue(blueQueue.isEmpty()); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), resultTo, false)); + 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(screenOff, screenOn)); + } + + @Test + public void testDeliveryGroupPolicy_prioritized_diffReceivers() { + final Intent screenOn = new Intent(Intent.ACTION_SCREEN_ON); + final Intent screenOff = new Intent(Intent.ACTION_SCREEN_OFF); + final BroadcastOptions screenOnOffOptions = BroadcastOptions.makeBasic() + .setDeliveryGroupPolicy(BroadcastOptions.DELIVERY_GROUP_POLICY_MOST_RECENT) + .setDeliveryGroupMatchingKey("screenOnOff", Intent.ACTION_SCREEN_ON); + + final Object greenReceiver = withPriority( + makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN), 10); + final Object redReceiver = withPriority( + makeManifestReceiver(PACKAGE_RED, CLASS_RED), 5); + final Object blueReceiver = withPriority( + makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE), 0); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), false)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), false)); + final BroadcastProcessQueue greenQueue = mImpl.getProcessQueue(PACKAGE_GREEN, + getUidForPackage(PACKAGE_GREEN)); + final BroadcastProcessQueue redQueue = mImpl.getProcessQueue(PACKAGE_RED, + getUidForPackage(PACKAGE_RED)); + final BroadcastProcessQueue blueQueue = mImpl.getProcessQueue(PACKAGE_BLUE, + getUidForPackage(PACKAGE_BLUE)); + verifyPendingRecords(greenQueue, List.of(screenOff)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOff)); + + assertTrue(greenQueue.isEmpty()); + assertTrue(redQueue.isEmpty()); + assertTrue(blueQueue.isEmpty()); + + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOff, screenOnOffOptions, + List.of(greenReceiver, redReceiver, blueReceiver), false)); + mImpl.enqueueBroadcastLocked(makeBroadcastRecord(screenOn, screenOnOffOptions, + List.of(greenReceiver, blueReceiver), false)); + verifyPendingRecords(greenQueue, List.of(screenOff, screenOn)); + verifyPendingRecords(redQueue, List.of(screenOff)); + verifyPendingRecords(blueQueue, List.of(screenOff, screenOn)); + } + /** * Verify that sending a broadcast with DELIVERY_GROUP_POLICY_MERGED works as expected. */