Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 07e30787 authored by Sudheer Shanka's avatar Sudheer Shanka
Browse files

Skip enqueueing receivers that are going to be skipped by policy. 

The current logic assumes that only receivers in apps that are
not already running will be skipped but this is not the case.
Since we are going to skip the broadcast anyway, don't enqueue
it in the broadcast process queue.

Bug: 277815320
Test: atest FrameworksMockingServicesTests:BroadcastQueueTest
Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest
Change-Id: I27cb4a0959c20e2dbc656f9650dea4d0643bcba5
parent 8bcb06a1

services/core/java/com/android/server/am/BroadcastProcessQueue.java

+14 −27
Original line number Diff line number Diff line
@@ -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();

services/core/java/com/android/server/am/BroadcastQueueModernImpl.java

+9 −15
Original line number Diff line number Diff line
@@ -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

            // 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 to avoid cold start");

                        "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);

            }

services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java

+48 −23
Original line number Diff line number Diff line
@@ -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);