diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-17 23:58:41 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-17 23:58:41 +0000 |
commit | 16449b24f3f63906f32f6a89032617c73ff72edf (patch) | |
tree | cde2539a385947dd3bf11ba686316a3e946f025a /sync/sessions | |
parent | 9835d34c8ece44b0dabcf650310a1bb8b7d017c6 (diff) | |
download | chromium_src-16449b24f3f63906f32f6a89032617c73ff72edf.zip chromium_src-16449b24f3f63906f32f6a89032617c73ff72edf.tar.gz chromium_src-16449b24f3f63906f32f6a89032617c73ff72edf.tar.bz2 |
sync: Remove UpdateProgress and related code
Following the merge of the process updates and verify updates step,
there is no longer any need for the UpdateProgress class. Its only use
outside of tests was to pass data between the verify and update steps
during a sync cycle.
The class had some use in tests, but most of the assertions based on it
were somewhat redundant. They can be replaced by assertions on similar,
but non-ModelSafe, counts.
BUG=156238
Review URL: https://chromiumcodereview.appspot.com/11190013
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162576 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/sessions')
-rw-r--r-- | sync/sessions/session_state.cc | 57 | ||||
-rw-r--r-- | sync/sessions/session_state.h | 52 | ||||
-rw-r--r-- | sync/sessions/status_controller.cc | 37 | ||||
-rw-r--r-- | sync/sessions/status_controller.h | 9 | ||||
-rw-r--r-- | sync/sessions/status_controller_unittest.cc | 28 |
5 files changed, 13 insertions, 170 deletions
diff --git a/sync/sessions/session_state.cc b/sync/sessions/session_state.cc index f5d1709..a08b093 100644 --- a/sync/sessions/session_state.cc +++ b/sync/sessions/session_state.cc @@ -16,63 +16,6 @@ using std::vector; namespace syncer { namespace sessions { -UpdateProgress::UpdateProgress() {} - -UpdateProgress::~UpdateProgress() {} - -void UpdateProgress::AddVerifyResult(const VerifyResult& verify_result, - const sync_pb::SyncEntity& entity) { - verified_updates_.push_back(std::make_pair(verify_result, entity)); -} - -void UpdateProgress::AddAppliedUpdate(const UpdateAttemptResponse& response, - const syncable::Id& id) { - applied_updates_.push_back(std::make_pair(response, id)); -} - -std::vector<AppliedUpdate>::iterator UpdateProgress::AppliedUpdatesBegin() { - return applied_updates_.begin(); -} - -std::vector<VerifiedUpdate>::const_iterator -UpdateProgress::VerifiedUpdatesBegin() const { - return verified_updates_.begin(); -} - -std::vector<AppliedUpdate>::const_iterator -UpdateProgress::AppliedUpdatesEnd() const { - return applied_updates_.end(); -} - -std::vector<VerifiedUpdate>::const_iterator -UpdateProgress::VerifiedUpdatesEnd() const { - return verified_updates_.end(); -} - -int UpdateProgress::SuccessfullyAppliedUpdateCount() const { - int count = 0; - for (std::vector<AppliedUpdate>::const_iterator it = - applied_updates_.begin(); - it != applied_updates_.end(); - ++it) { - if (it->first == SUCCESS) - count++; - } - return count; -} - -// Returns true if at least one update application failed due to a conflict -// during this sync cycle. -bool UpdateProgress::HasConflictingUpdates() const { - std::vector<AppliedUpdate>::const_iterator it; - for (it = applied_updates_.begin(); it != applied_updates_.end(); ++it) { - if (it->first != SUCCESS) { - return true; - } - } - return false; -} - PerModelSafeGroupState::PerModelSafeGroupState() { } diff --git a/sync/sessions/session_state.h b/sync/sessions/session_state.h index 548b4ed..4ef6860 100644 --- a/sync/sessions/session_state.h +++ b/sync/sessions/session_state.h @@ -13,69 +13,17 @@ #define SYNC_SESSIONS_SESSION_STATE_H_ #include <set> -#include <vector> -#include "sync/engine/syncer_types.h" -#include "sync/protocol/sync.pb.h" #include "sync/syncable/syncable_id.h" namespace syncer { namespace sessions { -typedef std::pair<VerifyResult, sync_pb::SyncEntity> VerifiedUpdate; -typedef std::pair<UpdateAttemptResponse, syncable::Id> AppliedUpdate; - -// Tracks update application and verification. -class UpdateProgress { - public: - UpdateProgress(); - ~UpdateProgress(); - - void AddVerifyResult(const VerifyResult& verify_result, - const sync_pb::SyncEntity& entity); - - // Log a successful or failing update attempt. - void AddAppliedUpdate(const UpdateAttemptResponse& response, - const syncable::Id& id); - - // Various iterators. - std::vector<AppliedUpdate>::iterator AppliedUpdatesBegin(); - std::vector<VerifiedUpdate>::const_iterator VerifiedUpdatesBegin() const; - std::vector<AppliedUpdate>::const_iterator AppliedUpdatesEnd() const; - std::vector<VerifiedUpdate>::const_iterator VerifiedUpdatesEnd() const; - - // Returns the number of update application attempts. This includes both - // failures and successes. - int AppliedUpdatesSize() const { return applied_updates_.size(); } - int VerifiedUpdatesSize() const { return verified_updates_.size(); } - bool HasVerifiedUpdates() const { return !verified_updates_.empty(); } - bool HasAppliedUpdates() const { return !applied_updates_.empty(); } - void ClearVerifiedUpdates() { verified_updates_.clear(); } - - // Count the number of successful update applications that have happend this - // cycle. Note that if an item is successfully applied twice, it will be - // double counted here. - int SuccessfullyAppliedUpdateCount() const; - - // Returns true if at least one update application failed due to a conflict - // during this sync cycle. - bool HasConflictingUpdates() const; - - private: - // Container for updates that passed verification. - std::vector<VerifiedUpdate> verified_updates_; - - // Stores the result of the various ApplyUpdate attempts we've made. - // May contain duplicate entries. - std::vector<AppliedUpdate> applied_updates_; -}; - // Grouping of all state that applies to a single ModelSafeGroup. struct PerModelSafeGroupState { explicit PerModelSafeGroupState(); ~PerModelSafeGroupState(); - UpdateProgress update_progress; std::set<syncable::Id> simple_conflict_ids; }; diff --git a/sync/sessions/status_controller.cc b/sync/sessions/status_controller.cc index 93a37f1..f47b085 100644 --- a/sync/sessions/status_controller.cc +++ b/sync/sessions/status_controller.cc @@ -22,17 +22,6 @@ StatusController::StatusController(const ModelSafeRoutingInfo& routes) StatusController::~StatusController() {} -const UpdateProgress* StatusController::update_progress() const { - const PerModelSafeGroupState* state = - GetModelSafeGroupState(true, group_restriction_); - return state ? &state->update_progress : NULL; -} - -UpdateProgress* StatusController::mutable_update_progress() { - return &GetOrCreateModelSafeGroupState( - true, group_restriction_)->update_progress; -} - const std::set<syncable::Id>* StatusController::simple_conflict_ids() const { const PerModelSafeGroupState* state = GetModelSafeGroupState(true, group_restriction_); @@ -51,18 +40,6 @@ const std::set<syncable::Id>* return state ? &state->simple_conflict_ids : NULL; } -const UpdateProgress* StatusController::GetUnrestrictedUpdateProgress( - ModelSafeGroup group) const { - const PerModelSafeGroupState* state = GetModelSafeGroupState(false, group); - return state ? &state->update_progress : NULL; -} - -UpdateProgress* - StatusController::GetUnrestrictedMutableUpdateProgressForTest( - ModelSafeGroup group) { - return &GetOrCreateModelSafeGroupState(false, group)->update_progress; -} - const PerModelSafeGroupState* StatusController::GetModelSafeGroupState( bool restrict, ModelSafeGroup group) const { DCHECK_EQ(restrict, group_restriction_in_effect_); @@ -187,15 +164,11 @@ int64 StatusController::CountUpdates() const { } bool StatusController::HasConflictingUpdates() const { - DCHECK(!group_restriction_in_effect_) - << "HasConflictingUpdates applies to all ModelSafeGroups"; - std::map<ModelSafeGroup, PerModelSafeGroupState*>::const_iterator it = - per_model_group_.begin(); - for (; it != per_model_group_.end(); ++it) { - if (it->second->update_progress.HasConflictingUpdates()) - return true; - } - return false; + return TotalNumConflictingItems() > 0; +} + +int StatusController::num_updates_applied() const { + return model_neutral_.num_updates_applied; } int StatusController::num_encryption_conflicts() const { diff --git a/sync/sessions/status_controller.h b/sync/sessions/status_controller.h index c18fc7d..1465dbf 100644 --- a/sync/sessions/status_controller.h +++ b/sync/sessions/status_controller.h @@ -55,14 +55,8 @@ class StatusController { // auto-create. const std::set<syncable::Id>* simple_conflict_ids() const; std::set<syncable::Id>* mutable_simple_conflict_ids(); - const UpdateProgress* update_progress() const; - UpdateProgress* mutable_update_progress(); const std::set<syncable::Id>* GetUnrestrictedSimpleConflictIds( ModelSafeGroup group) const; - const UpdateProgress* GetUnrestrictedUpdateProgress( - ModelSafeGroup group) const; - UpdateProgress* GetUnrestrictedMutableUpdateProgressForTest( - ModelSafeGroup group); // ClientToServer messages. const ModelTypeSet updates_request_types() const { @@ -110,6 +104,9 @@ class StatusController { // Aggregate sum of all conflicting items over all conflict types. int TotalNumConflictingItems() const; + // Number of successfully applied updates. + int num_updates_applied() const; + // Returns the number of updates received from the sync server. int64 CountUpdates() const; diff --git a/sync/sessions/status_controller_unittest.cc b/sync/sessions/status_controller_unittest.cc index 619e1fa..3e5143b 100644 --- a/sync/sessions/status_controller_unittest.cc +++ b/sync/sessions/status_controller_unittest.cc @@ -45,37 +45,22 @@ TEST_F(StatusControllerTest, ReadYourWrites) { TEST_F(StatusControllerTest, HasConflictingUpdates) { StatusController status(routes_); EXPECT_FALSE(status.HasConflictingUpdates()); + { ScopedModelSafeGroupRestriction r(&status, GROUP_UI); - EXPECT_FALSE(status.update_progress()); - status.mutable_update_progress()->AddAppliedUpdate(SUCCESS, - syncable::Id()); - status.mutable_update_progress()->AddAppliedUpdate(CONFLICT_SIMPLE, - syncable::Id()); - EXPECT_TRUE(status.update_progress()->HasConflictingUpdates()); + status.increment_num_updates_applied(); + status.mutable_simple_conflict_ids()->insert(syncable::Id()); } EXPECT_TRUE(status.HasConflictingUpdates()); - - { - ScopedModelSafeGroupRestriction r(&status, GROUP_PASSIVE); - EXPECT_FALSE(status.update_progress()); - } } TEST_F(StatusControllerTest, HasConflictingUpdates_NonBlockingUpdates) { StatusController status(routes_); EXPECT_FALSE(status.HasConflictingUpdates()); - { - ScopedModelSafeGroupRestriction r(&status, GROUP_UI); - EXPECT_FALSE(status.update_progress()); - status.mutable_update_progress()->AddAppliedUpdate(SUCCESS, - syncable::Id()); - status.mutable_update_progress()->AddAppliedUpdate(CONFLICT_HIERARCHY, - syncable::Id()); - EXPECT_TRUE(status.update_progress()->HasConflictingUpdates()); - } + status.increment_num_updates_applied(); + status.increment_num_encryption_conflicts(); EXPECT_TRUE(status.HasConflictingUpdates()); } @@ -116,9 +101,6 @@ TEST_F(StatusControllerTest, TotalNumConflictingItems) { // Basic test that non group-restricted state accessors don't cause violations. TEST_F(StatusControllerTest, Unrestricted) { StatusController status(routes_); - const UpdateProgress* progress = - status.GetUnrestrictedUpdateProgress(GROUP_UI); - EXPECT_FALSE(progress); status.model_neutral_state(); status.download_updates_succeeded(); status.ServerSaysNothingMoreToDownload(); |