sync: Refactor session tracking

This change refactors the related structs ErrorCounters, SyncerStatus, and 
SyncCycleControlParameters. Their values have all been merged into 
AllModelTypeState, which has been renamed to ModelNeutralState. All the 
functions which depend on this data have been updated to use the new struct. 

This change also removes the DirtyOnWrite template class, the is_dirty flag, 
and the SyncerCommand logic to send change events if it detects a change in the 
syncer's status. The changes are so frequent and predictable that it's easier 
to just send the snapshots manually after any major syncer steps. 

Finally, this change removes the 'invalid_store' status member, which was never 
set nor read outside of unit tests. 

BUG=132630
TEST=


Review URL: https://chromiumcodereview.appspot.com/10636010

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

8 files changed, 129 insertions, 170 deletions

diff --git a/sync/internal_api/public/sessions/error_counters.cc b/sync/internal_api/public/sessions/error_counters.cc
deleted file mode 100644
index 9ce6772..0000000
--- a/sync/internal_api/public/sessions/error_counters.cc
+++ /dev/null
@@ -1,16 +0,0 @@
-// Copyright (c) 2012 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 "sync/internal_api/public/sessions/error_counters.h"
-
-namespace csync {
-namespace sessions {
-
-ErrorCounters::ErrorCounters()
-    : last_download_updates_result(UNSET),
-      commit_result(UNSET) {
-}
-
-}  // namespace sessions
-}  // namespace csync
diff --git a/sync/internal_api/public/sessions/error_counters.h b/sync/internal_api/public/sessions/error_counters.h
deleted file mode 100644
index a5baed7..0000000
--- a/sync/internal_api/public/sessions/error_counters.h
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright (c) 2012 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.
-
-#ifndef SYNC_INTERNAL_API_PUBLIC_SESSIONS_ERROR_COUNTERS_H_
-#define SYNC_INTERNAL_API_PUBLIC_SESSIONS_ERROR_COUNTERS_H_
-#pragma once
-
-#include "base/basictypes.h"
-#include "sync/internal_api/public/util/syncer_error.h"
-#include "sync/protocol/sync_protocol_error.h"
-
-namespace csync {
-namespace sessions {
-
-// Counters for various errors that can occur repeatedly during a sync session.
-// TODO(lipalani) : Rename this structure to Error.
-struct ErrorCounters {
-  ErrorCounters();
-
-  // Any protocol errors that we received during this sync session.
-  SyncProtocolError sync_protocol_error;
-
-  // Records the most recent results of PostCommit and GetUpdates commands.
-  SyncerError last_download_updates_result;
-  SyncerError commit_result;
-};
-
-}  // namespace sessions
-}  // namespace csync
-
-#endif  // SYNC_INTERNAL_API_PUBLIC_SESSIONS_ERROR_COUNTERS_H_
diff --git a/sync/internal_api/public/sessions/model_neutral_state.cc b/sync/internal_api/public/sessions/model_neutral_state.cc
new file mode 100644
index 0000000..82d9a35
--- /dev/null
+++ b/sync/internal_api/public/sessions/model_neutral_state.cc
@@ -0,0 +1,29 @@
+// Copyright (c) 2012 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 "sync/internal_api/public/sessions/model_neutral_state.h"
+
+namespace csync {
+namespace sessions {
+
+ModelNeutralState::ModelNeutralState()
+    : num_successful_commits(0),
+      num_successful_bookmark_commits(0),
+      num_updates_downloaded_total(0),
+      num_tombstone_updates_downloaded_total(0),
+      num_reflected_updates_downloaded_total(0),
+      num_local_overwrites(0),
+      num_server_overwrites(0),
+      last_download_updates_result(UNSET),
+      commit_result(UNSET),
+      conflicts_resolved(false),
+      items_committed(false),
+      debug_info_sent(false),
+      num_server_changes_remaining(0) {
+}
+
+ModelNeutralState::~ModelNeutralState() {}
+
+}  // namespace sessions
+}  // namespace csync
diff --git a/sync/internal_api/public/sessions/model_neutral_state.h b/sync/internal_api/public/sessions/model_neutral_state.h
new file mode 100644
index 0000000..c9f93c8
--- /dev/null
+++ b/sync/internal_api/public/sessions/model_neutral_state.h
@@ -0,0 +1,74 @@
+// Copyright (c) 2012 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.
+
+#ifndef SYNC_SESSIONS_MODEL_NEUTRAL_STATE_H
+#define SYNC_SESSIONS_MODEL_NEUTRAL_STATE_H
+#pragma once
+
+#include "base/basictypes.h"
+#include "sync/internal_api/public/util/syncer_error.h"
+#include "sync/protocol/sync.pb.h"
+#include "sync/protocol/sync_protocol_error.h"
+
+namespace csync {
+namespace sessions {
+
+// Grouping of all state that applies to all model types.  Note that some
+// components of the global grouping can internally implement finer grained
+// scope control, but the top level entity is still a singleton with respect to
+// model types.
+struct ModelNeutralState {
+  ModelNeutralState();
+  ~ModelNeutralState();
+
+  // We GetUpdates for some combination of types at once.
+  // requested_update_types stores the set of types which were requested.
+  syncable::ModelTypeSet updates_request_types;
+
+  sync_pb::ClientToServerResponse updates_response;
+
+  int num_successful_commits;
+
+  // This is needed for monitoring extensions activity.
+  int num_successful_bookmark_commits;
+
+  // Download event counters.
+  int num_updates_downloaded_total;
+  int num_tombstone_updates_downloaded_total;
+  int num_reflected_updates_downloaded_total;
+
+  // If the syncer encountered a MIGRATION_DONE code, these are the types that
+  // the client must now "migrate", by purging and re-downloading all updates.
+  syncable::ModelTypeSet types_needing_local_migration;
+
+  // Overwrites due to conflict resolution counters.
+  int num_local_overwrites;
+  int num_server_overwrites;
+
+  // Any protocol errors that we received during this sync session.
+  SyncProtocolError sync_protocol_error;
+
+  // Records the most recent results of PostCommit and GetUpdates commands.
+  SyncerError last_download_updates_result;
+  SyncerError commit_result;
+
+  // Set to true by ResolveConflictsCommand if any forward progress was made.
+  bool conflicts_resolved;
+
+  // Set to true by PostCommitMessageCommand if any commits were successful.
+  bool items_committed;
+
+  // True indicates debug info has been sent once this session.
+  bool debug_info_sent;
+
+  // Number of changes remaining, according to the server.
+  // Take it as an estimate unless it's value is zero, in which case there
+  // really is nothing more to download.
+  int64 num_server_changes_remaining;
+};
+
+}  // namespace sessions
+}  // namespace csync
+
+#endif  // SYNC_SESSIONS_MODEL_NEUTRAL_STATE_H_
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.cc b/sync/internal_api/public/sessions/sync_session_snapshot.cc
index cf910dad..50ffcbb 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot.cc
+++ b/sync/internal_api/public/sessions/sync_session_snapshot.cc
@@ -25,9 +25,7 @@ SyncSessionSnapshot::SyncSessionSnapshot()
 }
 
 SyncSessionSnapshot::SyncSessionSnapshot(
-    const SyncerStatus& syncer_status,
-    const ErrorCounters& errors,
-    int64 num_server_changes_remaining,
+    const ModelNeutralState& model_neutral_state,
     bool is_share_usable,
     syncable::ModelTypeSet initial_sync_ended,
     const syncable::ModelTypePayloadMap& download_progress_markers,
@@ -42,9 +40,7 @@ SyncSessionSnapshot::SyncSessionSnapshot(
     size_t num_entries,
     base::Time sync_start_time,
     bool retry_scheduled)
-    : syncer_status_(syncer_status),
-      errors_(errors),
-      num_server_changes_remaining_(num_server_changes_remaining),
+    : model_neutral_state_(model_neutral_state),
       is_share_usable_(is_share_usable),
       initial_sync_ended_(initial_sync_ended),
       download_progress_markers_(download_progress_markers),
@@ -65,10 +61,23 @@ SyncSessionSnapshot::~SyncSessionSnapshot() {}
 
 DictionaryValue* SyncSessionSnapshot::ToValue() const {
   DictionaryValue* value = new DictionaryValue();
-  value->Set("syncerStatus", syncer_status_.ToValue());
-  // We don't care too much if we lose precision here.
-  value->SetInteger("numServerChangesRemaining",
-                    static_cast<int>(num_server_changes_remaining_));
+  value->SetInteger("numSuccessfulCommits",
+                    model_neutral_state_.num_successful_commits);
+  value->SetInteger("numSuccessfulBookmarkCommits",
+                model_neutral_state_.num_successful_bookmark_commits);
+  value->SetInteger("numUpdatesDownloadedTotal",
+                model_neutral_state_.num_updates_downloaded_total);
+  value->SetInteger("numTombstoneUpdatesDownloadedTotal",
+                model_neutral_state_.num_tombstone_updates_downloaded_total);
+  value->SetInteger("numReflectedUpdatesDownloadedTotal",
+                model_neutral_state_.num_reflected_updates_downloaded_total);
+  value->SetInteger("numLocalOverwrites",
+                    model_neutral_state_.num_local_overwrites);
+  value->SetInteger("numServerOverwrites",
+                    model_neutral_state_.num_server_overwrites);
+  value->SetInteger(
+      "numServerChangesRemaining",
+      static_cast<int>(model_neutral_state_.num_server_changes_remaining));
   value->SetBoolean("isShareUsable", is_share_usable_);
   value->Set("initialSyncEnded",
              syncable::ModelTypeSetToValue(initial_sync_ended_));
@@ -100,16 +109,8 @@ std::string SyncSessionSnapshot::ToString() const {
   return json;
 }
 
-SyncerStatus SyncSessionSnapshot::syncer_status() const {
-  return syncer_status_;
-}
-
-ErrorCounters SyncSessionSnapshot::errors() const {
-  return errors_;
-}
-
 int64 SyncSessionSnapshot::num_server_changes_remaining() const {
-  return num_server_changes_remaining_;
+  return model_neutral_state().num_server_changes_remaining;
 }
 
 bool SyncSessionSnapshot::is_share_usable() const {
diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.h b/sync/internal_api/public/sessions/sync_session_snapshot.h
index 8bd2258..a74e16b 100644
--- a/sync/internal_api/public/sessions/sync_session_snapshot.h
+++ b/sync/internal_api/public/sessions/sync_session_snapshot.h
@@ -10,9 +10,8 @@
 
 #include "base/basictypes.h"
 #include "base/time.h"
-#include "sync/internal_api/public/sessions/error_counters.h"
+#include "sync/internal_api/public/sessions/model_neutral_state.h"
 #include "sync/internal_api/public/sessions/sync_source_info.h"
-#include "sync/internal_api/public/sessions/syncer_status.h"
 #include "sync/internal_api/public/syncable/model_type.h"
 #include "sync/internal_api/public/syncable/model_type_payload_map.h"
 
@@ -32,9 +31,7 @@ class SyncSessionSnapshot {
  public:
   SyncSessionSnapshot();
   SyncSessionSnapshot(
-      const SyncerStatus& syncer_status,
-      const ErrorCounters& errors,
-      int64 num_server_changes_remaining,
+      const ModelNeutralState& model_neutral_state,
       bool is_share_usable,
       syncable::ModelTypeSet initial_sync_ended,
       const syncable::ModelTypePayloadMap& download_progress_markers,
@@ -56,8 +53,9 @@ class SyncSessionSnapshot {
 
   std::string ToString() const;
 
-  SyncerStatus syncer_status() const;
-  ErrorCounters errors() const;
+  ModelNeutralState model_neutral_state() const {
+    return model_neutral_state_;
+  }
   int64 num_server_changes_remaining() const;
   bool is_share_usable() const;
   syncable::ModelTypeSet initial_sync_ended() const;
@@ -75,8 +73,7 @@ class SyncSessionSnapshot {
   bool retry_scheduled() const;
 
  private:
-  SyncerStatus syncer_status_;
-  ErrorCounters errors_;
+  ModelNeutralState model_neutral_state_;
   int64 num_server_changes_remaining_;
   bool is_share_usable_;
   syncable::ModelTypeSet initial_sync_ended_;
diff --git a/sync/internal_api/public/sessions/syncer_status.cc b/sync/internal_api/public/sessions/syncer_status.cc
deleted file mode 100644
index 5b26a24..0000000
--- a/sync/internal_api/public/sessions/syncer_status.cc
+++ /dev/null
@@ -1,44 +0,0 @@
-// Copyright (c) 2012 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 "sync/internal_api/public/sessions/syncer_status.h"
-
-#include "base/values.h"
-
-namespace csync {
-namespace sessions {
-
-SyncerStatus::SyncerStatus()
-    : invalid_store(false),
-      num_successful_commits(0),
-      num_successful_bookmark_commits(0),
-      num_updates_downloaded_total(0),
-      num_tombstone_updates_downloaded_total(0),
-      num_reflected_updates_downloaded_total(0),
-      num_local_overwrites(0),
-      num_server_overwrites(0) {
-}
-
-SyncerStatus::~SyncerStatus() {
-}
-
-DictionaryValue* SyncerStatus::ToValue() const {
-  DictionaryValue* value = new DictionaryValue();
-  value->SetBoolean("invalidStore", invalid_store);
-  value->SetInteger("numSuccessfulCommits", num_successful_commits);
-  value->SetInteger("numSuccessfulBookmarkCommits",
-                num_successful_bookmark_commits);
-  value->SetInteger("numUpdatesDownloadedTotal",
-                num_updates_downloaded_total);
-  value->SetInteger("numTombstoneUpdatesDownloadedTotal",
-                num_tombstone_updates_downloaded_total);
-  value->SetInteger("numReflectedUpdatesDownloadedTotal",
-                num_reflected_updates_downloaded_total);
-  value->SetInteger("numLocalOverwrites", num_local_overwrites);
-  value->SetInteger("numServerOverwrites", num_server_overwrites);
-  return value;
-}
-
-}  // namespace sessions
-}  // namespace csync
diff --git a/sync/internal_api/public/sessions/syncer_status.h b/sync/internal_api/public/sessions/syncer_status.h
deleted file mode 100644
index 070f564..0000000
--- a/sync/internal_api/public/sessions/syncer_status.h
+++ /dev/null
@@ -1,50 +0,0 @@
-// Copyright (c) 2012 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.
-
-#ifndef SYNC_INTERNAL_API_PUBLIC_SESSIONS_SYNCER_STATUS_H_
-#define SYNC_INTERNAL_API_PUBLIC_SESSIONS_SYNCER_STATUS_H_
-#pragma once
-
-#include "base/basictypes.h"
-#include "sync/internal_api/public/syncable/model_type.h"
-
-namespace base {
-class DictionaryValue;
-}
-
-namespace csync {
-namespace sessions {
-
-// Data pertaining to the status of an active Syncer object.
-struct SyncerStatus {
-  SyncerStatus();
-  ~SyncerStatus();
-
-  // Caller takes ownership of the returned dictionary.
-  base::DictionaryValue* ToValue() const;
-
-  // True when we get such an INVALID_STORE error from the server.
-  bool invalid_store;
-  int num_successful_commits;
-  // This is needed for monitoring extensions activity.
-  int num_successful_bookmark_commits;
-
-  // Download event counters.
-  int num_updates_downloaded_total;
-  int num_tombstone_updates_downloaded_total;
-  int num_reflected_updates_downloaded_total;
-
-  // If the syncer encountered a MIGRATION_DONE code, these are the types that
-  // the client must now "migrate", by purging and re-downloading all updates.
-  syncable::ModelTypeSet types_needing_local_migration;
-
-  // Overwrites due to conflict resolution counters.
-  int num_local_overwrites;
-  int num_server_overwrites;
-};
-
-}  // namespace sessions
-}  // namespace csync
-
-#endif  // SYNC_INTERNAL_API_PUBLIC_SESSIONS_SYNCER_STATUS_H_