[Sync] Convert SyncSessionSnapshot to a copy-able class.

Previously it was an immutable struct that was passed around by making
dynamic allocations and passing pointers. We now just have a class with
only getters and no setters, but support for default copy and assign.

This cleans up some code and makes some future work easier to pass snapshots
around.

BUG=none
TEST=sync_unit_tests


Review URL: http://codereview.chromium.org/10197004

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

6 files changed, 183 insertions, 122 deletions

diff --git a/sync/sessions/session_state.cc b/sync/sessions/session_state.cc
index f49c41d..d0d23fa 100644
--- a/sync/sessions/session_state.cc
+++ b/sync/sessions/session_state.cc
@@ -75,38 +75,35 @@ DictionaryValue* SyncerStatus::ToValue() const {
   return value;
 }
 
-DictionaryValue* DownloadProgressMarkersToValue(
-    const std::string
-        (&download_progress_markers)[syncable::MODEL_TYPE_COUNT]) {
-  DictionaryValue* value = new DictionaryValue();
-  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-       i < syncable::MODEL_TYPE_COUNT; ++i) {
-    // TODO(akalin): Unpack the value into a protobuf.
-    std::string base64_marker;
-    bool encoded =
-        base::Base64Encode(download_progress_markers[i], &base64_marker);
-    DCHECK(encoded);
-    value->SetString(
-        syncable::ModelTypeToString(syncable::ModelTypeFromInt(i)),
-        base64_marker);
-  }
-  return value;
-}
-
 ErrorCounters::ErrorCounters()
     : last_download_updates_result(UNSET),
       last_post_commit_result(UNSET),
       last_process_commit_response_result(UNSET) {
 }
 
+SyncSessionSnapshot::SyncSessionSnapshot()
+    : num_server_changes_remaining_(0),
+      is_share_usable_(false),
+      has_more_to_sync_(false),
+      is_silenced_(false),
+      unsynced_count_(0),
+      num_encryption_conflicts_(0),
+      num_hierarchy_conflicts_(0),
+      num_simple_conflicts_(0),
+      num_server_conflicts_(0),
+      did_commit_items_(false),
+      notifications_enabled_(false),
+      num_entries_(0),
+      retry_scheduled_(false) {
+}
+
 SyncSessionSnapshot::SyncSessionSnapshot(
     const SyncerStatus& syncer_status,
     const ErrorCounters& errors,
     int64 num_server_changes_remaining,
     bool is_share_usable,
     syncable::ModelTypeSet initial_sync_ended,
-    const std::string
-        (&download_progress_markers)[syncable::MODEL_TYPE_COUNT],
+    const syncable::ModelTypePayloadMap& download_progress_markers,
     bool more_to_sync,
     bool is_silenced,
     int64 unsynced_count,
@@ -120,62 +117,57 @@ 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),
-      is_share_usable(is_share_usable),
-      initial_sync_ended(initial_sync_ended),
-      download_progress_markers(),
-      has_more_to_sync(more_to_sync),
-      is_silenced(is_silenced),
-      unsynced_count(unsynced_count),
-      num_encryption_conflicts(num_encryption_conflicts),
-      num_hierarchy_conflicts(num_hierarchy_conflicts),
-      num_simple_conflicts(num_simple_conflicts),
-      num_server_conflicts(num_server_conflicts),
-      did_commit_items(did_commit_items),
-      source(source),
-      notifications_enabled(notifications_enabled),
-      num_entries(num_entries),
-      sync_start_time(sync_start_time),
-      retry_scheduled(retry_scheduled) {
-  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-       i < syncable::MODEL_TYPE_COUNT; ++i) {
-    const_cast<std::string&>(this->download_progress_markers[i]).assign(
-        download_progress_markers[i]);
-  }
+    : syncer_status_(syncer_status),
+      errors_(errors),
+      num_server_changes_remaining_(num_server_changes_remaining),
+      is_share_usable_(is_share_usable),
+      initial_sync_ended_(initial_sync_ended),
+      download_progress_markers_(download_progress_markers),
+      has_more_to_sync_(more_to_sync),
+      is_silenced_(is_silenced),
+      unsynced_count_(unsynced_count),
+      num_encryption_conflicts_(num_encryption_conflicts),
+      num_hierarchy_conflicts_(num_hierarchy_conflicts),
+      num_simple_conflicts_(num_simple_conflicts),
+      num_server_conflicts_(num_server_conflicts),
+      did_commit_items_(did_commit_items),
+      source_(source),
+      notifications_enabled_(notifications_enabled),
+      num_entries_(num_entries),
+      sync_start_time_(sync_start_time),
+      retry_scheduled_(retry_scheduled) {
 }
 
 SyncSessionSnapshot::~SyncSessionSnapshot() {}
 
 DictionaryValue* SyncSessionSnapshot::ToValue() const {
   DictionaryValue* value = new DictionaryValue();
-  value->Set("syncerStatus", syncer_status.ToValue());
+  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->SetBoolean("isShareUsable", is_share_usable);
+                    static_cast<int>(num_server_changes_remaining_));
+  value->SetBoolean("isShareUsable", is_share_usable_);
   value->Set("initialSyncEnded",
-             syncable::ModelTypeSetToValue(initial_sync_ended));
+             syncable::ModelTypeSetToValue(initial_sync_ended_));
   value->Set("downloadProgressMarkers",
-             DownloadProgressMarkersToValue(download_progress_markers));
-  value->SetBoolean("hasMoreToSync", has_more_to_sync);
-  value->SetBoolean("isSilenced", is_silenced);
+             syncable::ModelTypePayloadMapToValue(download_progress_markers_));
+  value->SetBoolean("hasMoreToSync", has_more_to_sync_);
+  value->SetBoolean("isSilenced", is_silenced_);
   // We don't care too much if we lose precision here, also.
   value->SetInteger("unsyncedCount",
-                    static_cast<int>(unsynced_count));
+                    static_cast<int>(unsynced_count_));
   value->SetInteger("numEncryptionConflicts",
-                    num_encryption_conflicts);
+                    num_encryption_conflicts_);
   value->SetInteger("numHierarchyConflicts",
-                    num_hierarchy_conflicts);
+                    num_hierarchy_conflicts_);
   value->SetInteger("numSimpleConflicts",
-                    num_simple_conflicts);
+                    num_simple_conflicts_);
   value->SetInteger("numServerConflicts",
-                    num_server_conflicts);
-  value->SetBoolean("didCommitItems", did_commit_items);
-  value->SetInteger("numEntries", num_entries);
-  value->Set("source", source.ToValue());
-  value->SetBoolean("notificationsEnabled", notifications_enabled);
+                    num_server_conflicts_);
+  value->SetBoolean("didCommitItems", did_commit_items_);
+  value->SetInteger("numEntries", num_entries_);
+  value->Set("source", source_.ToValue());
+  value->SetBoolean("notificationsEnabled", notifications_enabled_);
   return value;
 }
 
@@ -188,6 +180,83 @@ 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_;
+}
+
+bool SyncSessionSnapshot::is_share_usable() const {
+  return is_share_usable_;
+}
+
+syncable::ModelTypeSet SyncSessionSnapshot::initial_sync_ended() const {
+  return initial_sync_ended_;
+}
+
+syncable::ModelTypePayloadMap
+    SyncSessionSnapshot::download_progress_markers() const {
+  return download_progress_markers_;
+}
+
+bool SyncSessionSnapshot::has_more_to_sync() const {
+  return has_more_to_sync_;
+}
+
+bool SyncSessionSnapshot::is_silenced() const {
+  return is_silenced_;
+}
+
+int64 SyncSessionSnapshot::unsynced_count() const {
+  return unsynced_count_;
+}
+
+int SyncSessionSnapshot::num_encryption_conflicts() const {
+  return num_encryption_conflicts_;
+}
+
+int SyncSessionSnapshot::num_hierarchy_conflicts() const {
+  return num_hierarchy_conflicts_;
+}
+
+int SyncSessionSnapshot::num_simple_conflicts() const {
+  return num_simple_conflicts_;
+}
+
+int SyncSessionSnapshot::num_server_conflicts() const {
+  return num_server_conflicts_;
+}
+
+bool SyncSessionSnapshot::did_commit_items() const {
+  return did_commit_items_;
+}
+
+SyncSourceInfo SyncSessionSnapshot::source() const {
+  return source_;
+}
+
+bool SyncSessionSnapshot::notifications_enabled() const {
+  return notifications_enabled_;
+}
+
+size_t SyncSessionSnapshot::num_entries() const {
+  return num_entries_;
+}
+
+base::Time SyncSessionSnapshot::sync_start_time() const {
+  return sync_start_time_;
+}
+
+bool SyncSessionSnapshot::retry_scheduled() const {
+  return retry_scheduled_;
+}
+
 ConflictProgress::ConflictProgress(bool* dirty_flag)
   : num_server_conflicting_items(0), num_hierarchy_conflicting_items(0),
     num_encryption_conflicting_items(0), dirty_(dirty_flag) {
diff --git a/sync/sessions/session_state.h b/sync/sessions/session_state.h
index 7403758..3f37cd0 100644
--- a/sync/sessions/session_state.h
+++ b/sync/sessions/session_state.h
@@ -97,22 +97,21 @@ struct ErrorCounters {
   SyncerError last_process_commit_response_result;
 };
 
-// Caller takes ownership of the returned dictionary.
-base::DictionaryValue* DownloadProgressMarkersToValue(
-    const std::string
-        (&download_progress_markers)[syncable::MODEL_TYPE_COUNT]);
-
 // An immutable snapshot of state from a SyncSession.  Convenient to use as
 // part of notifications as it is inherently thread-safe.
-struct SyncSessionSnapshot {
+// TODO(zea): if copying this all over the place starts getting expensive,
+// consider passing around immutable references instead of values.
+// Default copy and assign welcome.
+class SyncSessionSnapshot {
+ public:
+  SyncSessionSnapshot();
   SyncSessionSnapshot(
       const SyncerStatus& syncer_status,
       const ErrorCounters& errors,
       int64 num_server_changes_remaining,
       bool is_share_usable,
       syncable::ModelTypeSet initial_sync_ended,
-      const std::string
-          (&download_progress_markers)[syncable::MODEL_TYPE_COUNT],
+      const syncable::ModelTypePayloadMap& download_progress_markers,
       bool more_to_sync,
       bool is_silenced,
       int64 unsynced_count,
@@ -133,25 +132,46 @@ struct SyncSessionSnapshot {
 
   std::string ToString() const;
 
-  const SyncerStatus syncer_status;
-  const ErrorCounters errors;
-  const int64 num_server_changes_remaining;
-  const bool is_share_usable;
-  const syncable::ModelTypeSet 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_encryption_conflicts;
-  const int num_hierarchy_conflicts;
-  const int num_simple_conflicts;
-  const int num_server_conflicts;
-  const bool did_commit_items;
-  const SyncSourceInfo source;
-  const bool notifications_enabled;
-  const size_t num_entries;
-  base::Time sync_start_time;
-  const bool retry_scheduled;
+  SyncerStatus syncer_status() const;
+  ErrorCounters errors() const;
+  int64 num_server_changes_remaining() const;
+  bool is_share_usable() const;
+  syncable::ModelTypeSet initial_sync_ended() const;
+  syncable::ModelTypePayloadMap download_progress_markers() const;
+  bool has_more_to_sync() const;
+  bool is_silenced() const;
+  int64 unsynced_count() const;
+  int num_encryption_conflicts() const;
+  int num_hierarchy_conflicts() const;
+  int num_simple_conflicts() const;
+  int num_server_conflicts() const;
+  bool did_commit_items() const;
+  SyncSourceInfo source() const;
+  bool notifications_enabled() const;
+  size_t num_entries() const;
+  base::Time sync_start_time() const;
+  bool retry_scheduled() const;
+
+ private:
+  SyncerStatus syncer_status_;
+  ErrorCounters errors_;
+  int64 num_server_changes_remaining_;
+  bool is_share_usable_;
+  syncable::ModelTypeSet initial_sync_ended_;
+  syncable::ModelTypePayloadMap download_progress_markers_;
+  bool has_more_to_sync_;
+  bool is_silenced_;
+  int64 unsynced_count_;
+  int num_encryption_conflicts_;
+  int num_hierarchy_conflicts_;
+  int num_simple_conflicts_;
+  int num_server_conflicts_;
+  bool did_commit_items_;
+  SyncSourceInfo source_;
+  bool notifications_enabled_;
+  size_t num_entries_;
+  base::Time sync_start_time_;
+  bool retry_scheduled_;
 };
 
 // Tracks progress of conflicts and their resolutions.
diff --git a/sync/sessions/session_state_unittest.cc b/sync/sessions/session_state_unittest.cc
index a37dc17..011f832 100644
--- a/sync/sessions/session_state_unittest.cc
+++ b/sync/sessions/session_state_unittest.cc
@@ -72,29 +72,6 @@ TEST_F(SessionStateTest, SyncerStatusToValue) {
                          *value, "numServerOverwrites");
 }
 
-TEST_F(SessionStateTest, DownloadProgressMarkersToValue) {
-  std::string download_progress_markers[syncable::MODEL_TYPE_COUNT];
-  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-       i < syncable::MODEL_TYPE_COUNT; ++i) {
-    std::string marker(i, i);
-    download_progress_markers[i] = marker;
-  }
-
-  scoped_ptr<DictionaryValue> value(
-      DownloadProgressMarkersToValue(download_progress_markers));
-  EXPECT_EQ(syncable::MODEL_TYPE_COUNT - syncable::FIRST_REAL_MODEL_TYPE,
-            static_cast<int>(value->size()));
-  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-       i < syncable::MODEL_TYPE_COUNT; ++i) {
-    syncable::ModelType model_type = syncable::ModelTypeFromInt(i);
-    std::string marker(i, i);
-    std::string expected_value;
-    EXPECT_TRUE(base::Base64Encode(marker, &expected_value));
-    ExpectDictStringValue(expected_value,
-                          *value, syncable::ModelTypeToString(model_type));
-  }
-}
-
 TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
   SyncerStatus syncer_status;
   syncer_status.num_successful_commits = 500;
@@ -111,11 +88,11 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
   scoped_ptr<ListValue> expected_initial_sync_ended_value(
       syncable::ModelTypeSetToValue(initial_sync_ended));
 
-  std::string download_progress_markers[syncable::MODEL_TYPE_COUNT];
+  syncable::ModelTypePayloadMap download_progress_markers;
   download_progress_markers[syncable::BOOKMARKS] = "test";
   download_progress_markers[syncable::APPS] = "apps";
   scoped_ptr<DictionaryValue> expected_download_progress_markers_value(
-      DownloadProgressMarkersToValue(download_progress_markers));
+      syncable::ModelTypePayloadMapToValue(download_progress_markers));
 
   const bool kHasMoreToSync = false;
   const bool kIsSilenced = true;
diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc
index 0d5835e..4c58215 100644
--- a/sync/sessions/sync_session.cc
+++ b/sync/sessions/sync_session.cc
@@ -137,7 +137,7 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
 
   bool is_share_useable = true;
   syncable::ModelTypeSet initial_sync_ended;
-  std::string download_progress_markers[syncable::MODEL_TYPE_COUNT];
+  syncable::ModelTypePayloadMap download_progress_markers;
   for (int i = syncable::FIRST_REAL_MODEL_TYPE;
        i < syncable::MODEL_TYPE_COUNT; ++i) {
     syncable::ModelType type(syncable::ModelTypeFromInt(i));
@@ -147,7 +147,7 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
       else
         is_share_useable = false;
     }
-    dir->GetDownloadProgressAsString(type, &download_progress_markers[i]);
+    dir->GetDownloadProgressAsString(type, &download_progress_markers[type]);
   }
 
   return SyncSessionSnapshot(
@@ -174,10 +174,9 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
 
 void SyncSession::SendEventNotification(SyncEngineEvent::EventCause cause) {
   SyncEngineEvent event(cause);
-  const SyncSessionSnapshot& snapshot = TakeSnapshot();
-  event.snapshot = &snapshot;
+  event.snapshot = TakeSnapshot();
 
-  DVLOG(1) << "Sending event with snapshot: " << snapshot.ToString();
+  DVLOG(1) << "Sending event with snapshot: " << event.snapshot.ToString();
   context()->NotifyListeners(event);
 }
 
diff --git a/sync/sessions/sync_session_context.cc b/sync/sessions/sync_session_context.cc
index 2ebe655..dbbaaf91 100644
--- a/sync/sessions/sync_session_context.cc
+++ b/sync/sessions/sync_session_context.cc
@@ -5,7 +5,6 @@
 #include "sync/sessions/sync_session_context.h"
 
 #include "sync/sessions/debug_info_getter.h"
-#include "sync/sessions/session_state.h"
 #include "sync/util/extensions_activity_monitor.h"
 
 namespace browser_sync {
diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h
index 5c21726..117afc4 100644
--- a/sync/sessions/sync_session_context.h
+++ b/sync/sessions/sync_session_context.h
@@ -27,6 +27,7 @@
 #include "base/time.h"
 #include "sync/engine/model_safe_worker.h"
 #include "sync/engine/syncer_types.h"
+#include "sync/engine/sync_engine_event.h"
 #include "sync/engine/traffic_recorder.h"
 #include "sync/sessions/debug_info_getter.h"
 
@@ -46,7 +47,6 @@ static const int kDefaultMaxCommitBatchSize = 25;
 
 namespace sessions {
 class ScopedSessionContextConflictResolver;
-struct SyncSessionSnapshot;
 class TestScopedSessionEventListener;
 
 class SyncSessionContext {
@@ -172,9 +172,6 @@ class SyncSessionContext {
   // by the user.
   ModelSafeRoutingInfo previous_session_routing_info_;
 
-  // Cache of last session snapshot information.
-  scoped_ptr<sessions::SyncSessionSnapshot> previous_session_snapshot_;
-
   // We use this to get debug info to send to the server for debugging
   // client behavior on server side.
   DebugInfoGetter* const debug_info_getter_;