Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7459)

Unified Diff: chrome/browser/sync/engine/sync_scheduler.cc

Issue 9036003: Avoid useless SYNC_CYCLE_CONTINUATION sync cycle (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 9 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/sync/engine/store_timestamps_command.cc ('k') | chrome/browser/sync/engine/syncer.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/sync/engine/sync_scheduler.cc
diff --git a/chrome/browser/sync/engine/sync_scheduler.cc b/chrome/browser/sync/engine/sync_scheduler.cc
index 241692eec05dd64cbd6a9f5dc61250e460bcd51c..8ec74ade1f17cee66eafee26a3afd8247ff76a3a 100644
--- a/chrome/browser/sync/engine/sync_scheduler.cc
+++ b/chrome/browser/sync/engine/sync_scheduler.cc
@@ -838,30 +838,14 @@ void SyncScheduler::FinishSyncSessionJob(const SyncSessionJob& job) {
void SyncScheduler::ScheduleNextSync(const SyncSessionJob& old_job) {
DCHECK_EQ(MessageLoop::current(), sync_loop_);
DCHECK(!old_job.session->HasMoreToSync());
- // Note: |num_server_changes_remaining| > 0 here implies that we received a
- // broken response while trying to download all updates, because the Syncer
- // will loop until this value is exhausted. Also, if unsynced_handles exist
- // but HasMoreToSync is false, this implies that the Syncer determined no
- // forward progress was possible at this time (an error, such as an HTTP
- // 500, is likely to have occurred during commit).
- int num_server_changes_remaining =
- old_job.session->status_controller().num_server_changes_remaining();
- size_t num_unsynced_handles =
- old_job.session->status_controller().unsynced_handles().size();
- const bool work_to_do =
- num_server_changes_remaining > 0 || num_unsynced_handles > 0;
- SDVLOG(2) << "num server changes remaining: " << num_server_changes_remaining
- << ", num unsynced handles: " << num_unsynced_handles
- << ", syncer has work to do: " << work_to_do;
AdjustPolling(&old_job);
- // TODO(tim): Old impl had special code if notifications disabled. Needed?
- if (!work_to_do) {
+ if (old_job.session->Succeeded()) {
// Success implies backoff relief. Note that if this was a
// "one-off" job (i.e. purpose ==
// SyncSessionJob::{CLEAR_USER_DATA,CLEANUP_DISABLED_TYPES}), if
- // there was work_to_do before it ran this wont have changed, as
+ // there was work to do before it ran this wont have changed, as
// jobs like this don't run a full sync cycle. So we don't need
// special code here.
wait_interval_.reset();
@@ -869,6 +853,12 @@ void SyncScheduler::ScheduleNextSync(const SyncSessionJob& old_job) {
return;
}
+ // TODO(rlarocque): There's no reason why we should blindly backoff and retry
+ // if we don't succeed. Some types of errors are not likely to disappear on
+ // their own. With the return values now available in the old_job.session, we
+ // should be able to detect such errors and only retry when we detect
+ // transient errors.
+
// We are in backoff mode and our time did not run out. That means we had
// a local change, notification from server or a network connection change
// notification. In any case set had_nudge = true so we dont retry next
« no previous file with comments | « chrome/browser/sync/engine/store_timestamps_command.cc ('k') | chrome/browser/sync/engine/syncer.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698