sync: Don't use ConflictProgress in the resolver

Remove all references to the ConflictProgress struct from the place
where it is needed most: the ConflictResolver. It turns out that the
ConflictResolver's only relevant input is a set of simple conflict IDs,
and it has no need for the rest of the ConflictProgress struct.

This change is a step towards the removal of the ConflictProgress
struct, which will eventually allow us to improve the way that the
syncer handles conflicts.

This commit also cleans up some includes and forward declarations in the
conflict resolver.

BUG=147681

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

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

8 files changed, 24 insertions, 47 deletions

diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index 73c25ea..eec418a 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -4,44 +4,31 @@
 
 #include "sync/engine/conflict_resolver.h"
 
-#include <algorithm>
 #include <list>
-#include <map>
 #include <set>
+#include <string>
 
-#include "base/location.h"
 #include "base/metrics/histogram.h"
 #include "sync/engine/conflict_util.h"
-#include "sync/engine/syncer.h"
 #include "sync/engine/syncer_util.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/mutable_entry.h"
-#include "sync/syncable/nigori_handler.h"
 #include "sync/syncable/write_transaction.h"
 #include "sync/util/cryptographer.h"
 
 using std::list;
-using std::map;
 using std::set;
 
 namespace syncer {
 
-using sessions::ConflictProgress;
 using sessions::StatusController;
-using syncable::BaseTransaction;
 using syncable::Directory;
 using syncable::Entry;
 using syncable::Id;
 using syncable::MutableEntry;
 using syncable::WriteTransaction;
 
-namespace {
-
-const int SYNC_CYCLES_BEFORE_ADMITTING_DEFEAT = 8;
-
-}  // namespace
-
 ConflictResolver::ConflictResolver() {
 }
 
@@ -291,16 +278,17 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
   }
 }
 
-bool ConflictResolver::ResolveConflicts(syncable::WriteTransaction* trans,
-                                        const Cryptographer* cryptographer,
-                                        const ConflictProgress& progress,
-                                        sessions::StatusController* status) {
+bool ConflictResolver::ResolveConflicts(
+    syncable::WriteTransaction* trans,
+    const Cryptographer* cryptographer,
+    const std::set<syncable::Id>& simple_conflict_ids,
+    sessions::StatusController* status) {
   bool forward_progress = false;
   // Iterate over simple conflict items.
   set<Id>::const_iterator conflicting_item_it;
   set<Id> processed_items;
-  for (conflicting_item_it = progress.SimpleConflictingItemsBegin();
-       conflicting_item_it != progress.SimpleConflictingItemsEnd();
+  for (conflicting_item_it = simple_conflict_ids.begin();
+       conflicting_item_it != simple_conflict_ids.end();
        ++conflicting_item_it) {
     Id id = *conflicting_item_it;
     if (processed_items.count(id) > 0)
@@ -331,7 +319,7 @@ bool ConflictResolver::ResolveConflicts(syncable::WriteTransaction* trans,
         break;
       prev_id = new_prev_id;
     } while (processed_items.count(prev_id) == 0 &&
-             progress.HasSimpleConflictItem(prev_id));  // Excludes root.
+             simple_conflict_ids.count(prev_id) > 0);  // Excludes root.
     while (!predecessors.empty()) {
       id = predecessors.back();
       predecessors.pop_back();
diff --git a/sync/engine/conflict_resolver.h b/sync/engine/conflict_resolver.h
index 7e5be74..554d2e0 100644
--- a/sync/engine/conflict_resolver.h
+++ b/sync/engine/conflict_resolver.h
@@ -8,9 +8,7 @@
 #ifndef SYNC_ENGINE_CONFLICT_RESOLVER_H_
 #define SYNC_ENGINE_CONFLICT_RESOLVER_H_
 
-#include <map>
 #include <set>
-#include <string>
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
@@ -19,16 +17,13 @@
 namespace syncer {
 
 namespace syncable {
-class BaseTransaction;
 class Id;
-class MutableEntry;
 class WriteTransaction;
 }  // namespace syncable
 
 class Cryptographer;
 
 namespace sessions {
-class ConflictProgress;
 class StatusController;
 }  // namespace sessions
 
@@ -56,7 +51,7 @@ class ConflictResolver {
   // Returns true if the syncer should try to apply its updates again.
   bool ResolveConflicts(syncable::WriteTransaction* trans,
                         const Cryptographer* cryptographer,
-                        const sessions::ConflictProgress& progress,
+                        const std::set<syncable::Id>& simple_conflict_ids,
                         sessions::StatusController* status);
 
  private:
@@ -71,11 +66,6 @@ class ConflictResolver {
       const Cryptographer* cryptographer,
       sessions::StatusController* status);
 
-  bool ResolveSimpleConflicts(syncable::WriteTransaction* trans,
-                              const Cryptographer* cryptographer,
-                              const sessions::ConflictProgress& progress,
-                              sessions::StatusController* status);
-
   DISALLOW_COPY_AND_ASSIGN(ConflictResolver);
 };
 
diff --git a/sync/engine/conflict_util.h b/sync/engine/conflict_util.h
index c3efa5e..7242dd6 100644
--- a/sync/engine/conflict_util.h
+++ b/sync/engine/conflict_util.h
@@ -7,12 +7,12 @@
 #ifndef SYNC_ENGINE_CONFLICT_UTIL_H_
 #define SYNC_ENGINE_CONFLICT_UTIL_H_
 
+namespace syncer {
+
 namespace syncable {
 class MutableEntry;
 }
 
-namespace syncer {
-
 // Marks the item as no longer requiring sync, allowing the server's version
 // to 'win' during the next update application step.
 void IgnoreLocalChanges(syncable::MutableEntry* entry);
diff --git a/sync/engine/resolve_conflicts_command.cc b/sync/engine/resolve_conflicts_command.cc
index a1ffbde..3adfa9d 100644
--- a/sync/engine/resolve_conflicts_command.cc
+++ b/sync/engine/resolve_conflicts_command.cc
@@ -33,7 +33,8 @@ SyncerError ResolveConflictsCommand::ModelChangingExecuteImpl(
   syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir);
   const Cryptographer* cryptographer = dir->GetCryptographer(&trans);
   status->update_conflicts_resolved(
-      resolver->ResolveConflicts(&trans, cryptographer, *progress, status));
+      resolver->ResolveConflicts(&trans, cryptographer,
+                                 progress->SimpleConflictingItems(), status));
 
   return SYNCER_OK;
 }
diff --git a/sync/engine/syncer_util.h b/sync/engine/syncer_util.h
index eb790ae..108622c 100644
--- a/sync/engine/syncer_util.h
+++ b/sync/engine/syncer_util.h
@@ -20,9 +20,14 @@
 
 namespace sync_pb {
 class SyncEntity;
-}
+}  // namespace sync_pb
 
 namespace syncer {
+
+namespace syncable {
+class BaseTransaction;
+}  // namespace syncable
+
 class Cryptographer;
 
 // If the server sent down a client-tagged entry, or an entry whose
diff --git a/sync/sessions/session_state.cc b/sync/sessions/session_state.cc
index cd99800..015b20c 100644
--- a/sync/sessions/session_state.cc
+++ b/sync/sessions/session_state.cc
@@ -28,13 +28,8 @@ bool ConflictProgress::HasSimpleConflictItem(const syncable::Id& id) const {
   return simple_conflicting_item_ids_.count(id) > 0;
 }
 
-std::set<syncable::Id>::const_iterator
-ConflictProgress::SimpleConflictingItemsBegin() const {
-  return simple_conflicting_item_ids_.begin();
-}
-std::set<syncable::Id>::const_iterator
-ConflictProgress::SimpleConflictingItemsEnd() const {
-  return simple_conflicting_item_ids_.end();
+const std::set<syncable::Id>& ConflictProgress::SimpleConflictingItems() const {
+  return simple_conflicting_item_ids_;
 }
 
 void ConflictProgress::AddSimpleConflictingItemById(
diff --git a/sync/sessions/session_state.h b/sync/sessions/session_state.h
index 290ecee..eff9591 100644
--- a/sync/sessions/session_state.h
+++ b/sync/sessions/session_state.h
@@ -33,8 +33,7 @@ class ConflictProgress {
   // Various mutators for tracking commit conflicts.
   void AddSimpleConflictingItemById(const syncable::Id& the_id);
   void EraseSimpleConflictingItemById(const syncable::Id& the_id);
-  std::set<syncable::Id>::const_iterator SimpleConflictingItemsBegin() const;
-  std::set<syncable::Id>::const_iterator SimpleConflictingItemsEnd() const;
+  const std::set<syncable::Id>& SimpleConflictingItems() const;
   int SimpleConflictingItemsSize() const {
     return simple_conflicting_item_ids_.size();
   }
diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc
index b7bb5ca..b2be0e5 100644
--- a/sync/sessions/sync_session.cc
+++ b/sync/sessions/sync_session.cc
@@ -215,8 +215,7 @@ std::set<ModelSafeGroup> SyncSession::GetEnabledGroupsWithConflicts() const {
     const sessions::ConflictProgress* conflict_progress =
         status_controller_->GetUnrestrictedConflictProgress(*it);
     if (conflict_progress &&
-        (conflict_progress->SimpleConflictingItemsBegin() !=
-         conflict_progress->SimpleConflictingItemsEnd())) {
+        conflict_progress->SimpleConflictingItemsSize() > 0) {
       enabled_groups_with_conflicts.insert(*it);
     }
   }