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, 33 insertions, 40 deletions

diff --git a/sync/engine/all_status.cc b/sync/engine/all_status.cc
index 6a642db..e9e1a20 100644
--- a/sync/engine/all_status.cc
+++ b/sync/engine/all_status.cc
@@ -47,7 +47,8 @@ csync::SyncStatus AllStatus::CalcSyncing(
   status.hierarchy_conflicts = snapshot.num_hierarchy_conflicts();
   status.simple_conflicts = snapshot.num_simple_conflicts();
   status.server_conflicts = snapshot.num_server_conflicts();
-  status.committed_count = snapshot.syncer_status().num_successful_commits;
+  status.committed_count =
+      snapshot.model_neutral_state().num_successful_commits;
 
   if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_BEGIN) {
     status.syncing = true;
@@ -58,36 +59,37 @@ csync::SyncStatus AllStatus::CalcSyncing(
   status.initial_sync_ended |= snapshot.is_share_usable();
 
   status.updates_available += snapshot.num_server_changes_remaining();
-  status.sync_protocol_error = snapshot.errors().sync_protocol_error;
+  status.sync_protocol_error =
+      snapshot.model_neutral_state().sync_protocol_error;
 
   // Accumulate update count only once per session to avoid double-counting.
   // TODO(ncarter): Make this realtime by having the syncer_status
   // counter preserve its value across sessions.  http://crbug.com/26339
   if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_ENDED) {
     status.updates_received +=
-        snapshot.syncer_status().num_updates_downloaded_total;
+        snapshot.model_neutral_state().num_updates_downloaded_total;
     status.tombstone_updates_received +=
-        snapshot.syncer_status().num_tombstone_updates_downloaded_total;
+        snapshot.model_neutral_state().num_tombstone_updates_downloaded_total;
     status.reflected_updates_received +=
-        snapshot.syncer_status().num_reflected_updates_downloaded_total;
+        snapshot.model_neutral_state().num_reflected_updates_downloaded_total;
     status.num_commits_total +=
-        snapshot.syncer_status().num_successful_commits;
+        snapshot.model_neutral_state().num_successful_commits;
     status.num_local_overwrites_total +=
-        snapshot.syncer_status().num_local_overwrites;
+        snapshot.model_neutral_state().num_local_overwrites;
     status.num_server_overwrites_total +=
-        snapshot.syncer_status().num_server_overwrites;
-    if (snapshot.syncer_status().num_updates_downloaded_total == 0) {
+        snapshot.model_neutral_state().num_server_overwrites;
+    if (snapshot.model_neutral_state().num_updates_downloaded_total == 0) {
       ++status.empty_get_updates;
     } else {
       ++status.nonempty_get_updates;
     }
-    if (snapshot.syncer_status().num_successful_commits == 0) {
+    if (snapshot.model_neutral_state().num_successful_commits == 0) {
       ++status.sync_cycles_without_commits;
     } else {
       ++status.sync_cycles_with_commits;
     }
-    if (snapshot.syncer_status().num_successful_commits == 0 &&
-        snapshot.syncer_status().num_updates_downloaded_total == 0) {
+    if (snapshot.model_neutral_state().num_successful_commits == 0 &&
+        snapshot.model_neutral_state().num_updates_downloaded_total == 0) {
       ++status.useless_sync_cycles;
     } else {
       ++status.useful_sync_cycles;
@@ -109,7 +111,8 @@ void AllStatus::OnSyncEngineEvent(const SyncEngineEvent& event) {
        break;
     case SyncEngineEvent::ACTIONABLE_ERROR:
       status_ = CreateBlankStatus();
-      status_.sync_protocol_error = event.snapshot.errors().sync_protocol_error;
+      status_.sync_protocol_error =
+          event.snapshot.model_neutral_state().sync_protocol_error;
       break;
     default:
       LOG(ERROR) << "Unrecognized Syncer Event: " << event.what_happened;
diff --git a/sync/engine/commit.cc b/sync/engine/commit.cc
index 7cff6cb..f9a56e9 100644
--- a/sync/engine/commit.cc
+++ b/sync/engine/commit.cc
@@ -134,6 +134,7 @@ SyncerError BuildAndPostCommitsImpl(Syncer* syncer,
     if (processing_result != SYNCER_OK) {
       return processing_result;
     }
+    session->SendEventNotification(SyncEngineEvent::STATUS_CHANGED);
   }
 
   return SYNCER_OK;
diff --git a/sync/engine/process_commit_response_command.cc b/sync/engine/process_commit_response_command.cc
index 2a01a30..b315062 100644
--- a/sync/engine/process_commit_response_command.cc
+++ b/sync/engine/process_commit_response_command.cc
@@ -87,8 +87,8 @@ SyncerError ProcessCommitResponseCommand::ModelChangingExecuteImpl(
   // This is to be run on one model only: the bookmark model.
   if (session->status_controller().HasBookmarkCommitActivity()) {
     // If the commit failed, return the data to the ExtensionsActivityMonitor.
-    if (session->status_controller().syncer_status()
-          .num_successful_bookmark_commits == 0) {
+    if (session->status_controller().
+        model_neutral_state().num_successful_bookmark_commits == 0) {
       monitor->PutRecords(session->extensions_activity());
     }
     // Clear our cached data in either case.
diff --git a/sync/engine/sync_scheduler.cc b/sync/engine/sync_scheduler.cc
index 199c507..1d1ddad 100644
--- a/sync/engine/sync_scheduler.cc
+++ b/sync/engine/sync_scheduler.cc
@@ -1114,11 +1114,12 @@ void SyncScheduler::OnActionableError(
 void SyncScheduler::OnSyncProtocolError(
     const sessions::SyncSessionSnapshot& snapshot) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  if (ShouldRequestEarlyExit(snapshot.errors().sync_protocol_error)) {
+  if (ShouldRequestEarlyExit(
+          snapshot.model_neutral_state().sync_protocol_error)) {
     SDVLOG(2) << "Sync Scheduler requesting early exit.";
     syncer_->RequestEarlyExit();  // Thread-safe.
   }
-  if (IsActionableError(snapshot.errors().sync_protocol_error))
+  if (IsActionableError(snapshot.model_neutral_state().sync_protocol_error))
     OnActionableError(snapshot);
 }
 
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index 8f87cda..8d3063d 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -149,6 +149,7 @@ void Syncer::SyncShare(sessions::SyncSession* session,
       case STORE_TIMESTAMPS: {
         StoreTimestampsCommand store_timestamps;
         store_timestamps.Execute(session);
+        session->SendEventNotification(SyncEngineEvent::STATUS_CHANGED);
         // We download all of the updates before attempting to apply them.
         if (!session->status_controller().download_updates_succeeded()) {
           // We may have downloaded some updates, but if the latest download
@@ -168,6 +169,7 @@ void Syncer::SyncShare(sessions::SyncSession* session,
       case APPLY_UPDATES: {
         ApplyUpdatesCommand apply_updates;
         apply_updates.Execute(session);
+        session->SendEventNotification(SyncEngineEvent::STATUS_CHANGED);
         if (last_step == APPLY_UPDATES) {
           // We're in configuration mode, but we still need to run the
           // SYNCER_END step.
diff --git a/sync/engine/syncer_command.cc b/sync/engine/syncer_command.cc
index cac1fc5..bf7bf96 100644
--- a/sync/engine/syncer_command.cc
+++ b/sync/engine/syncer_command.cc
@@ -4,27 +4,14 @@
 
 #include "sync/engine/syncer_command.h"
 
-#include "sync/engine/net/server_connection_manager.h"
-#include "sync/sessions/sync_session.h"
-
 namespace csync {
-using sessions::SyncSession;
 
 SyncerCommand::SyncerCommand() {}
 SyncerCommand::~SyncerCommand() {}
 
-SyncerError SyncerCommand::Execute(SyncSession* session) {
+SyncerError SyncerCommand::Execute(sessions::SyncSession* session) {
   SyncerError result = ExecuteImpl(session);
-  SendNotifications(session);
   return result;
 }
 
-void SyncerCommand::SendNotifications(SyncSession* session) {
-  if (session->mutable_status_controller()->TestAndClearIsDirty()) {
-    SyncEngineEvent event(SyncEngineEvent::STATUS_CHANGED);
-    event.snapshot = session->TakeSnapshot();
-    session->context()->NotifyListeners(event);
-  }
-}
-
 }  // namespace csync
diff --git a/sync/engine/syncer_command.h b/sync/engine/syncer_command.h
index bb68161..710b3ac 100644
--- a/sync/engine/syncer_command.h
+++ b/sync/engine/syncer_command.h
@@ -39,7 +39,6 @@ class SyncerCommand {
   // ExecuteImpl is where derived classes actually do work.
   virtual SyncerError ExecuteImpl(sessions::SyncSession* session) = 0;
  private:
-  void SendNotifications(sessions::SyncSession* session);
   DISALLOW_COPY_AND_ASSIGN(SyncerCommand);
 };
 
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index aee0d1e..764161e 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -795,7 +795,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) {
   {
     const StatusController& status_controller = session_->status_controller();
     // Expect success.
-    EXPECT_EQ(status_controller.error().commit_result, SYNCER_OK);
+    EXPECT_EQ(status_controller.model_neutral_state().commit_result, SYNCER_OK);
     // None should be unsynced anymore.
     syncable::ReadTransaction rtrans(FROM_HERE, directory());
     VERIFY_ENTRY(1, false, false, false, 0, 21, 21, ids_, &rtrans);
@@ -953,18 +953,18 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   }
   // First cycle resolves conflicts, second cycle commits changes.
   SyncShareNudge();
-  EXPECT_EQ(2, status().syncer_status().num_server_overwrites);
-  EXPECT_EQ(1, status().syncer_status().num_local_overwrites);
+  EXPECT_EQ(2, status().model_neutral_state().num_server_overwrites);
+  EXPECT_EQ(1, status().model_neutral_state().num_local_overwrites);
   // We successfully commited item(s).
-  EXPECT_EQ(status().error().commit_result, SYNCER_OK);
+  EXPECT_EQ(status().model_neutral_state().commit_result, SYNCER_OK);
   SyncShareNudge();
 
   // Everything should be resolved now. The local changes should have
   // overwritten the server changes for 2 and 4, while the server changes
   // overwrote the local for entry 3.
-  EXPECT_EQ(0, status().syncer_status().num_server_overwrites);
-  EXPECT_EQ(0, status().syncer_status().num_local_overwrites);
-  EXPECT_EQ(status().error().commit_result, SYNCER_OK);
+  EXPECT_EQ(0, status().model_neutral_state().num_server_overwrites);
+  EXPECT_EQ(0, status().model_neutral_state().num_local_overwrites);
+  EXPECT_EQ(status().model_neutral_state().commit_result, SYNCER_OK);
   syncable::ReadTransaction rtrans(FROM_HERE, directory());
   VERIFY_ENTRY(1, false, false, false, 0, 41, 41, ids_, &rtrans);
   VERIFY_ENTRY(2, false, false, false, 1, 31, 31, ids_, &rtrans);
@@ -2731,7 +2731,7 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_PostBufferFail) {
   EXPECT_EQ(1U, mock_server_->commit_messages().size());
   EXPECT_FALSE(session_->Succeeded());
   EXPECT_EQ(SYNC_SERVER_ERROR,
-            session_->status_controller().error().commit_result);
+            session_->status_controller().model_neutral_state().commit_result);
   EXPECT_EQ(items_to_commit - kDefaultMaxCommitBatchSize,
             directory()->unsynced_entity_count());
 }