sync: Populate debug counters for directory types

Modifies the directory update handlers and commit contributors to make
use of the debug info emitters.  Previously, the debug info emitters
were available but unused.  This change makes it so the counters are
updated and their observers notified at appropriate times.

Updates some tests to rely on these counters rather than the
StatusController.  Eventually, these counters will replace most existing
uses of StatusController.

The code does not currently contain any non-test observers, so these
newly updated values are currently not reported anywhere.

BUG=349301

Review URL: https://codereview.chromium.org/271613006

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

9 files changed, 262 insertions, 77 deletions

diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index c481472..1630620 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -11,6 +11,7 @@
 #include "base/metrics/histogram.h"
 #include "sync/engine/conflict_util.h"
 #include "sync/engine/syncer_util.h"
+#include "sync/internal_api/public/sessions/update_counters.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/mutable_entry.h"
@@ -38,7 +39,8 @@ ConflictResolver::~ConflictResolver() {
 void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
                                              const Id& id,
                                              const Cryptographer* cryptographer,
-                                             StatusController* status) {
+                                             StatusController* status,
+                                             UpdateCounters* counters) {
   MutableEntry entry(trans, syncable::GET_BY_ID, id);
   // Must be good as the entry won't have been cleaned up.
   CHECK(entry.good());
@@ -158,6 +160,7 @@ void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
       DVLOG(1) << "Resolving simple conflict, ignoring server encryption "
                << " changes for: " << entry;
       status->increment_num_server_overwrites();
+      counters->num_server_overwrites++;
       conflict_util::OverwriteServerChanges(&entry);
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 IGNORE_ENCRYPTION,
@@ -169,6 +172,7 @@ void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
       // assumption.
       conflict_util::OverwriteServerChanges(&entry);
       status->increment_num_server_overwrites();
+      counters->num_server_overwrites++;
       DVLOG(1) << "Resolving simple conflict, overwriting server changes "
                << "for: " << entry;
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
@@ -179,6 +183,7 @@ void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
                << entry;
       conflict_util::IgnoreLocalChanges(&entry);
       status->increment_num_local_overwrites();
+      counters->num_local_overwrites++;
       UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
                                 OVERWRITE_LOCAL,
                                 CONFLICT_RESOLUTION_SIZE);
@@ -203,6 +208,7 @@ void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
     // data.
     conflict_util::OverwriteServerChanges(&entry);
     status->increment_num_server_overwrites();
+    counters->num_server_overwrites++;
     DVLOG(1) << "Resolving simple conflict, undeleting server entry: "
              << entry;
     UMA_HISTOGRAM_ENUMERATION("Sync.ResolveSimpleConflict",
@@ -215,7 +221,8 @@ void ConflictResolver::ResolveConflicts(
     syncable::WriteTransaction* trans,
     const Cryptographer* cryptographer,
     const std::set<syncable::Id>& simple_conflict_ids,
-    sessions::StatusController* status) {
+    sessions::StatusController* status,
+    UpdateCounters* counters) {
   // Iterate over simple conflict items.
   set<Id>::const_iterator it;
   for (it = simple_conflict_ids.begin();
@@ -229,7 +236,7 @@ void ConflictResolver::ResolveConflicts(
       continue;
     }
 
-    ProcessSimpleConflict(trans, *it, cryptographer, status);
+    ProcessSimpleConflict(trans, *it, cryptographer, status, counters);
   }
   return;
 }
diff --git a/sync/engine/conflict_resolver.h b/sync/engine/conflict_resolver.h
index 4ca614d..5838623 100644
--- a/sync/engine/conflict_resolver.h
+++ b/sync/engine/conflict_resolver.h
@@ -22,6 +22,7 @@ class WriteTransaction;
 }  // namespace syncable
 
 class Cryptographer;
+struct UpdateCounters;
 
 namespace sessions {
 class StatusController;
@@ -52,14 +53,16 @@ class ConflictResolver {
   void ResolveConflicts(syncable::WriteTransaction* trans,
                         const Cryptographer* cryptographer,
                         const std::set<syncable::Id>& simple_conflict_ids,
-                        sessions::StatusController* status);
+                        sessions::StatusController* status,
+                        UpdateCounters* counters);
 
  private:
   void ProcessSimpleConflict(
       syncable::WriteTransaction* trans,
       const syncable::Id& id,
       const Cryptographer* cryptographer,
-      sessions::StatusController* status);
+      sessions::StatusController* status,
+      UpdateCounters* counters);
 
   DISALLOW_COPY_AND_ASSIGN(ConflictResolver);
 };
diff --git a/sync/engine/directory_commit_contribution.cc b/sync/engine/directory_commit_contribution.cc
index 11fe518..1e101a5 100644
--- a/sync/engine/directory_commit_contribution.cc
+++ b/sync/engine/directory_commit_contribution.cc
@@ -7,6 +7,7 @@
 #include "sync/engine/commit_util.h"
 #include "sync/engine/get_commit_ids.h"
 #include "sync/engine/syncer_util.h"
+#include "sync/internal_api/public/sessions/commit_counters.h"
 #include "sync/syncable/model_neutral_mutable_entry.h"
 #include "sync/syncable/syncable_model_neutral_write_transaction.h"
 
@@ -66,6 +67,9 @@ void DirectoryCommitContribution::AddToCommitMessage(
             RepeatedPtrFieldBackInserter(commit_message->mutable_entries()));
   if (!context_.context().empty())
     commit_message->add_client_contexts()->Swap(&context_);
+
+  CommitCounters* counters = debug_info_emitter_->GetMutableCommitCounters();
+  counters->num_commits_attempted += entities_.size();
 }
 
 SyncerError DirectoryCommitContribution::ProcessCommitResponse(
@@ -120,6 +124,11 @@ SyncerError DirectoryCommitContribution::ProcessCommitResponse(
     MarkDeletedChildrenSynced(dir_, &trans, &deleted_folders);
   }
 
+  CommitCounters* counters = debug_info_emitter_->GetMutableCommitCounters();
+  counters->num_commits_success += successes;
+  counters->num_commits_conflict += transient_error_commits;
+  counters->num_commits_error += transient_error_commits;
+
   int commit_count = static_cast<int>(metahandles_.size());
   if (commit_count == successes) {
     return SYNCER_OK;
@@ -148,6 +157,7 @@ SyncerError DirectoryCommitContribution::ProcessCommitResponse(
 void DirectoryCommitContribution::CleanUp() {
   DCHECK(syncing_bits_set_);
   UnsetSyncingBits();
+  debug_info_emitter_->EmitCommitCountersUpdate();
 }
 
 size_t DirectoryCommitContribution::GetNumEntries() const {
diff --git a/sync/engine/directory_commit_contribution_unittest.cc b/sync/engine/directory_commit_contribution_unittest.cc
index 867fcde..c38af19 100644
--- a/sync/engine/directory_commit_contribution_unittest.cc
+++ b/sync/engine/directory_commit_contribution_unittest.cc
@@ -69,6 +69,9 @@ class DirectoryCommitContributionTest : public ::testing::Test {
 
   TestIdFactory id_factory_;
 
+  // Used in construction of DirectoryTypeDebugInfoEmitters.
+  ObserverList<TypeDebugInfoObserver> type_observers_;
+
  private:
   base::MessageLoop loop_;  // Neeed to initialize the directory.
   TestDirectorySetterUpper dir_maker_;
@@ -85,7 +88,7 @@ TEST_F(DirectoryCommitContributionTest, GatherByTypes) {
     CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
   }
 
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(PREFERENCES, &type_observers_);
   scoped_ptr<DirectoryCommitContribution> cc(
       DirectoryCommitContribution::Build(dir(), PREFERENCES, 5, &emitter));
   ASSERT_EQ(2U, cc->GetNumEntries());
@@ -111,7 +114,7 @@ TEST_F(DirectoryCommitContributionTest, GatherAndTruncate) {
     CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
   }
 
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(PREFERENCES, &type_observers_);
   scoped_ptr<DirectoryCommitContribution> cc(
       DirectoryCommitContribution::Build(dir(), PREFERENCES, 1, &emitter));
   ASSERT_EQ(1U, cc->GetNumEntries());
@@ -134,8 +137,8 @@ TEST_F(DirectoryCommitContributionTest, PrepareCommit) {
     CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
   }
 
-  DirectoryTypeDebugInfoEmitter emitter1;
-  DirectoryTypeDebugInfoEmitter emitter2;
+  DirectoryTypeDebugInfoEmitter emitter1(PREFERENCES, &type_observers_);
+  DirectoryTypeDebugInfoEmitter emitter2(EXTENSIONS, &type_observers_);
   scoped_ptr<DirectoryCommitContribution> pref_cc(
       DirectoryCommitContribution::Build(dir(), PREFERENCES, 25, &emitter1));
   scoped_ptr<DirectoryCommitContribution> ext_cc(
@@ -188,8 +191,8 @@ TEST_F(DirectoryCommitContributionTest, ProcessCommitResponse) {
     ext1_handle = CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
   }
 
-  DirectoryTypeDebugInfoEmitter emitter1;
-  DirectoryTypeDebugInfoEmitter emitter2;
+  DirectoryTypeDebugInfoEmitter emitter1(PREFERENCES, &type_observers_);
+  DirectoryTypeDebugInfoEmitter emitter2(EXTENSIONS, &type_observers_);
   scoped_ptr<DirectoryCommitContribution> pref_cc(
       DirectoryCommitContribution::Build(dir(), PREFERENCES, 25, &emitter1));
   scoped_ptr<DirectoryCommitContribution> ext_cc(
diff --git a/sync/engine/directory_update_handler.cc b/sync/engine/directory_update_handler.cc
index 8f0da59..a2f0ebf 100644
--- a/sync/engine/directory_update_handler.cc
+++ b/sync/engine/directory_update_handler.cc
@@ -60,6 +60,7 @@ SyncerError DirectoryUpdateHandler::ProcessGetUpdatesResponse(
       // A GetUpdates using the old context was in progress when the context was
       // set. Fail this get updates cycle, to force a retry.
       DVLOG(1) << "GU Context conflict detected, forcing GU retry.";
+      debug_info_emitter_->EmitUpdateCountersUpdate();
       return DATATYPE_TRIGGERED_RETRY;
     }
   }
@@ -70,6 +71,7 @@ SyncerError DirectoryUpdateHandler::ProcessGetUpdatesResponse(
     ExpireEntriesIfNeeded(&trans, progress_marker);
     UpdateProgressMarker(progress_marker);
   }
+  debug_info_emitter_->EmitUpdateCountersUpdate();
   return SYNCER_OK;
 }
 
@@ -86,6 +88,8 @@ void DirectoryUpdateHandler::ApplyUpdates(sessions::StatusController* status) {
       base::Unretained(this),
       base::Unretained(status));
   worker_->DoWorkAndWaitUntilDone(c);
+
+  debug_info_emitter_->EmitUpdateCountersUpdate();
 }
 
 void DirectoryUpdateHandler::PassiveApplyUpdates(
@@ -96,6 +100,8 @@ void DirectoryUpdateHandler::PassiveApplyUpdates(
 
   // Just do the work here instead of deferring to another thread.
   ApplyUpdatesImpl(status);
+
+  debug_info_emitter_->EmitUpdateCountersUpdate();
 }
 
 SyncerError DirectoryUpdateHandler::ApplyUpdatesImpl(
@@ -111,19 +117,30 @@ SyncerError DirectoryUpdateHandler::ApplyUpdatesImpl(
   // First set of update application passes.
   UpdateApplicator applicator(dir_->GetCryptographer(&trans));
   applicator.AttemptApplications(&trans, handles);
+
+  // The old StatusController counters.
   status->increment_num_updates_applied_by(applicator.updates_applied());
   status->increment_num_hierarchy_conflicts_by(
       applicator.hierarchy_conflicts());
   status->increment_num_encryption_conflicts_by(
       applicator.encryption_conflicts());
 
+  // The new UpdateCounter counters.
+  UpdateCounters* counters = debug_info_emitter_->GetMutableUpdateCounters();
+  counters->num_updates_applied += applicator.updates_applied();
+  counters->num_hierarchy_conflict_application_failures =
+      applicator.hierarchy_conflicts();
+  counters->num_encryption_conflict_application_failures +=
+      applicator.encryption_conflicts();
+
   if (applicator.simple_conflict_ids().size() != 0) {
     // Resolve the simple conflicts we just detected.
     ConflictResolver resolver;
     resolver.ResolveConflicts(&trans,
                               dir_->GetCryptographer(&trans),
                               applicator.simple_conflict_ids(),
-                              status);
+                              status,
+                              counters);
 
     // Conflict resolution sometimes results in more updates to apply.
     handles.clear();
@@ -138,6 +155,7 @@ SyncerError DirectoryUpdateHandler::ApplyUpdatesImpl(
     // We count the number of updates from both applicator passes.
     status->increment_num_updates_applied_by(
         conflict_applicator.updates_applied());
+    counters->num_updates_applied += conflict_applicator.updates_applied();
 
     // Encryption conflicts should remain unchanged by the resolution of simple
     // conflicts.  Those can only be solved by updating our nigori key bag.
@@ -177,7 +195,10 @@ void DirectoryUpdateHandler::UpdateSyncEntities(
     syncable::ModelNeutralWriteTransaction* trans,
     const SyncEntityList& applicable_updates,
     sessions::StatusController* status) {
-  ProcessDownloadedUpdates(dir_, trans, type_, applicable_updates, status);
+  UpdateCounters* counters = debug_info_emitter_->GetMutableUpdateCounters();
+  counters->num_updates_received += applicable_updates.size();
+  ProcessDownloadedUpdates(dir_, trans, type_,
+                           applicable_updates, status, counters);
 }
 
 bool DirectoryUpdateHandler::IsValidProgressMarker(
diff --git a/sync/engine/directory_update_handler_unittest.cc b/sync/engine/directory_update_handler_unittest.cc
index ec0a14e..837e9f4 100644
--- a/sync/engine/directory_update_handler_unittest.cc
+++ b/sync/engine/directory_update_handler_unittest.cc
@@ -86,6 +86,10 @@ class DirectoryUpdateHandlerProcessUpdateTest : public ::testing::Test {
     return e.good() && !e.GetIsDel();
   }
 
+ protected:
+  // Used in the construction of DirectoryTypeDebugInfoEmitters.
+  ObserverList<TypeDebugInfoObserver> type_observers_;
+
  private:
   base::MessageLoop loop_;  // Needed to initialize the directory.
   TestDirectorySetterUpper dir_maker_;
@@ -125,7 +129,7 @@ static const char kCacheGuid[] = "IrcjZ2jyzHDV9Io4+zKcXQ==";
 
 // Test that the bookmark tag is set on newly downloaded items.
 TEST_F(DirectoryUpdateHandlerProcessUpdateTest, NewBookmarkTag) {
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(BOOKMARKS, &type_observers_);
   DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker(), &emitter);
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
@@ -164,7 +168,7 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, NewBookmarkTag) {
 // Test the receipt of a type root node.
 TEST_F(DirectoryUpdateHandlerProcessUpdateTest,
        ReceiveServerCreatedBookmarkFolders) {
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(BOOKMARKS, &type_observers_);
   DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker(), &emitter);
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
@@ -199,7 +203,7 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest,
 
 // Test the receipt of a non-bookmark item.
 TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ReceiveNonBookmarkItem) {
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(AUTOFILL, &type_observers_);
   DirectoryUpdateHandler handler(dir(), AUTOFILL, ui_worker(), &emitter);
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
@@ -231,7 +235,7 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ReceiveNonBookmarkItem) {
 
 // Tests the setting of progress markers.
 TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ProcessNewProgressMarkers) {
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(BOOKMARKS, &type_observers_);
   DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker(), &emitter);
 
   sync_pb::DataTypeProgressMarker progress;
@@ -248,7 +252,7 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ProcessNewProgressMarkers) {
 }
 
 TEST_F(DirectoryUpdateHandlerProcessUpdateTest, GarbageCollectionByVersion) {
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(SYNCED_NOTIFICATIONS, &type_observers_);
   DirectoryUpdateHandler handler(dir(), SYNCED_NOTIFICATIONS,
                                  ui_worker(), &emitter);
   sessions::StatusController status;
@@ -313,7 +317,7 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, GarbageCollectionByVersion) {
 }
 
 TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ContextVersion) {
-  DirectoryTypeDebugInfoEmitter emitter;
+  DirectoryTypeDebugInfoEmitter emitter(SYNCED_NOTIFICATIONS, &type_observers_);
   DirectoryUpdateHandler handler(dir(), SYNCED_NOTIFICATIONS,
                                  ui_worker(), &emitter);
   sessions::StatusController status;
@@ -413,6 +417,8 @@ class DirectoryUpdateHandlerApplyUpdateTest : public ::testing::Test {
       : ui_worker_(new FakeModelWorker(GROUP_UI)),
         password_worker_(new FakeModelWorker(GROUP_PASSWORD)),
         passive_worker_(new FakeModelWorker(GROUP_PASSIVE)),
+        bookmarks_emitter_(BOOKMARKS, &type_observers_),
+        passwords_emitter_(PASSWORDS, &type_observers_),
         update_handler_map_deleter_(&update_handler_map_) {}
 
   virtual void SetUp() OVERRIDE {
@@ -435,6 +441,14 @@ class DirectoryUpdateHandlerApplyUpdateTest : public ::testing::Test {
     dir_maker_.TearDown();
   }
 
+  const UpdateCounters& GetBookmarksUpdateCounters() {
+    return bookmarks_emitter_.GetUpdateCounters();
+  }
+
+  const UpdateCounters& GetPasswordsUpdateCounters() {
+    return passwords_emitter_.GetUpdateCounters();
+  }
+
  protected:
   void ApplyBookmarkUpdates(sessions::StatusController* status) {
     update_handler_map_[BOOKMARKS]->ApplyUpdates(status);
@@ -463,6 +477,7 @@ class DirectoryUpdateHandlerApplyUpdateTest : public ::testing::Test {
   scoped_refptr<FakeModelWorker> password_worker_;
   scoped_refptr<FakeModelWorker> passive_worker_;
 
+  ObserverList<TypeDebugInfoObserver> type_observers_;
   DirectoryTypeDebugInfoEmitter bookmarks_emitter_;
   DirectoryTypeDebugInfoEmitter passwords_emitter_;
 
@@ -492,11 +507,12 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, SimpleBookmark) {
 
   ApplyBookmarkUpdates(&status);
 
-  EXPECT_EQ(0, status.num_encryption_conflicts())
+  const UpdateCounters& counter = GetBookmarksUpdateCounters();
+  EXPECT_EQ(0, counter.num_encryption_conflict_application_failures)
       << "Simple update shouldn't result in conflicts";
-  EXPECT_EQ(0, status.num_hierarchy_conflicts())
+  EXPECT_EQ(0, counter.num_hierarchy_conflict_application_failures)
       << "Simple update shouldn't result in conflicts";
-  EXPECT_EQ(2, status.num_updates_applied())
+  EXPECT_EQ(2, counter.num_updates_applied)
       << "All items should have been successfully applied";
 
   {
@@ -588,9 +604,11 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, SimpleBookmarkConflict) {
 
   sessions::StatusController status;
   ApplyBookmarkUpdates(&status);
-  EXPECT_EQ(1, status.num_server_overwrites())
+
+  const UpdateCounters& counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(1, counters.num_server_overwrites)
       << "Unsynced and unapplied item conflict should be resolved";
-  EXPECT_EQ(0, status.num_updates_applied())
+  EXPECT_EQ(0, counters.num_updates_applied)
       << "Update should not be applied; we should override the server.";
 
   {
@@ -626,9 +644,11 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, HierarchyAndSimpleConflict) {
 
   sessions::StatusController status;
   ApplyBookmarkUpdates(&status);
-  EXPECT_EQ(0, status.num_updates_applied());
-  EXPECT_EQ(0, status.num_server_overwrites());
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+
+  const UpdateCounters& counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(0, counters.num_updates_applied);
+  EXPECT_EQ(0, counters.num_server_overwrites);
+  EXPECT_EQ(1, counters.num_hierarchy_conflict_application_failures);
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -673,7 +693,8 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, BookmarkFolderLoop) {
   ApplyBookmarkUpdates(&status);
 
   // This should count as a hierarchy conflict.
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+  const UpdateCounters& counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(1, counters.num_hierarchy_conflict_application_failures);
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -711,7 +732,8 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest,
 
   sessions::StatusController status;
   ApplyBookmarkUpdates(&status);
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+  const UpdateCounters& counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(1, counters.num_hierarchy_conflict_application_failures);
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -755,7 +777,8 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest,
   ApplyBookmarkUpdates(&status);
 
   // This should count as a hierarchy conflict.
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+  const UpdateCounters& counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(1, counters.num_hierarchy_conflict_application_failures);
 
   {
     syncable::ReadTransaction trans(FROM_HERE, directory());
@@ -779,9 +802,10 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest,
   sessions::StatusController status;
   ApplyBookmarkUpdates(&status);
 
-  EXPECT_EQ(2, status.num_hierarchy_conflicts())
+  const UpdateCounters& counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(2, counters.num_hierarchy_conflict_application_failures)
       << "All updates with an unknown ancestors should be in conflict";
-  EXPECT_EQ(0, status.num_updates_applied())
+  EXPECT_EQ(0, counters.num_updates_applied)
       << "No item with an unknown ancestor should be applied";
 
   {
@@ -818,9 +842,10 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, ItemsBothKnownAndUnknown) {
   sessions::StatusController status;
   ApplyBookmarkUpdates(&status);
 
-  EXPECT_EQ(2, status.num_hierarchy_conflicts())
+  const UpdateCounters& counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(2, counters.num_hierarchy_conflict_application_failures)
       << "The updates with unknown ancestors should be in conflict";
-  EXPECT_EQ(4, status.num_updates_applied())
+  EXPECT_EQ(4, counters.num_updates_applied)
       << "The updates with known ancestors should be successfully applied";
 
   {
@@ -872,7 +897,8 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, DecryptablePassword) {
   sessions::StatusController status;
   ApplyPasswordUpdates(&status);
 
-  EXPECT_EQ(1, status.num_updates_applied())
+  const UpdateCounters& counters = GetPasswordsUpdateCounters();
+  EXPECT_EQ(1, counters.num_updates_applied)
       << "The updates that can be decrypted should be applied";
 
   {
@@ -910,9 +936,16 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, UndecryptableData) {
   ApplyBookmarkUpdates(&status);
   ApplyPasswordUpdates(&status);
 
-  EXPECT_EQ(3, status.num_encryption_conflicts())
+  const UpdateCounters& bm_counters = GetBookmarksUpdateCounters();
+  EXPECT_EQ(2, bm_counters.num_encryption_conflict_application_failures)
+      << "Updates that can't be decrypted should be in encryption conflict";
+  EXPECT_EQ(0, bm_counters.num_updates_applied)
+      << "No update that can't be decrypted should be applied";
+
+  const UpdateCounters& pw_counters = GetPasswordsUpdateCounters();
+  EXPECT_EQ(1, pw_counters.num_encryption_conflict_application_failures)
       << "Updates that can't be decrypted should be in encryption conflict";
-  EXPECT_EQ(0, status.num_updates_applied())
+  EXPECT_EQ(0, pw_counters.num_updates_applied)
       << "No update that can't be decrypted should be applied";
 
   {
@@ -973,10 +1006,11 @@ TEST_F(DirectoryUpdateHandlerApplyUpdateTest, SomeUndecryptablePassword) {
   sessions::StatusController status;
   ApplyPasswordUpdates(&status);
 
-  EXPECT_EQ(1, status.num_encryption_conflicts())
+  const UpdateCounters& counters = GetPasswordsUpdateCounters();
+  EXPECT_EQ(1, counters.num_encryption_conflict_application_failures)
       << "The updates that can't be decrypted should be in encryption "
       << "conflict";
-  EXPECT_EQ(1, status.num_updates_applied())
+  EXPECT_EQ(1, counters.num_updates_applied)
       << "The undecryptable password update shouldn't be applied";
 
   {
diff --git a/sync/engine/process_updates_util.cc b/sync/engine/process_updates_util.cc
index 7cfb920..dca46ab 100644
--- a/sync/engine/process_updates_util.cc
+++ b/sync/engine/process_updates_util.cc
@@ -8,6 +8,7 @@
 #include "sync/engine/syncer_proto_util.h"
 #include "sync/engine/syncer_types.h"
 #include "sync/engine/syncer_util.h"
+#include "sync/internal_api/public/sessions/update_counters.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/model_neutral_mutable_entry.h"
 #include "sync/syncable/syncable_model_neutral_write_transaction.h"
@@ -285,14 +286,19 @@ void ProcessDownloadedUpdates(
     syncable::ModelNeutralWriteTransaction* trans,
     ModelType type,
     const SyncEntityList& applicable_updates,
-    sessions::StatusController* status) {
+    sessions::StatusController* status,
+    UpdateCounters* counters) {
   for (SyncEntityList::const_iterator update_it = applicable_updates.begin();
        update_it != applicable_updates.end(); ++update_it) {
     DCHECK_EQ(type, GetModelType(**update_it));
-    if (!UpdateContainsNewVersion(trans, **update_it))
+    if (!UpdateContainsNewVersion(trans, **update_it)) {
       status->increment_num_reflected_updates_downloaded_by(1);
-    if ((*update_it)->deleted())
+      counters->num_reflected_updates_received++;
+    }
+    if ((*update_it)->deleted()) {
       status->increment_num_tombstone_updates_downloaded_by(1);
+      counters->num_tombstone_updates_received++;
+    }
     VerifyResult verify_result = VerifyUpdate(trans, **update_it, type);
     if (verify_result != VERIFY_SUCCESS && verify_result != VERIFY_UNDELETE)
       continue;
diff --git a/sync/engine/process_updates_util.h b/sync/engine/process_updates_util.h
index 4cbd103..9564d86 100644
--- a/sync/engine/process_updates_util.h
+++ b/sync/engine/process_updates_util.h
@@ -24,6 +24,8 @@ class ModelNeutralWriteTransaction;
 class Directory;
 }
 
+struct UpdateCounters;
+
 typedef std::vector<const sync_pb::SyncEntity*> SyncEntityList;
 
 // Processes all the updates associated with a single ModelType.
@@ -32,7 +34,8 @@ void ProcessDownloadedUpdates(
     syncable::ModelNeutralWriteTransaction* trans,
     ModelType type,
     const SyncEntityList& applicable_updates,
-    sessions::StatusController* status);
+    sessions::StatusController* status,
+    UpdateCounters* counters);
 
 // Tombstones all entries of |type| whose versions are older than
 // |version_watermark| unless they are type root or unsynced/unapplied.
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 803fd43..b88feb3 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -31,6 +31,9 @@
 #include "sync/internal_api/public/base/cancelation_signal.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/engine/model_safe_worker.h"
+#include "sync/internal_api/public/sessions/commit_counters.h"
+#include "sync/internal_api/public/sessions/status_counters.h"
+#include "sync/internal_api/public/sessions/update_counters.h"
 #include "sync/protocol/bookmark_specifics.pb.h"
 #include "sync/protocol/nigori_specifics.pb.h"
 #include "sync/protocol/preference_specifics.pb.h"
@@ -78,30 +81,11 @@ using syncable::kEncryptedString;
 using syncable::MutableEntry;
 using syncable::WriteTransaction;
 
-using syncable::BASE_VERSION;
 using syncable::CREATE;
 using syncable::GET_BY_HANDLE;
 using syncable::GET_BY_ID;
 using syncable::GET_BY_CLIENT_TAG;
 using syncable::GET_BY_SERVER_TAG;
-using syncable::ID;
-using syncable::IS_DEL;
-using syncable::IS_DIR;
-using syncable::IS_UNAPPLIED_UPDATE;
-using syncable::IS_UNSYNCED;
-using syncable::META_HANDLE;
-using syncable::MTIME;
-using syncable::NON_UNIQUE_NAME;
-using syncable::PARENT_ID;
-using syncable::BASE_SERVER_SPECIFICS;
-using syncable::SERVER_IS_DEL;
-using syncable::SERVER_PARENT_ID;
-using syncable::SERVER_SPECIFICS;
-using syncable::SERVER_VERSION;
-using syncable::UNIQUE_CLIENT_TAG;
-using syncable::UNIQUE_SERVER_TAG;
-using syncable::SPECIFICS;
-using syncable::SYNCING;
 using syncable::UNITTEST;
 
 using sessions::MockDebugInfoGetter;
@@ -109,6 +93,92 @@ using sessions::StatusController;
 using sessions::SyncSessionContext;
 using sessions::SyncSession;
 
+namespace {
+
+// A helper to hold on to the counters emitted by the sync engine.
+class TypeDebugInfoCache : public TypeDebugInfoObserver {
+ public:
+  TypeDebugInfoCache();
+  virtual ~TypeDebugInfoCache();
+
+  CommitCounters GetLatestCommitCounters(ModelType type) const;
+  UpdateCounters GetLatestUpdateCounters(ModelType type) const;
+  StatusCounters GetLatestStatusCounters(ModelType type) const;
+
+  // TypeDebugInfoObserver implementation.
+  virtual void OnCommitCountersUpdated(
+      syncer::ModelType type,
+      const CommitCounters& counters) OVERRIDE;
+  virtual void OnUpdateCountersUpdated(
+      syncer::ModelType type,
+      const UpdateCounters& counters) OVERRIDE;
+  virtual void OnStatusCountersUpdated(
+      syncer::ModelType type,
+      const StatusCounters& counters) OVERRIDE;
+
+ private:
+  std::map<ModelType, CommitCounters> commit_counters_map_;
+  std::map<ModelType, UpdateCounters> update_counters_map_;
+  std::map<ModelType, StatusCounters> status_counters_map_;
+};
+
+TypeDebugInfoCache::TypeDebugInfoCache() {}
+
+TypeDebugInfoCache::~TypeDebugInfoCache() {}
+
+CommitCounters TypeDebugInfoCache::GetLatestCommitCounters(
+    ModelType type) const {
+  std::map<ModelType, CommitCounters>::const_iterator it =
+      commit_counters_map_.find(type);
+  if (it == commit_counters_map_.end()) {
+    return CommitCounters();
+  } else {
+    return it->second;
+  }
+}
+
+UpdateCounters TypeDebugInfoCache::GetLatestUpdateCounters(
+    ModelType type) const {
+  std::map<ModelType, UpdateCounters>::const_iterator it =
+      update_counters_map_.find(type);
+  if (it == update_counters_map_.end()) {
+    return UpdateCounters();
+  } else {
+    return it->second;
+  }
+}
+
+StatusCounters TypeDebugInfoCache::GetLatestStatusCounters(
+    ModelType type) const {
+  std::map<ModelType, StatusCounters>::const_iterator it =
+      status_counters_map_.find(type);
+  if (it == status_counters_map_.end()) {
+    return StatusCounters();
+  } else {
+    return it->second;
+  }
+}
+
+void TypeDebugInfoCache::OnCommitCountersUpdated(
+    syncer::ModelType type,
+    const CommitCounters& counters) {
+  commit_counters_map_[type] = counters;
+}
+
+void TypeDebugInfoCache::OnUpdateCountersUpdated(
+    syncer::ModelType type,
+    const UpdateCounters& counters) {
+  update_counters_map_[type] = counters;
+}
+
+void TypeDebugInfoCache::OnStatusCountersUpdated(
+    syncer::ModelType type,
+    const StatusCounters& counters) {
+  status_counters_map_[type] = counters;
+}
+
+} // namespace
+
 class SyncerTest : public testing::Test,
                    public SyncSession::Delegate,
                    public SyncEngineEventListener {
@@ -224,6 +294,8 @@ class SyncerTest : public testing::Test,
     GetModelSafeRoutingInfo(&routing_info);
 
     model_type_registry_.reset(new ModelTypeRegistry(workers_, directory()));
+    model_type_registry_->RegisterDirectoryTypeDebugInfoObserver(
+        &debug_info_cache_);
 
     context_.reset(
         new SyncSessionContext(
@@ -250,11 +322,14 @@ class SyncerTest : public testing::Test,
   }
 
   virtual void TearDown() {
+    model_type_registry_->UnregisterDirectoryTypeDebugInfoObserver(
+        &debug_info_cache_);
     mock_server_.reset();
     delete syncer_;
     syncer_ = NULL;
     dir_maker_.TearDown();
   }
+
   void WriteTestDataToEntry(WriteTransaction* trans, MutableEntry* entry) {
     EXPECT_FALSE(entry->GetIsDir());
     EXPECT_FALSE(entry->GetIsDel());
@@ -400,8 +475,16 @@ class SyncerTest : public testing::Test,
     }
   }
 
-  const StatusController& status() {
-    return session_->status_controller();
+  CommitCounters GetCommitCounters(ModelType type) {
+    return debug_info_cache_.GetLatestCommitCounters(type);
+  }
+
+  UpdateCounters GetUpdateCounters(ModelType type) {
+    return debug_info_cache_.GetLatestUpdateCounters(type);
+  }
+
+  StatusCounters GetStatusCounters(ModelType type) {
+    return debug_info_cache_.GetLatestStatusCounters(type);
   }
 
   Directory* directory() {
@@ -499,6 +582,7 @@ class SyncerTest : public testing::Test,
   Syncer* syncer_;
 
   scoped_ptr<SyncSession> session_;
+  TypeDebugInfoCache debug_info_cache_;
   scoped_ptr<ModelTypeRegistry> model_type_registry_;
   scoped_ptr<SyncSessionContext> context_;
   bool saw_syncer_event_;
@@ -883,18 +967,29 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
   }
   // First cycle resolves conflicts, second cycle commits changes.
   SyncShareNudge();
-  EXPECT_EQ(2, status().model_neutral_state().num_server_overwrites);
-  EXPECT_EQ(1, status().model_neutral_state().num_local_overwrites);
+  EXPECT_EQ(1, GetUpdateCounters(BOOKMARKS).num_server_overwrites);
+  EXPECT_EQ(1, GetUpdateCounters(PREFERENCES).num_server_overwrites);
+  EXPECT_EQ(1, GetUpdateCounters(BOOKMARKS).num_local_overwrites);
+
   // We successfully commited item(s).
-  EXPECT_EQ(status().model_neutral_state().commit_result, SYNCER_OK);
+  EXPECT_EQ(2, GetCommitCounters(BOOKMARKS).num_commits_attempted);
+  EXPECT_EQ(2, GetCommitCounters(BOOKMARKS).num_commits_success);
+  EXPECT_EQ(1, GetCommitCounters(PREFERENCES).num_commits_attempted);
+  EXPECT_EQ(1, GetCommitCounters(PREFERENCES).num_commits_success);
+
   SyncShareNudge();
 
   // Everything should be resolved now. The local changes should have
   // overwritten the server changes for 2 and 4, while the server changes
   // overwrote the local for entry 3.
-  EXPECT_EQ(0, status().model_neutral_state().num_server_overwrites);
-  EXPECT_EQ(0, status().model_neutral_state().num_local_overwrites);
-  EXPECT_EQ(status().model_neutral_state().commit_result, SYNCER_OK);
+  //
+  // Expect there will be no new overwrites.
+  EXPECT_EQ(1, GetUpdateCounters(BOOKMARKS).num_server_overwrites);
+  EXPECT_EQ(1, GetUpdateCounters(BOOKMARKS).num_local_overwrites);
+
+  EXPECT_EQ(2, GetCommitCounters(BOOKMARKS).num_commits_success);
+  EXPECT_EQ(1, GetCommitCounters(PREFERENCES).num_commits_success);
+
   syncable::ReadTransaction rtrans(FROM_HERE, directory());
   VERIFY_ENTRY(1, false, false, false, 0, 41, 41, ids_, &rtrans);
   VERIFY_ENTRY(2, false, false, false, 1, 31, 31, ids_, &rtrans);
@@ -1655,8 +1750,9 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   SyncShareNudge();
 
   // Id 3 should be in conflict now.
-  EXPECT_EQ(1, status().TotalNumConflictingItems());
-  EXPECT_EQ(1, status().num_hierarchy_conflicts());
+  EXPECT_EQ(
+      1,
+      GetUpdateCounters(BOOKMARKS).num_hierarchy_conflict_application_failures);
 
   // The only request in that loop should have been a GetUpdate.
   // At that point, we didn't know whether or not we had conflicts.
@@ -1680,8 +1776,9 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   SyncShareNudge();
   // The three items with an unresolved parent should be unapplied (3, 9, 100).
   // The name clash should also still be in conflict.
-  EXPECT_EQ(3, status().TotalNumConflictingItems());
-  EXPECT_EQ(3, status().num_hierarchy_conflicts());
+  EXPECT_EQ(
+      3,
+      GetUpdateCounters(BOOKMARKS).num_hierarchy_conflict_application_failures);
 
   // This time around, we knew that there were conflicts.
   ASSERT_TRUE(mock_server_->last_request().has_get_updates());
@@ -1778,8 +1875,9 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   }
 
   EXPECT_FALSE(saw_syncer_event_);
-  EXPECT_EQ(4, status().TotalNumConflictingItems());
-  EXPECT_EQ(4, status().num_hierarchy_conflicts());
+  EXPECT_EQ(
+      4,
+      GetUpdateCounters(BOOKMARKS).num_hierarchy_conflict_application_failures);
 }
 
 // A commit with a lost response produces an update that has to be reunited with
@@ -2410,7 +2508,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) {
     existing.PutIsDel(true);
   }
   SyncShareNudge();
-  EXPECT_EQ(0, status().num_server_conflicts());
+  EXPECT_EQ(0, GetCommitCounters(BOOKMARKS).num_commits_conflict);
 }
 
 TEST_F(SyncerTest, DeletingEntryWithLocalEdits) {