Detect & destroy dead conflictresolver paths.

Remove useless member from conflictresolver (it was always empty and had no effect).  Add histograms to conflictresolver to verify the hypothesis that this code is unreachable in practice.

BUG=83006
TEST=unittests


Review URL: http://codereview.chromium.org/7042028

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

chrome/browser/sync/engine/conflict_resolver.cc

@@ -8,6 +8,7 @@
 #include <map>
 #include <set>
 
+#include "base/metrics/histogram.h"
 #include "chrome/browser/sync/engine/syncer.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
 #include "chrome/browser/sync/protocol/service_constants.h"
@@ -183,6 +184,7 @@ namespace {
 
 bool AttemptToFixCircularConflict(WriteTransaction* trans,
                                   ConflictSet* conflict_set) {
+  UMA_HISTOGRAM_COUNTS("Sync.ConflictFixCircularity", 1);
   ConflictSet::const_iterator i, j;
   for (i = conflict_set->begin() ; i != conflict_set->end() ; ++i) {
     MutableEntry entryi(trans, syncable::GET_BY_ID, *i);
@@ -347,6 +349,7 @@ bool AttemptToFixUpdateEntryInDeletedLocalTree(WriteTransaction* trans,
 
 bool AttemptToFixRemovedDirectoriesWithContent(WriteTransaction* trans,
                                                ConflictSet* conflict_set) {
+  UMA_HISTOGRAM_COUNTS("Sync.ConflictFixRemovedDirectoriesWithContent", 1);
   ConflictSet::const_iterator i, j;
   for (i = conflict_set->begin() ; i != conflict_set->end() ; ++i) {
     Entry entry(trans, syncable::GET_BY_ID, *i);
@@ -419,6 +422,7 @@ bool ConflictResolver::LogAndSignalIfConflictStuck(
   }
 
   status->set_syncer_stuck(true);
+  UMA_HISTOGRAM_COUNTS("Sync.SyncerConflictStuck", 1);
 
   return true;
   // TODO(sync): If we're stuck for a while we need to alert the user, clear
@@ -474,8 +478,6 @@ bool ConflictResolver::ResolveConflicts(const ScopedDirLookup& dir,
   if (ResolveSimpleConflicts(dir, status))
     rv = true;
   WriteTransaction trans(dir, syncable::SYNCER, __FILE__, __LINE__);
-  set<Id> children_of_dirs_merged_last_round;
-  std::swap(children_of_merged_dirs_, children_of_dirs_merged_last_round);
   set<ConflictSet*>::const_iterator set_it;
   for (set_it = progress.ConflictSetsBegin();
        set_it != progress.ConflictSetsEnd();
@@ -488,14 +490,6 @@ bool ConflictResolver::ResolveConflicts(const ScopedDirLookup& dir,
     if (2 == conflict_count) {
       // METRIC conflict sets seen ++
     }
-    // See if this set contains entries whose parents were merged last round.
-    if (SortedCollectionsIntersect(children_of_dirs_merged_last_round.begin(),
-                                   children_of_dirs_merged_last_round.end(),
-                                   conflict_set->begin(),
-                                   conflict_set->end())) {
-      VLOG(1) << "Accelerating resolution for hierarchical merge.";
-      conflict_count += 2;
-    }
     // See if we should process this set.
     if (ProcessConflictSet(&trans, conflict_set, conflict_count)) {
       rv = true;

chrome/browser/sync/engine/conflict_resolver.h

@@ -87,12 +87,6 @@ class ConflictResolver {
   ConflictSetCountMap conflict_set_count_map_;
   SimpleConflictCountMap simple_conflict_count_map_;
 
-  // Contains the ids of uncommitted items that are children of entries merged
-  // in the previous cycle. This is used to speed up the merge resolution of
-  // deep trees. Used to happen in store refresh.
-  // TODO(chron): Can we get rid of this optimization?
-  std::set<syncable::Id> children_of_merged_dirs_;
-
   DISALLOW_COPY_AND_ASSIGN(ConflictResolver);
 };
 

chrome/browser/sync/engine/syncer.h

@@ -130,27 +130,6 @@ class Syncer {
   DISALLOW_COPY_AND_ASSIGN(Syncer);
 };
 
-// Inline utility functions.
-
-// Given iterator ranges from two collections sorted according to a common
-// strict weak ordering, return true if the two ranges contain any common
-// items, and false if they do not. This function is in this header so that it
-// can be tested.
-template <class Iterator1, class Iterator2>
-bool SortedCollectionsIntersect(Iterator1 begin1, Iterator1 end1,
-                                Iterator2 begin2, Iterator2 end2) {
-  Iterator1 i1 = begin1;
-  Iterator2 i2 = begin2;
-  while (i1 != end1 && i2 != end2) {
-    if (*i1 == *i2)
-      return true;
-    if (*i1 > *i2)
-      ++i2;
-    else
-      ++i1;
-  }
-  return false;
-}
 // Utility function declarations.
 void CopyServerFields(syncable::Entry* src, syncable::MutableEntry* dest);
 void ClearServerData(syncable::MutableEntry* entry);

chrome/browser/sync/engine/syncer_unittest.cc

@@ -3729,24 +3729,6 @@ TEST_F(SyncerTest, TestMoveSanitizedNamedFolder) {
   SyncShareAsDelegate();
 }
 
-TEST(SortedCollectionsIntersect, SortedCollectionsIntersectTest) {
-  int negative[] = {-3, -2, -1};
-  int straddle[] = {-1, 0, 1};
-  int positive[] = {1, 2, 3};
-  EXPECT_TRUE(SortedCollectionsIntersect(negative, negative + 3,
-                                         straddle, straddle + 3));
-  EXPECT_FALSE(SortedCollectionsIntersect(negative, negative + 3,
-                                          positive, positive + 3));
-  EXPECT_TRUE(SortedCollectionsIntersect(straddle, straddle + 3,
-                                         positive, positive + 3));
-  EXPECT_FALSE(SortedCollectionsIntersect(straddle + 2, straddle + 3,
-                                          positive, positive));
-  EXPECT_FALSE(SortedCollectionsIntersect(straddle, straddle + 3,
-                                          positive + 1, positive + 1));
-  EXPECT_TRUE(SortedCollectionsIntersect(straddle, straddle + 3,
-                                         positive, positive + 1));
-}
-
 // Don't crash when this occurs.
 TEST_F(SyncerTest, UpdateWhereParentIsNotAFolder) {
   ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());