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

Commit a8fa0615 authored by Kweku Adams's avatar Kweku Adams
Browse files

Fix job readiness checking. 

Only mark dynamic constraints as satisfied if there are any dynamic
constraints specified, otherwise, JS will bypass the quota check and
always consider jobs without dynamic constraints as ready.

Bug: 151274168
Test: atest CtsJobSchedulerTestCases
Test: atest QuotaControllerTest
Change-Id: Ie70c8b1ae138eb063af981ae2b248b1a0bdbdc60
parent b8d5f9d7

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

+11 −11
Original line number Original line Diff line number Diff line
@@ -472,7 +472,7 @@ public final class JobStatus { 
        if (standbyBucket == RESTRICTED_INDEX) {

        if (standbyBucket == RESTRICTED_INDEX) {

            addDynamicConstraints(DYNAMIC_RESTRICTED_CONSTRAINTS);

            addDynamicConstraints(DYNAMIC_RESTRICTED_CONSTRAINTS);

        } else {

        } else {

            mReadyDynamicSatisfied = true;

            mReadyDynamicSatisfied = false;

        }

        }





        mLastSuccessfulRunTime = lastSuccessfulRunTime;

        mLastSuccessfulRunTime = lastSuccessfulRunTime;
@@ -1132,8 +1132,8 @@ public final class JobStatus { 
        }

        }

        satisfiedConstraints = (satisfiedConstraints&~constraint) | (state ? constraint : 0);

        satisfiedConstraints = (satisfiedConstraints&~constraint) | (state ? constraint : 0);

        mSatisfiedConstraintsOfInterest = satisfiedConstraints & CONSTRAINTS_OF_INTEREST;

        mSatisfiedConstraintsOfInterest = satisfiedConstraints & CONSTRAINTS_OF_INTEREST;

        mReadyDynamicSatisfied =

        mReadyDynamicSatisfied = mDynamicConstraints != 0

                mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

                && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

        if (STATS_LOG_ENABLED && (STATSD_CONSTRAINTS_TO_LOG & constraint) != 0) {

        if (STATS_LOG_ENABLED && (STATSD_CONSTRAINTS_TO_LOG & constraint) != 0) {

            FrameworkStatsLog.write_non_chained(

            FrameworkStatsLog.write_non_chained(

                    FrameworkStatsLog.SCHEDULED_JOB_CONSTRAINT_CHANGED,

                    FrameworkStatsLog.SCHEDULED_JOB_CONSTRAINT_CHANGED,
@@ -1184,8 +1184,8 @@ public final class JobStatus { 
        }

        }





        mDynamicConstraints |= constraints;

        mDynamicConstraints |= constraints;

        mReadyDynamicSatisfied =

        mReadyDynamicSatisfied = mDynamicConstraints != 0

                mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

                && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

    }

    }





    /**

    /**
@@ -1196,8 +1196,8 @@ public final class JobStatus { 
     */

     */

    private void removeDynamicConstraints(int constraints) {

    private void removeDynamicConstraints(int constraints) {

        mDynamicConstraints &= ~constraints;

        mDynamicConstraints &= ~constraints;

        mReadyDynamicSatisfied =

        mReadyDynamicSatisfied = mDynamicConstraints != 0

                mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

                && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

    }

    }





    public long getLastSuccessfulRunTime() {

    public long getLastSuccessfulRunTime() {
@@ -1241,8 +1241,8 @@ public final class JobStatus { 
                break;

                break;

            default:

            default:

                satisfied |= constraint;

                satisfied |= constraint;

                mReadyDynamicSatisfied =

                mReadyDynamicSatisfied = mDynamicConstraints != 0

                        mDynamicConstraints == (satisfied & mDynamicConstraints);

                        && mDynamicConstraints == (satisfied & mDynamicConstraints);

                break;

                break;

        }

        }
@@ -1262,8 +1262,8 @@ public final class JobStatus { 
                mReadyWithinQuota = oldValue;

                mReadyWithinQuota = oldValue;

                break;

                break;

            default:

            default:

                mReadyDynamicSatisfied =

                mReadyDynamicSatisfied = mDynamicConstraints != 0

                        mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

                        && mDynamicConstraints == (satisfiedConstraints & mDynamicConstraints);

                break;

                break;

        }

        }

        return toReturn;

        return toReturn;

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

+8 −1
Original line number Original line Diff line number Diff line
@@ -291,7 +291,6 @@ public class QuotaControllerTest { 
    private JobStatus createJobStatus(String testTag, int jobId) {

    private JobStatus createJobStatus(String testTag, int jobId) {

        JobInfo jobInfo = new JobInfo.Builder(jobId,

        JobInfo jobInfo = new JobInfo.Builder(jobId,

                new ComponentName(mContext, "TestQuotaJobService"))

                new ComponentName(mContext, "TestQuotaJobService"))

                .setMinimumLatency(Math.abs(jobId) + 1)

                .build();

                .build();

        return createJobStatus(testTag, SOURCE_PACKAGE, CALLING_UID, jobInfo);

        return createJobStatus(testTag, SOURCE_PACKAGE, CALLING_UID, jobInfo);

    }

    }
@@ -302,6 +301,9 @@ public class QuotaControllerTest { 
                jobInfo, callingUid, packageName, SOURCE_USER_ID, testTag);

                jobInfo, callingUid, packageName, SOURCE_USER_ID, testTag);

        // Make sure tests aren't passing just because the default bucket is likely ACTIVE.

        // Make sure tests aren't passing just because the default bucket is likely ACTIVE.

        js.setStandbyBucket(FREQUENT_INDEX);

        js.setStandbyBucket(FREQUENT_INDEX);

        // Make sure Doze and background-not-restricted don't affect tests.

        js.setDeviceNotDozingConstraintSatisfied(/* state */ true, /* whitelisted */false);

        js.setBackgroundNotRestrictedConstraintSatisfied(true);

        return js;

        return js;

    }

    }
@@ -649,6 +651,7 @@ public class QuotaControllerTest { 
            assertEquals(expectedStats, inputStats);

            assertEquals(expectedStats, inputStats);

            assertTrue(mQuotaController.isWithinQuotaLocked(SOURCE_USER_ID, SOURCE_PACKAGE,

            assertTrue(mQuotaController.isWithinQuotaLocked(SOURCE_USER_ID, SOURCE_PACKAGE,

                    RARE_INDEX));

                    RARE_INDEX));

            assertTrue("Job not ready: " + jobStatus, jobStatus.isReady());

        }

        }





        // Add old session. Make sure values are combined correctly.

        // Add old session. Make sure values are combined correctly.
@@ -689,6 +692,7 @@ public class QuotaControllerTest { 
        assertEquals(expectedStats, inputStats);

        assertEquals(expectedStats, inputStats);

        assertFalse(

        assertFalse(

                mQuotaController.isWithinQuotaLocked(SOURCE_USER_ID, SOURCE_PACKAGE, RARE_INDEX));

                mQuotaController.isWithinQuotaLocked(SOURCE_USER_ID, SOURCE_PACKAGE, RARE_INDEX));

        assertFalse("Job unexpectedly ready: " + jobStatus, jobStatus.isReady());

    }

    }





    /**

    /**
@@ -1325,6 +1329,7 @@ public class QuotaControllerTest { 
        assertEquals(2, mQuotaController.getExecutionStatsLocked(

        assertEquals(2, mQuotaController.getExecutionStatsLocked(

                SOURCE_USER_ID, SOURCE_PACKAGE, ACTIVE_INDEX).jobCountInRateLimitingWindow);

                SOURCE_USER_ID, SOURCE_PACKAGE, ACTIVE_INDEX).jobCountInRateLimitingWindow);

        assertTrue(mQuotaController.isWithinQuotaLocked(jobStatus));

        assertTrue(mQuotaController.isWithinQuotaLocked(jobStatus));

        assertTrue(jobStatus.isReady());

    }

    }





    @Test

    @Test
@@ -1387,7 +1392,9 @@ public class QuotaControllerTest { 
        mQuotaController.maybeStopTrackingJobLocked(unaffected, null, false);

        mQuotaController.maybeStopTrackingJobLocked(unaffected, null, false);





        assertTrue(mQuotaController.isWithinQuotaLocked(unaffected));

        assertTrue(mQuotaController.isWithinQuotaLocked(unaffected));

        assertTrue(unaffected.isReady());

        assertFalse(mQuotaController.isWithinQuotaLocked(fgStateChanger));

        assertFalse(mQuotaController.isWithinQuotaLocked(fgStateChanger));

        assertFalse(fgStateChanger.isReady());

        assertEquals(1,

        assertEquals(1,

                mQuotaController.getTimingSessions(SOURCE_USER_ID, unaffectedPkgName).size());

                mQuotaController.getTimingSessions(SOURCE_USER_ID, unaffectedPkgName).size());

        assertEquals(42,

        assertEquals(42,