diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-06 00:20:23 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-06 00:20:23 +0000 |
commit | ff390b8451249f70326d38fe12ed1166f26164d2 (patch) | |
tree | ae0854550e3e0e6bdc12c49f28530082c97aa1b0 /sync/engine | |
parent | 574eedcdeb1f43a7f26f647f2f4c87cb9cc73c35 (diff) | |
download | chromium_src-ff390b8451249f70326d38fe12ed1166f26164d2.zip chromium_src-ff390b8451249f70326d38fe12ed1166f26164d2.tar.gz chromium_src-ff390b8451249f70326d38fe12ed1166f26164d2.tar.bz2 |
sync: Remove ConflictProgress
This change removes the ConflictProgress struct from the
PerModelSafeGroupState. The struct was intended to pass state between
the update application and conflict resolution commands. This part of
its functionality has been replaced with a vector of simple conflict
item IDs.
The ConflictProgress struct also had an important role to play in stats
gathering and unit tests. To replace it, the update applicator has been
updated to store its counts directly in the StatusController. Unlike
ConflictProgress, this state is not thread-local. That's acceptable
because we can guarantee it will only be read or written by one thread
at a time and those threads use appropriate thread-safety mechanisms to
transfer control to each other.
This change is another step towards merging update application and
conflict resolution.
BUG=147681
Review URL: https://chromiumcodereview.appspot.com/11049002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@160530 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/engine')
-rw-r--r-- | sync/engine/apply_updates_command.cc | 5 | ||||
-rw-r--r-- | sync/engine/apply_updates_command_unittest.cc | 118 | ||||
-rw-r--r-- | sync/engine/process_commit_response_command.cc | 5 | ||||
-rw-r--r-- | sync/engine/resolve_conflicts_command.cc | 8 | ||||
-rw-r--r-- | sync/engine/resolve_conflicts_command_unittest.cc | 51 | ||||
-rw-r--r-- | sync/engine/syncer.cc | 5 | ||||
-rw-r--r-- | sync/engine/syncer_unittest.cc | 24 | ||||
-rw-r--r-- | sync/engine/update_applicator.cc | 18 | ||||
-rw-r--r-- | sync/engine/update_applicator.h | 10 |
9 files changed, 78 insertions, 166 deletions
diff --git a/sync/engine/apply_updates_command.cc b/sync/engine/apply_updates_command.cc index 639aef7..e43f071 100644 --- a/sync/engine/apply_updates_command.cc +++ b/sync/engine/apply_updates_command.cc @@ -69,9 +69,10 @@ SyncerError ApplyUpdatesCommand::ModelChangingExecuteImpl( dir->GetCryptographer(&trans), session->routing_info(), session->status_controller().group_restriction()); - applicator.AttemptApplications(&trans, handles); + applicator.AttemptApplications(&trans, handles, + session->mutable_status_controller()); applicator.SaveProgressIntoSessionState( - session->mutable_status_controller()->mutable_conflict_progress(), + session->mutable_status_controller()->mutable_simple_conflict_ids(), session->mutable_status_controller()->mutable_update_progress()); // This might be the first time we've fully completed a sync cycle, for diff --git a/sync/engine/apply_updates_command_unittest.cc b/sync/engine/apply_updates_command_unittest.cc index eee3edb..01ddd93 100644 --- a/sync/engine/apply_updates_command_unittest.cc +++ b/sync/engine/apply_updates_command_unittest.cc @@ -83,17 +83,16 @@ TEST_F(ApplyUpdatesCommandTest, Simple) { sessions::StatusController* status = session()->mutable_status_controller(); + EXPECT_EQ(0, status->num_simple_conflicts()) + << "Simple update shouldn't result in conflicts"; + EXPECT_EQ(0, status->num_encryption_conflicts()) + << "Simple update shouldn't result in conflicts"; + EXPECT_EQ(0, status->num_hierarchy_conflicts()) + << "Simple update shouldn't result in conflicts"; sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); ASSERT_TRUE(status->update_progress()); EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()) - << "Simple update shouldn't result in conflicts"; - EXPECT_EQ(0, status->conflict_progress()->EncryptionConflictingItemsSize()) - << "Simple update shouldn't result in conflicts"; - EXPECT_EQ(0, status->conflict_progress()->HierarchyConflictingItemsSize()) - << "Simple update shouldn't result in conflicts"; EXPECT_EQ(2, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "All items should have been successfully applied"; } @@ -117,13 +116,12 @@ TEST_F(ApplyUpdatesCommandTest, UpdateWithChildrenBeforeParents) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); + EXPECT_EQ(0, status->num_simple_conflicts()) + << "Simple update shouldn't result in conflicts, even if out-of-order"; sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); ASSERT_TRUE(status->update_progress()); EXPECT_EQ(5, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()) - << "Simple update shouldn't result in conflicts, even if out-of-order"; EXPECT_EQ(5, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "All updates should have been successfully applied"; } @@ -139,9 +137,7 @@ TEST_F(ApplyUpdatesCommandTest, SimpleConflict) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(1, status->conflict_progress()->SimpleConflictingItemsSize()) + EXPECT_EQ(1, status->num_simple_conflicts()) << "Unsynced and unapplied item should be a simple conflict"; } @@ -169,15 +165,14 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyAndSimpleConflict) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - - EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()); // An update that is both a simple conflict and a hierarchy conflict should be // treated as a hierarchy conflict. - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()); + EXPECT_EQ(1, status->num_hierarchy_conflicts()); + EXPECT_EQ(0, status->num_simple_conflicts()); + + sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()); } @@ -216,14 +211,13 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyConflictDirectoryLoop) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - - EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()); // This should count as a hierarchy conflict. - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()); + EXPECT_EQ(1, status->num_hierarchy_conflicts()); + EXPECT_EQ(0, status->num_simple_conflicts()); + + sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()); } // Runs the ApplyUpdatesCommand on a directory where the server sent us an @@ -254,12 +248,8 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyConflictDeletedParent) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - - // This should count as a hierarchy conflict. - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()); + EXPECT_EQ(1, status->num_hierarchy_conflicts()); + EXPECT_EQ(0, status->num_simple_conflicts()); } // Runs the ApplyUpdatesCommand on a directory where the server is trying to @@ -296,12 +286,9 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyConflictDeleteNonEmptyDirectory) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - // This should count as a hierarchy conflict. - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()); + EXPECT_EQ(1, status->num_hierarchy_conflicts()); + EXPECT_EQ(0, status->num_simple_conflicts()); } // Runs the ApplyUpdatesCommand on a server-created item that has a locally @@ -318,16 +305,17 @@ TEST_F(ApplyUpdatesCommandTest, HierarchyConflictUnknownParent) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); + + EXPECT_EQ(0, status->num_simple_conflicts()) + << "Updates with unknown parent should not be treated as 'simple'" + << " conflicts"; + EXPECT_EQ(2, status->num_hierarchy_conflicts()) + << "All updates with an unknown ancestors should be in conflict"; + sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); ASSERT_TRUE(status->update_progress()); EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()) - << "Updates with unknown parent should not be treated as 'simple'" - << " conflicts"; - EXPECT_EQ(2, status->conflict_progress()->HierarchyConflictingItemsSize()) - << "All updates with an unknown ancestors should be in conflict"; EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "No item with an unknown ancestor should be applied"; } @@ -352,13 +340,14 @@ TEST_F(ApplyUpdatesCommandTest, ItemsBothKnownAndUnknown) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); + + EXPECT_EQ(2, status->num_hierarchy_conflicts()) + << "The updates with unknown ancestors should be in conflict"; + sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); ASSERT_TRUE(status->update_progress()); EXPECT_EQ(6, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(2, status->conflict_progress()->HierarchyConflictingItemsSize()) - << "The updates with unknown ancestors should be in conflict"; EXPECT_EQ(4, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The updates with known ancestors should be successfully applied"; } @@ -388,13 +377,14 @@ TEST_F(ApplyUpdatesCommandTest, DecryptablePassword) { apply_updates_command_.ExecuteImpl(session()); sessions::StatusController* status = session()->mutable_status_controller(); + + EXPECT_EQ(0, status->num_simple_conflicts()) + << "No update should be in conflict because they're all decryptable"; + sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD); ASSERT_TRUE(status->update_progress()); EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()) - << "No update should be in conflict because they're all decryptable"; EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The updates that can be decrypted should be applied"; } @@ -419,18 +409,15 @@ TEST_F(ApplyUpdatesCommandTest, UndecryptableData) { EXPECT_TRUE(status->HasConflictingUpdates()) << "Updates that can't be decrypted should trigger the syncer to have " << "conflicting updates."; + EXPECT_EQ(0, status->num_simple_conflicts()) + << "Updates that can't be decrypted should not be in regular conflict"; + EXPECT_EQ(3, status->num_encryption_conflicts()) + << "Updates that can't be decrypted should be in encryption conflict"; { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); ASSERT_TRUE(status->update_progress()); EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()) - << "The updates that can't be decrypted should not be in regular " - << "conflict"; - EXPECT_EQ(2, status->conflict_progress()->EncryptionConflictingItemsSize()) - << "The updates that can't be decrypted should be in encryption " - << "conflict"; EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "No update that can't be decrypted should be applied"; } @@ -439,13 +426,6 @@ TEST_F(ApplyUpdatesCommandTest, UndecryptableData) { ASSERT_TRUE(status->update_progress()); EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()) - << "The updates that can't be decrypted should not be in regular " - << "conflict"; - EXPECT_EQ(1, status->conflict_progress()->EncryptionConflictingItemsSize()) - << "The updates that can't be decrypted should be in encryption " - << "conflict"; EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "No update that can't be decrypted should be applied"; } @@ -493,17 +473,17 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) { << "Updates that can't be decrypted should trigger the syncer to have " << "conflicting updates."; { - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD); - ASSERT_TRUE(status->update_progress()); - EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) - << "All updates should have been attempted"; - ASSERT_TRUE(status->conflict_progress()); - EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize()) + EXPECT_EQ(0, status->num_simple_conflicts()) << "The updates that can't be decrypted should not be in regular " << "conflict"; - EXPECT_EQ(1, status->conflict_progress()->EncryptionConflictingItemsSize()) + EXPECT_EQ(1, status->num_encryption_conflicts()) << "The updates that can't be decrypted should be in encryption " << "conflict"; + + sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD); + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) + << "All updates should have been attempted"; EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The undecryptable password update shouldn't be applied"; } diff --git a/sync/engine/process_commit_response_command.cc b/sync/engine/process_commit_response_command.cc index 8a00608..2f84619 100644 --- a/sync/engine/process_commit_response_command.cc +++ b/sync/engine/process_commit_response_command.cc @@ -32,7 +32,6 @@ namespace syncer { using sessions::OrderedCommitSet; using sessions::StatusController; using sessions::SyncSession; -using sessions::ConflictProgress; using syncable::WriteTransaction; using syncable::MutableEntry; using syncable::Entry; @@ -110,7 +109,6 @@ SyncerError ProcessCommitResponseCommand::ProcessCommitResponse( int successes = 0; set<syncable::Id> deleted_folders; - ConflictProgress* conflict_progress = status->mutable_conflict_progress(); OrderedCommitSet::Projection proj = status->commit_id_projection( commit_set_); @@ -129,8 +127,7 @@ SyncerError ProcessCommitResponseCommand::ProcessCommitResponse( break; case CommitResponse::CONFLICT: ++conflicting_commits; - conflict_progress->AddServerConflictingItemById( - commit_set_.GetCommitIdAt(proj[i])); + status->increment_num_server_conflicts(); break; case CommitResponse::SUCCESS: // TODO(sync): worry about sync_rate_ rate calc? diff --git a/sync/engine/resolve_conflicts_command.cc b/sync/engine/resolve_conflicts_command.cc index 3adfa9d..5b7f5e2 100644 --- a/sync/engine/resolve_conflicts_command.cc +++ b/sync/engine/resolve_conflicts_command.cc @@ -27,14 +27,14 @@ SyncerError ResolveConflictsCommand::ModelChangingExecuteImpl( syncable::Directory* dir = session->context()->directory(); sessions::StatusController* status = session->mutable_status_controller(); - const sessions::ConflictProgress* progress = status->conflict_progress(); - if (!progress) - return SYNCER_OK; // Nothing to do. + const std::set<syncable::Id>* simple_conflict_ids = + status->simple_conflict_ids(); + syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir); const Cryptographer* cryptographer = dir->GetCryptographer(&trans); status->update_conflicts_resolved( resolver->ResolveConflicts(&trans, cryptographer, - progress->SimpleConflictingItems(), status)); + *simple_conflict_ids, status)); return SYNCER_OK; } diff --git a/sync/engine/resolve_conflicts_command_unittest.cc b/sync/engine/resolve_conflicts_command_unittest.cc deleted file mode 100644 index 7a9bc14..0000000 --- a/sync/engine/resolve_conflicts_command_unittest.cc +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/basictypes.h" -#include "base/memory/ref_counted.h" -#include "sync/engine/resolve_conflicts_command.h" -#include "sync/internal_api/public/base/model_type.h" -#include "sync/sessions/sync_session.h" -#include "sync/syncable/syncable_id.h" -#include "sync/test/engine/fake_model_worker.h" -#include "sync/test/engine/syncer_command_test.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace syncer { - -namespace { - -class ResolveConflictsCommandTest : public SyncerCommandTest { - protected: - ResolveConflictsCommandTest() {} - virtual ~ResolveConflictsCommandTest() {} - - virtual void SetUp() { - workers()->push_back( - make_scoped_refptr(new FakeModelWorker(GROUP_UI))); - workers()->push_back( - make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD))); - (*mutable_routing_info())[BOOKMARKS] = GROUP_UI; - (*mutable_routing_info())[PASSWORDS] = GROUP_PASSWORD; - SyncerCommandTest::SetUp(); - } - - ResolveConflictsCommand command_; - - private: - DISALLOW_COPY_AND_ASSIGN(ResolveConflictsCommandTest); -}; - -TEST_F(ResolveConflictsCommandTest, GetGroupsToChange) { - ExpectNoGroupsToChange(command_); - // Put GROUP_PASSWORD in conflict. - session()->mutable_status_controller()-> - GetUnrestrictedMutableConflictProgressForTest(GROUP_PASSWORD)-> - AddSimpleConflictingItemById(syncable::Id()); - ExpectGroupToChange(command_, GROUP_PASSWORD); -} - -} // namespace - -} // namespace syncer diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc index adb34b4..af8e3a9 100644 --- a/sync/engine/syncer.cc +++ b/sync/engine/syncer.cc @@ -36,7 +36,6 @@ namespace syncer { using sessions::ScopedSessionContextConflictResolver; using sessions::StatusController; using sessions::SyncSession; -using sessions::ConflictProgress; using syncable::IS_UNAPPLIED_UPDATE; using syncable::SERVER_CTIME; using syncable::SERVER_IS_DEL; @@ -197,10 +196,10 @@ void Syncer::SyncShare(sessions::SyncSession* session, // We only care to resolve conflicts again if we made progress on the // simple conflicts. int before_blocking_conflicting_updates = - status->TotalNumSimpleConflictingItems(); + status->num_simple_conflicts(); apply_updates.Execute(session); int after_blocking_conflicting_updates = - status->TotalNumSimpleConflictingItems(); + status->num_simple_conflicts(); // If the following call sets the conflicts_resolved value to true, // SyncSession::HasMoreToSync() will send us into another sync cycle // after this one completes. diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index da4f01c0..c57def1 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -105,7 +105,6 @@ using syncable::SPECIFICS; using syncable::SYNCING; using syncable::UNITTEST; -using sessions::ConflictProgress; using sessions::ScopedSetSessionWriteTransaction; using sessions::StatusController; using sessions::SyncSessionContext; @@ -1583,12 +1582,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { // Id 3 should be in conflict now. EXPECT_EQ(1, status().TotalNumConflictingItems()); - { - sessions::ScopedModelSafeGroupRestriction r( - session_->mutable_status_controller(), GROUP_PASSIVE); - ASSERT_TRUE(status().conflict_progress()); - EXPECT_EQ(1, status().conflict_progress()->HierarchyConflictingItemsSize()); - } + EXPECT_EQ(1, status().num_hierarchy_conflicts()); // These entries will be used in the second set of updates. mock_server_->AddUpdateDirectory(4, 0, "newer_version", 20, 10); @@ -1602,12 +1596,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { // 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()); - { - sessions::ScopedModelSafeGroupRestriction r( - session_->mutable_status_controller(), GROUP_PASSIVE); - ASSERT_TRUE(status().conflict_progress()); - EXPECT_EQ(3, status().conflict_progress()->HierarchyConflictingItemsSize()); - } + EXPECT_EQ(3, status().num_hierarchy_conflicts()); { WriteTransaction trans(FROM_HERE, UNITTEST, directory()); @@ -1697,12 +1686,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { EXPECT_FALSE(saw_syncer_event_); EXPECT_EQ(4, status().TotalNumConflictingItems()); - { - sessions::ScopedModelSafeGroupRestriction r( - session_->mutable_status_controller(), GROUP_PASSIVE); - ASSERT_TRUE(status().conflict_progress()); - EXPECT_EQ(4, status().conflict_progress()->HierarchyConflictingItemsSize()); - } + EXPECT_EQ(4, status().num_hierarchy_conflicts()); } TEST_F(SyncerTest, CommitTimeRename) { @@ -2392,7 +2376,7 @@ TEST_F(SyncerTest, DeletingEntryInFolder) { existing.Put(IS_DEL, true); } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_EQ(0, status().TotalNumServerConflictingItems()); + EXPECT_EQ(0, status().num_server_conflicts()); } TEST_F(SyncerTest, DeletingEntryWithLocalEdits) { diff --git a/sync/engine/update_applicator.cc b/sync/engine/update_applicator.cc index 3dedc14..cb09fc7 100644 --- a/sync/engine/update_applicator.cc +++ b/sync/engine/update_applicator.cc @@ -45,7 +45,8 @@ UpdateApplicator::~UpdateApplicator() { // don't bother re-processing them on subsequent passes. void UpdateApplicator::AttemptApplications( syncable::WriteTransaction* trans, - const std::vector<int64>& handles) { + const std::vector<int64>& handles, + sessions::StatusController* status) { std::vector<int64> to_apply = handles; DVLOG(1) << "UpdateApplicator running over " << to_apply.size() << " items."; while (!to_apply.empty()) { @@ -64,12 +65,14 @@ void UpdateApplicator::AttemptApplications( switch (result) { case SUCCESS: application_results_.AddSuccess(entry.Get(ID)); + status->increment_num_updates_applied(); break; case CONFLICT_SIMPLE: application_results_.AddSimpleConflict(entry.Get(ID)); break; case CONFLICT_ENCRYPTION: application_results_.AddEncryptionConflict(entry.Get(ID)); + status->increment_num_encryption_conflicts(); break; case CONFLICT_HIERARCHY: application_results_.AddHierarchyConflict(entry.Get(ID)); @@ -86,7 +89,7 @@ void UpdateApplicator::AttemptApplications( if (to_reapply.size() == to_apply.size()) { // We made no progress. Must be stubborn hierarchy conflicts. - // Break out early, leaving some updates unapplied. + status->set_num_hierarchy_conflicts(to_apply.size()); break; } @@ -119,9 +122,9 @@ bool UpdateApplicator::SkipUpdate(const syncable::Entry& entry) { } void UpdateApplicator::SaveProgressIntoSessionState( - sessions::ConflictProgress* conflict_progress, + std::set<syncable::Id>* simple_conflict_ids, sessions::UpdateProgress* update_progress) { - application_results_.SaveProgress(conflict_progress, update_progress); + application_results_.SaveProgress(simple_conflict_ids, update_progress); } UpdateApplicator::ResultTracker::ResultTracker() { @@ -147,25 +150,22 @@ void UpdateApplicator::ResultTracker::AddSuccess(syncable::Id id) { } void UpdateApplicator::ResultTracker::SaveProgress( - sessions::ConflictProgress* conflict_progress, + std::set<syncable::Id>* simple_conflict_ids, sessions::UpdateProgress* update_progress) { std::set<syncable::Id>::const_iterator i; + *simple_conflict_ids = conflicting_ids_; for (i = conflicting_ids_.begin(); i != conflicting_ids_.end(); ++i) { - conflict_progress->AddSimpleConflictingItemById(*i); update_progress->AddAppliedUpdate(CONFLICT_SIMPLE, *i); } for (i = encryption_conflict_ids_.begin(); i != encryption_conflict_ids_.end(); ++i) { - conflict_progress->AddEncryptionConflictingItemById(*i); update_progress->AddAppliedUpdate(CONFLICT_ENCRYPTION, *i); } for (i = hierarchy_conflict_ids_.begin(); i != hierarchy_conflict_ids_.end(); ++i) { - conflict_progress->AddHierarchyConflictingItemById(*i); update_progress->AddAppliedUpdate(CONFLICT_HIERARCHY, *i); } for (i = successful_ids_.begin(); i != successful_ids_.end(); ++i) { - conflict_progress->EraseSimpleConflictingItemById(*i); update_progress->AddAppliedUpdate(SUCCESS, *i); } } diff --git a/sync/engine/update_applicator.h b/sync/engine/update_applicator.h index 36a907f..290a76c 100644 --- a/sync/engine/update_applicator.h +++ b/sync/engine/update_applicator.h @@ -17,11 +17,12 @@ #include "base/port.h" #include "sync/internal_api/public/engine/model_safe_worker.h" #include "sync/syncable/syncable_id.h" +#include "sync/sessions/status_controller.h" namespace syncer { namespace sessions { -class ConflictProgress; +class StatusController; class UpdateProgress; } @@ -44,14 +45,15 @@ class UpdateApplicator { // Attempt to apply the specified updates. void AttemptApplications(syncable::WriteTransaction* trans, - const std::vector<int64>& handles); + const std::vector<int64>& handles, + sessions::StatusController* status); // This class does not automatically save its progress into the // SyncSession -- to get that to happen, call this method after update // application is finished (i.e., when AttemptOneAllocation stops returning // true). void SaveProgressIntoSessionState( - sessions::ConflictProgress* conflict_progress, + std::set<syncable::Id>* simple_conflict_ids, sessions::UpdateProgress* update_progress); private: @@ -64,7 +66,7 @@ class UpdateApplicator { void AddEncryptionConflict(syncable::Id); void AddHierarchyConflict(syncable::Id); void AddSuccess(syncable::Id); - void SaveProgress(sessions::ConflictProgress* conflict_progress, + void SaveProgress(std::set<syncable::Id>* simple_conflict_ids, sessions::UpdateProgress* update_progress); void ClearHierarchyConflicts(); |