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

Commit 5b5978a2 authored by Kweku Adams's avatar Kweku Adams
Browse files

Reverse order of operations. 

We were checking if the network is available before checking if the job
would even be ready with connectivity. It's better to do things in the
opposite order.

Bug: 176987520
Test: atest CtsJobSchedulerTestCases
Test: atest FrameworksMockingServicesTests:ConnectivityControllerTest
Change-Id: I09a3bd190262c013824f3a4478debe28bc5f7d8d
parent 7e8ae00d

apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java

+2 −15
Original line number Diff line number Diff line
@@ -217,20 +217,6 @@ public final class ConnectivityController extends RestrictingController implemen 
        return jobs != null && jobs.size() > 0;

    }



    @VisibleForTesting

    @GuardedBy("mLock")

    boolean wouldBeReadyWithConnectivityLocked(JobStatus jobStatus) {

        final boolean networkAvailable = isNetworkAvailable(jobStatus);

        if (DEBUG) {

            Slog.v(TAG, "wouldBeReadyWithConnectivityLocked: " + jobStatus.toShortString()

                    + " networkAvailable=" + networkAvailable);

        }

        // If the network isn't available, then requesting an exception won't help.



        return networkAvailable && wouldBeReadyWithConstraintLocked(jobStatus,

                JobStatus.CONSTRAINT_CONNECTIVITY);

    }



    /**

     * Tell NetworkPolicyManager not to block a UID's network connection if that's the only

     * thing stopping a job from running.
@@ -243,7 +229,8 @@ public final class ConnectivityController extends RestrictingController implemen 
        }



        // Always check the full job readiness stat in case the component has been disabled.

        if (wouldBeReadyWithConnectivityLocked(jobStatus)) {

        if (wouldBeReadyWithConstraintLocked(jobStatus, JobStatus.CONSTRAINT_CONNECTIVITY)

                && isNetworkAvailable(jobStatus)) {

            if (DEBUG) {

                Slog.i(TAG, "evaluateStateLocked finds job " + jobStatus + " would be ready.");

            }

services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java

+14 −27
Original line number Diff line number Diff line
@@ -382,27 +382,6 @@ public class ConnectivityControllerTest { 
        assertTrue(controller.isStandbyExceptionRequestedLocked(UID_BLUE));

    }



    @Test

    public void testWouldBeReadyWithConnectivityLocked() {

        final ConnectivityController controller = spy(new ConnectivityController(mService));

        final JobStatus red = createJobStatus(createJob()

                .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), 0)

                .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED), UID_RED);



        doReturn(false).when(controller).isNetworkAvailable(any());

        assertFalse(controller.wouldBeReadyWithConnectivityLocked(red));



        doReturn(true).when(controller).isNetworkAvailable(any());

        doReturn(false).when(controller).wouldBeReadyWithConstraintLocked(any(),

                eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        assertFalse(controller.wouldBeReadyWithConnectivityLocked(red));



        doReturn(true).when(controller).isNetworkAvailable(any());

        doReturn(true).when(controller).wouldBeReadyWithConstraintLocked(any(),

                eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        assertTrue(controller.wouldBeReadyWithConnectivityLocked(red));

    }



    @Test

    public void testEvaluateStateLocked_JobWithoutConnectivity() {

        final ConnectivityController controller = new ConnectivityController(mService);
@@ -417,7 +396,9 @@ public class ConnectivityControllerTest { 
    @Test

    public void testEvaluateStateLocked_JobWouldBeReady() {

        final ConnectivityController controller = spy(new ConnectivityController(mService));

        doReturn(true).when(controller).wouldBeReadyWithConnectivityLocked(any());

        doReturn(true).when(controller)

                .wouldBeReadyWithConstraintLocked(any(), eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        doReturn(true).when(controller).isNetworkAvailable(any());

        final JobStatus red = createJobStatus(createJob()

                .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), 0)

                .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED), UID_RED);
@@ -455,7 +436,8 @@ public class ConnectivityControllerTest { 
    @Test

    public void testEvaluateStateLocked_JobWouldNotBeReady() {

        final ConnectivityController controller = spy(new ConnectivityController(mService));

        doReturn(false).when(controller).wouldBeReadyWithConnectivityLocked(any());

        doReturn(false).when(controller)

                .wouldBeReadyWithConstraintLocked(any(), eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        final JobStatus red = createJobStatus(createJob()

                .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), 0)

                .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED), UID_RED);
@@ -523,21 +505,26 @@ public class ConnectivityControllerTest { 
                .setAppIdleWhitelist(eq(12345), anyBoolean());



        // Both jobs would still be ready. Exception should not be revoked.

        doReturn(true).when(controller).wouldBeReadyWithConnectivityLocked(any());

        doReturn(true).when(controller)

                .wouldBeReadyWithConstraintLocked(any(), eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        doReturn(true).when(controller).isNetworkAvailable(any());

        controller.reevaluateStateLocked(UID_RED);

        inOrder.verify(mNetPolicyManagerInternal, never())

                .setAppIdleWhitelist(eq(UID_RED), anyBoolean());



        // One job is still ready. Exception should not be revoked.

        doReturn(true).when(controller).wouldBeReadyWithConnectivityLocked(eq(redOne));

        doReturn(false).when(controller).wouldBeReadyWithConnectivityLocked(eq(redTwo));

        doReturn(true).when(controller).wouldBeReadyWithConstraintLocked(

                eq(redOne), eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        doReturn(false).when(controller).wouldBeReadyWithConstraintLocked(

                eq(redTwo), eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        controller.reevaluateStateLocked(UID_RED);

        inOrder.verify(mNetPolicyManagerInternal, never())

                .setAppIdleWhitelist(eq(UID_RED), anyBoolean());

        assertTrue(controller.isStandbyExceptionRequestedLocked(UID_RED));



        // Both jobs are not ready. Exception should be revoked.

        doReturn(false).when(controller).wouldBeReadyWithConnectivityLocked(any());

        doReturn(false).when(controller)

                .wouldBeReadyWithConstraintLocked(any(), eq(JobStatus.CONSTRAINT_CONNECTIVITY));

        controller.reevaluateStateLocked(UID_RED);

        inOrder.verify(mNetPolicyManagerInternal, times(1))

                .setAppIdleWhitelist(eq(UID_RED), eq(false));