Make it clear what last_sync_timestamp actually tracks. Update

chrome/browser/sync/sessions/sync_session_unittest.cc

 // Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/sessions/sync_session.h"
 
 #include "chrome/browser/sync/engine/conflict_resolver.h"
 #include "chrome/browser/sync/engine/syncer_types.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
@@ skipping 115 matching lines @@
   EXPECT_TRUE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, MoreToSyncIfConflictSetsBuilt) {
   // If we built conflict sets, then we need to loop back and try
   // to get updates & commit again.
   status()->update_conflict_sets_built(true);
   EXPECT_TRUE(session_->HasMoreToSync());
 }
 
-TEST_F(SyncSessionTest, MoreToSyncIfDidNotGetZeroUpdates) {
-  // We're not done getting updates until we get an empty response.
-  ClientToServerResponse response;
-  response.mutable_get_updates()->add_entries();
-  status()->mutable_updates_response()->CopyFrom(response);
-  EXPECT_TRUE(session_->HasMoreToSync());
-  status()->mutable_updates_response()->Clear();
+TEST_F(SyncSessionTest, MoreToDownloadIfDownloadFailed) {
+  // When DownloadUpdatesCommand fails, these should be false.
+  EXPECT_FALSE(status()->server_says_nothing_more_to_download());
+  EXPECT_FALSE(status()->download_updates_succeeded());
+
+  // Download updates has its own loop in the syncer; it shouldn't factor
+  // into HasMoreToSync.
   EXPECT_FALSE(session_->HasMoreToSync());
-  status()->mutable_updates_response()->CopyFrom(response);
-  EXPECT_TRUE(session_->HasMoreToSync());
 }
 
+TEST_F(SyncSessionTest, MoreToDownloadIfGotTimestamp) {
+  // When the server returns a timestamp, that means there's more to download.
+  status()->mutable_updates_response()->mutable_get_updates()
+      ->set_new_timestamp(1000000L);
+  EXPECT_FALSE(status()->server_says_nothing_more_to_download());
+  EXPECT_TRUE(status()->download_updates_succeeded());
+
+  // Download updates has its own loop in the syncer; it shouldn't factor
+  // into HasMoreToSync.
+  EXPECT_FALSE(session_->HasMoreToSync());
+}
+
+TEST_F(SyncSessionTest, MoreToDownloadIfGotNoTimestamp) {
+  // When the server returns a timestamp, that means we're up to date.
+  status()->mutable_updates_response()->mutable_get_updates()
+      ->clear_new_timestamp();
+  EXPECT_TRUE(status()->server_says_nothing_more_to_download());
+  EXPECT_TRUE(status()->download_updates_succeeded());
+
+  // Download updates has its own loop in the syncer; it shouldn't factor
+  // into HasMoreToSync.
+  EXPECT_FALSE(session_->HasMoreToSync());
+}
+
+TEST_F(SyncSessionTest, MoreToDownloadIfGotTimestampAndEntries) {
+  // The actual entry count should not factor into the HasMoreToSync
+  // determination.
+  status()->mutable_updates_response()->mutable_get_updates()->add_entries();
+  status()->mutable_updates_response()->mutable_get_updates()
+      ->set_new_timestamp(1000000L);;
+  EXPECT_FALSE(status()->server_says_nothing_more_to_download());
+  EXPECT_TRUE(status()->download_updates_succeeded());
+
+  // Download updates has its own loop in the syncer; it shouldn't factor
+  // into HasMoreToSync.
+  EXPECT_FALSE(session_->HasMoreToSync());
+}
+
+
 TEST_F(SyncSessionTest, MoreToSyncIfConflictsResolved) {
   // Conflict resolution happens after get updates and commit,
   // so we need to loop back and get updates / commit again now
   // that we have made forward progress.
   status()->update_conflicts_resolved(true);
   EXPECT_TRUE(session_->HasMoreToSync());
 }
 
-TEST_F(SyncSessionTest, MoreToSyncIfTimestampDirty) {
-  // If there are more changes on the server that weren't processed during this
-  // GetUpdates request, the client should send another GetUpdates request and
-  // use new_timestamp as the from_timestamp value within GetUpdatesMessage.
-  status()->set_got_new_timestamp();
-  status()->update_conflicts_resolved(true);
-  EXPECT_TRUE(session_->HasMoreToSync());
-}
-
-
 }  // namespace
 }  // namespace sessions
 }  // namespace browser_sync