• rlarocque@chromium.org's avatar
    Abort sync cycles when download step fails · 4c07c654
    rlarocque@chromium.org authored
    This change causes the syncer to exit from its download then commit
    function if the download fails. This helps prevent the creation of
    server-side conflicts, which would be more likely to happen if a
    download failed but the following commit succeeded.
    
    The main changes are as follows:
    - Rather than proceed to the next step when a download updates failure
    is detected, the syncer will exit. This should cause the
    SyncScheduler to schedule a retry at a later time. 
    - The definition of a download updates failure is now based on the
    return code of the download attempt, rather than checking the contents
    of the (possible non-existent) returned protobuf. This makes the error
    detection logic used here more closely match the logic used to decide
    whether or not to schedule retries.
    
    This implementation has a side-effect on configure sync cycles. The old
    behaviour was to handle failures by applying whatever updates we had
    downloaded at that point. The new behaviour will leave updates
    unapplied if any error occurs. This better matches a nearby comment's
    assertion which states that we should not attempt to apply updates until
    we have downloaded all of them. I believe the author of that comment
    would approve of this change.
    
    This change moves around some of the ExtensionActivityMonitor logic.
    It's important that we not take the data from the extensions acitivity
    monitor at the start of the cycle. Restoring that data is handled in
    the commit building and response code, which might not be executed if we
    need to break out early. This also fixes issue 123270.
    
    Most of the diffs for this change are concentrated in the unit tests:
    - Expose more of the SyncScheduler to the SyncerTest test harness.
    - Add functions to SyncerTest for testing specific types of sync jobs.
    - Add some functions that allow us to better control failures in
    MockConnectionManager.
    - Added tests for configure job success and failure.
    - Added tests for update then commit job success and failure.
    - Removed some redundant tests.
    - Removed unused test infrastructure.
    
    This change allows the integration tests to expose a bug that was
    introduced back in r118572.  That commit assumed it was OK to not
    retry a job that failed due to a MIGRATION_DONE response from the
    server.  That is incorrect, and this error has been fixed.
    
    BUG=122033, 123270
    TEST=sync_unit_tests, specifically:
    SyncerTest.UpdateThenCommit,
    SyncerTest.UpdateFailsThenDontCommit,
    SyncerTest.ConfigureDownloadsTwoBatchesSuccess,
    SyncerTest.ConfigureFailsDontApplyUpdates
    
    
    Review URL: http://codereview.chromium.org/10103017
    
    git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133754 0039d316-1c4b-4281-b951-d872f2087c98
    4c07c654
mock_connection_manager.h 13.5 KB