sync: use progress markers instead of timestamps during GetUpdates

chrome/browser/sync/engine/download_updates_command.cc

Index: chrome/browser/sync/engine/download_updates_command.cc
diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc
index a8d495dafd270f4349c6930e6ce81f9e6957a95b..84fd6bf3e1c943246b887eb15b13d3cc0a9a53ed 100644
--- a/chrome/browser/sync/engine/download_updates_command.cc
+++ b/chrome/browser/sync/engine/download_updates_command.cc
@@ -41,25 +41,18 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
     return;
   }
 
-  // Pick some subset of the enabled types where all types in the set
-  // are at the same last_download_timestamp.  Do an update for those types.
+  // Request updates for all enabled types.

[tim (not reviewing), 2011/01/11 19:14:23]

Is it okay if the routing_info is actually a subset of all enabled types? 
Scenario: "configuration" mode where we only want to download updates for 2 out
of 5 enabled types. I'm working on a patch for that at the moment, and it uses
routing_info to control which types we query.  I know you know this case, but
might want to tweak the comment?

[ncarter (slow), 2011/01/13 00:06:13]

Sure; with the new protocol it's completely orthogonal what gets requested,
since the progress markers track their state indepenently.  We no longer have to
worry about timestamps converging.

   syncable::ModelTypeBitSet enabled_types;
   for (ModelSafeRoutingInfo::const_iterator i = session->routing_info().begin();
        i != session->routing_info().end(); ++i) {
+    syncable::ModelType model_type = syncable::ModelTypeFromInt(i->first);
     enabled_types[i->first] = true;
+    dir->GetDownloadProgress(model_type,
+        get_updates->add_from_progress_marker());
   }
-  syncable::MultiTypeTimeStamp target =
-      dir->GetTypesWithOldestLastDownloadTimestamp(enabled_types);
-  VLOG(1) << "Getting updates from ts " << target.timestamp
-          << " for types " << target.data_types.to_string()
-          << " (of possible " << enabled_types.to_string() << ")";
-  DCHECK(target.data_types.any());
-  target.data_types.set(syncable::TOP_LEVEL_FOLDER);  // Always fetched.
 
-  get_updates->set_from_timestamp(target.timestamp);
-
-  // Set the requested_types protobuf field so that we fetch all enabled types.
-  SetRequestedTypes(target.data_types, get_updates->mutable_requested_types());
+  VLOG(1) << "Getting updates for types " << enabled_types.to_string();
+  DCHECK(enabled_types.any());
 
   // We want folders for our associated types, always.  If we were to set
   // this to false, the server would send just the non-container items
@@ -83,7 +76,7 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
       update_response);
 
   StatusController* status = session->status_controller();
-  status->set_updates_request_parameters(target);
+  status->set_updates_request_types(enabled_types);
   if (!ok) {
     status->increment_num_consecutive_errors();
     status->mutable_updates_response()->Clear();
@@ -93,7 +86,7 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
 
   status->mutable_updates_response()->CopyFrom(update_response);
 
-  VLOG(1) << "GetUpdates from ts " << get_updates->from_timestamp()
+  VLOG(1) << "GetUpdates "
           << " returned " << update_response.get_updates().entries_size()
           << " updates and indicated "
           << update_response.get_updates().changes_remaining()