sync: Remove the remaining conflict sets code

chrome/browser/sync/sessions/session_state.h

Index: chrome/browser/sync/sessions/session_state.h
diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h
index 72bcb007d61dc27434cf87e8e5538ed540578448..44fc59ce377f827711e726e75cc117608b5cc32d 100644
--- a/chrome/browser/sync/sessions/session_state.h
+++ b/chrome/browser/sync/sessions/session_state.h
@@ -161,22 +161,13 @@ struct SyncSessionSnapshot {
   const bool retry_scheduled;
 };
 
-// Tracks progress of conflicts and their resolution using conflict sets.
+// Tracks progress of conflicts and their resolution.

[Nicolas Zea, 2012/02/01 19:16:19]

resolutions

[rlarocque, 2012/02/02 00:32:56]

Done.

 class ConflictProgress {
  public:
   explicit ConflictProgress(bool* dirty_flag);
   ~ConflictProgress();
-  // Various iterators, size, and retrieval functions for conflict sets.
-  IdToConflictSetMap::const_iterator IdToConflictSetBegin() const;
-  IdToConflictSetMap::const_iterator IdToConflictSetEnd() const;
-  IdToConflictSetMap::size_type IdToConflictSetSize() const;
-  IdToConflictSetMap::const_iterator IdToConflictSetFind(
-      const syncable::Id& the_id) const;
-  const ConflictSet* IdToConflictSetGet(const syncable::Id& the_id);
-  std::set<ConflictSet*>::const_iterator ConflictSetsBegin() const;
-  std::set<ConflictSet*>::const_iterator ConflictSetsEnd() const;
-  std::set<ConflictSet*>::size_type ConflictSetsSize() const;
-  bool HasSimpleConflictItem(const syncable::Id& id) const;
+
+  bool HasSimpleConflictItem(const syncable::Id &id) const;
 
   // Various mutators for tracking commit conflicts.
   void AddConflictingItemById(const syncable::Id& the_id);
@@ -187,24 +178,28 @@ class ConflictProgress {
 
   // Mutators for nonblocking conflicting items (see description below).
   void AddNonblockingConflictingItemById(const syncable::Id& the_id);
-  void EraseNonblockingConflictingItemById(const syncable::Id& the_id);
   int NonblockingConflictingItemsSize() const {
     return nonblocking_conflicting_item_ids_.size();
   }
 
-  void MergeSets(const syncable::Id& set1, const syncable::Id& set2);
-  void CleanupSets();
+  void AddHierarchyConflictingItemById(const syncable::Id& id);
+  int HierarchyConflictingItemsSize() const {
+    return hierarchy_conflicting_item_ids_.size();
+  }
 
  private:
-  // TODO(sync): move away from sets if it makes more sense.
+  // Conflicts that occur when an local and server changes collide, minus

[Nicolas Zea, 2012/02/01 19:16:19]

remove an

[rlarocque, 2012/02/02 00:32:56]

Done.

+  // the nonblocking and hierarchy conflicts.
   std::set<syncable::Id> conflicting_item_ids_;
-  std::map<syncable::Id, ConflictSet*> id_to_conflict_set_;
-  std::set<ConflictSet*> conflict_sets_;
 
   // Nonblocking conflicts are not processed by the conflict resolver, but
   // they will be processed in the APPLY_UDPATES_TO_RESOLVE_CONFLICTS step.
   std::set<syncable::Id> nonblocking_conflicting_item_ids_;
 
+  // Hiearchy conflicts are updates that would violate tree invariants if
+  // they were applied.
+  std::set<syncable::Id> hierarchy_conflicting_item_ids_;

[rlarocque, 2012/01/31 19:12:17]

I'm not sure this is the right thing to do.  I decided to maintain symmetry with
the nonblocking_conflicting_item_ids, even though it doesn't make a lot of sense
to me.  

We never access the members of this set.  I think the set is intended to allow
us to count more accurately (by avoiding double-counting), but I'm not sure
that's actually necessary.  

Alternative approaches:
A. Track both encryption and hierarchy conflicts as nonblocking conflicts
because we treat them the same way.  (this may end up limiting the amount of
information we could export to about:sync)
B. Use counters instead of sets to track these items.  (This makes the most
sense to me, but I'm reluctant to delete code I don't understand.)
C. Some combination of the two: one set, two counters?

[Nicolas Zea, 2012/02/01 19:16:19]

I prefer C, probably by adding an extra parameter to the nonblocking conflict
methods specifying the type of nonblocking conflict (e.g. CONFLICT_ENCRYPTION 
or CONFLICT_HIERARCHY).

[rlarocque, 2012/02/02 00:32:56]

Minor point: If we're going with option C, I'd prefer to use separate functions.
 It's harder to misuse two functions than one function with an enum parameter.  

Implemented in the following patch set.

+
   // Whether a conflicting item was added or removed since
   // the last call to reset_progress_changed(), if any. In practice this
   // points to StatusController::is_dirty_.