diff options
| author | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-23 07:55:20 +0000 |
|---|---|---|
| committer | akalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-23 07:55:20 +0000 |
| commit | 74a82ca6ee3940312df1a0a966a6cf9eafcf3314 (patch) | |
| tree | a5179d1b4a79c9950666e2b4fdae73af291f41cc /chrome/browser/sync/engine | |
| parent | 7477b27103e20e1898b1c2846f2fc3e08d498576 (diff) | |
| download | chromium_src-74a82ca6ee3940312df1a0a966a6cf9eafcf3314.zip chromium_src-74a82ca6ee3940312df1a0a966a6cf9eafcf3314.tar.gz chromium_src-74a82ca6ee3940312df1a0a966a6cf9eafcf3314.tar.bz2 | |
[Sync] Made some sync session member functions const
In particular, split SyncSession::status_controller() into status_controller()
and mutable_status_controller().
Also remove some dead code.
Propagate const methods throughout sync code.
This is in preparation for an upcoming change that makes
ModelChangingSyncerCommand post on only the threads it needs to.
BUG=97832
TEST=
Review URL: http://codereview.chromium.org/8638001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@111329 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync/engine')
24 files changed, 275 insertions, 306 deletions
diff --git a/chrome/browser/sync/engine/apply_updates_command.cc b/chrome/browser/sync/engine/apply_updates_command.cc index 6d1f958..8b5a6b8 100644 --- a/chrome/browser/sync/engine/apply_updates_command.cc +++ b/chrome/browser/sync/engine/apply_updates_command.cc @@ -32,16 +32,16 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) { session->context()->resolver(), session->context()->directory_manager()->GetCryptographer(&trans), handles.begin(), handles.end(), session->routing_info(), - session->status_controller()->group_restriction()); + session->status_controller().group_restriction()); while (applicator.AttemptOneApplication(&trans)) {} applicator.SaveProgressIntoSessionState( - session->status_controller()->mutable_conflict_progress(), - session->status_controller()->mutable_update_progress()); + session->mutable_status_controller()->mutable_conflict_progress(), + session->mutable_status_controller()->mutable_update_progress()); // This might be the first time we've fully completed a sync cycle, for // some subset of the currently synced datatypes. - sessions::StatusController* status(session->status_controller()); - if (status->ServerSaysNothingMoreToDownload()) { + const sessions::StatusController& status(session->status_controller()); + if (status.ServerSaysNothingMoreToDownload()) { syncable::ScopedDirLookup dir(session->context()->directory_manager(), session->context()->account_name()); if (!dir.good()) { @@ -52,7 +52,7 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) { for (int i = syncable::FIRST_REAL_MODEL_TYPE; i < syncable::MODEL_TYPE_COUNT; ++i) { syncable::ModelType model_type = syncable::ModelTypeFromInt(i); - if (status->updates_request_types()[i]) { + if (status.updates_request_types()[i]) { // This gets persisted to the directory's backing store. dir->set_initial_sync_ended_for_type(model_type, true); } diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc index a5b9cbf..b951e4f 100644 --- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc +++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc @@ -162,14 +162,16 @@ TEST_F(ApplyUpdatesCommandTest, Simple) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "Simple update shouldn't result in conflicts"; - EXPECT_EQ(2, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(2, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "All items should have been successfully applied"; } @@ -195,13 +197,15 @@ TEST_F(ApplyUpdatesCommandTest, UpdateWithChildrenBeforeParents) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - EXPECT_EQ(5, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(5, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "Simple update shouldn't result in conflicts, even if out-of-order"; - EXPECT_EQ(5, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(5, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "All updates should have been successfully applied"; } @@ -216,13 +220,15 @@ TEST_F(ApplyUpdatesCommandTest, NestedItemsWithUnknownParent) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(2, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize()) << "All updates with an unknown ancestors should be in conflict"; - EXPECT_EQ(0, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "No item with an unknown ancestor should be applied"; } @@ -250,13 +256,15 @@ TEST_F(ApplyUpdatesCommandTest, ItemsBothKnownAndUnknown) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - EXPECT_EQ(6, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(6, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(2, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize()) << "The updates with unknown ancestors should be in conflict"; - EXPECT_EQ(4, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(4, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The updates with known ancestors should be successfully applied"; } @@ -286,13 +294,15 @@ TEST_F(ApplyUpdatesCommandTest, DecryptablePassword) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "No update should be in conflict because they're all decryptable"; - EXPECT_EQ(1, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The updates that can be decrypted should be applied"; } @@ -312,34 +322,38 @@ TEST_F(ApplyUpdatesCommandTest, UndecryptableData) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); EXPECT_TRUE(status->HasConflictingUpdates()) << "Updates that can't be decrypted should trigger the syncer to have " << "conflicting updates."; { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "The updates that can't be decrypted should not be in regular " << "conflict"; - EXPECT_EQ(2, status->conflict_progress().NonblockingConflictingItemsSize()) + EXPECT_EQ(2, status->conflict_progress()->NonblockingConflictingItemsSize()) << "The updates that can't be decrypted should be in nonblocking " << "conflict"; - EXPECT_EQ(0, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "No update that can't be decrypted should be applied"; } { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "The updates that can't be decrypted should not be in regular " << "conflict"; - EXPECT_EQ(1, status->conflict_progress().NonblockingConflictingItemsSize()) + EXPECT_EQ(1, status->conflict_progress()->NonblockingConflictingItemsSize()) << "The updates that can't be decrypted should be in nonblocking " << "conflict"; - EXPECT_EQ(0, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "No update that can't be decrypted should be applied"; } } @@ -382,21 +396,23 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); EXPECT_TRUE(status->HasConflictingUpdates()) << "Updates that can't be decrypted should trigger the syncer to have " << "conflicting updates."; { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD); - EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "The updates that can't be decrypted should not be in regular " << "conflict"; - EXPECT_EQ(1, status->conflict_progress().NonblockingConflictingItemsSize()) + EXPECT_EQ(1, status->conflict_progress()->NonblockingConflictingItemsSize()) << "The updates that can't be decrypted should be in nonblocking " << "conflict"; - EXPECT_EQ(1, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The undecryptable password update shouldn't be applied"; } } @@ -434,13 +450,15 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdate) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "The nigori update shouldn't be in conflict"; - EXPECT_EQ(1, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The nigori update should be applied"; EXPECT_FALSE(cryptographer->is_ready()); @@ -483,13 +501,15 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdateForDisabledTypes) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "The nigori update shouldn't be in conflict"; - EXPECT_EQ(1, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The nigori update should be applied"; EXPECT_FALSE(cryptographer->is_ready()); @@ -568,15 +588,17 @@ TEST_F(ApplyUpdatesCommandTest, EncryptUnsyncedChanges) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "No updates should be in conflict"; - EXPECT_EQ(0, status->conflict_progress().NonblockingConflictingItemsSize()) + EXPECT_EQ(0, status->conflict_progress()->NonblockingConflictingItemsSize()) << "No updates should be in conflict"; - EXPECT_EQ(1, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The nigori update should be applied"; EXPECT_FALSE(cryptographer->has_pending_keys()); EXPECT_TRUE(cryptographer->is_ready()); @@ -668,17 +690,19 @@ TEST_F(ApplyUpdatesCommandTest, CannotEncryptUnsyncedChanges) { apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); + sessions::StatusController* status = session()->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize()) << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize()) << "The unsynced changes don't trigger a blocking conflict with the " << "nigori update."; - EXPECT_EQ(1, status->conflict_progress().NonblockingConflictingItemsSize()) + EXPECT_EQ(1, status->conflict_progress()->NonblockingConflictingItemsSize()) << "The unsynced changes trigger a non-blocking conflict with the " << "nigori update."; - EXPECT_EQ(0, status->update_progress().SuccessfullyAppliedUpdateCount()) + EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount()) << "The nigori update should not be applied"; EXPECT_FALSE(cryptographer->is_ready()); EXPECT_TRUE(cryptographer->has_pending_keys()); diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc index 05799fd..ed6076e 100644 --- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc +++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc @@ -32,7 +32,7 @@ BuildAndProcessConflictSetsCommand::~BuildAndProcessConflictSetsCommand() {} void BuildAndProcessConflictSetsCommand::ModelChangingExecuteImpl( SyncSession* session) { - session->status_controller()->update_conflict_sets_built( + session->mutable_status_controller()->update_conflict_sets_built( BuildAndProcessConflictSets(session)); } @@ -46,11 +46,11 @@ bool BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSets( { // Scope for transaction. syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir); BuildConflictSets(&trans, - session->status_controller()->mutable_conflict_progress()); + session->mutable_status_controller()->mutable_conflict_progress()); had_single_direction_sets = ProcessSingleDirectionConflictSets(&trans, session->context()->resolver(), session->context()->directory_manager()->GetCryptographer(&trans), - session->status_controller(), session->routing_info()); + session->mutable_status_controller(), session->routing_info()); // We applied some updates transactionally, lets try syncing again. if (had_single_direction_sets) return true; @@ -62,10 +62,12 @@ bool BuildAndProcessConflictSetsCommand::ProcessSingleDirectionConflictSets( syncable::WriteTransaction* trans, ConflictResolver* resolver, Cryptographer* cryptographer, StatusController* status, const ModelSafeRoutingInfo& routes) { + if (!status->conflict_progress()) + return false; bool rv = false; set<ConflictSet*>::const_iterator all_sets_iterator; - for (all_sets_iterator = status->conflict_progress().ConflictSetsBegin(); - all_sets_iterator != status->conflict_progress().ConflictSetsEnd();) { + for (all_sets_iterator = status->conflict_progress()->ConflictSetsBegin(); + all_sets_iterator != status->conflict_progress()->ConflictSetsEnd();) { const ConflictSet* conflict_set = *all_sets_iterator; CHECK_GE(conflict_set->size(), 2U); // We scan the set to see if it consists of changes of only one type. @@ -227,7 +229,8 @@ void BuildAndProcessConflictSetsCommand::BuildConflictSets( syncable::BaseTransaction* trans, ConflictProgress* conflict_progress) { conflict_progress->CleanupSets(); - set<syncable::Id>::iterator i = conflict_progress->ConflictingItemsBegin(); + set<syncable::Id>::const_iterator i = + conflict_progress->ConflictingItemsBegin(); while (i != conflict_progress->ConflictingItemsEnd()) { syncable::Entry entry(trans, syncable::GET_BY_ID, *i); if (!entry.good() || diff --git a/chrome/browser/sync/engine/build_commit_command.cc b/chrome/browser/sync/engine/build_commit_command.cc index 6b0e02f..35ff047 100644 --- a/chrome/browser/sync/engine/build_commit_command.cc +++ b/chrome/browser/sync/engine/build_commit_command.cc @@ -57,7 +57,7 @@ void BuildCommitCommand::AddExtensionsActivityToMessage( // We only send ExtensionsActivity to the server if bookmarks are being // committed. ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor(); - if (!session->status_controller()->HasBookmarkCommitActivity()) { + if (!session->status_controller().HasBookmarkCommitActivity()) { // Return the records to the activity monitor. monitor->PutRecords(session->extensions_activity()); session->mutable_extensions_activity()->clear(); @@ -105,7 +105,7 @@ void BuildCommitCommand::ExecuteImpl(SyncSession* session) { // whose ID is the map's key. std::map<Id, std::pair<int64, int64> > position_map; - const vector<Id>& commit_ids = session->status_controller()->commit_ids(); + const vector<Id>& commit_ids = session->status_controller().commit_ids(); for (size_t i = 0; i < commit_ids.size(); i++) { Id id = commit_ids[i]; SyncEntity* sync_entry = @@ -208,7 +208,8 @@ void BuildCommitCommand::ExecuteImpl(SyncSession* session) { SetEntrySpecifics(&meta_entry, sync_entry); } } - session->status_controller()->mutable_commit_message()->CopyFrom(message); + session->mutable_status_controller()-> + mutable_commit_message()->CopyFrom(message); } int64 BuildCommitCommand::FindAnchorPosition(syncable::IdField direction, diff --git a/chrome/browser/sync/engine/conflict_resolver.cc b/chrome/browser/sync/engine/conflict_resolver.cc index d8b6848..10f67df 100644 --- a/chrome/browser/sync/engine/conflict_resolver.cc +++ b/chrome/browser/sync/engine/conflict_resolver.cc @@ -455,14 +455,15 @@ bool ConflictResolver::LogAndSignalIfConflictStuck( // that's obviously not working. } -bool ConflictResolver::ResolveSimpleConflicts(const ScopedDirLookup& dir, - StatusController* status) { +bool ConflictResolver::ResolveSimpleConflicts( + const ScopedDirLookup& dir, + const ConflictProgress& progress, + sessions::StatusController* status) { WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir); bool forward_progress = false; - const ConflictProgress& progress = status->conflict_progress(); // First iterate over simple conflict items (those that belong to no set). set<Id>::const_iterator conflicting_item_it; - for (conflicting_item_it = progress.ConflictingItemsBeginConst(); + for (conflicting_item_it = progress.ConflictingItemsBegin(); conflicting_item_it != progress.ConflictingItemsEnd(); ++conflicting_item_it) { Id id = *conflicting_item_it; @@ -497,10 +498,10 @@ bool ConflictResolver::ResolveSimpleConflicts(const ScopedDirLookup& dir, } bool ConflictResolver::ResolveConflicts(const ScopedDirLookup& dir, - StatusController* status) { - const ConflictProgress& progress = status->conflict_progress(); + const ConflictProgress& progress, + sessions::StatusController* status) { bool rv = false; - if (ResolveSimpleConflicts(dir, status)) + if (ResolveSimpleConflicts(dir, progress, status)) rv = true; WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir); set<ConflictSet*>::const_iterator set_it; diff --git a/chrome/browser/sync/engine/conflict_resolver.h b/chrome/browser/sync/engine/conflict_resolver.h index 9bec0d5..c622794 100644 --- a/chrome/browser/sync/engine/conflict_resolver.h +++ b/chrome/browser/sync/engine/conflict_resolver.h @@ -26,9 +26,11 @@ class WriteTransaction; } // namespace syncable namespace browser_sync { + namespace sessions { +class ConflictProgress; class StatusController; -} +} // namespace sessions class ConflictResolver { friend class SyncerTest; @@ -40,6 +42,7 @@ class ConflictResolver { // Called by the syncer at the end of a update/commit cycle. // Returns true if the syncer should try to apply its updates again. bool ResolveConflicts(const syncable::ScopedDirLookup& dir, + const sessions::ConflictProgress& progress, sessions::StatusController* status); private: @@ -70,6 +73,7 @@ class ConflictResolver { sessions::StatusController* status); bool ResolveSimpleConflicts(const syncable::ScopedDirLookup& dir, + const sessions::ConflictProgress& progress, sessions::StatusController* status); bool ProcessConflictSet(syncable::WriteTransaction* trans, diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc index 5ca2290..2d573d8 100644 --- a/chrome/browser/sync/engine/download_updates_command.cc +++ b/chrome/browser/sync/engine/download_updates_command.cc @@ -97,7 +97,7 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { VLOG(2) << SyncerProtoUtil::ClientToServerResponseDebugString( update_response); - StatusController* status = session->status_controller(); + StatusController* status = session->mutable_status_controller(); status->set_updates_request_types(enabled_types); if (!ok) { status->increment_num_consecutive_errors(); @@ -115,36 +115,19 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { << " updates left on server."; } -void DownloadUpdatesCommand::SetRequestedTypes( - const syncable::ModelTypeBitSet& target_datatypes, - sync_pb::EntitySpecifics* filter_protobuf) { - // The datatypes which should be synced are dictated by the value of the - // ModelSafeRoutingInfo. If a datatype is in the routing info map, it - // should be synced (even if it's GROUP_PASSIVE). - int requested_type_count = 0; - for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) { - if (target_datatypes[i]) { - requested_type_count++; - syncable::AddDefaultExtensionValue(syncable::ModelTypeFromInt(i), - filter_protobuf); - } - } - DCHECK_LT(0, requested_type_count) << "Doing GetUpdates with empty filter."; -} - void DownloadUpdatesCommand::AppendClientDebugInfoIfNeeded( sessions::SyncSession* session, DebugInfo* debug_info) { // We want to send the debug info only once per sync cycle. Check if it has // already been sent. - if (!session->status_controller()->debug_info_sent()) { + if (!session->status_controller().debug_info_sent()) { VLOG(1) << "Sending client debug info ..."; // could be null in some unit tests. if (session->context()->debug_info_getter()) { session->context()->debug_info_getter()->GetAndClearDebugInfo( debug_info); } - session->status_controller()->set_debug_info_sent(); + session->mutable_status_controller()->set_debug_info_sent(); } } diff --git a/chrome/browser/sync/engine/download_updates_command.h b/chrome/browser/sync/engine/download_updates_command.h index f40a453..293c99a 100644 --- a/chrome/browser/sync/engine/download_updates_command.h +++ b/chrome/browser/sync/engine/download_updates_command.h @@ -46,9 +46,6 @@ class DownloadUpdatesCommand : public SyncerCommand { // SyncerCommand implementation. virtual void ExecuteImpl(sessions::SyncSession* session) OVERRIDE; - void SetRequestedTypes(const syncable::ModelTypeBitSet& target_datatypes, - sync_pb::EntitySpecifics* filter_protobuf); - private: FRIEND_TEST_ALL_PREFIXES(DownloadUpdatesCommandTest, VerifyAppendDebugInfo); void AppendClientDebugInfoIfNeeded(sessions::SyncSession* session, diff --git a/chrome/browser/sync/engine/download_updates_command_unittest.cc b/chrome/browser/sync/engine/download_updates_command_unittest.cc index 25a4be9..bc71492 100644 --- a/chrome/browser/sync/engine/download_updates_command_unittest.cc +++ b/chrome/browser/sync/engine/download_updates_command_unittest.cc @@ -42,68 +42,6 @@ class DownloadUpdatesCommandTest : public SyncerCommandTest { DISALLOW_COPY_AND_ASSIGN(DownloadUpdatesCommandTest); }; -TEST_F(DownloadUpdatesCommandTest, SetRequestedTypes) { - { - SCOPED_TRACE("Several enabled datatypes, spread out across groups."); - syncable::ModelTypeBitSet enabled_types; - enabled_types[syncable::BOOKMARKS] = true; - enabled_types[syncable::AUTOFILL] = true; - enabled_types[syncable::PREFERENCES] = true; - sync_pb::EntitySpecifics get_updates_filter; - command_.SetRequestedTypes(enabled_types, &get_updates_filter); - ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter); - v.ExpectHasExtension(sync_pb::autofill); - v.ExpectHasExtension(sync_pb::preference); - v.ExpectHasExtension(sync_pb::bookmark); - v.ExpectNoOtherFieldsOrExtensions(); - } - - { - SCOPED_TRACE("Top level folders."); - syncable::ModelTypeBitSet enabled_types; - enabled_types[syncable::TOP_LEVEL_FOLDER] = true; - enabled_types[syncable::BOOKMARKS] = true; - sync_pb::EntitySpecifics get_updates_filter; - command_.SetRequestedTypes(enabled_types, &get_updates_filter); - ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter); - v.ExpectHasExtension(sync_pb::bookmark); - v.ExpectNoOtherFieldsOrExtensions(); - } - - { - SCOPED_TRACE("Bookmarks only."); - syncable::ModelTypeBitSet enabled_types; - enabled_types[syncable::BOOKMARKS] = true; - sync_pb::EntitySpecifics get_updates_filter; - command_.SetRequestedTypes(enabled_types, &get_updates_filter); - ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter); - v.ExpectHasExtension(sync_pb::bookmark); - v.ExpectNoOtherFieldsOrExtensions(); - } - - { - SCOPED_TRACE("Autofill only."); - syncable::ModelTypeBitSet enabled_types; - enabled_types[syncable::AUTOFILL] = true; - sync_pb::EntitySpecifics get_updates_filter; - command_.SetRequestedTypes(enabled_types, &get_updates_filter); - ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter); - v.ExpectHasExtension(sync_pb::autofill); - v.ExpectNoOtherFieldsOrExtensions(); - } - - { - SCOPED_TRACE("Preferences only."); - syncable::ModelTypeBitSet enabled_types; - enabled_types[syncable::PREFERENCES] = true; - sync_pb::EntitySpecifics get_updates_filter; - command_.SetRequestedTypes(enabled_types, &get_updates_filter); - ProtoExtensionValidator<sync_pb::EntitySpecifics> v(get_updates_filter); - v.ExpectHasExtension(sync_pb::preference); - v.ExpectNoOtherFieldsOrExtensions(); - } -} - TEST_F(DownloadUpdatesCommandTest, ExecuteNoPayloads) { ConfigureMockServerConnection(); mock_server()->ExpectGetUpdatesRequestTypes( diff --git a/chrome/browser/sync/engine/get_commit_ids_command.cc b/chrome/browser/sync/engine/get_commit_ids_command.cc index 4bfc5e00..2b17aa7 100644 --- a/chrome/browser/sync/engine/get_commit_ids_command.cc +++ b/chrome/browser/sync/engine/get_commit_ids_command.cc @@ -49,7 +49,7 @@ void GetCommitIdsCommand::ExecuteImpl(SyncSession* session) { FilterUnreadyEntries(session->write_transaction(), &all_unsynced_handles); - StatusController* status = session->status_controller(); + StatusController* status = session->mutable_status_controller(); status->set_unsynced_handles(all_unsynced_handles); BuildCommitIds(status->unsynced_handles(), session->write_transaction(), session->routing_info()); diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.cc b/chrome/browser/sync/engine/model_changing_syncer_command.cc index 407c299..213cf23 100644 --- a/chrome/browser/sync/engine/model_changing_syncer_command.cc +++ b/chrome/browser/sync/engine/model_changing_syncer_command.cc @@ -45,7 +45,8 @@ void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) { continue; } - sessions::StatusController* status = work_session_->status_controller(); + sessions::StatusController* status = + work_session_->mutable_status_controller(); sessions::ScopedModelSafeGroupRestriction r(status, group); WorkCallback c = base::Bind( &ModelChangingSyncerCommand::StartChangingModel, diff --git a/chrome/browser/sync/engine/post_commit_message_command.cc b/chrome/browser/sync/engine/post_commit_message_command.cc index c9b4af4..c70a193 100644 --- a/chrome/browser/sync/engine/post_commit_message_command.cc +++ b/chrome/browser/sync/engine/post_commit_message_command.cc @@ -20,14 +20,14 @@ PostCommitMessageCommand::PostCommitMessageCommand() {} PostCommitMessageCommand::~PostCommitMessageCommand() {} void PostCommitMessageCommand::ExecuteImpl(sessions::SyncSession* session) { - if (session->status_controller()->commit_ids().empty()) + if (session->status_controller().commit_ids().empty()) return; // Nothing to commit. ClientToServerResponse response; syncable::ScopedDirLookup dir(session->context()->directory_manager(), session->context()->account_name()); if (!dir.good()) return; - sessions::StatusController* status = session->status_controller(); + sessions::StatusController* status = session->mutable_status_controller(); if (!SyncerProtoUtil::PostClientToServerMessage(status->commit_message(), &response, session)) { // None of our changes got through. Clear the SYNCING bit which was diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc index 0fc5dbd..7a7113f 100644 --- a/chrome/browser/sync/engine/process_commit_response_command.cc +++ b/chrome/browser/sync/engine/process_commit_response_command.cc @@ -68,14 +68,14 @@ bool ProcessCommitResponseCommand::ModelNeutralExecuteImpl( return false; } - StatusController* status = session->status_controller(); - const ClientToServerResponse& response(status->commit_response()); - const vector<syncable::Id>& commit_ids(status->commit_ids()); + const StatusController& status = session->status_controller(); + const ClientToServerResponse& response(status.commit_response()); + const vector<syncable::Id>& commit_ids(status.commit_ids()); if (!response.has_commit()) { // TODO(sync): What if we didn't try to commit anything? LOG(WARNING) << "Commit response has no commit body!"; - IncrementErrorCounters(status); + IncrementErrorCounters(session->mutable_status_controller()); return false; } @@ -90,7 +90,7 @@ bool ProcessCommitResponseCommand::ModelNeutralExecuteImpl( if (cr.entryresponse(i).has_error_message()) LOG(ERROR) << " " << cr.entryresponse(i).error_message(); } - IncrementErrorCounters(status); + IncrementErrorCounters(session->mutable_status_controller()); return false; } return true; @@ -100,8 +100,8 @@ void ProcessCommitResponseCommand::ModelChangingExecuteImpl( SyncSession* session) { ProcessCommitResponse(session); ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor(); - if (session->status_controller()->HasBookmarkCommitActivity() && - session->status_controller()->syncer_status() + if (session->status_controller().HasBookmarkCommitActivity() && + session->status_controller().syncer_status() .num_successful_bookmark_commits == 0) { monitor->PutRecords(session->extensions_activity()); session->mutable_extensions_activity()->clear(); @@ -119,7 +119,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( return; } - StatusController* status = session->status_controller(); + StatusController* status = session->mutable_status_controller(); const ClientToServerResponse& response(status->commit_response()); const CommitResponse& cr = response.commit(); const sync_pb::CommitMessage& commit_message = @@ -159,7 +159,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse( case CommitResponse::SUCCESS: // TODO(sync): worry about sync_rate_ rate calc? ++successes; - if (status->GetCommitIdModelTypeAt(proj[i]) == syncable::BOOKMARKS) + if (status->GetCommitModelTypeAt(proj[i]) == syncable::BOOKMARKS) status->increment_num_successful_bookmark_commits(); status->increment_num_successful_commits(); break; diff --git a/chrome/browser/sync/engine/process_commit_response_command_unittest.cc b/chrome/browser/sync/engine/process_commit_response_command_unittest.cc index 44d2a8a..83b8885 100644 --- a/chrome/browser/sync/engine/process_commit_response_command_unittest.cc +++ b/chrome/browser/sync/engine/process_commit_response_command_unittest.cc @@ -126,7 +126,8 @@ class ProcessCommitResponseCommandTestWithParam const Id& parent_id, const string& name, syncable::ModelType model_type) { - sessions::StatusController* sync_state = session()->status_controller(); + sessions::StatusController* sync_state = + session()->mutable_status_controller(); bool is_folder = true; int64 metahandle = 0; CreateUnsyncedItem(item_id, parent_id, name, is_folder, model_type, @@ -187,7 +188,8 @@ class ProcessCommitResponseCommandTestWithParam } void SetLastErrorCode(CommitResponse::ResponseType error_code) { - sessions::StatusController* sync_state = session()->status_controller(); + sessions::StatusController* sync_state = + session()->mutable_status_controller(); sync_pb::ClientToServerResponse* response = sync_state->mutable_commit_response(); sync_pb::CommitResponse_EntryResponse* entry_response = diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc index c6d41411..e6763bd 100644 --- a/chrome/browser/sync/engine/process_updates_command.cc +++ b/chrome/browser/sync/engine/process_updates_command.cc @@ -28,7 +28,7 @@ ProcessUpdatesCommand::~ProcessUpdatesCommand() {} bool ProcessUpdatesCommand::ModelNeutralExecuteImpl(SyncSession* session) { const GetUpdatesResponse& updates = - session->status_controller()->updates_response().get_updates(); + session->status_controller().updates_response().get_updates(); const int update_count = updates.entries_size(); // Don't bother processing updates if there were none. @@ -43,13 +43,15 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) { return; } - StatusController* status = session->status_controller(); + const sessions::UpdateProgress* progress = + session->status_controller().update_progress(); + if (!progress) + return; // Nothing to do. syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir); - const sessions::UpdateProgress& progress(status->update_progress()); vector<sessions::VerifiedUpdate>::const_iterator it; - for (it = progress.VerifiedUpdatesBegin(); - it != progress.VerifiedUpdatesEnd(); + for (it = progress->VerifiedUpdatesBegin(); + it != progress->VerifiedUpdatesEnd(); ++it) { const sync_pb::SyncEntity& update = it->second; @@ -65,10 +67,9 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) { } } + StatusController* status = session->mutable_status_controller(); status->set_num_consecutive_errors(0); - status->mutable_update_progress()->ClearVerifiedUpdates(); - return; } namespace { diff --git a/chrome/browser/sync/engine/resolve_conflicts_command.cc b/chrome/browser/sync/engine/resolve_conflicts_command.cc index a8fcefb..71351c4 100644 --- a/chrome/browser/sync/engine/resolve_conflicts_command.cc +++ b/chrome/browser/sync/engine/resolve_conflicts_command.cc @@ -1,10 +1,11 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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 "chrome/browser/sync/engine/resolve_conflicts_command.h" #include "chrome/browser/sync/engine/conflict_resolver.h" +#include "chrome/browser/sync/sessions/session_state.h" #include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" @@ -24,8 +25,12 @@ void ResolveConflictsCommand::ModelChangingExecuteImpl( session->context()->account_name()); if (!dir.good()) return; - sessions::StatusController* status = session->status_controller(); - status->update_conflicts_resolved(resolver->ResolveConflicts(dir, status)); + sessions::StatusController* status = session->mutable_status_controller(); + const sessions::ConflictProgress* progress = status->conflict_progress(); + if (!progress) + return; // Nothing to do. + status->update_conflicts_resolved( + resolver->ResolveConflicts(dir, *progress, status)); } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/store_timestamps_command.cc b/chrome/browser/sync/engine/store_timestamps_command.cc index a063bf4..0f88920 100644 --- a/chrome/browser/sync/engine/store_timestamps_command.cc +++ b/chrome/browser/sync/engine/store_timestamps_command.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -25,9 +25,9 @@ void StoreTimestampsCommand::ExecuteImpl(sessions::SyncSession* session) { } const GetUpdatesResponse& updates = - session->status_controller()->updates_response().get_updates(); + session->status_controller().updates_response().get_updates(); - sessions::StatusController* status = session->status_controller(); + sessions::StatusController* status = session->mutable_status_controller(); // Update the progress marker tokens from the server result. If a marker // was omitted for any one type, that indicates no change from the previous diff --git a/chrome/browser/sync/engine/sync_scheduler.cc b/chrome/browser/sync/engine/sync_scheduler.cc index f4aa2a0..dea4a18 100644 --- a/chrome/browser/sync/engine/sync_scheduler.cc +++ b/chrome/browser/sync/engine/sync_scheduler.cc @@ -843,9 +843,9 @@ void SyncScheduler::ScheduleNextSync(const SyncSessionJob& old_job) { // forward progress was possible at this time (an error, such as an HTTP // 500, is likely to have occurred during commit). int num_server_changes_remaining = - old_job.session->status_controller()->num_server_changes_remaining(); + old_job.session->status_controller().num_server_changes_remaining(); size_t num_unsynced_handles = - old_job.session->status_controller()->unsynced_handles().size(); + old_job.session->status_controller().unsynced_handles().size(); const bool work_to_do = num_server_changes_remaining > 0 || num_unsynced_handles > 0; SVLOG(2) << "num server changes remaining: " << num_server_changes_remaining diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc index 12c79c0..9a879da 100644 --- a/chrome/browser/sync/engine/syncer.cc +++ b/chrome/browser/sync/engine/syncer.cc @@ -84,11 +84,13 @@ const char* SyncerStepToString(const SyncerStep step) Syncer::ScopedSyncStartStopTracker::ScopedSyncStartStopTracker( sessions::SyncSession* session) : session_(session) { - session_->status_controller()->SetSyncInProgressAndUpdateStartTime(true); + session_->mutable_status_controller()-> + SetSyncInProgressAndUpdateStartTime(true); } Syncer::ScopedSyncStartStopTracker::~ScopedSyncStartStopTracker() { - session_->status_controller()->SetSyncInProgressAndUpdateStartTime(false); + session_->mutable_status_controller()-> + SetSyncInProgressAndUpdateStartTime(false); } Syncer::Syncer() @@ -179,8 +181,8 @@ void Syncer::SyncShare(sessions::SyncSession* session, store_timestamps.Execute(session); // We should download all of the updates before attempting to process // them. - if (session->status_controller()->ServerSaysNothingMoreToDownload() || - !session->status_controller()->download_updates_succeeded()) { + if (session->status_controller().ServerSaysNothingMoreToDownload() || + !session->status_controller().download_updates_succeeded()) { next_step = APPLY_UPDATES; } else { next_step = DOWNLOAD_UPDATES; @@ -217,7 +219,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, session->context()->max_commit_batch_size()); get_commit_ids_command.Execute(session); - if (!session->status_controller()->commit_ids().empty()) { + if (!session->status_controller().commit_ids().empty()) { VLOG(1) << "Building a commit message"; BuildCommitCommand build_commit_command; build_commit_command.Execute(session); @@ -236,7 +238,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, break; } case PROCESS_COMMIT_RESPONSE: { - session->status_controller()->reset_num_conflicting_commits(); + session->mutable_status_controller()->reset_num_conflicting_commits(); ProcessCommitResponseCommand process_response_command; process_response_command.Execute(session); next_step = BUILD_AND_PROCESS_CONFLICT_SETS; @@ -245,7 +247,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, case BUILD_AND_PROCESS_CONFLICT_SETS: { BuildAndProcessConflictSetsCommand build_process_conflict_sets; build_process_conflict_sets.Execute(session); - if (session->status_controller()->conflict_sets_built()) + if (session->status_controller().conflict_sets_built()) next_step = SYNCER_END; else next_step = RESOLVE_CONFLICTS; @@ -259,7 +261,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, pre_conflict_resolution_closure_->Run(); } - StatusController* status = session->status_controller(); + StatusController* status = session->mutable_status_controller(); status->reset_conflicts_resolved(); ResolveConflictsCommand resolve_conflicts_command; resolve_conflicts_command.Execute(session); @@ -273,7 +275,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, break; } case APPLY_UPDATES_TO_RESOLVE_CONFLICTS: { - StatusController* status = session->status_controller(); + StatusController* status = session->mutable_status_controller(); VLOG(1) << "Applying updates to resolve conflicts"; ApplyUpdatesCommand apply_updates; @@ -320,7 +322,7 @@ void Syncer::SyncShare(sessions::SyncSession* session, void Syncer::ProcessClientCommand(sessions::SyncSession* session) { const ClientToServerResponse& response = - session->status_controller()->updates_response(); + session->status_controller().updates_response(); if (!response.has_client_command()) return; const ClientCommand& command = response.client_command(); diff --git a/chrome/browser/sync/engine/syncer_command.cc b/chrome/browser/sync/engine/syncer_command.cc index d4cae8d..44e5477 100644 --- a/chrome/browser/sync/engine/syncer_command.cc +++ b/chrome/browser/sync/engine/syncer_command.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -27,7 +27,7 @@ void SyncerCommand::SendNotifications(SyncSession* session) { return; } - if (session->status_controller()->TestAndClearIsDirty()) { + if (session->mutable_status_controller()->TestAndClearIsDirty()) { SyncEngineEvent event(SyncEngineEvent::STATUS_CHANGED); const sessions::SyncSessionSnapshot& snapshot(session->TakeSnapshot()); event.snapshot = &snapshot; diff --git a/chrome/browser/sync/engine/syncer_proto_util.cc b/chrome/browser/sync/engine/syncer_proto_util.cc index 84abca6..7da3140 100644 --- a/chrome/browser/sync/engine/syncer_proto_util.cc +++ b/chrome/browser/sync/engine/syncer_proto_util.cc @@ -93,7 +93,8 @@ void SyncerProtoUtil::HandleMigrationDoneResponse( response->migrated_data_type_id(i))); } // TODO(akalin): This should be a set union. - session->status_controller()->set_types_needing_local_migration(to_migrate); + session->mutable_status_controller()-> + set_types_needing_local_migration(to_migrate); } // static @@ -313,7 +314,7 @@ bool SyncerProtoUtil::PostClientToServerMessage( } // Now set the error into the status so the layers above us could read it. - sessions::StatusController* status = session->status_controller(); + sessions::StatusController* status = session->mutable_status_controller(); status->set_sync_protocol_error(sync_protocol_error); // Inform the delegate of the error we got. diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc index 2e47dfa..360c6d2 100644 --- a/chrome/browser/sync/engine/syncer_unittest.cc +++ b/chrome/browser/sync/engine/syncer_unittest.cc @@ -213,7 +213,7 @@ class SyncerTest : public testing::Test, ReadTransaction trans(FROM_HERE, dir); syncable::Directory::ChildHandles children; dir->GetChildHandlesById(&trans, trans.root_id(), &children); - ASSERT_TRUE(0 == children.size()); + ASSERT_EQ(0u, children.size()); saw_syncer_event_ = false; root_id_ = TestIdFactory::root(); parent_id_ = ids_.MakeServer("parent id"); @@ -351,7 +351,7 @@ class SyncerTest : public testing::Test, mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. for (size_t i = 0; i < expected_positions.size(); ++i) { - EXPECT_TRUE(1 == expected_positions.count(i)); + EXPECT_EQ(1u, expected_positions.count(i)); EXPECT_TRUE(expected_positions[i] == mock_server_->committed_ids()[i]); } } @@ -361,7 +361,7 @@ class SyncerTest : public testing::Test, const vector<syncable::Id>& expected_id_order) { // The expected order is "x", "b", "c", "e", truncated appropriately. for (size_t limit = expected_id_order.size() + 2; limit > 0; --limit) { - StatusController* status = session_->status_controller(); + StatusController* status = session_->mutable_status_controller(); WriteTransaction wtrans(FROM_HERE, UNITTEST, dir); ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans); status->set_unsynced_handles(unsynced_handle_view); @@ -369,7 +369,8 @@ class SyncerTest : public testing::Test, ModelSafeRoutingInfo routes; GetModelSafeRoutingInfo(&routes); GetCommitIdsCommand command(limit); - command.BuildCommitIds(session_->status_controller()->unsynced_handles(), + command.BuildCommitIds( + session_->status_controller().unsynced_handles(), session_->write_transaction(), routes); vector<syncable::Id> output = command.ordered_commit_set_->GetAllCommitIds(); @@ -509,7 +510,7 @@ TEST_F(SyncerTest, TestCallGatherUnsyncedEntries) { ReadTransaction trans(FROM_HERE, dir); SyncerUtil::GetUnsyncedEntries(&trans, &handles); } - ASSERT_TRUE(0 == handles.size()); + ASSERT_EQ(0u, handles.size()); } // TODO(sync): When we can dynamically connect and disconnect the mock // ServerConnectionManager test disconnected GetUnsyncedEntries here. It's a @@ -611,7 +612,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) { { // We remove any unready entries from the status controller's unsynced // handles, so this should remain 0 even though the entries didn't commit. - ASSERT_EQ(0U, session_->status_controller()->unsynced_handles().size()); + ASSERT_EQ(0U, session_->status_controller().unsynced_handles().size()); // Nothing should have commited due to bookmarks being encrypted and // the cryptographer having pending keys. A would have been resolved // as a simple conflict, but still be unsynced until the next sync cycle. @@ -635,7 +636,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) { } SyncShareAsDelegate(); { - ASSERT_EQ(0U, session_->status_controller()->unsynced_handles().size()); + ASSERT_EQ(0U, session_->status_controller().unsynced_handles().size()); // All properly encrypted and non-conflicting items should commit. "A" was // conflicting, but last sync cycle resolved it as simple conflict, so on // this sync cycle it committed succesfullly. @@ -675,7 +676,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) { } SyncShareAsDelegate(); { - ASSERT_EQ(0U, session_->status_controller()->unsynced_handles().size()); + ASSERT_EQ(0U, session_->status_controller().unsynced_handles().size()); // None should be unsynced anymore. ReadTransaction rtrans(FROM_HERE, dir); Entry entryA(&rtrans, syncable::GET_BY_ID, ids_.FromNumber(1)); @@ -701,7 +702,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) { TEST_F(SyncerTest, TestCommitMetahandleIterator) { ScopedDirLookup dir(syncdb_.manager(), syncdb_.name()); ASSERT_TRUE(dir.good()); - StatusController* status = session_->status_controller(); + StatusController* status = session_->mutable_status_controller(); const vector<int64>& unsynced(status->unsynced_handles()); { @@ -764,10 +765,10 @@ TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) { WriteTestDataToEntry(&wtrans, &child); } - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_TRUE(2 == status->unsynced_handles().size()); - ASSERT_TRUE(2 == mock_server_->committed_ids().size()); + EXPECT_EQ(2u, status.unsynced_handles().size()); + ASSERT_EQ(2u, mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); EXPECT_TRUE(child_id_ == mock_server_->committed_ids()[1]); @@ -812,9 +813,9 @@ TEST_F(SyncerTest, TestPurgeWhileUnsynced) { types_to_purge.insert(syncable::PREFERENCES); dir->PurgeEntriesWithTypeIn(types_to_purge); - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_EQ(2U, status->unsynced_handles().size()); + EXPECT_EQ(2U, status.unsynced_handles().size()); ASSERT_EQ(2U, mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); @@ -1058,8 +1059,8 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNesting) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_TRUE(6 == session_->status_controller()->unsynced_handles().size()); - ASSERT_TRUE(6 == mock_server_->committed_ids().size()); + EXPECT_EQ(6u, session_->status_controller().unsynced_handles().size()); + ASSERT_EQ(6u, mock_server_->committed_ids().size()); // This test will NOT unroll deletes because SERVER_PARENT_ID is not set. // It will treat these like moves. vector<syncable::Id> commit_ids(mock_server_->committed_ids()); @@ -1128,8 +1129,8 @@ TEST_F(SyncerTest, TestCommitListOrderingWithNewItems) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_TRUE(6 == session_->status_controller()->unsynced_handles().size()); - ASSERT_TRUE(6 == mock_server_->committed_ids().size()); + EXPECT_EQ(6u, session_->status_controller().unsynced_handles().size()); + ASSERT_EQ(6u, mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); EXPECT_TRUE(child_id_ == mock_server_->committed_ids()[1]); @@ -1170,8 +1171,8 @@ TEST_F(SyncerTest, TestCommitListOrderingCounterexample) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_TRUE(3 == session_->status_controller()->unsynced_handles().size()); - ASSERT_TRUE(3 == mock_server_->committed_ids().size()); + EXPECT_EQ(3u, session_->status_controller().unsynced_handles().size()); + ASSERT_EQ(3u, mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); EXPECT_TRUE(child_id_ == mock_server_->committed_ids()[1]); @@ -1219,8 +1220,8 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParent) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_TRUE(3 == session_->status_controller()->unsynced_handles().size()); - ASSERT_TRUE(3 == mock_server_->committed_ids().size()); + EXPECT_EQ(3u, session_->status_controller().unsynced_handles().size()); + ASSERT_EQ(3u, mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); EXPECT_TRUE(parent2_id == mock_server_->committed_ids()[1]); @@ -1293,8 +1294,8 @@ TEST_F(SyncerTest, TestCommitListOrderingAndNewParentAndChild) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_TRUE(3 == session_->status_controller()->unsynced_handles().size()); - ASSERT_TRUE(3 == mock_server_->committed_ids().size()); + EXPECT_EQ(3u, session_->status_controller().unsynced_handles().size()); + ASSERT_EQ(3u, mock_server_->committed_ids().size()); // If this test starts failing, be aware other sort orders could be valid. EXPECT_TRUE(parent_id_ == mock_server_->committed_ids()[0]); EXPECT_TRUE(parent2_local_id == mock_server_->committed_ids()[1]); @@ -1347,7 +1348,7 @@ TEST_F(SyncerTest, DontGetStuckWithTwoSameNames) { SyncShareAsDelegate(); mock_server_->AddUpdateDirectory(2, 0, "foo:", 1, 20); SyncRepeatedlyToTriggerStuckSignal(session_.get()); - EXPECT_FALSE(session_->status_controller()->syncer_status().syncer_stuck); + EXPECT_FALSE(session_->status_controller().syncer_status().syncer_stuck); saw_syncer_event_ = false; } @@ -1395,13 +1396,14 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { mock_server_->AddUpdateDirectory(3, -80, "bad_parent", 10, 10); syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - StatusController* status = session_->status_controller(); + StatusController* status = session_->mutable_status_controller(); // Id 3 should be in conflict now. EXPECT_EQ(1, status->TotalNumConflictingItems()); { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(1, status->conflict_progress().ConflictingItemsSize()); + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(1, status->conflict_progress()->ConflictingItemsSize()); } // These entries will be used in the second set of updates. @@ -1418,7 +1420,8 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { EXPECT_EQ(3, status->TotalNumConflictingItems()); { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(3, status->conflict_progress().ConflictingItemsSize()); + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(3, status->conflict_progress()->ConflictingItemsSize()); } { @@ -1464,8 +1467,8 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { Entry still_a_dir(&trans, GET_BY_ID, ids_.FromNumber(10)); ASSERT_TRUE(still_a_dir.good()); EXPECT_FALSE(still_a_dir.Get(IS_UNAPPLIED_UPDATE)); - EXPECT_TRUE(10 == still_a_dir.Get(BASE_VERSION)); - EXPECT_TRUE(10 == still_a_dir.Get(SERVER_VERSION)); + EXPECT_EQ(10u, still_a_dir.Get(BASE_VERSION)); + EXPECT_EQ(10u, still_a_dir.Get(SERVER_VERSION)); EXPECT_TRUE(still_a_dir.Get(IS_DIR)); Entry rename(&trans, GET_BY_ID, ids_.FromNumber(1)); @@ -1474,13 +1477,13 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { EXPECT_EQ("new_name", rename.Get(NON_UNIQUE_NAME)); EXPECT_FALSE(rename.Get(IS_UNAPPLIED_UPDATE)); EXPECT_TRUE(ids_.FromNumber(1) == rename.Get(ID)); - EXPECT_TRUE(20 == rename.Get(BASE_VERSION)); + EXPECT_EQ(20u, rename.Get(BASE_VERSION)); Entry name_clash(&trans, GET_BY_ID, ids_.FromNumber(2)); ASSERT_TRUE(name_clash.good()); EXPECT_EQ(root, name_clash.Get(PARENT_ID)); EXPECT_TRUE(ids_.FromNumber(2) == name_clash.Get(ID)); - EXPECT_TRUE(10 == name_clash.Get(BASE_VERSION)); + EXPECT_EQ(10u, name_clash.Get(BASE_VERSION)); EXPECT_EQ("in_root", name_clash.Get(NON_UNIQUE_NAME)); Entry ignored_old_version(&trans, GET_BY_ID, ids_.FromNumber(4)); @@ -1488,7 +1491,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { EXPECT_TRUE( ignored_old_version.Get(NON_UNIQUE_NAME) == "newer_version"); EXPECT_FALSE(ignored_old_version.Get(IS_UNAPPLIED_UPDATE)); - EXPECT_TRUE(20 == ignored_old_version.Get(BASE_VERSION)); + EXPECT_EQ(20u, ignored_old_version.Get(BASE_VERSION)); Entry circular_parent_issue(&trans, GET_BY_ID, ids_.FromNumber(5)); ASSERT_TRUE(circular_parent_issue.good()); @@ -1497,21 +1500,22 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) { EXPECT_TRUE(circular_parent_issue.Get(PARENT_ID) == root_id_); EXPECT_TRUE(circular_parent_issue.Get(SERVER_PARENT_ID) == ids_.FromNumber(6)); - EXPECT_TRUE(10 == circular_parent_issue.Get(BASE_VERSION)); + EXPECT_EQ(10u, circular_parent_issue.Get(BASE_VERSION)); Entry circular_parent_target(&trans, GET_BY_ID, ids_.FromNumber(6)); ASSERT_TRUE(circular_parent_target.good()); EXPECT_FALSE(circular_parent_target.Get(IS_UNAPPLIED_UPDATE)); EXPECT_TRUE(circular_parent_issue.Get(ID) == circular_parent_target.Get(PARENT_ID)); - EXPECT_TRUE(10 == circular_parent_target.Get(BASE_VERSION)); + EXPECT_EQ(10u, circular_parent_target.Get(BASE_VERSION)); } EXPECT_FALSE(saw_syncer_event_); EXPECT_EQ(4, status->TotalNumConflictingItems()); { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(4, status->conflict_progress().ConflictingItemsSize()); + ASSERT_TRUE(status->conflict_progress()); + EXPECT_EQ(4, status->conflict_progress()->ConflictingItemsSize()); } } @@ -1959,7 +1963,7 @@ TEST_F(EntryCreatedInNewFolderTest, EntryCreatedInNewFolderMidSync) { NewCallback<EntryCreatedInNewFolderTest>(this, &EntryCreatedInNewFolderTest::CreateFolderInBob)); syncer_->SyncShare(session_.get(), BUILD_COMMIT_REQUEST, SYNCER_END); - EXPECT_TRUE(1 == mock_server_->committed_ids().size()); + EXPECT_EQ(1u, mock_server_->committed_ids().size()); { ReadTransaction trans(FROM_HERE, dir); Entry parent_entry(&trans, syncable::GET_BY_ID, @@ -1998,7 +2002,7 @@ TEST_F(SyncerTest, UnappliedUpdateOnCreatedItemItemDoesNotCrash) { } // Commit it. SyncShareAsDelegate(); - EXPECT_TRUE(1 == mock_server_->committed_ids().size()); + EXPECT_EQ(1u, mock_server_->committed_ids().size()); mock_server_->set_conflict_all_commits(true); syncable::Id fred_match_id; { @@ -2058,7 +2062,7 @@ TEST_F(SyncerTest, DoublyChangedWithResolver) { } // Only one entry, since we just overwrite one. - EXPECT_TRUE(1 == children.size()); + EXPECT_EQ(1u, children.size()); saw_syncer_event_ = false; } @@ -2141,15 +2145,15 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) { ReadTransaction trans(FROM_HERE, dir); Directory::ChildHandles children; dir->GetChildHandlesById(&trans, root_id_, &children); - EXPECT_TRUE(1 == children.size()); + EXPECT_EQ(1u, children.size()); dir->GetChildHandlesById(&trans, parent_id, &children); - EXPECT_TRUE(1 == children.size()); + EXPECT_EQ(1u, children.size()); Directory::UnappliedUpdateMetaHandles unapplied; dir->GetUnappliedUpdateMetaHandles(&trans, &unapplied); - EXPECT_TRUE(0 == unapplied.size()); + EXPECT_EQ(0u, unapplied.size()); syncable::Directory::UnsyncedMetaHandles unsynced; dir->GetUnsyncedMetaHandles(&trans, &unsynced); - EXPECT_TRUE(0 == unsynced.size()); + EXPECT_EQ(0u, unsynced.size()); saw_syncer_event_ = false; } } @@ -2164,7 +2168,7 @@ TEST_F(SyncerTest, CommittingNewDeleted) { entry.Put(IS_DEL, true); } SyncShareAsDelegate(); - EXPECT_TRUE(0 == mock_server_->committed_ids().size()); + EXPECT_EQ(0u, mock_server_->committed_ids().size()); } // Original problem synopsis: @@ -2195,7 +2199,7 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_TRUE(0 == session_->status_controller()->TotalNumConflictingItems()); + EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems()); saw_syncer_event_ = false; } @@ -2243,8 +2247,8 @@ TEST_F(SyncerTest, DeletingEntryInFolder) { existing.Put(IS_DEL, true); } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - StatusController* status(session_->status_controller()); - EXPECT_TRUE(0 == status->error().num_conflicting_commits); + const StatusController& status(session_->status_controller()); + EXPECT_EQ(0, status.error().num_conflicting_commits); } TEST_F(SyncerTest, DeletingEntryWithLocalEdits) { @@ -3238,7 +3242,7 @@ class SusanDeletingTest : public SyncerTest { MutableEntry susan(&trans, GET_BY_ID, susan_id); Directory::ChildHandles children; dir->GetChildHandlesById(&trans, susan.Get(ID), &children); - ASSERT_TRUE(0 == children.size()); + ASSERT_EQ(0u, children.size()); susan.Put(IS_DEL, true); susan.Put(IS_UNSYNCED, true); } @@ -3303,7 +3307,7 @@ TEST_F(SusanDeletingTest, EXPECT_TRUE(bob.Get(IS_UNSYNCED)); EXPECT_TRUE(joe.Get(IS_UNSYNCED)); } - EXPECT_TRUE(0 == countdown_till_delete_); + EXPECT_EQ(0, countdown_till_delete_); delete syncer_->pre_conflict_resolution_closure_; syncer_->pre_conflict_resolution_closure_ = NULL; LoopSyncShare(); @@ -3503,12 +3507,12 @@ TEST_F(SyncerTest, DuplicateIDReturn) { folder2.Put(ID, syncable::Id::CreateFromServerId("mock_server:10000")); } mock_server_->set_next_new_id(10000); - EXPECT_TRUE(1 == dir->unsynced_entity_count()); + EXPECT_EQ(1u, dir->unsynced_entity_count()); // we get back a bad id in here (should never happen). SyncShareAsDelegate(); - EXPECT_TRUE(1 == dir->unsynced_entity_count()); + EXPECT_EQ(1u, dir->unsynced_entity_count()); SyncShareAsDelegate(); // another bad id in here. - EXPECT_TRUE(0 == dir->unsynced_entity_count()); + EXPECT_EQ(0u, dir->unsynced_entity_count()); saw_syncer_event_ = false; } @@ -3650,7 +3654,7 @@ TEST(SyncerSyncProcessState, MergeSetsTest) { c.MergeSets(id[2], id[3]); c.MergeSets(id[4], id[5]); c.MergeSets(id[5], id[6]); - EXPECT_TRUE(6 == c.IdToConflictSetSize()); + EXPECT_EQ(6u, c.IdToConflictSetSize()); EXPECT_FALSE(is_dirty); for (int i = 1; i < 7; i++) { EXPECT_TRUE(NULL != c.IdToConflictSetGet(id[i])); @@ -3703,7 +3707,7 @@ TEST_F(SyncerTest, OneBajillionUpdates) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_FALSE(session_->status_controller()->syncer_status().syncer_stuck); + EXPECT_FALSE(session_->status_controller().syncer_status().syncer_stuck); } // In this test a long changelog contains a child at the start of the changelog @@ -3733,7 +3737,7 @@ TEST_F(SyncerTest, LongChangelistWithApplicationConflict) { } syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END); - EXPECT_FALSE(session_->status_controller()->syncer_status().syncer_stuck); + EXPECT_FALSE(session_->status_controller().syncer_status().syncer_stuck); // Ensure our folder hasn't somehow applied. { @@ -4026,7 +4030,7 @@ TEST_F(SyncerTest, EnsureWeSendUpOldParent) { } SyncShareAsDelegate(); const sync_pb::CommitMessage& commit = mock_server_->last_sent_commit(); - ASSERT_TRUE(2 == commit.entries_size()); + ASSERT_EQ(2, commit.entries_size()); EXPECT_TRUE(commit.entries(0).parent_id_string() == "2"); EXPECT_TRUE(commit.entries(0).old_parent_id() == "0"); EXPECT_FALSE(commit.entries(1).has_old_parent_id()); @@ -4765,14 +4769,14 @@ class SyncerUndeletionTest : public SyncerTest { }; TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Delete, begin committing the delete, then undelete while committing. @@ -4785,7 +4789,7 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) { // The item ought to exist as an unsynced undeletion (meaning, // we think that the next commit ought to be a recreation commit). - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectUnsyncedUndeletion(); @@ -4796,20 +4800,20 @@ TEST_F(SyncerUndeletionTest, UndeleteDuringCommit) { mock_server_->SetMidCommitCallback(NULL); mock_server_->AddUpdateTombstone(Get(metahandle_, ID)); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); } TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Delete and undelete, then sync to pick up the result. @@ -4820,7 +4824,7 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) { SyncShareAsDelegate(); // The item ought to have committed successfully. - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); EXPECT_EQ(2, Get(metahandle_, BASE_VERSION)); @@ -4829,20 +4833,20 @@ TEST_F(SyncerUndeletionTest, UndeleteBeforeCommit) { // update. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); } TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Delete and commit. @@ -4851,7 +4855,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) { SyncShareAsDelegate(); // The item ought to have committed successfully. - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -4863,26 +4867,26 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterCommitButBeforeGetUpdates) { // deletion update. The undeletion should prevail. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); } TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Delete and commit. @@ -4891,7 +4895,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) { SyncShareAsDelegate(); // The item ought to have committed successfully. - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -4899,7 +4903,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) { // deletion update. Should be consistent. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -4910,28 +4914,28 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterDeleteAndGetUpdates) { // Now, encounter a GetUpdates corresponding to the just-committed // deletion update. The undeletion should prevail. SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); } // Test processing of undeletion GetUpdateses. TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Add a delete from the server. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Some other client deletes the item. @@ -4939,7 +4943,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) { SyncShareAsDelegate(); // The update ought to have applied successfully. - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -4947,7 +4951,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) { Undelete(); ExpectUnsyncedUndeletion(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); @@ -4955,20 +4959,20 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletes) { // deletion update. The undeletion should prevail. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); } TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Some other client deletes the item before we get a chance @@ -4977,7 +4981,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) { SyncShareAsDelegate(); // The update ought to have applied successfully. - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -4985,7 +4989,7 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) { Undelete(); ExpectUnsyncedUndeletion(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); @@ -4993,27 +4997,27 @@ TEST_F(SyncerUndeletionTest, UndeleteAfterOtherClientDeletesImmediately) { // deletion update. The undeletion should prevail. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); } TEST_F(SyncerUndeletionTest, OtherClientUndeletes) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Get the updates of our just-committed entry. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // We delete the item. @@ -5022,7 +5026,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) { SyncShareAsDelegate(); // The update ought to have applied successfully. - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -5030,7 +5034,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) { // deletion update. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -5040,28 +5044,28 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletes) { "Thadeusz", 100, 1000); mock_server_->SetLastUpdateClientTag(client_tag_); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); EXPECT_EQ("Thadeusz", Get(metahandle_, NON_UNIQUE_NAME)); } TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) { - StatusController* status = session_->status_controller(); + const StatusController& status = session_->status_controller(); Create(); ExpectUnsyncedCreation(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // Get the updates of our just-committed entry. mock_server_->AddUpdateFromLastCommit(); SyncShareAsDelegate(); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); ExpectSyncedAndCreated(); // We delete the item. @@ -5070,7 +5074,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) { SyncShareAsDelegate(); // The update ought to have applied successfully. - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndDeleted(); @@ -5081,7 +5085,7 @@ TEST_F(SyncerUndeletionTest, OtherClientUndeletesImmediately) { "Thadeusz", 100, 1000); mock_server_->SetLastUpdateClientTag(client_tag_); SyncShareAsDelegate(); - EXPECT_EQ(0, status->TotalNumConflictingItems()); + EXPECT_EQ(0, status.TotalNumConflictingItems()); EXPECT_EQ(1, mock_server_->GetAndClearNumGetUpdatesRequests()); ExpectSyncedAndCreated(); EXPECT_EQ("Thadeusz", Get(metahandle_, NON_UNIQUE_NAME)); diff --git a/chrome/browser/sync/engine/verify_updates_command.cc b/chrome/browser/sync/engine/verify_updates_command.cc index a5c0439..87d39f5 100644 --- a/chrome/browser/sync/engine/verify_updates_command.cc +++ b/chrome/browser/sync/engine/verify_updates_command.cc @@ -37,7 +37,7 @@ void VerifyUpdatesCommand::ModelChangingExecuteImpl( return; } WriteTransaction trans(FROM_HERE, SYNCER, dir); - sessions::StatusController* status = session->status_controller(); + sessions::StatusController* status = session->mutable_status_controller(); const GetUpdatesResponse& updates = status->updates_response().get_updates(); int update_count = updates.entries().size(); diff --git a/chrome/browser/sync/engine/verify_updates_command_unittest.cc b/chrome/browser/sync/engine/verify_updates_command_unittest.cc index 08fd028..20aab01 100644 --- a/chrome/browser/sync/engine/verify_updates_command_unittest.cc +++ b/chrome/browser/sync/engine/verify_updates_command_unittest.cc @@ -81,7 +81,7 @@ TEST_F(VerifyUpdatesCommandTest, AllVerified) { CreateLocalItem("p1", root, syncable::PREFERENCES); CreateLocalItem("a1", root, syncable::AUTOFILL); - GetUpdatesResponse* updates = session()->status_controller()-> + GetUpdatesResponse* updates = session()->mutable_status_controller()-> mutable_updates_response()->mutable_get_updates(); AddUpdate(updates, "b1", root, syncable::BOOKMARKS); AddUpdate(updates, "b2", root, syncable::BOOKMARKS); @@ -90,18 +90,20 @@ TEST_F(VerifyUpdatesCommandTest, AllVerified) { command_.ExecuteImpl(session()); - StatusController* status = session()->status_controller(); + StatusController* status = session()->mutable_status_controller(); { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI); - EXPECT_EQ(3, status->update_progress().VerifiedUpdatesSize()); + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(3, status->update_progress()->VerifiedUpdatesSize()); } { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_DB); - EXPECT_EQ(1, status->update_progress().VerifiedUpdatesSize()); + ASSERT_TRUE(status->update_progress()); + EXPECT_EQ(1, status->update_progress()->VerifiedUpdatesSize()); } { sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(0, status->update_progress().VerifiedUpdatesSize()); + EXPECT_FALSE(status->update_progress()); } } |