diff options
3 files changed, 71 insertions, 65 deletions
diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java index 0767218ec065..5c68e6759083 100644 --- a/services/core/java/com/android/server/am/BroadcastProcessQueue.java +++ b/services/core/java/com/android/server/am/BroadcastProcessQueue.java @@ -242,8 +242,7 @@ class BroadcastProcessQueue { */ @Nullable public BroadcastRecord enqueueOrReplaceBroadcast(@NonNull BroadcastRecord record, - int recordIndex, boolean wouldBeSkipped, - @NonNull BroadcastConsumer deferredStatesApplyConsumer) { + int recordIndex, @NonNull BroadcastConsumer deferredStatesApplyConsumer) { // When updateDeferredStates() has already applied a deferred state to // all pending items, apply to this new broadcast too if (mLastDeferredStates && record.deferUntilActive @@ -252,8 +251,7 @@ class BroadcastProcessQueue { } if (record.isReplacePending()) { - final BroadcastRecord replacedBroadcastRecord = replaceBroadcast(record, recordIndex, - wouldBeSkipped); + final BroadcastRecord replacedBroadcastRecord = replaceBroadcast(record, recordIndex); if (replacedBroadcastRecord != null) { return replacedBroadcastRecord; } @@ -264,14 +262,13 @@ class BroadcastProcessQueue { SomeArgs newBroadcastArgs = SomeArgs.obtain(); newBroadcastArgs.arg1 = record; newBroadcastArgs.argi1 = recordIndex; - newBroadcastArgs.argi2 = (wouldBeSkipped ? 1 : 0); // Cross-broadcast prioritization policy: some broadcasts might warrant being // issued ahead of others that are already pending, for example if this new // broadcast is in a different delivery class or is tied to a direct user interaction // with implicit responsiveness expectations. getQueueForBroadcast(record).addLast(newBroadcastArgs); - onBroadcastEnqueued(record, recordIndex, wouldBeSkipped); + onBroadcastEnqueued(record, recordIndex); return null; } @@ -285,10 +282,9 @@ class BroadcastProcessQueue { * wasn't any broadcast that was replaced. */ @Nullable - private BroadcastRecord replaceBroadcast(@NonNull BroadcastRecord record, int recordIndex, - boolean wouldBeSkipped) { + private BroadcastRecord replaceBroadcast(@NonNull BroadcastRecord record, int recordIndex) { final ArrayDeque<SomeArgs> queue = getQueueForBroadcast(record); - return replaceBroadcastInQueue(queue, record, recordIndex, wouldBeSkipped); + return replaceBroadcastInQueue(queue, record, recordIndex); } /** @@ -302,15 +298,13 @@ class BroadcastProcessQueue { */ @Nullable private BroadcastRecord replaceBroadcastInQueue(@NonNull ArrayDeque<SomeArgs> queue, - @NonNull BroadcastRecord record, int recordIndex, - boolean wouldBeSkipped) { + @NonNull BroadcastRecord record, int recordIndex) { final Iterator<SomeArgs> it = queue.descendingIterator(); final Object receiver = record.receivers.get(recordIndex); while (it.hasNext()) { final SomeArgs args = it.next(); final BroadcastRecord testRecord = (BroadcastRecord) args.arg1; final int testRecordIndex = args.argi1; - final boolean testWouldBeSkipped = (args.argi2 == 1); final Object testReceiver = testRecord.receivers.get(testRecordIndex); if ((record.callingUid == testRecord.callingUid) && (record.userId == testRecord.userId) @@ -320,10 +314,9 @@ class BroadcastProcessQueue { // Exact match found; perform in-place swap args.arg1 = record; args.argi1 = recordIndex; - args.argi2 = (wouldBeSkipped ? 1 : 0); record.copyEnqueueTimeFrom(testRecord); - onBroadcastDequeued(testRecord, testRecordIndex, testWouldBeSkipped); - onBroadcastEnqueued(record, recordIndex, wouldBeSkipped); + onBroadcastDequeued(testRecord, testRecordIndex); + onBroadcastEnqueued(record, recordIndex); return testRecord; } } @@ -383,13 +376,12 @@ class BroadcastProcessQueue { final SomeArgs args = it.next(); final BroadcastRecord record = (BroadcastRecord) args.arg1; final int recordIndex = args.argi1; - final boolean recordWouldBeSkipped = (args.argi2 == 1); if (predicate.test(record, recordIndex)) { consumer.accept(record, recordIndex); if (andRemove) { args.recycle(); it.remove(); - onBroadcastDequeued(record, recordIndex, recordWouldBeSkipped); + onBroadcastDequeued(record, recordIndex); } else { // Even if we're leaving broadcast in queue, it may have // been mutated in such a way to change our runnable time @@ -539,12 +531,11 @@ class BroadcastProcessQueue { final SomeArgs next = removeNextBroadcast(); mActive = (BroadcastRecord) next.arg1; mActiveIndex = next.argi1; - final boolean wouldBeSkipped = (next.argi2 == 1); mActiveCountSinceIdle++; mActiveViaColdStart = false; mActiveWasStopped = false; next.recycle(); - onBroadcastDequeued(mActive, mActiveIndex, wouldBeSkipped); + onBroadcastDequeued(mActive, mActiveIndex); } /** @@ -561,8 +552,7 @@ class BroadcastProcessQueue { /** * Update summary statistics when the given record has been enqueued. */ - private void onBroadcastEnqueued(@NonNull BroadcastRecord record, int recordIndex, - boolean wouldBeSkipped) { + private void onBroadcastEnqueued(@NonNull BroadcastRecord record, int recordIndex) { mCountEnqueued++; if (record.deferUntilActive) { mCountDeferred++; @@ -594,8 +584,7 @@ class BroadcastProcessQueue { if (record.callerInstrumented) { mCountInstrumented++; } - if (!wouldBeSkipped - && (record.receivers.get(recordIndex) instanceof ResolveInfo)) { + if (record.receivers.get(recordIndex) instanceof ResolveInfo) { mCountManifest++; } invalidateRunnableAt(); @@ -604,8 +593,7 @@ class BroadcastProcessQueue { /** * Update summary statistics when the given record has been dequeued. */ - private void onBroadcastDequeued(@NonNull BroadcastRecord record, int recordIndex, - boolean wouldBeSkipped) { + private void onBroadcastDequeued(@NonNull BroadcastRecord record, int recordIndex) { mCountEnqueued--; if (record.deferUntilActive) { mCountDeferred--; @@ -637,8 +625,7 @@ class BroadcastProcessQueue { if (record.callerInstrumented) { mCountInstrumented--; } - if (!wouldBeSkipped - && (record.receivers.get(recordIndex) instanceof ResolveInfo)) { + if (record.receivers.get(recordIndex) instanceof ResolveInfo) { mCountManifest--; } invalidateRunnableAt(); diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index 8735f8a37b8b..96e152320282 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -606,24 +606,18 @@ class BroadcastQueueModernImpl extends BroadcastQueue { final BroadcastProcessQueue queue = getOrCreateProcessQueue( getReceiverProcessName(receiver), getReceiverUid(receiver)); - boolean wouldBeSkipped = false; - if (receiver instanceof ResolveInfo) { - // If the app is running but would not have been started if the process weren't - // running, we're going to deliver the broadcast but mark that it's not a manifest - // broadcast that would have started the app. This allows BroadcastProcessQueue to - // defer the broadcast as though it were a normal runtime receiver. - wouldBeSkipped = (mSkipPolicy.shouldSkipMessage(r, receiver) != null); - if (wouldBeSkipped && queue.app == null && !queue.getActiveViaColdStart()) { - // Skip receiver if there's no running app, the app is not being started, and - // the app wouldn't be launched for this broadcast - setDeliveryState(null, null, r, i, receiver, BroadcastRecord.DELIVERY_SKIPPED, - "skipped by policy to avoid cold start"); - continue; - } + // If this receiver is going to be skipped, skip it now itself and don't even enqueue + // it. + final boolean wouldBeSkipped = (mSkipPolicy.shouldSkipMessage(r, receiver) != null); + if (wouldBeSkipped) { + setDeliveryState(null, null, r, i, receiver, BroadcastRecord.DELIVERY_SKIPPED, + "skipped by policy at enqueue"); + continue; } + enqueuedBroadcast = true; final BroadcastRecord replacedBroadcast = queue.enqueueOrReplaceBroadcast( - r, i, wouldBeSkipped, mBroadcastConsumerDeferApply); + r, i, mBroadcastConsumerDeferApply); if (replacedBroadcast != null) { replacedBroadcasts.add(replacedBroadcast); } 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 3fb7fb4e6aea..acfea85d60a2 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java @@ -40,6 +40,7 @@ import static com.android.server.am.BroadcastQueueTest.PACKAGE_YELLOW; import static com.android.server.am.BroadcastQueueTest.getUidForPackage; import static com.android.server.am.BroadcastQueueTest.makeManifestReceiver; import static com.android.server.am.BroadcastQueueTest.withPriority; +import static com.android.server.am.BroadcastRecord.isReceiverEquals; import static com.google.common.truth.Truth.assertThat; @@ -49,13 +50,16 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import android.annotation.NonNull; @@ -116,6 +120,7 @@ public final class BroadcastQueueModernImplTest { TestLooperManager mLooper; BroadcastConstants mConstants; + private BroadcastSkipPolicy mSkipPolicy; BroadcastQueueModernImpl mImpl; BroadcastProcessQueue mHead; @@ -139,29 +144,18 @@ public final class BroadcastQueueModernImplTest { mConstants.DELAY_NORMAL_MILLIS = 10_000; mConstants.DELAY_CACHED_MILLIS = 120_000; - final BroadcastSkipPolicy emptySkipPolicy = new BroadcastSkipPolicy(mAms) { - public boolean shouldSkip(BroadcastRecord r, Object o) { - // Ignored - return false; - } - public String shouldSkipMessage(BroadcastRecord r, Object o) { - // Ignored - return null; - } - public boolean disallowBackgroundStart(BroadcastRecord r) { - // Ignored - return false; - } - }; + mSkipPolicy = spy(new BroadcastSkipPolicy(mAms)); + doReturn(null).when(mSkipPolicy).shouldSkipMessage(any(), any()); + doReturn(false).when(mSkipPolicy).disallowBackgroundStart(any()); + final BroadcastHistory emptyHistory = new BroadcastHistory(mConstants) { public void addBroadcastToHistoryLocked(BroadcastRecord original) { // Ignored } }; - mImpl = new BroadcastQueueModernImpl(mAms, mHandlerThread.getThreadHandler(), - mConstants, mConstants, emptySkipPolicy, emptyHistory); + mConstants, mConstants, mSkipPolicy, emptyHistory); doReturn(1L).when(mQueue1).getRunnableAt(); doReturn(2L).when(mQueue2).getRunnableAt(); @@ -256,17 +250,12 @@ public final class BroadcastQueueModernImplTest { private void enqueueOrReplaceBroadcast(BroadcastProcessQueue queue, BroadcastRecord record, int recordIndex) { - enqueueOrReplaceBroadcast(queue, record, recordIndex, false, 42_000_000L); + enqueueOrReplaceBroadcast(queue, record, recordIndex, 42_000_000L); } private void enqueueOrReplaceBroadcast(BroadcastProcessQueue queue, BroadcastRecord record, int recordIndex, long enqueueTime) { - enqueueOrReplaceBroadcast(queue, record, recordIndex, false, enqueueTime); - } - - private void enqueueOrReplaceBroadcast(BroadcastProcessQueue queue, - BroadcastRecord record, int recordIndex, boolean wouldBeSkipped, long enqueueTime) { - queue.enqueueOrReplaceBroadcast(record, recordIndex, wouldBeSkipped, (r, i) -> { + queue.enqueueOrReplaceBroadcast(record, recordIndex, (r, i) -> { throw new UnsupportedOperationException(); }); record.enqueueTime = enqueueTime; @@ -1199,6 +1188,42 @@ public final class BroadcastQueueModernImplTest { assertEquals(ProcessList.SCHED_GROUP_DEFAULT, queue.getPreferredSchedulingGroupLocked()); } + @Test + public void testSkipPolicy_atEnqueueTime() throws Exception { + final Intent userPresent = new Intent(Intent.ACTION_USER_PRESENT); + final Object greenReceiver = makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN); + final Object redReceiver = makeManifestReceiver(PACKAGE_RED, CLASS_RED); + + final BroadcastRecord userPresentRecord = makeBroadcastRecord(userPresent, + List.of(greenReceiver, redReceiver)); + + final Intent timeTick = new Intent(Intent.ACTION_TIME_TICK); + final BroadcastRecord timeTickRecord = makeBroadcastRecord(timeTick, + List.of(greenReceiver, redReceiver)); + + doAnswer(invocation -> { + final BroadcastRecord r = invocation.getArgument(0); + final Object o = invocation.getArgument(1); + if (userPresent.getAction().equals(r.intent.getAction()) + && isReceiverEquals(o, greenReceiver)) { + return "receiver skipped by test"; + } + return null; + }).when(mSkipPolicy).shouldSkipMessage(any(BroadcastRecord.class), any()); + + mImpl.enqueueBroadcastLocked(userPresentRecord); + mImpl.enqueueBroadcastLocked(timeTickRecord); + + final BroadcastProcessQueue greenQueue = mImpl.getProcessQueue(PACKAGE_GREEN, + getUidForPackage(PACKAGE_GREEN)); + // There should be only one broadcast for green process as the other would have + // been skipped. + verifyPendingRecords(greenQueue, List.of(timeTick)); + final BroadcastProcessQueue redQueue = mImpl.getProcessQueue(PACKAGE_RED, + getUidForPackage(PACKAGE_RED)); + verifyPendingRecords(redQueue, List.of(userPresent, timeTick)); + } + private Intent createPackageChangedIntent(int uid, List<String> componentNameList) { final Intent packageChangedIntent = new Intent(Intent.ACTION_PACKAGE_CHANGED); packageChangedIntent.putExtra(Intent.EXTRA_UID, uid); |