OSDN Git Service

Fixes JobScheduler not persisting flex and sourcePackage
authorShreyas Basarge <snb@google.com>
Fri, 12 Feb 2016 15:49:31 +0000 (15:49 +0000)
committerShreyas Basarge <snb@google.com>
Mon, 15 Feb 2016 15:51:52 +0000 (15:51 +0000)
There was a bug with persisteing and restoring flex and
sourcePackage. Fixed it and added tests.

Change-Id: Ie8e4714b4727ecef4254773fd4339b28f4a47c01

services/core/java/com/android/server/job/JobSchedulerService.java
services/core/java/com/android/server/job/JobStore.java
services/core/java/com/android/server/job/controllers/JobStatus.java
services/tests/servicestests/src/com/android/server/job/JobStoreTest.java

index 4028372..57cede8 100644 (file)
@@ -231,10 +231,7 @@ public class JobSchedulerService extends com.android.server.SystemService
     }
 
     public int scheduleAsPackage(JobInfo job, int uId, String packageName, int userId) {
-        JobStatus jobStatus = new JobStatus(job, uId);
-        if (packageName != null) {
-            jobStatus.setSource(packageName, userId);
-        }
+        JobStatus jobStatus = new JobStatus(job, uId, packageName, userId);
         try {
             if (ActivityManagerNative.getDefault().getAppStartMode(uId,
                     job.getService().getPackageName()) == ActivityManager.APP_START_MODE_DISABLED) {
index f6a6778..3565fc1 100644 (file)
@@ -282,8 +282,7 @@ public class JobStore {
                         continue;
                     }
 
-                    JobStatus copy = new JobStatus(jobStatus.getJob(), jobStatus.getUid(),
-                            jobStatus.getEarliestRunTime(), jobStatus.getLatestRunTimeElapsed());
+                    JobStatus copy = new JobStatus(jobStatus);
                     mStoreCopy.add(copy);
                 }
             }
@@ -544,7 +543,7 @@ public class JobStore {
         private JobStatus restoreJobFromXml(XmlPullParser parser) throws XmlPullParserException,
                 IOException {
             JobInfo.Builder jobBuilder;
-            int uid, userId;
+            int uid, sourceUserId;
 
             // Read out job identifier attributes and priority.
             try {
@@ -557,7 +556,7 @@ public class JobStore {
                     jobBuilder.setPriority(Integer.valueOf(val));
                 }
                 val = parser.getAttributeValue(null, "sourceUserId");
-                userId = val == null ? -1 : Integer.valueOf(val);
+                sourceUserId = val == null ? -1 : Integer.valueOf(val);
             } catch (NumberFormatException e) {
                 Slog.e(TAG, "Error parsing job's required fields, skipping");
                 return null;
@@ -608,9 +607,9 @@ public class JobStore {
                 try {
                     String val = parser.getAttributeValue(null, "period");
                     final long periodMillis = Long.valueOf(val);
-                    jobBuilder.setPeriodic(periodMillis);
                     val = parser.getAttributeValue(null, "flex");
                     final long flexMillis = (val != null) ? Long.valueOf(val) : periodMillis;
+                    jobBuilder.setPeriodic(periodMillis, flexMillis);
                     // As a sanity check, cap the recreated run time to be no later than flex+period
                     // from now. This is the latest the periodic could be pushed out. This could
                     // happen if the periodic ran early (at flex time before period), and then the
@@ -679,10 +678,8 @@ public class JobStore {
             parser.nextTag(); // Consume </extras>
 
             JobStatus js = new JobStatus(
-                    jobBuilder.build(), uid, elapsedRuntimes.first, elapsedRuntimes.second);
-            if (userId != -1) {
-                js.setSource(sourcePackageName, userId);
-            }
+                    jobBuilder.build(), uid, sourcePackageName, sourceUserId, elapsedRuntimes.first,
+                    elapsedRuntimes.second);
             return js;
         }
 
index e749433..c4d564c 100644 (file)
@@ -48,13 +48,13 @@ public class JobStatus {
 
     final JobInfo job;
     /** Uid of the package requesting this job. */
-    final int uId;
+    final int callingUid;
     final String name;
     final String tag;
 
-    String sourcePackageName;
-    int sourceUserId = -1;
-    int sourceUid = -1;
+    final String sourcePackageName;
+    final int sourceUserId;
+    final int sourceUid;
 
     // Constraints.
     final AtomicBoolean chargingConstraintSatisfied = new AtomicBoolean();
@@ -91,30 +91,55 @@ public class JobStatus {
 
     /** Provide a handle to the service that this job will be run on. */
     public int getServiceToken() {
-        return uId;
+        return callingUid;
     }
 
-    private JobStatus(JobInfo job, int uId, int numFailures) {
+    private JobStatus(JobInfo job, int callingUid, String sourcePackageName, int sourceUserId,
+                      int numFailures) {
         this.job = job;
-        this.uId = uId;
-        this.sourceUid = uId;
+        this.callingUid = callingUid;
         this.name = job.getService().flattenToShortString();
         this.tag = "*job*/" + this.name;
         this.numFailures = numFailures;
+
+        int tempSourceUid = -1;
+        if (sourceUserId != -1 && sourcePackageName != null) {
+            try {
+                tempSourceUid = AppGlobals.getPackageManager().getPackageUid(sourcePackageName, 0,
+                        sourceUserId);
+            } catch (RemoteException ex) {
+                // Can't happen, PackageManager runs in the same process.
+            }
+        }
+        if (tempSourceUid == -1) {
+            this.sourceUid = callingUid;
+            this.sourceUserId = UserHandle.getUserId(callingUid);
+            this.sourcePackageName = job.getService().getPackageName();
+        } else {
+            this.sourceUid = tempSourceUid;
+            this.sourceUserId = sourceUserId;
+            this.sourcePackageName = sourcePackageName;
+        }
     }
 
     /** Copy constructor. */
     public JobStatus(JobStatus jobStatus) {
-        this(jobStatus.getJob(), jobStatus.getUid(), jobStatus.getNumFailures());
-        this.sourceUserId = jobStatus.sourceUserId;
-        this.sourcePackageName = jobStatus.sourcePackageName;
+        this(jobStatus.getJob(), jobStatus.getUid(), jobStatus.getSourcePackageName(),
+                jobStatus.getSourceUserId(), jobStatus.getNumFailures());
         this.earliestRunTimeElapsedMillis = jobStatus.getEarliestRunTime();
         this.latestRunTimeElapsedMillis = jobStatus.getLatestRunTimeElapsed();
     }
 
-    /** Create a newly scheduled job. */
-    public JobStatus(JobInfo job, int uId) {
-        this(job, uId, 0);
+    /**
+     * Create a newly scheduled job.
+     * @param callingUid Uid of the package that scheduled this job.
+     * @param sourcePackageName Package name on whose behalf this job is scheduled. Null indicates
+     *                          the calling package is the source.
+     * @param sourceUserId User id for whom this job is scheduled. -1 indicates this is same as the
+     *                     calling userId.
+     */
+    public JobStatus(JobInfo job, int callingUid, String sourcePackageName, int sourceUserId) {
+        this(job, callingUid, sourcePackageName, sourceUserId, 0);
 
         final long elapsedNow = SystemClock.elapsedRealtime();
 
@@ -136,9 +161,9 @@ public class JobStatus {
      * wallclock runtime rather than resetting it on every boot.
      * We consider a freshly loaded job to no longer be in back-off.
      */
-    public JobStatus(JobInfo job, int uId, long earliestRunTimeElapsedMillis,
-                      long latestRunTimeElapsedMillis) {
-        this(job, uId, 0);
+    public JobStatus(JobInfo job, int callingUid, String sourcePackageName, int sourceUserId,
+                     long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) {
+        this(job, callingUid, sourcePackageName, sourceUserId, 0);
 
         this.earliestRunTimeElapsedMillis = earliestRunTimeElapsedMillis;
         this.latestRunTimeElapsedMillis = latestRunTimeElapsedMillis;
@@ -147,9 +172,8 @@ public class JobStatus {
     /** Create a new job to be rescheduled with the provided parameters. */
     public JobStatus(JobStatus rescheduling, long newEarliestRuntimeElapsedMillis,
                       long newLatestRuntimeElapsedMillis, int backoffAttempt) {
-        this(rescheduling.job, rescheduling.getUid(), backoffAttempt);
-        this.sourceUserId = rescheduling.sourceUserId;
-        this.sourcePackageName = rescheduling.sourcePackageName;
+        this(rescheduling.job, rescheduling.getUid(), rescheduling.getSourcePackageName(),
+                rescheduling.getSourceUserId(), backoffAttempt);
 
         earliestRunTimeElapsedMillis = newEarliestRuntimeElapsedMillis;
         latestRunTimeElapsedMillis = newLatestRuntimeElapsedMillis;
@@ -172,7 +196,7 @@ public class JobStatus {
     }
 
     public String getSourcePackageName() {
-        return sourcePackageName != null ? sourcePackageName : job.getService().getPackageName();
+        return sourcePackageName;
     }
 
     public int getSourceUid() {
@@ -180,18 +204,15 @@ public class JobStatus {
     }
 
     public int getSourceUserId() {
-        if (sourceUserId == -1) {
-            sourceUserId = getUserId();
-        }
         return sourceUserId;
     }
 
     public int getUserId() {
-        return UserHandle.getUserId(uId);
+        return UserHandle.getUserId(callingUid);
     }
 
     public int getUid() {
-        return uId;
+        return callingUid;
     }
 
     public String getName() {
@@ -278,7 +299,7 @@ public class JobStatus {
     }
 
     public boolean matches(int uid, int jobId) {
-        return this.job.getId() == jobId && this.uId == uid;
+        return this.job.getId() == jobId && this.callingUid == uid;
     }
 
     @Override
@@ -312,22 +333,6 @@ public class JobStatus {
         }
     }
 
-    public void setSource(String sourcePackageName, int sourceUserId) {
-        this.sourcePackageName = sourcePackageName;
-        this.sourceUserId = sourceUserId;
-        try {
-            sourceUid = AppGlobals.getPackageManager().getPackageUid(sourcePackageName, 0,
-                    sourceUserId);
-        } catch (RemoteException ex) {
-            // Can't happen, PackageManager runs in the same process.
-        }
-        if (sourceUid == -1) {
-            sourceUid = uId;
-            this.sourceUserId = getUserId();
-            this.sourcePackageName = null;
-        }
-    }
-
     /**
      * Convenience function to identify a job uniquely without pulling all the data that
      * {@link #toString()} returns.
@@ -338,7 +343,7 @@ public class JobStatus {
         sb.append(" jId=");
         sb.append(job.getId());
         sb.append(" uid=");
-        UserHandle.formatUid(sb, uId);
+        UserHandle.formatUid(sb, callingUid);
         sb.append(' ');
         sb.append(job.getService().flattenToShortString());
         return sb.toString();
@@ -346,7 +351,7 @@ public class JobStatus {
 
     // Dumpsys infrastructure
     public void dump(PrintWriter pw, String prefix) {
-        pw.print(prefix); UserHandle.formatUid(pw, uId);
+        pw.print(prefix); UserHandle.formatUid(pw, callingUid);
         pw.print(" tag="); pw.println(tag);
         pw.print(prefix);
         pw.print("Source: uid="); UserHandle.formatUid(pw, getSourceUid());
index ae0a25e..577c3a1 100644 (file)
@@ -58,7 +58,7 @@ public class JobStoreTest extends AndroidTestCase {
                 .setMinimumLatency(runFromMillis)
                 .setPersisted(true)
                 .build();
-        final JobStatus ts = new JobStatus(task, SOME_UID);
+        final JobStatus ts = new JobStatus(task, SOME_UID, null, -1);
         mTaskStoreUnderTest.add(ts);
         Thread.sleep(IO_WAIT);
         // Manually load tasks from xml file.
@@ -91,8 +91,8 @@ public class JobStoreTest extends AndroidTestCase {
                 .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED)
                 .setPersisted(true)
                 .build();
-        final JobStatus taskStatus1 = new JobStatus(task1, SOME_UID);
-        final JobStatus taskStatus2 = new JobStatus(task2, SOME_UID);
+        final JobStatus taskStatus1 = new JobStatus(task1, SOME_UID, null, -1);
+        final JobStatus taskStatus2 = new JobStatus(task2, SOME_UID, null, -1);
         mTaskStoreUnderTest.add(taskStatus1);
         mTaskStoreUnderTest.add(taskStatus2);
         Thread.sleep(IO_WAIT);
@@ -140,7 +140,7 @@ public class JobStoreTest extends AndroidTestCase {
         extras.putInt("into", 3);
         b.setExtras(extras);
         final JobInfo task = b.build();
-        JobStatus taskStatus = new JobStatus(task, SOME_UID);
+        JobStatus taskStatus = new JobStatus(task, SOME_UID, null, -1);
 
         mTaskStoreUnderTest.add(taskStatus);
         Thread.sleep(IO_WAIT);
@@ -151,17 +151,59 @@ public class JobStoreTest extends AndroidTestCase {
         JobStatus loaded = jobStatusSet.iterator().next();
         assertTasksEqual(task, loaded.getJob());
     }
+    public void testWritingTaskWithSourcePackage() throws Exception {
+        JobInfo.Builder b = new Builder(8, mComponent)
+                .setRequiresDeviceIdle(true)
+                .setPeriodic(10000L)
+                .setRequiresCharging(true)
+                .setPersisted(true);
+        JobStatus taskStatus = new JobStatus(b.build(), SOME_UID, "com.google.android.gms", 0);
+
+        mTaskStoreUnderTest.add(taskStatus);
+        Thread.sleep(IO_WAIT);
+
+        final ArraySet<JobStatus> jobStatusSet = new ArraySet<JobStatus>();
+        mTaskStoreUnderTest.readJobMapFromDisk(jobStatusSet);
+        assertEquals("Incorrect # of persisted tasks.", 1, jobStatusSet.size());
+        JobStatus loaded = jobStatusSet.iterator().next();
+        assertEquals("Source package not equal.", loaded.getSourcePackageName(),
+                taskStatus.getSourcePackageName());
+        assertEquals("Source user not equal.", loaded.getSourceUserId(),
+                taskStatus.getSourceUserId());
+    }
+
+    public void testWritingTaskWithFlex() throws Exception {
+        JobInfo.Builder b = new Builder(8, mComponent)
+                .setRequiresDeviceIdle(true)
+                .setPeriodic(5*60*60*1000, 1*60*60*1000)
+                .setRequiresCharging(true)
+                .setPersisted(true);
+        JobStatus taskStatus = new JobStatus(b.build(), SOME_UID, null, -1);
+
+        mTaskStoreUnderTest.add(taskStatus);
+        Thread.sleep(IO_WAIT);
+
+        final ArraySet<JobStatus> jobStatusSet = new ArraySet<JobStatus>();
+        mTaskStoreUnderTest.readJobMapFromDisk(jobStatusSet);
+        assertEquals("Incorrect # of persisted tasks.", 1, jobStatusSet.size());
+        JobStatus loaded = jobStatusSet.iterator().next();
+        assertEquals("Period not equal.", loaded.getJob().getIntervalMillis(),
+                taskStatus.getJob().getIntervalMillis());
+        assertEquals("Flex not equal.", loaded.getJob().getFlexMillis(),
+                taskStatus.getJob().getFlexMillis());
+    }
 
     public void testMassivePeriodClampedOnRead() throws Exception {
-        final long TEN_SECONDS = 10000L;
+        final long ONE_HOUR = 60*60*1000L; // flex
+        final long TWO_HOURS = 2 * ONE_HOUR; // period
         JobInfo.Builder b = new Builder(8, mComponent)
-                .setPeriodic(TEN_SECONDS)
+                .setPeriodic(TWO_HOURS, ONE_HOUR)
                 .setPersisted(true);
         final long invalidLateRuntimeElapsedMillis =
-                SystemClock.elapsedRealtime() + (TEN_SECONDS * 2) + 5000;  // >2P from now.
+                SystemClock.elapsedRealtime() + (TWO_HOURS * ONE_HOUR) + TWO_HOURS;  // > period+flex
         final long invalidEarlyRuntimeElapsedMillis =
-                invalidLateRuntimeElapsedMillis - TEN_SECONDS;  // Early is (late - period).
-        final JobStatus js = new JobStatus(b.build(), SOME_UID,
+                invalidLateRuntimeElapsedMillis - TWO_HOURS;  // Early is (late - period).
+        final JobStatus js = new JobStatus(b.build(), SOME_UID, "somePackage", 0 /* sourceUserId */,
                 invalidEarlyRuntimeElapsedMillis, invalidLateRuntimeElapsedMillis);
 
         mTaskStoreUnderTest.add(js);
@@ -176,10 +218,10 @@ public class JobStoreTest extends AndroidTestCase {
         // call SystemClock.elapsedRealtime after doing the disk i/o.
         final long newNowElapsed = SystemClock.elapsedRealtime();
         assertTrue("Early runtime wasn't correctly clamped.",
-                loaded.getEarliestRunTime() <= newNowElapsed + TEN_SECONDS);
-        // Assert late runtime was clamped to be now + period*2.
+                loaded.getEarliestRunTime() <= newNowElapsed + TWO_HOURS);
+        // Assert late runtime was clamped to be now + period + flex.
         assertTrue("Early runtime wasn't correctly clamped.",
-                loaded.getEarliestRunTime() <= newNowElapsed + TEN_SECONDS * 2);
+                loaded.getEarliestRunTime() <= newNowElapsed + TWO_HOURS + ONE_HOUR);
     }
 
     public void testPriorityPersisted() throws Exception {
@@ -187,7 +229,7 @@ public class JobStoreTest extends AndroidTestCase {
                 .setOverrideDeadline(5000)
                 .setPriority(42)
                 .setPersisted(true);
-        final JobStatus js = new JobStatus(b.build(), SOME_UID);
+        final JobStatus js = new JobStatus(b.build(), SOME_UID, null, -1);
         mTaskStoreUnderTest.add(js);
         Thread.sleep(IO_WAIT);
         final ArraySet<JobStatus> jobStatusSet = new ArraySet<JobStatus>();
@@ -203,12 +245,12 @@ public class JobStoreTest extends AndroidTestCase {
         JobInfo.Builder b = new Builder(42, mComponent)
                 .setOverrideDeadline(10000)
                 .setPersisted(false);
-        JobStatus jsNonPersisted = new JobStatus(b.build(), SOME_UID);
+        JobStatus jsNonPersisted = new JobStatus(b.build(), SOME_UID, null, -1);
         mTaskStoreUnderTest.add(jsNonPersisted);
         b = new Builder(43, mComponent)
                 .setOverrideDeadline(10000)
                 .setPersisted(true);
-        JobStatus jsPersisted = new JobStatus(b.build(), SOME_UID);
+        JobStatus jsPersisted = new JobStatus(b.build(), SOME_UID, null, -1);
         mTaskStoreUnderTest.add(jsPersisted);
         Thread.sleep(IO_WAIT);
         final ArraySet<JobStatus> jobStatusSet = new ArraySet<JobStatus>();