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

Commit 64280467 authored by Matthew Williams's avatar Matthew Williams Committed by Liam Byrne
Browse files

Downgrade expedited to normal on reschedule.

bug: 12033540
Expedited was previously tracked by a redundant internal variable, ostensibly
as an optimisation. This variable could differ from the value in the bundle
depending on how the operation is initialised, which led to confusion. Now an
expedited sync will only be treated as such on its first execution.

Change-Id: I9979102317aecbe8bc53a36381d4b2782ac131be

Conflicts:
	services/core/java/com/android/server/content/SyncOperation.java
	services/core/java/com/android/server/content/SyncQueue.java
parent cb1b23b5
Loading
Loading
Loading
Loading
+1 −1
Original line number Original line Diff line number Diff line
@@ -2431,7 +2431,7 @@ public class SyncManager {
                            Log.v(TAG, "canceling and rescheduling sync since an initialization "
                            Log.v(TAG, "canceling and rescheduling sync since an initialization "
                                    + "takes higher priority, " + conflict);
                                    + "takes higher priority, " + conflict);
                        }
                        }
                    } else if (candidate.expedited && !conflict.mSyncOperation.expedited
                    } else if (candidate.isExpedited() && !conflict.mSyncOperation.isExpedited()
                            && (candidateIsInitialization
                            && (candidateIsInitialization
                                == conflict.mSyncOperation.isInitialization())) {
                                == conflict.mSyncOperation.isInitialization())) {
                        toReschedule = conflict;
                        toReschedule = conflict;
+36 −38
Original line number Original line Diff line number Diff line
@@ -65,17 +65,18 @@ public class SyncOperation implements Comparable {
    /** Why this sync was kicked off. {@link #REASON_NAMES} */
    /** Why this sync was kicked off. {@link #REASON_NAMES} */
    public final int reason;
    public final int reason;
    /** Where this sync was initiated. */
    /** Where this sync was initiated. */
    public int syncSource;
    public final int syncSource;
    public final boolean allowParallelSyncs;
    public final boolean allowParallelSyncs;
    public Bundle extras;
    public final String key;
    public final String key;
    public boolean expedited;
    /** Internal boolean to avoid reading a bundle everytime we want to compare operations. */
    private final boolean expedited;
    public Bundle extras;
    /** Bare-bones version of this operation that is persisted across reboots. */
    /** Bare-bones version of this operation that is persisted across reboots. */
    public SyncStorageEngine.PendingOperation pendingOperation;
    public SyncStorageEngine.PendingOperation pendingOperation;
    /** Elapsed real time in millis at which to run this sync. */
    /** Elapsed real time in millis at which to run this sync. */
    public long latestRunTime;
    public long latestRunTime;
    /** Set by the SyncManager in order to delay retries. */
    /** Set by the SyncManager in order to delay retries. */
    public Long backoff;
    public long backoff;
    /** Specified by the adapter to delay subsequent sync operations. */
    /** Specified by the adapter to delay subsequent sync operations. */
    public long delayUntil;
    public long delayUntil;
    /**
    /**
@@ -89,61 +90,58 @@ public class SyncOperation implements Comparable {
    public SyncOperation(Account account, int userId, int reason, int source, String provider,
    public SyncOperation(Account account, int userId, int reason, int source, String provider,
            Bundle extras, long runTimeFromNow, long flexTime, long backoff,
            Bundle extras, long runTimeFromNow, long flexTime, long backoff,
            long delayUntil, boolean allowParallelSyncs) {
            long delayUntil, boolean allowParallelSyncs) {
        this.target = new SyncStorageEngine.EndPoint(account, provider, userId);
        this(new SyncStorageEngine.EndPoint(account, provider, userId),
        this.reason = reason;
                reason, source, extras, runTimeFromNow, flexTime, backoff, delayUntil,
        this.allowParallelSyncs = allowParallelSyncs;
                allowParallelSyncs);
        this.key = initialiseOperation(this.target, source, extras, runTimeFromNow, flexTime,
                backoff, delayUntil);
    }
    }


    public SyncOperation(ComponentName service, int userId, int reason, int source,
    public SyncOperation(ComponentName service, int userId, int reason, int source,
            Bundle extras, long runTimeFromNow, long flexTime, long backoff,
            Bundle extras, long runTimeFromNow, long flexTime, long backoff,
            long delayUntil) {
            long delayUntil) {
        this.target = new SyncStorageEngine.EndPoint(service, userId);
        this(new SyncStorageEngine.EndPoint(service, userId), reason, source, extras,
        // Default to true for sync service. The service itself decides how to handle this.
                runTimeFromNow, flexTime, backoff, delayUntil, true /* allowParallelSyncs */);
        this.allowParallelSyncs = true;
        this.reason = reason;
        this.key =
                initialiseOperation(this.target,
                        source, extras, runTimeFromNow, flexTime, backoff, delayUntil);
    }

    /** Used to reschedule a sync at a new point in time. */
    SyncOperation(SyncOperation other, long newRunTimeFromNow) {
        this.target = other.target;
        this.reason = other.reason;
        this.expedited = other.expedited;
        this.allowParallelSyncs = other.allowParallelSyncs;
        // re-use old flex, but only 
        long newFlexTime = Math.min(other.flexTime, newRunTimeFromNow);
        this.key =
                initialiseOperation(this.target,
                    other.syncSource, other.extras,
                    newRunTimeFromNow /* runTimeFromNow*/,
                    newFlexTime /* flexTime */,
                    other.backoff,
                    0L /* delayUntil */);
    }
    }


    private String initialiseOperation(SyncStorageEngine.EndPoint info, int source, Bundle extras,
    private SyncOperation(SyncStorageEngine.EndPoint info, int reason, int source, Bundle extras,
            long runTimeFromNow, long flexTime, long backoff, long delayUntil) {
            long runTimeFromNow, long flexTime, long backoff, long delayUntil,
            boolean allowParallelSyncs) {
        this.target = info;
        this.reason = reason;
        this.syncSource = source;
        this.syncSource = source;
        this.extras = new Bundle(extras);
        this.extras = new Bundle(extras);
        cleanBundle(this.extras);
        cleanBundle(this.extras);
        this.delayUntil = delayUntil;
        this.delayUntil = delayUntil;
        this.backoff = backoff;
        this.backoff = backoff;
        this.allowParallelSyncs = allowParallelSyncs;
        final long now = SystemClock.elapsedRealtime();
        final long now = SystemClock.elapsedRealtime();
        if (runTimeFromNow < 0 || isExpedited()) {
        // Set expedited based on runTimeFromNow. The SyncManager specifies whether the op is
        // expedited (Not done solely based on bundle).
        if (runTimeFromNow < 0) {
            this.expedited = true;
            this.expedited = true;
            // Sanity check: Will always be true.
            if (!this.extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false)) {
                this.extras.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
            }
            this.latestRunTime = now;
            this.latestRunTime = now;
            this.flexTime = 0;
            this.flexTime = 0;
        } else {
        } else {
            this.expedited = false;
            this.expedited = false;
            this.extras.remove(ContentResolver.SYNC_EXTRAS_EXPEDITED);
            this.latestRunTime = now + runTimeFromNow;
            this.latestRunTime = now + runTimeFromNow;
            this.flexTime = flexTime;
            this.flexTime = flexTime;
        }
        }
        updateEffectiveRunTime();
        updateEffectiveRunTime();
        return toKey(info, this.extras);
        this.key = toKey(info, this.extras);
    }

    /** Used to reschedule a sync at a new point in time. */
    public SyncOperation(SyncOperation other, long newRunTimeFromNow) {
        this(other.target, other.reason, other.syncSource, new Bundle(other.extras),
                newRunTimeFromNow,
                0L /* In back-off so no flex */,
                other.backoff,
                other.delayUntil,
                other.allowParallelSyncs);
    }
    }


    public boolean matchesAuthority(SyncOperation other) {
    public boolean matchesAuthority(SyncOperation other) {
@@ -261,7 +259,7 @@ public class SyncOperation implements Comparable {
    }
    }


    public boolean isExpedited() {
    public boolean isExpedited() {
        return extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false) || expedited;
        return expedited;
    }
    }


    public boolean ignoreBackoff() {
    public boolean ignoreBackoff() {
+3 −6
Original line number Original line Diff line number Diff line
@@ -76,12 +76,11 @@ public class SyncQueue {
                operationToAdd = new SyncOperation(
                operationToAdd = new SyncOperation(
                        info.account, info.userId, op.reason, op.syncSource, info.provider,
                        info.account, info.userId, op.reason, op.syncSource, info.provider,
                        op.extras,
                        op.extras,
                        0 /* delay */,
                        op.expedited ? -1 : 0 /* delay */,
                        0 /* flex */,
                        0 /* flex */,
                        backoff != null ? backoff.first : 0,
                        backoff != null ? backoff.first : 0L,
                        mSyncStorageEngine.getDelayUntilTime(info),
                        mSyncStorageEngine.getDelayUntilTime(info),
                        syncAdapterInfo.type.allowParallelSyncs());
                        syncAdapterInfo.type.allowParallelSyncs());
                operationToAdd.expedited = op.expedited;
                operationToAdd.pendingOperation = op;
                operationToAdd.pendingOperation = op;
                add(operationToAdd, op);
                add(operationToAdd, op);
            } else if (info.target_service) {
            } else if (info.target_service) {
@@ -96,11 +95,10 @@ public class SyncQueue {
                operationToAdd = new SyncOperation(
                operationToAdd = new SyncOperation(
                        info.service, info.userId, op.reason, op.syncSource,
                        info.service, info.userId, op.reason, op.syncSource,
                        op.extras,
                        op.extras,
                        0 /* delay */,
                        op.expedited ? -1 : 0 /* delay */,
                        0 /* flex */,
                        0 /* flex */,
                        backoff != null ? backoff.first : 0,
                        backoff != null ? backoff.first : 0,
                        mSyncStorageEngine.getDelayUntilTime(info));
                        mSyncStorageEngine.getDelayUntilTime(info));
                operationToAdd.expedited = op.expedited;
                operationToAdd.pendingOperation = op;
                operationToAdd.pendingOperation = op;
                add(operationToAdd, op);
                add(operationToAdd, op);
            }
            }
@@ -129,7 +127,6 @@ public class SyncQueue {
        if (existingOperation != null) {
        if (existingOperation != null) {
            boolean changed = false;
            boolean changed = false;
            if (operation.compareTo(existingOperation) <= 0 ) {
            if (operation.compareTo(existingOperation) <= 0 ) {
                existingOperation.expedited = operation.expedited;
                long newRunTime =
                long newRunTime =
                        Math.min(existingOperation.latestRunTime, operation.latestRunTime);
                        Math.min(existingOperation.latestRunTime, operation.latestRunTime);
                // Take smaller runtime.
                // Take smaller runtime.
+1 −1
Original line number Original line Diff line number Diff line
@@ -1088,7 +1088,7 @@ public class SyncStorageEngine extends Handler {
            }
            }


            pop = new PendingOperation(authority, op.reason, op.syncSource, op.extras,
            pop = new PendingOperation(authority, op.reason, op.syncSource, op.extras,
                    op.expedited);
                    op.isExpedited());
            mPendingOperations.add(pop);
            mPendingOperations.add(pop);
            appendPendingOperationLocked(pop);
            appendPendingOperationLocked(pop);


+61 −16
Original line number Original line Diff line number Diff line
@@ -18,12 +18,13 @@ package com.android.server.content;


import android.accounts.Account;
import android.accounts.Account;
import android.content.ContentResolver;
import android.content.ContentResolver;
import android.content.Context;
import android.os.Bundle;
import android.os.Bundle;
import android.os.SystemClock;
import android.provider.Settings;
import android.test.AndroidTestCase;
import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.SmallTest;
import android.test.suitebuilder.annotation.SmallTest;


import com.android.server.content.SyncOperation;

/**
/**
 * You can run those tests with:
 * You can run those tests with:
 *
 *
@@ -35,6 +36,21 @@ import com.android.server.content.SyncOperation;


public class SyncOperationTest extends AndroidTestCase {
public class SyncOperationTest extends AndroidTestCase {


    Account mDummy;
    /** Indicate an unimportant long that we're not testing. */
    long mUnimportantLong = 0L;
    /** Empty bundle. */
    Bundle mEmpty;
    /** Silly authority. */
    String mAuthority;

    @Override
    public void setUp() {
        mDummy = new Account("account1", "type1");
        mEmpty = new Bundle();
        mAuthority = "authority1";
    }

    @SmallTest
    @SmallTest
    public void testToKey() {
    public void testToKey() {
        Account account1 = new Account("account1", "type1");
        Account account1 = new Account("account1", "type1");
@@ -111,35 +127,64 @@ public class SyncOperationTest extends AndroidTestCase {


    @SmallTest
    @SmallTest
    public void testCompareTo() {
    public void testCompareTo() {
        Account dummy = new Account("account1", "type1");
        Bundle b1 = new Bundle();
        final long unimportant = 0L;
        long soon = 1000;
        long soon = 1000;
        long soonFlex = 50;
        long soonFlex = 50;
        long after = 1500;
        long after = 1500;
        long afterFlex = 100;
        long afterFlex = 100;
        SyncOperation op1 = new SyncOperation(dummy, 0, 0, SyncOperation.REASON_PERIODIC,
        SyncOperation op1 = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_PERIODIC,
                "authority1", b1, soon, soonFlex, unimportant, unimportant, true);
                "authority1", mEmpty, soon, soonFlex, mUnimportantLong, mUnimportantLong, true);


        // Interval disjoint from and after op1.
        // Interval disjoint from and after op1.
        SyncOperation op2 = new SyncOperation(dummy, 0, 0, SyncOperation.REASON_PERIODIC,
        SyncOperation op2 = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_PERIODIC,
                "authority1", b1, after, afterFlex, unimportant, unimportant, true);
                "authority1", mEmpty, after, afterFlex, mUnimportantLong, mUnimportantLong, true);


        // Interval equivalent to op1, but expedited.
        // Interval equivalent to op1, but expedited.
        Bundle b2 = new Bundle();
        Bundle b2 = new Bundle();
        b2.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
        b2.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
        SyncOperation op3 = new SyncOperation(dummy, 0, 0, 0,
        SyncOperation op3 = new SyncOperation(mDummy, 0, 0, 0,
                "authority1", b2, soon, soonFlex, unimportant, unimportant, true);
                "authority1", b2, -1, soonFlex, mUnimportantLong, mUnimportantLong, true);


        // Interval overlaps but not equivalent to op1.
        // Interval overlaps but not equivalent to op1.
        SyncOperation op4 = new SyncOperation(dummy, 0, 0, SyncOperation.REASON_PERIODIC,
        SyncOperation op4 = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_PERIODIC,
                "authority1", b1, soon + 100, soonFlex + 100, unimportant, unimportant, true);
                "authority1", mEmpty, soon + 100, soonFlex + 100, mUnimportantLong, mUnimportantLong, true);


        assertTrue(op1.compareTo(op2) == -1);
        assertTrue(op1.compareTo(op2) == -1);
        assertTrue("less than not transitive.", op2.compareTo(op1) == 1);
        assertTrue("less than not transitive.", op2.compareTo(op1) == 1);
        assertTrue(op1.compareTo(op3) == 1);
        assertTrue("Expedited sync not smaller than non-expedited.", op1.compareTo(op3) == 1);
        assertTrue("greater than not transitive. ", op3.compareTo(op1) == -1);
        assertTrue("greater than not transitive. ", op3.compareTo(op1) == -1);
        assertTrue("overlapping intervals not the same.", op1.compareTo(op4) == 0);
        assertTrue("overlapping intervals not correctly compared.", op1.compareTo(op4) == -1);
        assertTrue("equality not transitive.", op4.compareTo(op1) == 0);
        assertTrue("equality not transitive.", op4.compareTo(op1) == 1);
    }

    @SmallTest
    public void testCopyConstructor() {
        long fiveSecondsFromNow = 5 * 1000L;
        long twoSecondsFlex = 2 * 1000L;
        long eightSeconds = 8 * 1000L;
        long fourSeconds = 4 * 1000L;

        Bundle withExpedited = new Bundle();
        withExpedited.putBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, true);
        SyncOperation op = new SyncOperation(mDummy, 0, 0, SyncOperation.REASON_USER_START,
                mAuthority, withExpedited, fiveSecondsFromNow, twoSecondsFlex,
                eightSeconds /* backoff */, fourSeconds /* delayUntil */, true);
        // Create another sync op to be rerun in 5 minutes.
        long now = SystemClock.elapsedRealtime();
        SyncOperation copy = new SyncOperation(op, fiveSecondsFromNow * 60);
        // Copying an expedited sync to be re-run should not keep expedited property.
        assertFalse("A rescheduled sync based off an expedited should not be expedited!",
                copy.isExpedited());
        assertFalse("A rescheduled sync based off an expedited should not have expedited=true in"
                + "its bundle.",
                copy.extras.getBoolean(ContentResolver.SYNC_EXTRAS_EXPEDITED, false));
        assertTrue("Copied sync is not respecting new provided run-time.",
                copy.latestRunTime == (now + fiveSecondsFromNow * 60));
        assertTrue("A rescheduled sync should not have any flex.",
                copy.flexTime == 0L);
        assertTrue("A rescheduled op should honour the old op's backoff.",
                copy.backoff == eightSeconds);
        assertTrue("A rescheduled op should honour the old op's delayUntil param.",
                copy.delayUntil == fourSeconds);

    }
    }
}
}
Loading