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

Commit f9bac16d authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Fix issue #32180780: Sync adapters inappropriately being run...

...during full-data backup/restore

The activity manager now tells job scheduler service about the
current backup that is running (only if it is a full backup), it
there is a new condition where we won't consider jobs associated
with the current backup to be ready to run.

Also...  just a little optimization here. :)  The focus is on
scheduling  jobs with a 0 deadline, meaning they should run right
away.  Now the timing controller does a quick check for a new
job to see if its constraints are already satisifed, and doesn't
do anything further if that is the case (doesn't add to the list,
doesn't re-evaluate alarms, etc).  And in the path to scheduling
a job, we do a check to see if the new job is already ready and if
so then just directly add it to the pending list and schedule it.

Doing this required removing what I think is the last bit of code
relying on handler serializing for thread safety, so now everything
in the job scheduler is protected by our global lock and we can
do whatever we want with the lock held and be assured the state
remains consistent.

Also did some small optimizations to many of the other controllers,
mostly switching from an ArrayList to an ArraySet for their tracked
jobs, since one of the things we do frequently is add/remove jobs.

Finally, added some nullability annotations to the JobScheduler
APIs.

Test: bit CtsJobSchedulerTestCases:*

Change-Id: I533fad94ba59468a52fe3d077a0ceab3427f0012
parent 6e8f1166
Loading
Loading
Loading
Loading
+9 −10
Original line number Original line Diff line number Diff line
@@ -244,7 +244,7 @@ public class JobInfo implements Parcelable {
    /**
    /**
     * Bundle of extras which are returned to your application at execution time.
     * Bundle of extras which are returned to your application at execution time.
     */
     */
    public PersistableBundle getExtras() {
    public @NonNull PersistableBundle getExtras() {
        return extras;
        return extras;
    }
    }


@@ -252,7 +252,7 @@ public class JobInfo implements Parcelable {
     * Bundle of transient extras which are returned to your application at execution time,
     * Bundle of transient extras which are returned to your application at execution time,
     * but not persisted by the system.
     * but not persisted by the system.
     */
     */
    public Bundle getTransientExtras() {
    public @NonNull Bundle getTransientExtras() {
        return transientExtras;
        return transientExtras;
    }
    }


@@ -260,7 +260,7 @@ public class JobInfo implements Parcelable {
     * ClipData of information that is returned to your application at execution time,
     * ClipData of information that is returned to your application at execution time,
     * but not persisted by the system.
     * but not persisted by the system.
     */
     */
    public ClipData getClipData() {
    public @Nullable ClipData getClipData() {
        return clipData;
        return clipData;
    }
    }


@@ -274,7 +274,7 @@ public class JobInfo implements Parcelable {
    /**
    /**
     * Name of the service endpoint that will be called back into by the JobScheduler.
     * Name of the service endpoint that will be called back into by the JobScheduler.
     */
     */
    public ComponentName getService() {
    public @NonNull ComponentName getService() {
        return service;
        return service;
    }
    }


@@ -327,8 +327,7 @@ public class JobInfo implements Parcelable {
     * Which content: URIs must change for the job to be scheduled.  Returns null
     * Which content: URIs must change for the job to be scheduled.  Returns null
     * if there are none required.
     * if there are none required.
     */
     */
    @Nullable
    public @Nullable TriggerContentUri[] getTriggerContentUris() {
    public TriggerContentUri[] getTriggerContentUris() {
        return triggerContentUris;
        return triggerContentUris;
    }
    }


@@ -811,7 +810,7 @@ public class JobInfo implements Parcelable {
         * @param jobService The endpoint that you implement that will receive the callback from the
         * @param jobService The endpoint that you implement that will receive the callback from the
         * JobScheduler.
         * JobScheduler.
         */
         */
        public Builder(int jobId, ComponentName jobService) {
        public Builder(int jobId, @NonNull ComponentName jobService) {
            mJobService = jobService;
            mJobService = jobService;
            mJobId = jobId;
            mJobId = jobId;
        }
        }
@@ -832,7 +831,7 @@ public class JobInfo implements Parcelable {
         * Set optional extras. This is persisted, so we only allow primitive types.
         * Set optional extras. This is persisted, so we only allow primitive types.
         * @param extras Bundle containing extras you want the scheduler to hold on to for you.
         * @param extras Bundle containing extras you want the scheduler to hold on to for you.
         */
         */
        public Builder setExtras(PersistableBundle extras) {
        public Builder setExtras(@NonNull PersistableBundle extras) {
            mExtras = extras;
            mExtras = extras;
            return this;
            return this;
        }
        }
@@ -842,7 +841,7 @@ public class JobInfo implements Parcelable {
         * persisted with {@link #setPersisted(boolean)}; mixing the two is not allowed.
         * persisted with {@link #setPersisted(boolean)}; mixing the two is not allowed.
         * @param extras Bundle containing extras you want the scheduler to hold on to for you.
         * @param extras Bundle containing extras you want the scheduler to hold on to for you.
         */
         */
        public Builder setTransientExtras(Bundle extras) {
        public Builder setTransientExtras(@NonNull Bundle extras) {
            mTransientExtras = extras;
            mTransientExtras = extras;
            return this;
            return this;
        }
        }
@@ -869,7 +868,7 @@ public class JobInfo implements Parcelable {
         * {@link android.content.Intent#FLAG_GRANT_WRITE_URI_PERMISSION}, and
         * {@link android.content.Intent#FLAG_GRANT_WRITE_URI_PERMISSION}, and
         * {@link android.content.Intent#FLAG_GRANT_PREFIX_URI_PERMISSION}.
         * {@link android.content.Intent#FLAG_GRANT_PREFIX_URI_PERMISSION}.
         */
         */
        public Builder setClipData(ClipData clip, int grantFlags) {
        public Builder setClipData(@Nullable ClipData clip, int grantFlags) {
            mClipData = clip;
            mClipData = clip;
            mClipGrantFlags = grantFlags;
            mClipGrantFlags = grantFlags;
            return this;
            return this;
+9 −7
Original line number Original line Diff line number Diff line
@@ -16,6 +16,8 @@


package android.app.job;
package android.app.job;


import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.job.IJobCallback;
import android.app.job.IJobCallback;
import android.content.ClipData;
import android.content.ClipData;
import android.net.Uri;
import android.net.Uri;
@@ -91,7 +93,7 @@ public class JobParameters implements Parcelable {
     * {@link android.app.job.JobInfo.Builder#setExtras(android.os.PersistableBundle)}. This will
     * {@link android.app.job.JobInfo.Builder#setExtras(android.os.PersistableBundle)}. This will
     * never be null. If you did not set any extras this will be an empty bundle.
     * never be null. If you did not set any extras this will be an empty bundle.
     */
     */
    public PersistableBundle getExtras() {
    public @NonNull PersistableBundle getExtras() {
        return extras;
        return extras;
    }
    }


@@ -100,7 +102,7 @@ public class JobParameters implements Parcelable {
     * {@link android.app.job.JobInfo.Builder#setTransientExtras(android.os.Bundle)}. This will
     * {@link android.app.job.JobInfo.Builder#setTransientExtras(android.os.Bundle)}. This will
     * never be null. If you did not set any extras this will be an empty bundle.
     * never be null. If you did not set any extras this will be an empty bundle.
     */
     */
    public Bundle getTransientExtras() {
    public @NonNull Bundle getTransientExtras() {
        return transientExtras;
        return transientExtras;
    }
    }


@@ -109,7 +111,7 @@ public class JobParameters implements Parcelable {
     * {@link android.app.job.JobInfo.Builder#setClipData(ClipData, int)}. Will be null
     * {@link android.app.job.JobInfo.Builder#setClipData(ClipData, int)}. Will be null
     * if it was not set.
     * if it was not set.
     */
     */
    public ClipData getClipData() {
    public @Nullable ClipData getClipData() {
        return clipData;
        return clipData;
    }
    }


@@ -140,7 +142,7 @@ public class JobParameters implements Parcelable {
     * always use {@link #getTriggeredContentAuthorities()} to determine whether the job was
     * always use {@link #getTriggeredContentAuthorities()} to determine whether the job was
     * triggered due to any content changes and the authorities they are associated with.
     * triggered due to any content changes and the authorities they are associated with.
     */
     */
    public Uri[] getTriggeredContentUris() {
    public @Nullable Uri[] getTriggeredContentUris() {
        return mTriggeredContentUris;
        return mTriggeredContentUris;
    }
    }


@@ -152,7 +154,7 @@ public class JobParameters implements Parcelable {
     * to retrieve the details of which URIs changed (as long as that has not exceeded the maximum
     * to retrieve the details of which URIs changed (as long as that has not exceeded the maximum
     * number it can reported).
     * number it can reported).
     */
     */
    public String[] getTriggeredContentAuthorities() {
    public @Nullable String[] getTriggeredContentAuthorities() {
        return mTriggeredContentAuthorities;
        return mTriggeredContentAuthorities;
    }
    }


@@ -183,7 +185,7 @@ public class JobParameters implements Parcelable {
     * (This means that for correct operation, you must always call dequeueWork() after you have
     * (This means that for correct operation, you must always call dequeueWork() after you have
     * completed other work, to check either for more work or allow the system to stop the job.)
     * completed other work, to check either for more work or allow the system to stop the job.)
     */
     */
    public JobWorkItem dequeueWork() {
    public @Nullable JobWorkItem dequeueWork() {
        try {
        try {
            return getCallback().dequeueWork(getJobId());
            return getCallback().dequeueWork(getJobId());
        } catch (RemoteException e) {
        } catch (RemoteException e) {
@@ -207,7 +209,7 @@ public class JobParameters implements Parcelable {
     * @param work The work you have completed processing, as previously returned by
     * @param work The work you have completed processing, as previously returned by
     * {@link #dequeueWork()}
     * {@link #dequeueWork()}
     */
     */
    public void completeWork(JobWorkItem work) {
    public void completeWork(@NonNull JobWorkItem work) {
        try {
        try {
            if (!getCallback().completeWork(getJobId(), work.getWorkId())) {
            if (!getCallback().completeWork(getJobId(), work.getWorkId())) {
                throw new IllegalArgumentException("Given work is not active: " + work);
                throw new IllegalArgumentException("Given work is not active: " + work);
+4 −3
Original line number Original line Diff line number Diff line
@@ -72,7 +72,7 @@ public abstract class JobScheduler {
     * you can schedule.
     * you can schedule.
     * @return An int representing ({@link #RESULT_SUCCESS} or {@link #RESULT_FAILURE}).
     * @return An int representing ({@link #RESULT_SUCCESS} or {@link #RESULT_FAILURE}).
     */
     */
    public abstract int schedule(JobInfo job);
    public abstract int schedule(@NonNull JobInfo job);


    /**
    /**
     * Similar to {@link #schedule}, but allows you to enqueue work for an existing job.  If a job
     * Similar to {@link #schedule}, but allows you to enqueue work for an existing job.  If a job
@@ -108,7 +108,7 @@ public abstract class JobScheduler {
     * @param work New work to enqueue.  This will be available later when the job starts running.
     * @param work New work to enqueue.  This will be available later when the job starts running.
     * @return An int representing ({@link #RESULT_SUCCESS} or {@link #RESULT_FAILURE}).
     * @return An int representing ({@link #RESULT_SUCCESS} or {@link #RESULT_FAILURE}).
     */
     */
    public abstract int enqueue(JobInfo job, JobWorkItem work);
    public abstract int enqueue(@NonNull JobInfo job, @NonNull JobWorkItem work);


    /**
    /**
     *
     *
@@ -121,7 +121,8 @@ public abstract class JobScheduler {
     * @hide
     * @hide
     */
     */
    @SystemApi
    @SystemApi
    public abstract int scheduleAsPackage(JobInfo job, String packageName, int userId, String tag);
    public abstract int scheduleAsPackage(@NonNull JobInfo job, @NonNull String packageName,
            int userId, String tag);


    /**
    /**
     * Cancel a job that is pending in the JobScheduler.
     * Cancel a job that is pending in the JobScheduler.
+27 −0
Original line number Original line Diff line number Diff line
@@ -358,6 +358,7 @@ import android.view.LayoutInflater;
import android.view.View;
import android.view.View;
import android.view.WindowManager;
import android.view.WindowManager;
import com.android.server.job.JobSchedulerInternal;
import com.google.android.collect.Lists;
import com.google.android.collect.Lists;
import com.google.android.collect.Maps;
import com.google.android.collect.Maps;
@@ -18158,6 +18159,9 @@ public class ActivityManagerService extends IActivityManager.Stub
            return false;
            return false;
        }
        }
        int oldBackupUid;
        int newBackupUid;
        synchronized(this) {
        synchronized(this) {
            // !!! TODO: currently no check here that we're already bound
            // !!! TODO: currently no check here that we're already bound
            BatteryStatsImpl.Uid.Pkg.Serv ss = null;
            BatteryStatsImpl.Uid.Pkg.Serv ss = null;
@@ -18198,6 +18202,8 @@ public class ActivityManagerService extends IActivityManager.Stub
                proc.inFullBackup = true;
                proc.inFullBackup = true;
            }
            }
            r.app = proc;
            r.app = proc;
            oldBackupUid = mBackupTarget != null ? mBackupTarget.appInfo.uid : -1;
            newBackupUid = proc.inFullBackup ? r.appInfo.uid : -1;
            mBackupTarget = r;
            mBackupTarget = r;
            mBackupAppName = app.packageName;
            mBackupAppName = app.packageName;
@@ -18223,6 +18229,14 @@ public class ActivityManagerService extends IActivityManager.Stub
            // know that it's scheduled for a backup-agent operation.
            // know that it's scheduled for a backup-agent operation.
        }
        }
        JobSchedulerInternal js = LocalServices.getService(JobSchedulerInternal.class);
        if (oldBackupUid != -1) {
            js.removeBackingUpUid(oldBackupUid);
        }
        if (newBackupUid != -1) {
            js.addBackingUpUid(newBackupUid);
        }
        return true;
        return true;
    }
    }
@@ -18235,6 +18249,9 @@ public class ActivityManagerService extends IActivityManager.Stub
            mBackupTarget = null;
            mBackupTarget = null;
            mBackupAppName = null;
            mBackupAppName = null;
        }
        }
        JobSchedulerInternal js = LocalServices.getService(JobSchedulerInternal.class);
        js.clearAllBackingUpUids();
    }
    }
    // A backup agent has just come up
    // A backup agent has just come up
@@ -18272,6 +18289,8 @@ public class ActivityManagerService extends IActivityManager.Stub
            return;
            return;
        }
        }
        int oldBackupUid;
        synchronized(this) {
        synchronized(this) {
            try {
            try {
                if (mBackupAppName == null) {
                if (mBackupAppName == null) {
@@ -18289,6 +18308,8 @@ public class ActivityManagerService extends IActivityManager.Stub
                updateOomAdjLocked(proc);
                updateOomAdjLocked(proc);
                proc.inFullBackup = false;
                proc.inFullBackup = false;
                oldBackupUid = mBackupTarget != null ? mBackupTarget.appInfo.uid : -1;
                // If the app crashed during backup, 'thread' will be null here
                // If the app crashed during backup, 'thread' will be null here
                if (proc.thread != null) {
                if (proc.thread != null) {
                    try {
                    try {
@@ -18304,7 +18325,13 @@ public class ActivityManagerService extends IActivityManager.Stub
                mBackupAppName = null;
                mBackupAppName = null;
            }
            }
        }
        }
        if (oldBackupUid != -1) {
            JobSchedulerInternal js = LocalServices.getService(JobSchedulerInternal.class);
            js.removeBackingUpUid(oldBackupUid);
        }
        }
    }
    // =========================================================
    // =========================================================
    // BROADCASTS
    // BROADCASTS
    // =========================================================
    // =========================================================
+7 −0
Original line number Original line Diff line number Diff line
@@ -30,4 +30,11 @@ public interface JobSchedulerInternal {
     * Returns a list of pending jobs scheduled by the system service.
     * Returns a list of pending jobs scheduled by the system service.
     */
     */
    List<JobInfo> getSystemScheduledPendingJobs();
    List<JobInfo> getSystemScheduledPendingJobs();

    /**
     * These are for activity manager to communicate to use what is currently performing backups.
     */
    void addBackingUpUid(int uid);
    void removeBackingUpUid(int uid);
    void clearAllBackingUpUids();
}
}
Loading