sync: Follow-up to conflict resolution refactor

This is part two of r164286.  That commit refactored the way we handle
conflict resolution.  This commit takes advantage of those changes to
delete lots of code.

Because this change deletes session_state.cc, I decided to move the two
remaining useful tests session_state_unittest.cc into their own files,
sync_session_snapshot_unittest.cc and sync_source_info_unittest.cc.  The
tests were not modified in any way.

None of these changes should have any effect on syncer behaviour:
- We can remove the simple conflict counters and related code, since we
  now assert that all conflicts will be resolved by the end of a
  successful sync cycle.
- We can remove the PerModelSafeGroupState because that struct no longer
  has any members.
- The 'conflicts_resolved' indicators are no longer set, so we can
  remove them.
- Without those indicators, it's no longer possible to have any
  SYNC_CYCLE_CONTINUATION sync cycles.  We can remove a few counters
  associated with them, as well as the 'has_more_to_sync' flag in the
  snapshot.
- Without SYNC_CYCLE_CONTINUATION cycles, there's no longer any need for
  code that loops around SyncShare() in SyncSchedulerImpl.  The
  SyncSession::PrepareForAnotherSyncCycle() function is no longer used,
  either.
- The ScopedConflictResolver installed on the session during a sync
  cycle is no longer used, so all the code related to it can be deleted.

BUG=147681,111280,156238


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

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

9 files changed, 5 insertions, 40 deletions

diff --git a/sync/engine/all_status.cc b/sync/engine/all_status.cc
index 82457e4..9f5de50 100644
--- a/sync/engine/all_status.cc
+++ b/sync/engine/all_status.cc
@@ -10,7 +10,6 @@
 #include "base/port.h"
 #include "sync/engine/net/server_connection_manager.h"
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/sessions/session_state.h"
 
 namespace syncer {
 
@@ -31,7 +30,6 @@ SyncStatus AllStatus::CreateBlankStatus() const {
   SyncStatus status = status_;
   status.encryption_conflicts = 0;
   status.hierarchy_conflicts = 0;
-  status.simple_conflicts = 0;
   status.server_conflicts = 0;
   status.committed_count = 0;
   status.initial_sync_ended = false;
@@ -44,7 +42,6 @@ SyncStatus AllStatus::CalcSyncing(const SyncEngineEvent &event) const {
   const sessions::SyncSessionSnapshot& snapshot = event.snapshot;
   status.encryption_conflicts = snapshot.num_encryption_conflicts();
   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.model_neutral_state().num_successful_commits;
@@ -62,8 +59,6 @@ SyncStatus AllStatus::CalcSyncing(const SyncEngineEvent &event) const {
       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.model_neutral_state().num_updates_downloaded_total;
@@ -186,9 +181,6 @@ void AllStatus::IncrementNudgeCounter(NudgeSource source) {
     case NUDGE_SOURCE_NOTIFICATION:
       status_.nudge_source_notification++;
       return;
-    case NUDGE_SOURCE_CONTINUATION:
-      status_.nudge_source_continuation++;
-      return;
     case NUDGE_SOURCE_UNKNOWN:
       break;
   }
diff --git a/sync/engine/nudge_source.cc b/sync/engine/nudge_source.cc
index cd01dda..62a2409 100644
--- a/sync/engine/nudge_source.cc
+++ b/sync/engine/nudge_source.cc
@@ -15,7 +15,6 @@ const char* GetNudgeSourceString(NudgeSource nudge_source) {
     ENUM_CASE(NUDGE_SOURCE_UNKNOWN);
     ENUM_CASE(NUDGE_SOURCE_NOTIFICATION);
     ENUM_CASE(NUDGE_SOURCE_LOCAL);
-    ENUM_CASE(NUDGE_SOURCE_CONTINUATION);
     ENUM_CASE(NUDGE_SOURCE_LOCAL_REFRESH);
   };
   NOTREACHED();
diff --git a/sync/engine/nudge_source.h b/sync/engine/nudge_source.h
index 366f6d1..d60b515 100644
--- a/sync/engine/nudge_source.h
+++ b/sync/engine/nudge_source.h
@@ -13,8 +13,6 @@ enum NudgeSource {
   NUDGE_SOURCE_NOTIFICATION,
   // A local change occurred (e.g. bookmark moved).
   NUDGE_SOURCE_LOCAL,
-  // A previous sync cycle did not fully complete (e.g. HTTP error).
-  NUDGE_SOURCE_CONTINUATION,
   // A local event is triggering an optimistic datatype refresh.
   NUDGE_SOURCE_LOCAL_REFRESH,
 };
diff --git a/sync/engine/process_updates_command_unittest.cc b/sync/engine/process_updates_command_unittest.cc
index fc543fb..f4cc583 100644
--- a/sync/engine/process_updates_command_unittest.cc
+++ b/sync/engine/process_updates_command_unittest.cc
@@ -5,7 +5,6 @@
 #include "base/basictypes.h"
 #include "sync/engine/process_updates_command.h"
 #include "sync/internal_api/public/base/model_type.h"
-#include "sync/sessions/session_state.h"
 #include "sync/sessions/sync_session.h"
 #include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/syncable_id.h"
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index c270fc5..aeb95c4 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -112,8 +112,6 @@ GetUpdatesCallerInfo::GetUpdatesSource GetUpdatesFromNudgeSource(
       return GetUpdatesCallerInfo::NOTIFICATION;
     case NUDGE_SOURCE_LOCAL:
       return GetUpdatesCallerInfo::LOCAL;
-    case NUDGE_SOURCE_CONTINUATION:
-      return GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION;
     case NUDGE_SOURCE_LOCAL_REFRESH:
       return GetUpdatesCallerInfo::DATATYPE_REFRESH;
     case NUDGE_SOURCE_UNKNOWN:
@@ -763,23 +761,12 @@ bool SyncSchedulerImpl::DoSyncSessionJob(scoped_ptr<SyncSessionJob> job) {
     return false;
   }
 
-  SDVLOG(2) << "DoSyncSessionJob with "
+  SDVLOG(2) << "Calling SyncShare with "
             << SyncSessionJob::GetPurposeString(job->purpose()) << " job";
-
-  bool has_more_to_sync = true;
-  bool premature_exit = false;
-  while (DecideOnJob(*job) == CONTINUE && has_more_to_sync) {
-    SDVLOG(2) << "Calling SyncShare.";
-    // Synchronously perform the sync session from this thread.
-    premature_exit = !syncer_->SyncShare(job->mutable_session(),
-                                         job->start_step(),
-                                         job->end_step());
-
-    has_more_to_sync = job->session()->HasMoreToSync();
-    if (has_more_to_sync)
-      job->mutable_session()->PrepareForAnotherSyncCycle();
-  }
-  SDVLOG(2) << "Done SyncShare looping.";
+  bool premature_exit = !syncer_->SyncShare(job->mutable_session(),
+                                            job->start_step(),
+                                            job->end_step());
+  SDVLOG(2) << "Done SyncShare, returned: " << premature_exit;
 
   return FinishSyncSessionJob(job.Pass(), premature_exit);
 }
@@ -838,7 +825,6 @@ bool SyncSchedulerImpl::FinishSyncSessionJob(scoped_ptr<SyncSessionJob> job,
 void SyncSchedulerImpl::ScheduleNextSync(
     scoped_ptr<SyncSessionJob> finished_job, bool succeeded) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  DCHECK(!finished_job->session()->HasMoreToSync());
 
   AdjustPolling(finished_job.get());
 
diff --git a/sync/engine/sync_session_job.cc b/sync/engine/sync_session_job.cc
index 7dc5661..3f13d98 100644
--- a/sync/engine/sync_session_job.cc
+++ b/sync/engine/sync_session_job.cc
@@ -50,8 +50,6 @@ bool SyncSessionJob::Finish(bool early_exit) {
   if (early_exit)
     return false;
 
-  DCHECK(!session_->HasMoreToSync());
-
   // Did we hit any errors along the way?
   if (sessions::HasSyncerError(
       session_->status_controller().model_neutral_state())) {
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index 2b38501..22010ee 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -36,7 +36,6 @@ using sync_pb::ClientCommand;
 
 namespace syncer {
 
-using sessions::ScopedSessionContextConflictResolver;
 using sessions::StatusController;
 using sessions::SyncSession;
 using syncable::IS_UNAPPLIED_UPDATE;
@@ -86,8 +85,6 @@ void Syncer::RequestEarlyExit() {
 bool Syncer::SyncShare(sessions::SyncSession* session,
                        SyncerStep first_step,
                        SyncerStep last_step) {
-  ScopedSessionContextConflictResolver scoped(session->context(),
-                                              &resolver_);
   session->mutable_status_controller()->UpdateStartTime();
   SyncerStep current_step = first_step;
 
diff --git a/sync/engine/syncer.h b/sync/engine/syncer.h
index 80dfb51..4bcac92 100644
--- a/sync/engine/syncer.h
+++ b/sync/engine/syncer.h
@@ -67,8 +67,6 @@ class Syncer {
   bool early_exit_requested_;
   base::Lock early_exit_requested_lock_;
 
-  ConflictResolver resolver_;
-
   friend class SyncerTest;
   FRIEND_TEST_ALL_PREFIXES(SyncerTest, NameClashWithResolver);
   FRIEND_TEST_ALL_PREFIXES(SyncerTest, IllegalAndLegalUpdates);
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 54d0eec..d92cc0e 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -232,7 +232,6 @@ class SyncerTest : public testing::Test,
             listeners, NULL, &traffic_recorder_,
             true  /* enable keystore encryption */));
     context_->set_routing_info(routing_info);
-    ASSERT_FALSE(context_->resolver());
     syncer_ = new Syncer();
     session_.reset(MakeSession());
 
@@ -2568,7 +2567,6 @@ TEST_F(SyncerTest, CommitManyItemsInOneGo_CommitConflict) {
 
   // We should stop looping at the first sign of trouble.
   EXPECT_EQ(1U, mock_server_->commit_messages().size());
-  EXPECT_FALSE(session_->HasMoreToSync());
   EXPECT_EQ(items_to_commit - (kDefaultMaxCommitBatchSize - 1),
             directory()->unsynced_entity_count());
 }