summaryrefslogtreecommitdiffstats
path: root/sync/engine
diff options
context:
space:
mode:
Diffstat (limited to 'sync/engine')
-rw-r--r--sync/engine/conflict_resolver.cc13
-rw-r--r--sync/engine/conflict_resolver.h7
-rw-r--r--sync/engine/directory_commit_contribution.cc10
-rw-r--r--sync/engine/directory_commit_contribution_unittest.cc15
-rw-r--r--sync/engine/directory_update_handler.cc25
-rw-r--r--sync/engine/directory_update_handler_unittest.cc86
-rw-r--r--sync/engine/process_updates_util.cc12
-rw-r--r--sync/engine/process_updates_util.h5
-rw-r--r--sync/engine/syncer_unittest.cc166
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) {
generated by cgit v1.1 at 2024-09-09 21:29:54 +0000