Changes related to new conflict counters

While it might have made more sense to commit these separately, it's
hard to update the snapshot format without leaving some part of the
system in an inconsistent state.

At the heart of this change is a change in the snapshot to make it contain
counts of various types of conflicts, rather than just counting the simple
conflicts and total number of conflicts.  This enables us to make the changes
that follow.

This change adds more detailed conflict information to about:sync.  The main
page now lists the counts of encryption, hierarchy, simple and server conflicts
separately.  It removes the total number of conflicts counter, but that
number should be equal to encryption + hierarchy + simple conflicts.

Similarly, it updates the client_debug_info server-side debug mechanism with
the new counters.  The old counters are not only old, but were also being set
wrong.  I've added comments to the protobuf that explain the situation.  We
should move away from those old counters as soon as possible.

Finally, it removes the conflicting_commits counter from ErrorCounters.
There's a new server conflicting commits counter (intended to be symmetric with
the other new counters) that can track this.  Exposing that counter has made
conflicting_commits redundant.

BUG=107816,112659
TEST=


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

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

21 files changed, 180 insertions, 127 deletions

diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc
index db60b76..a32c72d 100644
--- a/chrome/browser/sync/engine/all_status.cc
+++ b/chrome/browser/sync/engine/all_status.cc
@@ -32,7 +32,10 @@ sync_api::SyncManager::Status AllStatus::CreateBlankStatus() const {
   // are not to be cleared here.
   sync_api::SyncManager::Status status = status_;
   status.unsynced_count = 0;
-  status.conflicting_count = 0;
+  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;
   status.updates_available = 0;
@@ -43,12 +46,12 @@ sync_api::SyncManager::Status AllStatus::CalcSyncing(
     const SyncEngineEvent &event) const {
   sync_api::SyncManager::Status status = CreateBlankStatus();
   const sessions::SyncSessionSnapshot* snapshot = event.snapshot;
-  status.unsynced_count += static_cast<int>(snapshot->unsynced_count);
-  status.conflicting_count += snapshot->errors.num_conflicting_commits;
-  status.committed_count += snapshot->syncer_status.num_successful_commits;
-  // The syncer may not be done yet, which could cause conflicting updates.
-  // But this is only used for status, so it is better to have visibility.
-  status.conflicting_count += snapshot->num_conflicting_updates;
+  status.unsynced_count = static_cast<int>(snapshot->unsynced_count);
+  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->syncer_status.num_successful_commits;
 
   if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_BEGIN) {
     status.syncing = true;
diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc
index b16601a..761aeba 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command.cc
@@ -168,8 +168,6 @@ SyncerError ProcessCommitResponseCommand::ProcessCommitResponse(
     }
   }
 
-  // TODO(sync): move status reporting elsewhere.
-  status->increment_num_conflicting_commits_by(conflicting_commits);
   SyncerUtil::MarkDeletedChildrenSynced(dir, &deleted_folders);
 
   int commit_count = static_cast<int>(proj.size());
diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc
index 9750fac..3fe46dc 100644
--- a/chrome/browser/sync/engine/syncer.cc
+++ b/chrome/browser/sync/engine/syncer.cc
@@ -221,7 +221,6 @@ void Syncer::SyncShare(sessions::SyncSession* session,
         break;
       }
       case PROCESS_COMMIT_RESPONSE: {
-        session->mutable_status_controller()->reset_num_conflicting_commits();
         ProcessCommitResponseCommand process_response_command;
         session->mutable_status_controller()->
             set_last_process_commit_response_result(
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index 3a45495..2821cbc 100644
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -2474,7 +2474,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
   }
   syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
   const StatusController& status(session_->status_controller());
-  EXPECT_EQ(0, status.error().num_conflicting_commits);
+  EXPECT_EQ(0, status.TotalNumServerConflictingItems());
 }
 
 TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {
diff --git a/chrome/browser/sync/internal_api/debug_info_event_listener.cc b/chrome/browser/sync/internal_api/debug_info_event_listener.cc
index 8c49eb5..80737ee 100644
--- a/chrome/browser/sync/internal_api/debug_info_event_listener.cc
+++ b/chrome/browser/sync/internal_api/debug_info_event_listener.cc
@@ -23,11 +23,14 @@ void DebugInfoEventListener::OnSyncCycleCompleted(
   sync_pb::SyncCycleCompletedEventInfo* sync_completed_event_info =
       event_info.mutable_sync_cycle_completed_event_info();
 
-  // TODO(rlarocque): These counters are reversed.  We should fix this bug.
-  sync_completed_event_info->set_num_blocking_conflicts(
-      snapshot->num_conflicting_updates);
-  sync_completed_event_info->set_num_non_blocking_conflicts(
-      snapshot->num_simple_conflicting_updates);
+  sync_completed_event_info->set_num_encryption_conflicts(
+      snapshot->num_encryption_conflicts);
+  sync_completed_event_info->set_num_hierarchy_conflicts(
+      snapshot->num_hierarchy_conflicts);
+  sync_completed_event_info->set_num_simple_conflicts(
+      snapshot->num_simple_conflicts);
+  sync_completed_event_info->set_num_server_conflicts(
+      snapshot->num_server_conflicts);
 
   AddEventToQueue(event_info);
 }
diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc
index c545f00..33e42e2 100644
--- a/chrome/browser/sync/internal_api/sync_manager.cc
+++ b/chrome/browser/sync/internal_api/sync_manager.cc
@@ -701,7 +701,10 @@ SyncManager::Status::Status()
       notifications_enabled(false),
       notifications_received(0),
       unsynced_count(0),
-      conflicting_count(0),
+      encryption_conflicts(0),
+      hierarchy_conflicts(0),
+      simple_conflicts(0),
+      server_conflicts(0),
       committed_count(0),
       syncing(false),
       initial_sync_ended(false),
diff --git a/chrome/browser/sync/internal_api/sync_manager.h b/chrome/browser/sync/internal_api/sync_manager.h
index c1d6bc4..79de6d6 100644
--- a/chrome/browser/sync/internal_api/sync_manager.h
+++ b/chrome/browser/sync/internal_api/sync_manager.h
@@ -132,8 +132,18 @@ class SyncManager {
     // Number of unsynced items counted at the start of most recent sync cycle.
     int unsynced_count;
 
-    // Number of conflicting items counted during most recent sync cycle.
-    int conflicting_count;
+    // Number of encryption conflicts counted during most recent sync cycle.
+    int encryption_conflicts;
+
+    // Number of hierarchy conflicts counted during most recent sync cycle.
+    int hierarchy_conflicts;
+
+    // Number of simple conflicts counted during most recent sync cycle.
+    int simple_conflicts;
+
+    // Number of items the server refused to commit due to conflict during most
+    // recent sync cycle.
+    int server_conflicts;
 
     // Number of items successfully committed during most recent sync cycle.
     int committed_count;
diff --git a/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc b/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc
index 1145458..5a59b32 100644
--- a/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc
+++ b/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc
@@ -84,6 +84,8 @@ TEST_F(JsSyncManagerObserverTest, OnSyncCycleCompleted) {
                                          100,
                                          8,
                                          5,
+                                         2,
+                                         7,
                                          false,
                                          sessions::SyncSourceInfo(),
                                          0,
diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc
index 48c2159..a835fa3 100644
--- a/chrome/browser/sync/profile_sync_service_harness.cc
+++ b/chrome/browser/sync/profile_sync_service_harness.cc
@@ -330,7 +330,7 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() {
       // TODO(rlarocque): Figure out a less brittle way of detecting this.
       if (IsTypeEncrypted(waiting_for_encryption_type_) &&
           IsFullySynced() &&
-          GetLastSessionSnapshot()->num_conflicting_updates == 0) {
+          GetLastSessionSnapshot()->num_encryption_conflicts == 0) {
         // Encryption is now complete for the the type in which we were waiting.
         SignalStateCompleteWithNextState(FULLY_SYNCED);
         break;
@@ -776,7 +776,7 @@ ProfileSyncService::Status ProfileSyncServiceHarness::GetStatus() {
 bool ProfileSyncServiceHarness::IsDataSyncedImpl(
     const browser_sync::sessions::SyncSessionSnapshot *snap) {
   return snap &&
-      snap->num_simple_conflicting_updates == 0 &&
+      snap->num_simple_conflicts == 0 &&
       ServiceIsPushingChanges() &&
       GetStatus().notifications_enabled &&
       !service()->HasUnsyncedItems() &&
@@ -1015,10 +1015,14 @@ std::string ProfileSyncServiceHarness::GetClientInfoString(
          << service()->HasUnsyncedItems()
          << ", unsynced_count: "
          << snap->unsynced_count
-         << ", num_conflicting_updates: "
-         << snap->num_conflicting_updates
-         << ", num_simple_conflicting_updates: "
-         << snap->num_simple_conflicting_updates
+         << ", encryption conflicts: "
+         << snap->num_encryption_conflicts
+         << ", hierarchy conflicts: "
+         << snap->num_hierarchy_conflicts
+         << ", simple conflicts: "
+         << snap->num_simple_conflicts
+         << ", server conflicts: "
+         << snap->num_server_conflicts
          << ", num_updates_downloaded : "
          << snap->syncer_status.num_updates_downloaded_total
          << ", passphrase_required_reason: "
@@ -1068,7 +1072,7 @@ bool ProfileSyncServiceHarness::WaitForTypeEncryption(
   // TODO(rlarocque): Figure out a less brittle way of detecting this.
   if (IsTypeEncrypted(type) &&
       IsFullySynced() &&
-      GetLastSessionSnapshot()->num_conflicting_updates == 0) {
+      GetLastSessionSnapshot()->num_encryption_conflicts == 0) {
     // Encryption is already complete for |type|; do not wait.
     return true;
   }
diff --git a/chrome/browser/sync/protocol/client_debug_info.proto b/chrome/browser/sync/protocol/client_debug_info.proto
index d58f61c..ed30a52 100644
--- a/chrome/browser/sync/protocol/client_debug_info.proto
+++ b/chrome/browser/sync/protocol/client_debug_info.proto
@@ -16,9 +16,21 @@ package sync_pb;
 message SyncCycleCompletedEventInfo {
   // optional bool syncer_stuck = 1; // Was always false, now obsolete.
 
-  // TODO(rlarocque): rename these counters.
-  optional int32 num_blocking_conflicts = 2;
-  optional int32 num_non_blocking_conflicts = 3;
+  // The client has never set these values correctly.  It set
+  // num_blocking_conflicts to the total number of conflicts detected and set
+  // num_non_blocking_conflicts to the number of blocking (aka. simple)
+  // conflicts.
+  //
+  // These counters have been deprecated to avoid further confusion.  The newer
+  // counters provide more detail and are less buggy.
+  optional int32 num_blocking_conflicts = 2 [deprecated = true];
+  optional int32 num_non_blocking_conflicts = 3 [deprecated = true];
+
+  // These new conflict counters replace the ones above.
+  optional int32 num_encryption_conflicts = 4;
+  optional int32 num_hierarchy_conflicts = 5;
+  optional int32 num_simple_conflicts = 6;
+  optional int32 num_server_conflicts = 7;
 }
 
 message DebugEventInfo {
diff --git a/chrome/browser/sync/sessions/session_state.cc b/chrome/browser/sync/sessions/session_state.cc
index eff0a22..c9fd813 100644
--- a/chrome/browser/sync/sessions/session_state.cc
+++ b/chrome/browser/sync/sessions/session_state.cc
@@ -91,18 +91,11 @@ DictionaryValue* DownloadProgressMarkersToValue(
 }
 
 ErrorCounters::ErrorCounters()
-    : num_conflicting_commits(0),
-      last_download_updates_result(UNSET),
+    : last_download_updates_result(UNSET),
       last_post_commit_result(UNSET),
       last_process_commit_response_result(UNSET) {
 }
 
-DictionaryValue* ErrorCounters::ToValue() const {
-  DictionaryValue* value = new DictionaryValue();
-  value->SetInteger("numConflictingCommits", num_conflicting_commits);
-  return value;
-}
-
 SyncSessionSnapshot::SyncSessionSnapshot(
     const SyncerStatus& syncer_status,
     const ErrorCounters& errors,
@@ -114,8 +107,10 @@ SyncSessionSnapshot::SyncSessionSnapshot(
     bool more_to_sync,
     bool is_silenced,
     int64 unsynced_count,
-    int num_simple_conflicting_updates,
-    int num_conflicting_updates,
+    int num_encryption_conflicts,
+    int num_hierarchy_conflicts,
+    int num_simple_conflicts,
+    int num_server_conflicts,
     bool did_commit_items,
     const SyncSourceInfo& source,
     size_t num_entries,
@@ -130,8 +125,10 @@ SyncSessionSnapshot::SyncSessionSnapshot(
       has_more_to_sync(more_to_sync),
       is_silenced(is_silenced),
       unsynced_count(unsynced_count),
-      num_simple_conflicting_updates(num_simple_conflicting_updates),
-      num_conflicting_updates(num_conflicting_updates),
+      num_encryption_conflicts(num_encryption_conflicts),
+      num_hierarchy_conflicts(num_hierarchy_conflicts),
+      num_simple_conflicts(num_simple_conflicts),
+      num_server_conflicts(num_server_conflicts),
       did_commit_items(did_commit_items),
       source(source),
       num_entries(num_entries),
@@ -149,7 +146,6 @@ SyncSessionSnapshot::~SyncSessionSnapshot() {}
 DictionaryValue* SyncSessionSnapshot::ToValue() const {
   DictionaryValue* value = new DictionaryValue();
   value->Set("syncerStatus", syncer_status.ToValue());
-  value->Set("errors", errors.ToValue());
   // We don't care too much if we lose precision here.
   value->SetInteger("numServerChangesRemaining",
                     static_cast<int>(num_server_changes_remaining));
@@ -163,9 +159,14 @@ DictionaryValue* SyncSessionSnapshot::ToValue() const {
   // We don't care too much if we lose precision here, also.
   value->SetInteger("unsyncedCount",
                     static_cast<int>(unsynced_count));
-  value->SetInteger("numSimpleConflictingUpdates",
-                    num_simple_conflicting_updates);
-  value->SetInteger("numConflictingUpdates", num_conflicting_updates);
+  value->SetInteger("numEncryptionConflicts",
+                    num_encryption_conflicts);
+  value->SetInteger("numHierarchyConflicts",
+                    num_hierarchy_conflicts);
+  value->SetInteger("numSimpleConflicts",
+                    num_simple_conflicts);
+  value->SetInteger("numServerConflicts",
+                    num_server_conflicts);
   value->SetBoolean("didCommitItems", did_commit_items);
   value->SetInteger("numEntries", num_entries);
   value->Set("source", source.ToValue());
diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h
index f98a94f..994e983 100644
--- a/chrome/browser/sync/sessions/session_state.h
+++ b/chrome/browser/sync/sessions/session_state.h
@@ -87,11 +87,6 @@ struct SyncerStatus {
 struct ErrorCounters {
   ErrorCounters();
 
-  // Caller takes ownership of the returned dictionary.
-  base::DictionaryValue* ToValue() const;
-
-  int num_conflicting_commits;
-
   // Any protocol errors that we received during this sync session.
   SyncProtocolError sync_protocol_error;
 
@@ -120,8 +115,10 @@ struct SyncSessionSnapshot {
       bool more_to_sync,
       bool is_silenced,
       int64 unsynced_count,
-      int num_simple_conflicting_updates,
-      int num_conflicting_updates,
+      int num_encryption_conflicts,
+      int num_hierarchy_conflicts,
+      int num_simple_conflicts,
+      int num_server_conflicts,
       bool did_commit_items,
       const SyncSourceInfo& source,
       size_t num_entries,
@@ -143,8 +140,10 @@ struct SyncSessionSnapshot {
   const bool has_more_to_sync;
   const bool is_silenced;
   const int64 unsynced_count;
-  const int num_simple_conflicting_updates;
-  const int num_conflicting_updates;
+  const int num_encryption_conflicts;
+  const int num_hierarchy_conflicts;
+  const int num_simple_conflicts;
+  const int num_server_conflicts;
   const bool did_commit_items;
   const SyncSourceInfo source;
   const size_t num_entries;
diff --git a/chrome/browser/sync/sessions/session_state_unittest.cc b/chrome/browser/sync/sessions/session_state_unittest.cc
index b2db4c9..7b0e8b9 100644
--- a/chrome/browser/sync/sessions/session_state_unittest.cc
+++ b/chrome/browser/sync/sessions/session_state_unittest.cc
@@ -69,16 +69,6 @@ TEST_F(SessionStateTest, SyncerStatusToValue) {
                          *value, "numServerOverwrites");
 }
 
-TEST_F(SessionStateTest, ErrorCountersToValue) {
-  ErrorCounters counters;
-  counters.num_conflicting_commits = 1;
-
-  scoped_ptr<DictionaryValue> value(counters.ToValue());
-  EXPECT_EQ(1u, value->size());
-  ExpectDictIntegerValue(counters.num_conflicting_commits,
-                         *value, "numConflictingCommits");
-}
-
 TEST_F(SessionStateTest, DownloadProgressMarkersToValue) {
   std::string download_progress_markers[syncable::MODEL_TYPE_COUNT];
   for (int i = syncable::FIRST_REAL_MODEL_TYPE;
@@ -109,9 +99,6 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
       syncer_status.ToValue());
 
   ErrorCounters errors;
-  errors.num_conflicting_commits = 250;
-  scoped_ptr<DictionaryValue> expected_errors_value(
-      errors.ToValue());
 
   const int kNumServerChangesRemaining = 105;
   const bool kIsShareUsable = true;
@@ -130,8 +117,10 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
   const bool kHasMoreToSync = false;
   const bool kIsSilenced = true;
   const int kUnsyncedCount = 1053;
-  const int kNumSimpleConflictingUpdates = 1054;
-  const int kNumConflictingUpdates = 1055;
+  const int kNumEncryptionConflicts = 1054;
+  const int kNumHierarchyConflicts = 1055;
+  const int kNumSimpleConflicts = 1056;
+  const int kNumServerConflicts = 1057;
   const bool kDidCommitItems = true;
 
   SyncSourceInfo source;
@@ -146,18 +135,19 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
                                kHasMoreToSync,
                                kIsSilenced,
                                kUnsyncedCount,
-                               kNumSimpleConflictingUpdates,
-                               kNumConflictingUpdates,
+                               kNumEncryptionConflicts,
+                               kNumHierarchyConflicts,
+                               kNumSimpleConflicts,
+                               kNumServerConflicts,
                                kDidCommitItems,
                                source,
                                0,
                                base::Time::Now(),
                                false);
   scoped_ptr<DictionaryValue> value(snapshot.ToValue());
-  EXPECT_EQ(14u, value->size());
+  EXPECT_EQ(15u, value->size());
   ExpectDictDictionaryValue(*expected_syncer_status_value, *value,
                             "syncerStatus");
-  ExpectDictDictionaryValue(*expected_errors_value, *value, "errors");
   ExpectDictIntegerValue(kNumServerChangesRemaining, *value,
                          "numServerChangesRemaining");
   ExpectDictBooleanValue(kIsShareUsable, *value, "isShareUsable");
@@ -168,10 +158,14 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
   ExpectDictBooleanValue(kHasMoreToSync, *value, "hasMoreToSync");
   ExpectDictBooleanValue(kIsSilenced, *value, "isSilenced");
   ExpectDictIntegerValue(kUnsyncedCount, *value, "unsyncedCount");
-  ExpectDictIntegerValue(kNumSimpleConflictingUpdates, *value,
-                         "numSimpleConflictingUpdates");
-  ExpectDictIntegerValue(kNumConflictingUpdates, *value,
-                         "numConflictingUpdates");
+  ExpectDictIntegerValue(kNumEncryptionConflicts, *value,
+                         "numEncryptionConflicts");
+  ExpectDictIntegerValue(kNumHierarchyConflicts, *value,
+                         "numHierarchyConflicts");
+  ExpectDictIntegerValue(kNumSimpleConflicts, *value,
+                         "numSimpleConflicts");
+  ExpectDictIntegerValue(kNumServerConflicts, *value,
+                         "numServerConflicts");
   ExpectDictBooleanValue(kDidCommitItems, *value,
                          "didCommitItems");
   ExpectDictDictionaryValue(*expected_source_value, *value, "source");
diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc
index bfd22732..262cb89 100644
--- a/chrome/browser/sync/sessions/status_controller.cc
+++ b/chrome/browser/sync/sessions/status_controller.cc
@@ -101,12 +101,6 @@ PerModelSafeGroupState* StatusController::GetOrCreateModelSafeGroupState(
   return it->second;
 }
 
-void StatusController::increment_num_conflicting_commits_by(int value) {
-  if (value == 0)
-    return;
-  shared_.error.mutate()->num_conflicting_commits += value;
-}
-
 void StatusController::increment_num_updates_downloaded_by(int value) {
   shared_.syncer_status.mutate()->num_updates_downloaded_total += value;
 }
@@ -122,11 +116,6 @@ void StatusController::increment_num_tombstone_updates_downloaded_by(
       value;
 }
 
-void StatusController::reset_num_conflicting_commits() {
-  if (shared_.error.value().num_conflicting_commits != 0)
-    shared_.error.mutate()->num_conflicting_commits = 0;
-}
-
 void StatusController::set_num_server_changes_remaining(
     int64 changes_remaining) {
   if (shared_.num_server_changes_remaining.value() != changes_remaining)
@@ -233,6 +222,30 @@ bool StatusController::HasConflictingUpdates() const {
   return false;
 }
 
+int StatusController::TotalNumEncryptionConflictingItems() const {
+  DCHECK(!group_restriction_in_effect_)
+      << "TotalNumEncryptionConflictingItems 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->conflict_progress.EncryptionConflictingItemsSize();
+  }
+  return sum;
+}
+
+int StatusController::TotalNumHierarchyConflictingItems() const {
+  DCHECK(!group_restriction_in_effect_)
+      << "TotalNumHierarchyConflictingItems 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->conflict_progress.HierarchyConflictingItemsSize();
+  }
+  return sum;
+}
+
 int StatusController::TotalNumSimpleConflictingItems() const {
   DCHECK(!group_restriction_in_effect_)
       << "TotalNumSimpleConflictingItems applies to all ModelSafeGroups";
@@ -245,6 +258,18 @@ int StatusController::TotalNumSimpleConflictingItems() const {
   return sum;
 }
 
+int StatusController::TotalNumServerConflictingItems() const {
+  DCHECK(!group_restriction_in_effect_)
+      << "TotalNumServerConflictingItems 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->conflict_progress.ServerConflictingItemsSize();
+  }
+  return sum;
+}
+
 int StatusController::TotalNumConflictingItems() const {
   DCHECK(!group_restriction_in_effect_)
       << "TotalNumConflictingItems applies to all ModelSafeGroups";
diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h
index a1287fd..f4fb4ec 100644
--- a/chrome/browser/sync/sessions/status_controller.h
+++ b/chrome/browser/sync/sessions/status_controller.h
@@ -150,9 +150,11 @@ class StatusController {
   // Note: this includes unresolvable conflicts.
   bool HasConflictingUpdates() const;
 
-  // Aggregate sum of SimpleConflictingItemSize() over all ConflictProgress
-  // objects (one for each ModelSafeGroup currently in-use).
-  // Note: this does not include unresolvable conflicts.
+  // Aggregate sums of various types of conflict counters accross all
+  // ConflictProgress objects (one for each ModelSafeGroup currently in-use).
+  int TotalNumEncryptionConflictingItems() const;
+  int TotalNumHierarchyConflictingItems() const;
+  int TotalNumServerConflictingItems() const;
   int TotalNumSimpleConflictingItems() const;
 
   // Aggregate sum of SimpleConflictingItemSize() and other
@@ -205,8 +207,6 @@ class StatusController {
   }
 
   // A toolbelt full of methods for updating counters and flags.
-  void increment_num_conflicting_commits_by(int value);
-  void reset_num_conflicting_commits();
   void set_num_server_changes_remaining(int64 changes_remaining);
   void set_invalid_store(bool invalid_store);
   void set_num_successful_bookmark_commits(int value);
diff --git a/chrome/browser/sync/sessions/status_controller_unittest.cc b/chrome/browser/sync/sessions/status_controller_unittest.cc
index c640e53..24827d8 100644
--- a/chrome/browser/sync/sessions/status_controller_unittest.cc
+++ b/chrome/browser/sync/sessions/status_controller_unittest.cc
@@ -20,14 +20,6 @@ class StatusControllerTest : public testing::Test {
 
 TEST_F(StatusControllerTest, GetsDirty) {
   StatusController status(routes_);
-  status.increment_num_conflicting_commits_by(1);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-  EXPECT_FALSE(status.TestAndClearIsDirty());  // Test that it actually resets.
-  status.increment_num_conflicting_commits_by(0);
-  EXPECT_FALSE(status.TestAndClearIsDirty());
-  status.increment_num_conflicting_commits_by(1);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-
   status.set_num_server_changes_remaining(30);
   EXPECT_TRUE(status.TestAndClearIsDirty());
 
@@ -77,9 +69,6 @@ TEST_F(StatusControllerTest, StaysClean) {
 // method was actually setting |bar_| instead!).
 TEST_F(StatusControllerTest, ReadYourWrites) {
   StatusController status(routes_);
-  status.increment_num_conflicting_commits_by(1);
-  EXPECT_EQ(1, status.error().num_conflicting_commits);
-
   status.set_num_server_changes_remaining(13);
   EXPECT_EQ(13, status.num_server_changes_remaining());
 
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index a3bf6f1..ff07ee27 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -159,8 +159,10 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
       HasMoreToSync(),
       delegate_->IsSyncingCurrentlySilenced(),
       status_controller_->unsynced_handles().size(),
+      status_controller_->TotalNumEncryptionConflictingItems(),
+      status_controller_->TotalNumHierarchyConflictingItems(),
       status_controller_->TotalNumSimpleConflictingItems(),
-      status_controller_->TotalNumConflictingItems(),
+      status_controller_->TotalNumServerConflictingItems(),
       status_controller_->did_commit_items(),
       source_,
       dir->GetEntriesCount(),
diff --git a/chrome/browser/sync/sync_ui_util.cc b/chrome/browser/sync/sync_ui_util.cc
index f1ff6ae..6728529 100644
--- a/chrome/browser/sync/sync_ui_util.cc
+++ b/chrome/browser/sync/sync_ui_util.cc
@@ -551,8 +551,17 @@ void ConstructAboutInformation(ProfileSyncService* service,
                                    "Unsynced Count",
                                    full_status.unsynced_count);
     sync_ui_util::AddIntSyncDetail(details,
-                                   "Conflicting Count",
-                                   full_status.conflicting_count);
+                                   "Encryption Conflicts Detected (this cycle)",
+                                   full_status.encryption_conflicts);
+    sync_ui_util::AddIntSyncDetail(details,
+                                   "Hierarchy Conflicts Detected (this cycle)",
+                                   full_status.hierarchy_conflicts);
+    sync_ui_util::AddIntSyncDetail(details,
+                                   "Simple Conflicts Detected (this cycle)",
+                                   full_status.simple_conflicts);
+    sync_ui_util::AddIntSyncDetail(details,
+                                   "Server Conflicts Detected (this cycle)",
+                                   full_status.server_conflicts);
     sync_ui_util::AddIntSyncDetail(details,
                                    "Committed Items (this session)",
                                    full_status.committed_count);
diff --git a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
index c4187aa..dca9344 100644
--- a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
+++ b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc
@@ -1815,7 +1815,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest,
   ASSERT_TRUE(IsEncrypted(1, syncable::BOOKMARKS));
   ASSERT_TRUE(GetClient(1)->service()->IsPassphraseRequired());
   ASSERT_GT(GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates, 3);  // The encrypted nodes.
+      num_encryption_conflicts, 3);  // The encrypted nodes.
 
   // Client 1 adds bookmarks between the first two and between the second two.
   ASSERT_TRUE(AddURL(0, 1, IndexedURLTitle(3), GURL(IndexedURL(3))) != NULL);
@@ -1833,7 +1833,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest,
   EXPECT_TRUE(AllModelsMatchVerifier());
   EXPECT_TRUE(AllModelsMatch());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);
+      num_encryption_conflicts);
 
   // Ensure everything is syncing normally by appending a final bookmark.
   ASSERT_TRUE(AddURL(1, 5, IndexedURLTitle(5), GURL(IndexedURL(5))) != NULL);
@@ -1841,7 +1841,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest,
   EXPECT_TRUE(AllModelsMatchVerifier());
   EXPECT_TRUE(AllModelsMatch());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);
+      num_encryption_conflicts);
 }
 
 // Deliberately racy rearranging of bookmarks to test that our conflict resolver
diff --git a/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc b/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc
index e04e5ae..8e44fae 100644
--- a/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc
+++ b/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc
@@ -168,12 +168,12 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_simple_conflicting_updates);
+      num_simple_conflicts);
   // We have two meta nodes (one for each client), the one tab node, plus the
   // basic preference/themes/search engines items.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 3,
       GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);  // The encrypted nodes.
+      num_encryption_conflicts);  // The encrypted nodes.
 
   GetClient(1)->service()->SetPassphrase(
       kValidPassphrase,
@@ -218,22 +218,22 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_simple_conflicting_updates);
+      num_simple_conflicts);
   // We have nine encryption conflicts due to the two meta nodes (one for each
   // client), plus the basic preference/themes/search engines nodes.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 2,
       GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);  // The encrypted nodes.
+      num_encryption_conflicts);  // The encrypted nodes.
 
   ScopedWindowMap client0_windows;
   ASSERT_TRUE(OpenTabAndGetLocalWindows(0, GURL(kURL1),
       client0_windows.GetMutable()));
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_simple_conflicting_updates);
+      num_simple_conflicts);
   ASSERT_EQ(NumberOfDefaultSyncItems() + 3,
       GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);  // The encrypted nodes.
+      num_encryption_conflicts);  // The encrypted nodes.
 
   GetClient(1)->service()->SetPassphrase(
       kValidPassphrase,
@@ -270,12 +270,12 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_simple_conflicting_updates);
+      num_simple_conflicts);
   // We have two encryption conflicts due to the two meta nodes (one for each
   // client), plus the basic preference/themes/search engines nodes.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 2,
       GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);  // The encrypted nodes.
+      num_encryption_conflicts);  // The encrypted nodes.
 
   // These changes are either made with the old passphrase or not encrypted at
   // all depending on when client 0's changes are propagated.
@@ -293,7 +293,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(GetClient(1)->WaitForTypeEncryption(syncable::SESSIONS));
   ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0)));
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);
+      num_encryption_conflicts);
 
   ASSERT_TRUE(IsEncrypted(0, syncable::SESSIONS));
   ASSERT_TRUE(IsEncrypted(1, syncable::SESSIONS));
@@ -335,13 +335,13 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(AwaitQuiescence());
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_simple_conflicting_updates);
+      num_simple_conflicts);
   // We have three encryption conflicts due to the two meta nodes (one for
   // each client), the one tab node, plus the basic preference/themes/search
   // engines nodes.
   ASSERT_GE(NumberOfDefaultSyncItems() + 3,
       GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);  // The encrypted nodes.
+      num_encryption_conflicts);  // The encrypted nodes.
 
   // At this point we enter the passphrase, triggering a resync.
   GetClient(1)->service()->SetPassphrase(
@@ -394,13 +394,13 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(EnableEncryption(0, syncable::SESSIONS));
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_simple_conflicting_updates);
+      num_simple_conflicts);
   // We have three encryption conflicts due to the two meta nodes (one for
   // each client), the one tab node, plus the basic preference/themes/search
   // engines nodes.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 3,
       GetClient(1)->GetLastSessionSnapshot()->
-      num_conflicting_updates);  // The encrypted nodes.
+      num_encryption_conflicts);  // The encrypted nodes.
 
   GetClient(1)->service()->SetPassphrase(
       kValidPassphrase,
diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc
index fc5e0dc..3f76310 100644
--- a/chrome/browser/sync/test_profile_sync_service.cc
+++ b/chrome/browser/sync/test_profile_sync_service.cc
@@ -50,8 +50,8 @@ void SyncBackendHostForProfileSyncTest::
   std::string download_progress_markers[syncable::MODEL_TYPE_COUNT];
   HandleSyncCycleCompletedOnFrontendLoop(new SyncSessionSnapshot(
       SyncerStatus(), ErrorCounters(), 0, false,
-      sync_ended, download_progress_markers, false, false, 0, 0, 0, false,
-      SyncSourceInfo(), 0, base::Time::Now(), false));
+      sync_ended, download_progress_markers, false, false, 0, 0, 0, 0, 0,
+      false, SyncSourceInfo(), 0, base::Time::Now(), false));
 }
 
 namespace {
@@ -91,8 +91,8 @@ void SyncBackendHostForProfileSyncTest::StartConfiguration(
     std::string download_progress_markers[syncable::MODEL_TYPE_COUNT];
     HandleSyncCycleCompletedOnFrontendLoop(new SyncSessionSnapshot(
         SyncerStatus(), ErrorCounters(), 0, false,
-        sync_ended, download_progress_markers, false, false, 0, 0, 0, false,
-        SyncSourceInfo(), 0, base::Time::Now(), false));
+        sync_ended, download_progress_markers, false, false, 0, 0, 0, 0, 0,
+        false, SyncSourceInfo(), 0, base::Time::Now(), false));
   }
 }