Hook up "remaining changes" to sync client. Remove "latest timestamp".

TEST=manually tested
BUG=19993
Review URL: http://codereview.chromium.org/295052

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29930 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 34 insertions, 36 deletions

diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc
index 07551a1..a6087b6 100644
--- a/chrome/browser/sync/engine/all_status.cc
+++ b/chrome/browser/sync/engine/all_status.cc
@@ -95,10 +95,10 @@ AllStatus::Status AllStatus::CalcSyncing(const SyncerEvent &event) const {
   SyncerStatus syncerStatus(event.last_session);
   status.unsynced_count += static_cast<int>(syncerStatus.unsynced_count());
   status.conflicting_count += syncerStatus.conflicting_commits();
-  if (syncerStatus.current_sync_timestamp() ==
-      syncerStatus.servers_latest_timestamp()) {
-    status.conflicting_count += syncerStatus.conflicting_updates();
-  }
+  // The syncer may not be done yet, which could cause conflicting updates.
+  // But this is only used for status, so it is better to have visibility.
+  status.conflicting_count += syncerStatus.conflicting_updates();
+
   status.syncing |= syncerStatus.syncing();
   // Show a syncer as syncing if it's got stalled updates.
   status.syncing = event.last_session->HasMoreToSync() &&
@@ -112,7 +112,7 @@ AllStatus::Status AllStatus::CalcSyncing(const SyncerEvent &event) const {
   if (syncerStatus.consecutive_transient_error_commits() > 100)
     status.server_broken = true;
 
-  status.updates_available += syncerStatus.servers_latest_timestamp();
+  status.updates_available += syncerStatus.num_server_changes_remaining();
   status.updates_received += syncerStatus.current_sync_timestamp();
   return status;
 }
diff --git a/chrome/browser/sync/engine/conflict_resolution_view.cc b/chrome/browser/sync/engine/conflict_resolution_view.cc
index 1ea2d5e..07daf9a 100644
--- a/chrome/browser/sync/engine/conflict_resolution_view.cc
+++ b/chrome/browser/sync/engine/conflict_resolution_view.cc
@@ -48,8 +48,8 @@ int64 ConflictResolutionView::current_sync_timestamp() const {
   return process_state_->current_sync_timestamp();
 }
 
-int64 ConflictResolutionView::servers_latest_timestamp() const {
-  return process_state_->servers_latest_timestamp();
+int64 ConflictResolutionView::num_server_changes_remaining() const {
+  return process_state_->num_server_changes_remaining();
 }
 
 // True iff we're stuck. User should contact support.
diff --git a/chrome/browser/sync/engine/conflict_resolution_view.h b/chrome/browser/sync/engine/conflict_resolution_view.h
index 8ea0d2f..2e1b9b5 100644
--- a/chrome/browser/sync/engine/conflict_resolution_view.h
+++ b/chrome/browser/sync/engine/conflict_resolution_view.h
@@ -65,7 +65,7 @@ class ConflictResolutionView {
 
   int64 current_sync_timestamp() const;
 
-  int64 servers_latest_timestamp() const;
+  int64 num_server_changes_remaining() const;
 
   IdToConflictSetMap::const_iterator IdToConflictSetFind(
       const syncable::Id& the_id) const;
diff --git a/chrome/browser/sync/engine/conflict_resolver.cc b/chrome/browser/sync/engine/conflict_resolver.cc
index c6f5234..a59faca 100644
--- a/chrome/browser/sync/engine/conflict_resolver.cc
+++ b/chrome/browser/sync/engine/conflict_resolver.cc
@@ -619,12 +619,14 @@ bool ConflictResolver::LogAndSignalIfConflictStuck(
     InputIt begin,
     InputIt end,
     ConflictResolutionView* view) {
-  if (attempt_count < SYNC_CYCLES_BEFORE_ADMITTING_DEFEAT)
+  if (attempt_count < SYNC_CYCLES_BEFORE_ADMITTING_DEFEAT) {
     return false;
+  }
 
   // Don't signal stuck if we're not up to date.
-  if (view->servers_latest_timestamp() != view->current_sync_timestamp())
+  if (view->num_server_changes_remaining() > 0) {
     return false;
+  }
 
   LOG(ERROR) << "[BUG] Conflict set cannot be resolved, has "
              << end - begin << " items:";
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index 9d6ce2d..31d267d 100644
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -37,10 +37,10 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncerSession* session) {
   LOG(INFO) << "Get updates from ts " << dir->last_sync_timestamp() <<
     " returned " << update_count << " updates.";
 
-  if (updates.has_newest_timestamp()) {
-    int64 newest_timestamp = updates.newest_timestamp();
-    LOG(INFO) << "Newest Timestamp:" << newest_timestamp;
-    status.set_servers_latest_timestamp(newest_timestamp);
+  if (updates.has_changes_remaining()) {
+    int64 changes_left = updates.changes_remaining();
+    LOG(INFO) << "Changes remaining:" << changes_left;
+    status.set_num_server_changes_remaining(changes_left);
   }
 
   int64 new_timestamp = 0;
@@ -56,11 +56,6 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncerSession* session) {
     }
   }
 
-  if (0 == status.servers_latest_timestamp()) {
-    // Hack since new server never gives us the server's latest timestamp. But
-    // if a getupdates returns zero, then we know we are up to date.
-    status.set_servers_latest_timestamp(status.current_sync_timestamp());
-  }
   // If we have updates that are ALL supposed to be skipped, we don't want to
   // get them again.  In fact, the account's final updates are all supposed to
   // be skipped and we DON'T step past them, we will sync forever.
diff --git a/chrome/browser/sync/engine/sync_process_state.cc b/chrome/browser/sync/engine/sync_process_state.cc
index 32ee2a6..15f1464 100644
--- a/chrome/browser/sync/engine/sync_process_state.cc
+++ b/chrome/browser/sync/engine/sync_process_state.cc
@@ -46,7 +46,7 @@ SyncProcessState::SyncProcessState(syncable::DirectoryManager* dirman,
       syncer_event_channel_(syncer_event_channel),
       error_rate_(0),
       current_sync_timestamp_(0),
-      servers_latest_timestamp_(0),
+      num_server_changes_remaining_(0),
       syncing_(false),
       invalid_store_(false),
       syncer_stuck_(false),
@@ -80,7 +80,7 @@ SyncProcessState& SyncProcessState::operator=(const SyncProcessState& counts) {
   silenced_until_ = counts.silenced_until_;
   error_rate_ = counts.error_rate_;
   current_sync_timestamp_ = counts.current_sync_timestamp_;
-  servers_latest_timestamp_ = counts.servers_latest_timestamp_;
+  num_server_changes_remaining_ = counts.num_server_changes_remaining_;
   error_commits_ = counts.error_commits_;
   stalled_commits_ = counts.stalled_commits_;
   conflicting_commits_ = counts.conflicting_commits_;
@@ -176,9 +176,9 @@ void SyncProcessState::set_current_sync_timestamp(const int64 val) {
   current_sync_timestamp_ = val;
 }
 
-void SyncProcessState::set_servers_latest_timestamp(const int64 val) {
-  UpdateDirty(val != servers_latest_timestamp_);
-  servers_latest_timestamp_ = val;
+void SyncProcessState::set_num_server_changes_remaining(const int64 val) {
+  UpdateDirty(val != num_server_changes_remaining_);
+  num_server_changes_remaining_ = val;
 }
 
 void SyncProcessState::set_error_commits(const int val) {
diff --git a/chrome/browser/sync/engine/sync_process_state.h b/chrome/browser/sync/engine/sync_process_state.h
index de48e17..c5db80a 100644
--- a/chrome/browser/sync/engine/sync_process_state.h
+++ b/chrome/browser/sync/engine/sync_process_state.h
@@ -193,11 +193,11 @@ class SyncProcessState {
   // During inital sync these two members can be used to measure sync progress.
   int64 current_sync_timestamp() const { return current_sync_timestamp_; }
 
-  int64 servers_latest_timestamp() const { return servers_latest_timestamp_; }
+  int64 num_server_changes_remaining() const { return num_server_changes_remaining_; }
 
   void set_current_sync_timestamp(const int64 val);
 
-  void set_servers_latest_timestamp(const int64 val);
+  void set_num_server_changes_remaining(const int64 val);
 
   bool invalid_store() const { return invalid_store_; }
 
@@ -300,7 +300,7 @@ class SyncProcessState {
         syncer_event_channel_(NULL),
         error_rate_(0),
         current_sync_timestamp_(0),
-        servers_latest_timestamp_(0),
+        num_server_changes_remaining_(0),
         syncing_(false),
         invalid_store_(false),
         syncer_stuck_(false),
@@ -342,7 +342,7 @@ class SyncProcessState {
   static const int ERROR_THRESHOLD = 500;
   int error_rate_;  // A EMA in the range [0,65536)
   int64 current_sync_timestamp_;  // During inital sync these two members
-  int64 servers_latest_timestamp_;  // can be used to measure sync progress.
+  int64 num_server_changes_remaining_;  // Can be used to measure sync progress.
 
   // There remains sync state updating in:
   //   CommitUnsyncedEntries
diff --git a/chrome/browser/sync/engine/syncer_status.h b/chrome/browser/sync/engine/syncer_status.h
index 8301b56..718985a 100644
--- a/chrome/browser/sync/engine/syncer_status.h
+++ b/chrome/browser/sync/engine/syncer_status.h
@@ -70,12 +70,12 @@ class SyncerStatus {
     sync_process_state_->set_current_sync_timestamp(val);
   }
 
-  int64 servers_latest_timestamp() const {
-    return sync_process_state_->servers_latest_timestamp();
+  int64 num_server_changes_remaining() const {
+    return sync_process_state_->num_server_changes_remaining();
   }
 
-  void set_servers_latest_timestamp(const int64 val) {
-    sync_process_state_->set_servers_latest_timestamp(val);
+  void set_num_server_changes_remaining(const int64 val) {
+    sync_process_state_->set_num_server_changes_remaining(val);
   }
 
   int64 unsynced_count() const {
@@ -226,7 +226,8 @@ class SyncerStatus {
     LOG(INFO) << "over_quota = " << over_quota();
 
     LOG(INFO) << "current_sync_timestamp = " << current_sync_timestamp();
-    LOG(INFO) << "servers_latest_timestamp = " << servers_latest_timestamp();
+    LOG(INFO) << "num_server_changes_remaining = "
+              << num_server_changes_remaining();
     LOG(INFO) << "unsynced_count = " << unsynced_count();
     LOG(INFO) << "conflicting_updates = " << conflicting_updates();
     LOG(INFO) << "conflicting_commits = " << conflicting_commits();
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index f37083c..288ed42 100644
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -3807,19 +3807,19 @@ TEST_F(SyncerTest, LongChangelistCreatesFakeOrphanedEntries) {
   // First we an item in a folder in the root. However the folder won't come
   // till much later.
   mock_server_->AddUpdateDirectory(99999, 1, "stuck", 1, 1);
-  mock_server_->SetNewestTimestamp(DEPTH);
+  mock_server_->SetChangesRemaining(DEPTH - 1);
   syncer_->SyncShare(state_.get());
 
   // Very long changelist. We should never be stuck.
   for (int i = 0; i < DEPTH; i++) {
     mock_server_->SetNewTimestamp(i);
-    mock_server_->SetNewestTimestamp(DEPTH);
+    mock_server_->SetChangesRemaining(DEPTH - i);
     syncer_->SyncShare(state_.get());
     EXPECT_FALSE(SyncerStuck(state_.get()));
   }
   // And finally the folder.
   mock_server_->AddUpdateDirectory(1, 0, "folder", 1, 1);
-  mock_server_->SetNewestTimestamp(DEPTH);
+  mock_server_->SetChangesRemaining(0);
   LoopSyncShare(syncer_);
   LoopSyncShare(syncer_);
   // Check that everything's as expected after the commit.