sync: use progress markers instead of timestamps during GetUpdates

chrome/browser/sync/sessions/session_state.h

 // 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.
 
 // The 'sessions' namespace comprises all the pieces of state that are
 // combined to form a SyncSession instance. In that way, it can be thought of
 // as an extension of the SyncSession type itself. Session scoping gives
 // context to things like "conflict progress", "update progress", etc, and the
 // separation this file provides allows clients to only include the parts they
 // need rather than the entire session stack.
@@ skipping 17 matching lines @@
 }
 
 namespace browser_sync {
 namespace sessions {
 
 class UpdateProgress;
 
 // Data pertaining to the status of an active Syncer object.
 struct SyncerStatus {
   SyncerStatus()
-      : invalid_store(false), syncer_stuck(false),
-        syncing(false), num_successful_commits(0),
-        num_successful_bookmark_commits(0) {}
+      : invalid_store(false),
+        syncer_stuck(false),
+        syncing(false),
+        num_successful_commits(0),
+        num_successful_bookmark_commits(0),
+        num_updates_downloaded(0),
+        num_tombstone_updates_downloaded(0) {}
 
   // True when we get such an INVALID_STORE error from the server.
   bool invalid_store;
   // True iff we're stuck.
   bool syncer_stuck;
   bool syncing;
   int num_successful_commits;
   // This is needed for monitoring extensions activity.
   int num_successful_bookmark_commits;
+
+  // Download event counters.

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

It looks like you update the counters at the same time as we update the
'UpdateProgress', which is where the number of verified downloaded updates is
currently tracked (VerifiedUpdatesSize()). It is split up by model safe group, 
so you would need a function in StatusController to iterate and sum (similar to
what ComputeMaxLocalTimestamp did). UpdateProgress doesn't count the tombstones,
but we pass in the update to AddVerifyResult so it'd be trivial to add a
num_tombstone_updates_downloaded there.  And you could revert
verify_updates_command, I think.

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

Moving these to the UpdateProgress would require additional handling in
constructing the SyncSessionSnapshot.  I rather like that there's just one POD
field that gets incremented in one place, and plumbed through.  And, my goal for
these fields is for them to be durable across sessions, in which case the
ephemerality of UpdateProgress would make for an inappropriate place.

They can't go in the Context because the context is const.

+  int num_updates_downloaded;
+  int num_tombstone_updates_downloaded;
 };
 
 // Counters for various errors that can occur repeatedly during a sync session.
 struct ErrorCounters {
   ErrorCounters() : num_conflicting_commits(0),
                     consecutive_transient_error_commits(0),
                     consecutive_errors(0) {}
   int num_conflicting_commits;
 
   // Number of commits hitting transient errors since the last successful
   // commit.
   int consecutive_transient_error_commits;
 
   // Incremented when get_updates fails, commit fails, and when hitting
   // transient errors. When any of these succeed, this counter is reset.
   // TODO(chron): Reduce number of weird counters we use.
   int consecutive_errors;
 };
 
 // An immutable snapshot of state from a SyncSession.  Convenient to use as
 // part of notifications as it is inherently thread-safe.
 struct SyncSessionSnapshot {
-  SyncSessionSnapshot(const SyncerStatus& syncer_status,
+  SyncSessionSnapshot(
+      const SyncerStatus& syncer_status,
       const ErrorCounters& errors,
       int64 num_server_changes_remaining,
-      int64 max_local_timestamp,
       bool is_share_usable,
       const syncable::ModelTypeBitSet& initial_sync_ended,
+      std::string download_progress_markers[syncable::MODEL_TYPE_COUNT],
       bool more_to_sync,
       bool is_silenced,
       int64 unsynced_count,
       int num_conflicting_updates,
-                      bool did_commit_items);
+      bool did_commit_items);
   ~SyncSessionSnapshot();
 
   const SyncerStatus syncer_status;
   const ErrorCounters errors;
   const int64 num_server_changes_remaining;
-  const int64 max_local_timestamp;
   const bool is_share_usable;
   const syncable::ModelTypeBitSet initial_sync_ended;
+  const std::string download_progress_markers[syncable::MODEL_TYPE_COUNT];
   const bool has_more_to_sync;
   const bool is_silenced;
   const int64 unsynced_count;
   const int num_conflicting_updates;
   const bool did_commit_items;
 };
 
 // Tracks progress of conflicts and their resolution using conflict sets.
 class ConflictProgress {
  public:
@@ skipping 125 matching lines @@
 // a singleton with respect to model types.
 struct AllModelTypeState {
   explicit AllModelTypeState(bool* dirty_flag);
   ~AllModelTypeState();
 
   // Commits for all model types are bundled together into a single message.
   ClientToServerMessage commit_message;
   ClientToServerResponse commit_response;
   // We GetUpdates for some combination of types at once.
   // requested_update_types stores the set of types which were requested.
-  syncable::MultiTypeTimeStamp updates_request_parameters;
+  syncable::ModelTypeBitSet updates_request_types;
   ClientToServerResponse updates_response;
   // Used to build the shared commit message.
   DirtyOnWrite<std::vector<int64> > unsynced_handles;
   DirtyOnWrite<SyncerStatus> syncer_status;
   DirtyOnWrite<ErrorCounters> error_counters;
   SyncCycleControlParameters control_params;
   DirtyOnWrite<int64> num_server_changes_remaining;
   OrderedCommitSet commit_set;
 };
 
 // Grouping of all state that applies to a single ModelSafeGroup.
 struct PerModelSafeGroupState {
   explicit PerModelSafeGroupState(bool* dirty_flag);
   ~PerModelSafeGroupState();
 
   UpdateProgress update_progress;
   ConflictProgress conflict_progress;
 };
 
 // Grouping of all state that applies to a single ModelType.
 struct PerModelTypeState {
   explicit PerModelTypeState(bool* dirty_flag);
   ~PerModelTypeState();
-
-  DirtyOnWrite<int64> current_download_timestamp;
 };
 
 }  // namespace sessions
 }  // namespace browser_sync
 
 #endif  // CHROME_BROWSER_SYNC_SESSIONS_SESSION_STATE_H_