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

13 files changed, 12 insertions, 437 deletions

diff --git a/sync/sessions/session_state.cc b/sync/sessions/session_state.cc
deleted file mode 100644
index a08b093..0000000
--- a/sync/sessions/session_state.cc
+++ /dev/null
@@ -1,26 +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/sessions/session_state.h"
-
-#include <set>
-#include <vector>
-
-#include "base/memory/scoped_ptr.h"
-#include "base/values.h"
-
-using std::set;
-using std::vector;
-
-namespace syncer {
-namespace sessions {
-
-PerModelSafeGroupState::PerModelSafeGroupState() {
-}
-
-PerModelSafeGroupState::~PerModelSafeGroupState() {
-}
-
-}  // namespace sessions
-}  // namespace syncer
diff --git a/sync/sessions/session_state.h b/sync/sessions/session_state.h
deleted file mode 100644
index 4ef6860..0000000
--- a/sync/sessions/session_state.h
+++ /dev/null
@@ -1,33 +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.
-
-// 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.
-
-#ifndef SYNC_SESSIONS_SESSION_STATE_H_
-#define SYNC_SESSIONS_SESSION_STATE_H_
-
-#include <set>
-
-#include "sync/syncable/syncable_id.h"
-
-namespace syncer {
-namespace sessions {
-
-// Grouping of all state that applies to a single ModelSafeGroup.
-struct PerModelSafeGroupState {
-  explicit PerModelSafeGroupState();
-  ~PerModelSafeGroupState();
-
-  std::set<syncable::Id> simple_conflict_ids;
-};
-
-}  // namespace sessions
-}  // namespace syncer
-
-#endif  // SYNC_SESSIONS_SESSION_STATE_H_
diff --git a/sync/sessions/session_state_unittest.cc b/sync/sessions/session_state_unittest.cc
deleted file mode 100644
index 92d4b6f..0000000
--- a/sync/sessions/session_state_unittest.cc
+++ /dev/null
@@ -1,133 +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/sessions/session_state.h"
-
-#include <string>
-
-#include "base/base64.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/test/values_test_util.h"
-#include "base/time.h"
-#include "base/values.h"
-#include "sync/internal_api/public/sessions/sync_session_snapshot.h"
-#include "sync/internal_api/public/sessions/sync_source_info.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace syncer {
-namespace sessions {
-namespace {
-
-using base::ExpectDictBooleanValue;
-using base::ExpectDictDictionaryValue;
-using base::ExpectDictIntegerValue;
-using base::ExpectDictListValue;
-using base::ExpectDictStringValue;
-
-class SessionStateTest : public testing::Test {};
-
-TEST_F(SessionStateTest, SyncSourceInfoToValue) {
-  sync_pb::GetUpdatesCallerInfo::GetUpdatesSource updates_source =
-      sync_pb::GetUpdatesCallerInfo::PERIODIC;
-  ModelTypeInvalidationMap types;
-  types[PREFERENCES].payload = "preferencespayload";
-  types[EXTENSIONS].payload = "";
-  scoped_ptr<DictionaryValue> expected_types_value(
-      ModelTypeInvalidationMapToValue(types));
-
-  SyncSourceInfo source_info(updates_source, types);
-
-  scoped_ptr<DictionaryValue> value(source_info.ToValue());
-  EXPECT_EQ(2u, value->size());
-  ExpectDictStringValue("PERIODIC", *value, "updatesSource");
-  ExpectDictDictionaryValue(*expected_types_value, *value, "types");
-}
-
-TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
-  ModelNeutralState model_neutral;
-  model_neutral.num_server_changes_remaining = 105;
-  model_neutral.num_successful_commits = 5;
-  model_neutral.num_successful_bookmark_commits = 10;
-  model_neutral.num_updates_downloaded_total = 100;
-  model_neutral.num_tombstone_updates_downloaded_total = 200;
-  model_neutral.num_reflected_updates_downloaded_total = 50;
-  model_neutral.num_local_overwrites = 15;
-  model_neutral.num_server_overwrites = 18;
-
-  const bool kIsShareUsable = true;
-
-  const ModelTypeSet initial_sync_ended(BOOKMARKS, PREFERENCES);
-  scoped_ptr<ListValue> expected_initial_sync_ended_value(
-      ModelTypeSetToValue(initial_sync_ended));
-
-  ProgressMarkerMap download_progress_markers;
-  download_progress_markers[BOOKMARKS] = "test";
-  download_progress_markers[APPS] = "apps";
-  scoped_ptr<DictionaryValue> expected_download_progress_markers_value(
-      ProgressMarkerMapToValue(download_progress_markers));
-
-  const bool kHasMoreToSync = false;
-  const bool kIsSilenced = true;
-  const int kNumEncryptionConflicts = 1054;
-  const int kNumHierarchyConflicts = 1055;
-  const int kNumSimpleConflicts = 1056;
-  const int kNumServerConflicts = 1057;
-
-  SyncSourceInfo source;
-  scoped_ptr<DictionaryValue> expected_source_value(source.ToValue());
-
-  SyncSessionSnapshot snapshot(model_neutral,
-                               kIsShareUsable,
-                               initial_sync_ended,
-                               download_progress_markers,
-                               kHasMoreToSync,
-                               kIsSilenced,
-                               kNumEncryptionConflicts,
-                               kNumHierarchyConflicts,
-                               kNumSimpleConflicts,
-                               kNumServerConflicts,
-                               source,
-                               false,
-                               0,
-                               base::Time::Now());
-  scoped_ptr<DictionaryValue> value(snapshot.ToValue());
-  EXPECT_EQ(20u, value->size());
-  ExpectDictIntegerValue(model_neutral.num_successful_commits,
-                         *value, "numSuccessfulCommits");
-  ExpectDictIntegerValue(model_neutral.num_successful_bookmark_commits,
-                         *value, "numSuccessfulBookmarkCommits");
-  ExpectDictIntegerValue(model_neutral.num_updates_downloaded_total,
-                         *value, "numUpdatesDownloadedTotal");
-  ExpectDictIntegerValue(model_neutral.num_tombstone_updates_downloaded_total,
-                         *value, "numTombstoneUpdatesDownloadedTotal");
-  ExpectDictIntegerValue(model_neutral.num_reflected_updates_downloaded_total,
-                         *value, "numReflectedUpdatesDownloadedTotal");
-  ExpectDictIntegerValue(model_neutral.num_local_overwrites,
-                         *value, "numLocalOverwrites");
-  ExpectDictIntegerValue(model_neutral.num_server_overwrites,
-                         *value, "numServerOverwrites");
-  ExpectDictIntegerValue(model_neutral.num_server_changes_remaining,
-                         *value, "numServerChangesRemaining");
-  ExpectDictBooleanValue(kIsShareUsable, *value, "isShareUsable");
-  ExpectDictListValue(*expected_initial_sync_ended_value, *value,
-                      "initialSyncEnded");
-  ExpectDictDictionaryValue(*expected_download_progress_markers_value,
-                            *value, "downloadProgressMarkers");
-  ExpectDictBooleanValue(kHasMoreToSync, *value, "hasMoreToSync");
-  ExpectDictBooleanValue(kIsSilenced, *value, "isSilenced");
-  ExpectDictIntegerValue(kNumEncryptionConflicts, *value,
-                         "numEncryptionConflicts");
-  ExpectDictIntegerValue(kNumHierarchyConflicts, *value,
-                         "numHierarchyConflicts");
-  ExpectDictIntegerValue(kNumSimpleConflicts, *value,
-                         "numSimpleConflicts");
-  ExpectDictIntegerValue(kNumServerConflicts, *value,
-                         "numServerConflicts");
-  ExpectDictDictionaryValue(*expected_source_value, *value, "source");
-  ExpectDictBooleanValue(false, *value, "notificationsEnabled");
-}
-
-}  // namespace
-}  // namespace sessions
-}  // namespace syncer
diff --git a/sync/sessions/status_controller.cc b/sync/sessions/status_controller.cc
index 810a8c5..2287a51 100644
--- a/sync/sessions/status_controller.cc
+++ b/sync/sessions/status_controller.cc
@@ -14,52 +14,13 @@ namespace syncer {
 namespace sessions {
 
 StatusController::StatusController(const ModelSafeRoutingInfo& routes)
-    : per_model_group_deleter_(&per_model_group_),
-      group_restriction_in_effect_(false),
+    : group_restriction_in_effect_(false),
       group_restriction_(GROUP_PASSIVE),
       routing_info_(routes) {
 }
 
 StatusController::~StatusController() {}
 
-const std::set<syncable::Id>* StatusController::simple_conflict_ids() const {
-  const PerModelSafeGroupState* state =
-      GetModelSafeGroupState(true, group_restriction_);
-  return state ? &state->simple_conflict_ids : NULL;
-}
-
-std::set<syncable::Id>* StatusController::mutable_simple_conflict_ids() {
-  return &GetOrCreateModelSafeGroupState(
-      true, group_restriction_)->simple_conflict_ids;
-}
-
-const std::set<syncable::Id>*
-    StatusController::GetUnrestrictedSimpleConflictIds(
-        ModelSafeGroup group) const {
-  const PerModelSafeGroupState* state = GetModelSafeGroupState(false, group);
-  return state ? &state->simple_conflict_ids : NULL;
-}
-
-const PerModelSafeGroupState* StatusController::GetModelSafeGroupState(
-    bool restrict, ModelSafeGroup group) const {
-  DCHECK_EQ(restrict, group_restriction_in_effect_);
-  std::map<ModelSafeGroup, PerModelSafeGroupState*>::const_iterator it =
-      per_model_group_.find(group);
-  return (it == per_model_group_.end()) ? NULL : it->second;
-}
-
-PerModelSafeGroupState* StatusController::GetOrCreateModelSafeGroupState(
-    bool restrict, ModelSafeGroup group) {
-  DCHECK_EQ(restrict, group_restriction_in_effect_);
-  std::map<ModelSafeGroup, PerModelSafeGroupState*>::iterator it =
-      per_model_group_.find(group);
-  if (it == per_model_group_.end()) {
-    PerModelSafeGroupState* state = new PerModelSafeGroupState();
-    it = per_model_group_.insert(std::make_pair(group, state)).first;
-  }
-  return it->second;
-}
-
 void StatusController::increment_num_updates_downloaded_by(int value) {
   model_neutral_.num_updates_downloaded_total += value;
 }
@@ -145,13 +106,6 @@ SyncerError StatusController::last_get_key_result() const {
   return model_neutral_.last_get_key_result;
 }
 
-void StatusController::update_conflicts_resolved(bool resolved) {
-  model_neutral_.conflicts_resolved |= resolved;
-}
-void StatusController::reset_conflicts_resolved() {
-  model_neutral_.conflicts_resolved = false;
-}
-
 // Returns the number of updates received from the sync server.
 int64 StatusController::CountUpdates() const {
   const sync_pb::ClientToServerResponse& updates =
@@ -163,10 +117,6 @@ int64 StatusController::CountUpdates() const {
   }
 }
 
-bool StatusController::HasConflictingUpdates() const {
-  return TotalNumConflictingItems() > 0;
-}
-
 int StatusController::num_updates_applied() const {
   return model_neutral_.num_updates_applied;
 }
@@ -185,18 +135,6 @@ int StatusController::num_hierarchy_conflicts() const {
   return model_neutral_.num_hierarchy_conflicts;
 }
 
-int StatusController::num_simple_conflicts() const {
-  DCHECK(!group_restriction_in_effect_)
-   << "num_simple_conflicts applies to all ModelSafeGroups";
-  std::map<ModelSafeGroup, PerModelSafeGroupState*>::const_iterator it =
-      per_model_group_.begin();
-  int sum = 0;
-  for (; it != per_model_group_.end(); ++it) {
-    sum += it->second->simple_conflict_ids.size();
-  }
-  return sum;
-}
-
 int StatusController::num_server_conflicts() const {
   DCHECK(!group_restriction_in_effect_)
       << "num_server_conflicts applies to all ModelSafeGroups";
@@ -207,7 +145,6 @@ int StatusController::TotalNumConflictingItems() const {
   DCHECK(!group_restriction_in_effect_)
       << "TotalNumConflictingItems applies to all ModelSafeGroups";
   int sum = 0;
-  sum += num_simple_conflicts();
   sum += num_encryption_conflicts();
   sum += num_hierarchy_conflicts();
   sum += num_server_conflicts();
diff --git a/sync/sessions/status_controller.h b/sync/sessions/status_controller.h
index 0717b6e..9e1e73c 100644
--- a/sync/sessions/status_controller.h
+++ b/sync/sessions/status_controller.h
@@ -3,20 +3,16 @@
 // found in the LICENSE file.
 
 // StatusController handles all counter and status related number crunching and
-// state tracking on behalf of a SyncSession.  It 'controls' the model data
-// defined in session_state.h.  The most important feature of StatusController
-// is the ScopedModelSafetyRestriction. When one of these is active, the
-// underlying data set exposed via accessors is swapped out to the appropriate
-// set for the restricted ModelSafeGroup behind the scenes.  For example, if
-// GROUP_UI is set, then accessors such as conflict_progress() and commit_ids()
-// are implicitly restricted to returning only data pertaining to GROUP_UI.
-// You can see which parts of status fall into this "restricted" category, or
-// the global "shared" category for all model types, by looking at the struct
-// declarations in session_state.h. If these accessors are invoked without a
-// restriction in place, this is a violation and will cause debug assertions
-// to surface improper use of the API in development.  Likewise for
-// invocation of "shared" accessors when a restriction is in place; for
-// safety's sake, an assertion will fire.
+// state tracking on behalf of a SyncSession.
+//
+// The most important feature of StatusController is the
+// ScopedModelSafeGroupRestriction.  Some of its functions expose per-thread
+// state, and can be called only when the restriction is in effect.  For
+// example, if GROUP_UI is set then the value returned from
+// commit_id_projection() will be useful for iterating over the commit IDs of
+// items that live on the UI thread.
+//
+// Other parts of its state are global, and do not require the restriction.
 //
 // NOTE: There is no concurrent access protection provided by this class. It
 // assumes one single thread is accessing this class for each unique
@@ -40,7 +36,6 @@
 #include "base/time.h"
 #include "sync/internal_api/public/sessions/model_neutral_state.h"
 #include "sync/sessions/ordered_commit_set.h"
-#include "sync/sessions/session_state.h"
 
 namespace syncer {
 namespace sessions {
@@ -50,14 +45,6 @@ class StatusController {
   explicit StatusController(const ModelSafeRoutingInfo& routes);
   ~StatusController();
 
-  // Progress counters.  All const methods may return NULL if the
-  // progress structure doesn't exist, but all non-const methods
-  // auto-create.
-  const std::set<syncable::Id>* simple_conflict_ids() const;
-  std::set<syncable::Id>* mutable_simple_conflict_ids();
-  const std::set<syncable::Id>* GetUnrestrictedSimpleConflictIds(
-      ModelSafeGroup group) const;
-
   // ClientToServer messages.
   const ModelTypeSet updates_request_types() const {
     return model_neutral_.updates_request_types;
@@ -84,23 +71,11 @@ class StatusController {
     return commit_set.GetCommitIdProjection(group_restriction_);
   }
 
-  // Control parameters for sync cycles.
-  bool conflicts_resolved() const {
-    return model_neutral_.conflicts_resolved;
-  }
-
-  // If a GetUpdates for any data type resulted in downloading an update that
-  // is in conflict, this method returns true.
-  // Note: this includes unresolvable conflicts.
-  bool HasConflictingUpdates() const;
-
   // Various conflict counters.
   int num_encryption_conflicts() const;
   int num_hierarchy_conflicts() const;
   int num_server_conflicts() const;
 
-  int num_simple_conflicts() const;
-
   // Aggregate sum of all conflicting items over all conflict types.
   int TotalNumConflictingItems() const;
 
@@ -160,10 +135,6 @@ class StatusController {
   void increment_num_local_overwrites();
   void increment_num_server_overwrites();
 
-  // TODO(rlarocque): Remove these after conflict resolution refactor.
-  void update_conflicts_resolved(bool resolved);
-  void reset_conflicts_resolved();
-
   // Commit counters.
   void increment_num_successful_commits();
   void increment_num_successful_bookmark_commits();
@@ -198,19 +169,7 @@ class StatusController {
     return group_restriction() == it->second;
   }
 
-  // Returns the state, if it exists, or NULL otherwise.
-  const PerModelSafeGroupState* GetModelSafeGroupState(
-      bool restrict, ModelSafeGroup group) const;
-
-  // Helper to lazily create objects for per-ModelSafeGroup state.
-  PerModelSafeGroupState* GetOrCreateModelSafeGroupState(
-      bool restrict, ModelSafeGroup group);
-
   ModelNeutralState model_neutral_;
-  std::map<ModelSafeGroup, PerModelSafeGroupState*> per_model_group_;
-
-  STLValueDeleter<std::map<ModelSafeGroup, PerModelSafeGroupState*> >
-      per_model_group_deleter_;
 
   // Used to fail read/write operations on state that don't obey the current
   // active ModelSafeWorker contract.
diff --git a/sync/sessions/status_controller_unittest.cc b/sync/sessions/status_controller_unittest.cc
index 95362e5..d361a6e 100644
--- a/sync/sessions/status_controller_unittest.cc
+++ b/sync/sessions/status_controller_unittest.cc
@@ -26,10 +26,6 @@ TEST_F(StatusControllerTest, ReadYourWrites) {
   status.set_num_server_changes_remaining(13);
   EXPECT_EQ(13, status.num_server_changes_remaining());
 
-  EXPECT_FALSE(status.conflicts_resolved());
-  status.update_conflicts_resolved(true);
-  EXPECT_TRUE(status.conflicts_resolved());
-
   status.set_last_download_updates_result(SYNCER_OK);
   EXPECT_EQ(SYNCER_OK,
             status.model_neutral_state().last_download_updates_result);
diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc
index 9be78d8..7ba93668 100644
--- a/sync/sessions/sync_session.cc
+++ b/sync/sessions/sync_session.cc
@@ -145,12 +145,6 @@ void SyncSession::RebaseRoutingInfoWithLatest(
   enabled_groups_ = ComputeEnabledGroups(routing_info_, workers_);
 }
 
-void SyncSession::PrepareForAnotherSyncCycle() {
-  source_.updates_source =
-      sync_pb::GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION;
-  status_controller_.reset(new StatusController(routing_info_));
-}
-
 SyncSessionSnapshot SyncSession::TakeSnapshot() const {
   syncable::Directory* dir = context_->directory();
 
@@ -173,11 +167,9 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
       is_share_useable,
       initial_sync_ended,
       download_progress_markers,
-      HasMoreToSync(),
       delegate_->IsSyncingCurrentlySilenced(),
       status_controller_->num_encryption_conflicts(),
       status_controller_->num_hierarchy_conflicts(),
-      status_controller_->num_simple_conflicts(),
       status_controller_->num_server_conflicts(),
       source_,
       context_->notifications_enabled(),
@@ -193,30 +185,10 @@ void SyncSession::SendEventNotification(SyncEngineEvent::EventCause cause) {
   context()->NotifyListeners(event);
 }
 
-bool SyncSession::HasMoreToSync() const {
-  const StatusController* status = status_controller_.get();
-  return status->conflicts_resolved();
-}
-
 const std::set<ModelSafeGroup>& SyncSession::GetEnabledGroups() const {
   return enabled_groups_;
 }
 
-// TODO(rlarocque): Delete this function after refactoring conflict resolution.
-std::set<ModelSafeGroup> SyncSession::GetEnabledGroupsWithConflicts() const {
-  const std::set<ModelSafeGroup>& enabled_groups = GetEnabledGroups();
-  std::set<ModelSafeGroup> enabled_groups_with_conflicts;
-  for (std::set<ModelSafeGroup>::const_iterator it =
-           enabled_groups.begin(); it != enabled_groups.end(); ++it) {
-    const std::set<syncable::Id>* ids =
-        status_controller_->GetUnrestrictedSimpleConflictIds(*it);
-    if (ids && ids->size() > 0) {
-      enabled_groups_with_conflicts.insert(*it);
-    }
-  }
-  return enabled_groups_with_conflicts;
-}
-
 bool SyncSession::DidReachServer() const {
   const ModelNeutralState& state = status_controller_->model_neutral_state();
   return state.last_get_key_result >= FIRST_SERVER_RETURN_VALUE ||
diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h
index 4026e88..19cba8c 100644
--- a/sync/sessions/sync_session.h
+++ b/sync/sessions/sync_session.h
@@ -27,7 +27,6 @@
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/sessions/sync_session_snapshot.h"
 #include "sync/sessions/ordered_commit_set.h"
-#include "sync/sessions/session_state.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/sessions/sync_session_context.h"
 #include "sync/util/extensions_activity_monitor.h"
@@ -108,10 +107,6 @@ class SyncSession {
   // Builds and sends a snapshot to the session context's listeners.
   void SendEventNotification(SyncEngineEvent::EventCause cause);
 
-  // Returns true if this session contains data that should go through the sync
-  // engine again.
-  bool HasMoreToSync() const;
-
   // Returns true if we reached the server. Note that "reaching the server"
   // here means that from an HTTP perspective, we succeeded (HTTP 200).  The
   // server **MAY** have returned a sync protocol error.
@@ -133,10 +128,6 @@ class SyncSession {
       const ModelSafeRoutingInfo& routing_info,
       const std::vector<ModelSafeWorker*>& workers);
 
-  // Should be called any time |this| is being re-used in a new call to
-  // SyncShare (e.g., HasMoreToSync returned true).
-  void PrepareForAnotherSyncCycle();
-
   // TODO(akalin): Split this into context() and mutable_context().
   SyncSessionContext* context() const { return context_; }
   Delegate* delegate() const { return delegate_; }
@@ -162,9 +153,6 @@ class SyncSession {
   // Returns the set of groups which have enabled types.
   const std::set<ModelSafeGroup>& GetEnabledGroups() const;
 
-  // Returns the set of enabled groups that have conflicts.
-  std::set<ModelSafeGroup> GetEnabledGroupsWithConflicts() const;
-
  private:
   // Extend the encapsulation boundary to utilities for internal member
   // assignments. This way, the scope of these actions is explicit, they can't
diff --git a/sync/sessions/sync_session_context.cc b/sync/sessions/sync_session_context.cc
index fa946d0..f4f625b 100644
--- a/sync/sessions/sync_session_context.cc
+++ b/sync/sessions/sync_session_context.cc
@@ -24,8 +24,7 @@ SyncSessionContext::SyncSessionContext(
     DebugInfoGetter* debug_info_getter,
     TrafficRecorder* traffic_recorder,
     bool keystore_encryption_enabled)
-    : resolver_(NULL),
-      connection_manager_(connection_manager),
+    : connection_manager_(connection_manager),
       directory_(directory),
       workers_(workers),
       extensions_activity_monitor_(extensions_activity_monitor),
diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h
index e1933c7..16b56be 100644
--- a/sync/sessions/sync_session_context.h
+++ b/sync/sessions/sync_session_context.h
@@ -31,7 +31,6 @@
 
 namespace syncer {
 
-class ConflictResolver;
 class ExtensionsActivityMonitor;
 class ServerConnectionManager;
 class ThrottledDataTypeTracker;
@@ -44,7 +43,6 @@ class Directory;
 static const int kDefaultMaxCommitBatchSize = 25;
 
 namespace sessions {
-class ScopedSessionContextConflictResolver;
 class TestScopedSessionEventListener;
 
 class SyncSessionContext {
@@ -61,7 +59,6 @@ class SyncSessionContext {
 
   ~SyncSessionContext();
 
-  ConflictResolver* resolver() { return resolver_; }
   ServerConnectionManager* connection_manager() {
     return connection_manager_;
   }
@@ -136,12 +133,8 @@ class SyncSessionContext {
   // Rather than force clients to set and null-out various context members, we
   // extend our encapsulation boundary to scoped helpers that take care of this
   // once they are allocated. See definitions of these below.
-  friend class ScopedSessionContextConflictResolver;
   friend class TestScopedSessionEventListener;
 
-  // This is installed by Syncer objects when needed and may be NULL.
-  ConflictResolver* resolver_;
-
   ObserverList<SyncEngineEventListener> listeners_;
 
   ServerConnectionManager* const connection_manager_;
@@ -187,29 +180,6 @@ class SyncSessionContext {
   DISALLOW_COPY_AND_ASSIGN(SyncSessionContext);
 };
 
-// Installs a ConflictResolver to a given session context for the lifetime of
-// the ScopedSessionContextConflictResolver.  There should never be more than
-// one ConflictResolver in the system, so it is an error to use this if the
-// context already has a resolver.
-class ScopedSessionContextConflictResolver {
- public:
-  // Note: |context| and |resolver| should outlive |this|.
-  ScopedSessionContextConflictResolver(SyncSessionContext* context,
-                                       ConflictResolver* resolver)
-      : context_(context), resolver_(resolver) {
-    DCHECK(NULL == context->resolver_);
-    context->resolver_ = resolver;
-  }
-  ~ScopedSessionContextConflictResolver() {
-    context_->resolver_ = NULL;
-  }
-
- private:
-  SyncSessionContext* context_;
-  ConflictResolver* resolver_;
-  DISALLOW_COPY_AND_ASSIGN(ScopedSessionContextConflictResolver);
-};
-
 }  // namespace sessions
 }  // namespace syncer
 
diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc
index e23f6f6..aaa8150 100644
--- a/sync/sessions/sync_session_unittest.cc
+++ b/sync/sessions/sync_session_unittest.cc
@@ -8,12 +8,10 @@
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
-#include "sync/engine/conflict_resolver.h"
 #include "sync/engine/syncer_types.h"
 #include "sync/engine/throttled_data_type_tracker.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/base/model_type_invalidation_map_test_util.h"
-#include "sync/sessions/session_state.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/syncable/syncable_id.h"
 #include "sync/syncable/write_transaction.h"
@@ -162,16 +160,6 @@ TEST_F(SyncSessionTest, EnabledGroups) {
   EXPECT_EQ(expected_enabled_groups, session->GetEnabledGroups());
 }
 
-TEST_F(SyncSessionTest, ScopedContextHelpers) {
-  ConflictResolver resolver;
-  EXPECT_FALSE(context_->resolver());
-  {
-    ScopedSessionContextConflictResolver s_resolver(context_.get(), &resolver);
-    EXPECT_EQ(&resolver, context_->resolver());
-  }
-  EXPECT_FALSE(context_->resolver());
-}
-
 TEST_F(SyncSessionTest, SetWriteTransaction) {
   TestDirectorySetterUpper dir_maker;
   dir_maker.SetUp();
@@ -194,10 +182,6 @@ TEST_F(SyncSessionTest, MoreToDownloadIfDownloadFailed) {
   // When DownloadUpdatesCommand fails, these should be false.
   EXPECT_FALSE(status()->ServerSaysNothingMoreToDownload());
   EXPECT_FALSE(status()->download_updates_succeeded());
-
-  // Download updates has its own loop in the syncer; it shouldn't factor
-  // into HasMoreToSync.
-  EXPECT_FALSE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, MoreToDownloadIfGotChangesRemaining) {
@@ -210,10 +194,6 @@ TEST_F(SyncSessionTest, MoreToDownloadIfGotChangesRemaining) {
      ->set_changes_remaining(1000L);
   EXPECT_FALSE(status()->ServerSaysNothingMoreToDownload());
   EXPECT_TRUE(status()->download_updates_succeeded());
-
-  // Download updates has its own loop in the syncer; it shouldn't factor
-  // into HasMoreToSync.
-  EXPECT_FALSE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, MoreToDownloadIfGotNoChangesRemaining) {
@@ -224,29 +204,6 @@ TEST_F(SyncSessionTest, MoreToDownloadIfGotNoChangesRemaining) {
       ->set_changes_remaining(0);
   EXPECT_TRUE(status()->ServerSaysNothingMoreToDownload());
   EXPECT_TRUE(status()->download_updates_succeeded());
-
-  // Download updates has its own loop in the syncer; it shouldn't factor
-  // into HasMoreToSync.
-  EXPECT_FALSE(session_->HasMoreToSync());
-}
-
-TEST_F(SyncSessionTest, MoreToSyncIfConflictsResolved) {
-  // Conflict resolution happens after get updates and commit,
-  // so we need to loop back and get updates / commit again now
-  // that we have made forward progress.
-  status()->update_conflicts_resolved(true);
-  EXPECT_TRUE(session_->HasMoreToSync());
-}
-
-TEST_F(SyncSessionTest, ResetTransientState) {
-  status()->update_conflicts_resolved(true);
-  status()->increment_num_successful_commits();
-  EXPECT_TRUE(session_->HasMoreToSync());
-  session_->PrepareForAnotherSyncCycle();
-  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::SYNC_CYCLE_CONTINUATION,
-            session_->source().updates_source);
-  EXPECT_FALSE(status()->conflicts_resolved());
-  EXPECT_FALSE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, Coalesce) {
diff --git a/sync/sessions/test_util.cc b/sync/sessions/test_util.cc
index cef5121..3b4b6c9 100644
--- a/sync/sessions/test_util.cc
+++ b/sync/sessions/test_util.cc
@@ -8,12 +8,6 @@ namespace syncer {
 namespace sessions {
 namespace test_util {
 
-void SimulateHasMoreToSync(sessions::SyncSession* session,
-                           SyncerStep begin, SyncerStep end) {
-  session->mutable_status_controller()->update_conflicts_resolved(true);
-  ASSERT_TRUE(session->HasMoreToSync());
-}
-
 void SimulateGetEncryptionKeyFailed(sessions::SyncSession* session,
                                     SyncerStep begin, SyncerStep end) {
   session->mutable_status_controller()->set_last_get_key_result(
@@ -46,9 +40,6 @@ void SimulateConnectionFailure(sessions::SyncSession* session,
 
 void SimulateSuccess(sessions::SyncSession* session,
                      SyncerStep begin, SyncerStep end) {
-  if (session->HasMoreToSync()) {
-    ADD_FAILURE() << "Shouldn't have more to sync";
-  }
   ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining());
   switch(end) {
     case SYNCER_END:
diff --git a/sync/sessions/test_util.h b/sync/sessions/test_util.h
index a6fa431..38638e1 100644
--- a/sync/sessions/test_util.h
+++ b/sync/sessions/test_util.h
@@ -15,8 +15,6 @@ namespace syncer {
 namespace sessions {
 namespace test_util {
 
-void SimulateHasMoreToSync(sessions::SyncSession* session,
-                           SyncerStep begin, SyncerStep end);
 void SimulateGetEncryptionKeyFailed(sessions::SyncSession* session,
                                     SyncerStep begin, SyncerStep end);
 void SimulateDownloadUpdatesFailed(sessions::SyncSession* session,