diff options
33 files changed, 376 insertions, 361 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()); } } diff --git a/chrome/browser/sync/sessions/session_state.cc b/chrome/browser/sync/sessions/session_state.cc index f37c9f7..5a55a92 100644 --- a/chrome/browser/sync/sessions/session_state.cc +++ b/chrome/browser/sync/sessions/session_state.cc @@ -228,12 +228,8 @@ ConflictProgress::ConflictSetsSize() const { return conflict_sets_.size(); } -std::set<syncable::Id>::iterator -ConflictProgress::ConflictingItemsBegin() { - return conflicting_item_ids_.begin(); -} std::set<syncable::Id>::const_iterator -ConflictProgress::ConflictingItemsBeginConst() const { +ConflictProgress::ConflictingItemsBegin() const { return conflicting_item_ids_.begin(); } std::set<syncable::Id>::const_iterator diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h index 85fd9a0..99a9cb3 100644 --- a/chrome/browser/sync/sessions/session_state.h +++ b/chrome/browser/sync/sessions/session_state.h @@ -177,8 +177,7 @@ class ConflictProgress { void AddConflictingItemById(const syncable::Id& the_id); void EraseConflictingItemById(const syncable::Id& the_id); int ConflictingItemsSize() const { return conflicting_item_ids_.size(); } - std::set<syncable::Id>::iterator ConflictingItemsBegin(); - std::set<syncable::Id>::const_iterator ConflictingItemsBeginConst() const; + std::set<syncable::Id>::const_iterator ConflictingItemsBegin() const; std::set<syncable::Id>::const_iterator ConflictingItemsEnd() const; // Mutators for nonblocking conflicting items (see description below). diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc index edfebe6..26f5c62 100644 --- a/chrome/browser/sync/sessions/status_controller.cc +++ b/chrome/browser/sync/sessions/status_controller.cc @@ -33,15 +33,60 @@ bool StatusController::TestAndClearIsDirty() { return is_dirty; } +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 ConflictProgress* StatusController::conflict_progress() const { + const PerModelSafeGroupState* state = + GetModelSafeGroupState(true, group_restriction_); + return state ? &state->conflict_progress : NULL; +} + +ConflictProgress* StatusController::mutable_conflict_progress() { + return &GetOrCreateModelSafeGroupState( + true, group_restriction_)->conflict_progress; +} + +const ConflictProgress* StatusController::GetUnrestrictedConflictProgress( + ModelSafeGroup group) const { + const PerModelSafeGroupState* state = + GetModelSafeGroupState(false, group); + return state ? &state->conflict_progress : NULL; +} + +const UpdateProgress* StatusController::GetUnrestrictedUpdateProgress( + ModelSafeGroup group) const { + const PerModelSafeGroupState* state = + GetModelSafeGroupState(false, group); + return state ? &state->update_progress : NULL; +} + +const PerModelSafeGroupState* StatusController::GetModelSafeGroupState( + bool restrict, ModelSafeGroup group) const { + DCHECK_EQ(restrict, group_restriction_in_effect_); + std::map<ModelSafeGroup, PerModelSafeGroupState*>::const_iterator it = + per_model_group_.find(group); + return (it == per_model_group_.end()) ? NULL : it->second; +} + PerModelSafeGroupState* StatusController::GetOrCreateModelSafeGroupState( bool restrict, ModelSafeGroup group) { - DCHECK(restrict == group_restriction_in_effect_) << "Group violation!"; - if (per_model_group_.find(group) == per_model_group_.end()) { + DCHECK_EQ(restrict, group_restriction_in_effect_); + std::map<ModelSafeGroup, PerModelSafeGroupState*>::iterator it = + per_model_group_.find(group); + if (it == per_model_group_.end()) { PerModelSafeGroupState* state = new PerModelSafeGroupState(&is_dirty_); - per_model_group_[group] = state; - return state; + it = per_model_group_.insert(std::make_pair(group, state)).first; } - return per_model_group_[group]; + return it->second; } void StatusController::increment_num_conflicting_commits_by(int value) { @@ -246,7 +291,7 @@ void StatusController::set_debug_info_sent() { shared_.control_params.debug_info_sent = true; } -bool StatusController::debug_info_sent() { +bool StatusController::debug_info_sent() const { return shared_.control_params.debug_info_sent; } diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h index ec1437d..c0782cb 100644 --- a/chrome/browser/sync/sessions/status_controller.h +++ b/chrome/browser/sync/sessions/status_controller.h @@ -36,6 +36,7 @@ #include <vector> #include <map> +#include "base/logging.h" #include "base/stl_util.h" #include "base/time.h" #include "chrome/browser/sync/sessions/ordered_commit_set.h" @@ -53,28 +54,17 @@ class StatusController { // since it was created or was last reset. bool TestAndClearIsDirty(); - // Progress counters. - const ConflictProgress& conflict_progress() { - return GetOrCreateModelSafeGroupState(true, - group_restriction_)->conflict_progress; - } - ConflictProgress* mutable_conflict_progress() { - return &GetOrCreateModelSafeGroupState(true, - group_restriction_)->conflict_progress; - } - const UpdateProgress& update_progress() { - return GetOrCreateModelSafeGroupState(true, - group_restriction_)->update_progress; - } - UpdateProgress* mutable_update_progress() { - return &GetOrCreateModelSafeGroupState(true, - group_restriction_)->update_progress; - } - // Some unrestricted, non-ModelChangingSyncerCommand commands need to store - // meta information about updates. - UpdateProgress* GetUnrestrictedUpdateProgress(ModelSafeGroup group) { - return &GetOrCreateModelSafeGroupState(false, group)->update_progress; - } + // Progress counters. All const methods may return NULL if the + // progress structure doesn't exist, but all non-const methods + // auto-create. + const ConflictProgress* conflict_progress() const; + ConflictProgress* mutable_conflict_progress(); + const UpdateProgress* update_progress() const; + UpdateProgress* mutable_update_progress(); + const ConflictProgress* GetUnrestrictedConflictProgress( + ModelSafeGroup group) const; + const UpdateProgress* GetUnrestrictedUpdateProgress( + ModelSafeGroup group) const; // ClientToServer messages. const ClientToServerMessage& commit_message() { @@ -129,10 +119,14 @@ class StatusController { DCHECK(CurrentCommitIdProjectionHasIndex(index)); return shared_.commit_set.GetCommitIdAt(index); } - syncable::ModelType GetCommitIdModelTypeAt(size_t index) { + syncable::ModelType GetCommitModelTypeAt(size_t index) { DCHECK(CurrentCommitIdProjectionHasIndex(index)); return shared_.commit_set.GetModelTypeAt(index); } + syncable::ModelType GetUnrestrictedCommitModelTypeAt(size_t index) const { + DCHECK(!group_restriction_in_effect_) << "Group restriction in effect!"; + return shared_.commit_set.GetModelTypeAt(index); + } const std::vector<int64>& unsynced_handles() const { DCHECK(!group_restriction_in_effect_) << "unsynced_handles is unrestricted."; @@ -241,7 +235,7 @@ class StatusController { void set_debug_info_sent(); - bool debug_info_sent(); + bool debug_info_sent() const; private: friend class ScopedModelSafeGroupRestriction; @@ -250,9 +244,13 @@ class StatusController { // references position |index| into the full set of commit ids in play. bool CurrentCommitIdProjectionHasIndex(size_t index); + // Returns the state, if it exists, or NULL otherwise. + const PerModelSafeGroupState* GetModelSafeGroupState( + bool restrict, ModelSafeGroup group) const; + // Helper to lazily create objects for per-ModelSafeGroup state. - PerModelSafeGroupState* GetOrCreateModelSafeGroupState(bool restrict, - ModelSafeGroup group); + PerModelSafeGroupState* GetOrCreateModelSafeGroupState( + bool restrict, ModelSafeGroup group); AllModelTypeState shared_; std::map<ModelSafeGroup, PerModelSafeGroupState*> per_model_group_; diff --git a/chrome/browser/sync/sessions/status_controller_unittest.cc b/chrome/browser/sync/sessions/status_controller_unittest.cc index bebbd18..7b850e8 100644 --- a/chrome/browser/sync/sessions/status_controller_unittest.cc +++ b/chrome/browser/sync/sessions/status_controller_unittest.cc @@ -154,19 +154,19 @@ TEST_F(StatusControllerTest, HasConflictingUpdates) { EXPECT_FALSE(status.HasConflictingUpdates()); { ScopedModelSafeGroupRestriction r(&status, GROUP_UI); - EXPECT_FALSE(status.update_progress().HasConflictingUpdates()); + EXPECT_FALSE(status.update_progress()); status.mutable_update_progress()->AddAppliedUpdate(SUCCESS, syncable::Id()); status.mutable_update_progress()->AddAppliedUpdate(CONFLICT, syncable::Id()); - EXPECT_TRUE(status.update_progress().HasConflictingUpdates()); + EXPECT_TRUE(status.update_progress()->HasConflictingUpdates()); } EXPECT_TRUE(status.HasConflictingUpdates()); { ScopedModelSafeGroupRestriction r(&status, GROUP_PASSIVE); - EXPECT_FALSE(status.update_progress().HasConflictingUpdates()); + EXPECT_FALSE(status.update_progress()); } } @@ -186,17 +186,18 @@ TEST_F(StatusControllerTest, TotalNumConflictingItems) { TestIdFactory f; { ScopedModelSafeGroupRestriction r(&status, GROUP_UI); + EXPECT_FALSE(status.conflict_progress()); status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId()); status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId()); - EXPECT_EQ(2, status.conflict_progress().ConflictingItemsSize()); + EXPECT_EQ(2, status.conflict_progress()->ConflictingItemsSize()); } EXPECT_EQ(2, status.TotalNumConflictingItems()); { ScopedModelSafeGroupRestriction r(&status, GROUP_DB); - EXPECT_EQ(0, status.conflict_progress().ConflictingItemsSize()); + EXPECT_FALSE(status.conflict_progress()); status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId()); status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId()); - EXPECT_EQ(2, status.conflict_progress().ConflictingItemsSize()); + EXPECT_EQ(2, status.conflict_progress()->ConflictingItemsSize()); } EXPECT_EQ(4, status.TotalNumConflictingItems()); } @@ -204,8 +205,9 @@ TEST_F(StatusControllerTest, TotalNumConflictingItems) { // Basic test that non group-restricted state accessors don't cause violations. TEST_F(StatusControllerTest, Unrestricted) { StatusController status(routes_); - status.GetUnrestrictedUpdateProgress( - GROUP_UI)->SuccessfullyAppliedUpdateCount(); + const UpdateProgress* progress = + status.GetUnrestrictedUpdateProgress(GROUP_UI); + EXPECT_FALSE(progress); status.mutable_commit_message(); status.commit_response(); status.mutable_commit_response(); diff --git a/chrome/browser/sync/sessions/sync_session.h b/chrome/browser/sync/sessions/sync_session.h index 04b1a1a..2762057 100644 --- a/chrome/browser/sync/sessions/sync_session.h +++ b/chrome/browser/sync/sessions/sync_session.h @@ -125,10 +125,16 @@ class SyncSession { // SyncShare (e.g., HasMoreToSync returned true). void ResetTransientState(); + // TODO(akalin): Split this into context() and mutable_context(). SyncSessionContext* context() const { return context_; } Delegate* delegate() const { return delegate_; } syncable::WriteTransaction* write_transaction() { return write_transaction_; } - StatusController* status_controller() { return status_controller_.get(); } + const StatusController& status_controller() const { + return *status_controller_.get(); + } + StatusController* mutable_status_controller() { + return status_controller_.get(); + } const ExtensionsActivityMonitor::Records& extensions_activity() const { return extensions_activity_; diff --git a/chrome/browser/sync/sessions/sync_session_context.h b/chrome/browser/sync/sessions/sync_session_context.h index fbd5c0d..89a75c4 100644 --- a/chrome/browser/sync/sessions/sync_session_context.h +++ b/chrome/browser/sync/sessions/sync_session_context.h @@ -83,7 +83,7 @@ class SyncSessionContext { DCHECK(account_name_.empty()); account_name_ = name; } - const std::string& account_name() { return account_name_; } + const std::string& account_name() const { return account_name_; } void set_max_commit_batch_size(int batch_size) { max_commit_batch_size_ = batch_size; diff --git a/chrome/browser/sync/sessions/sync_session_unittest.cc b/chrome/browser/sync/sessions/sync_session_unittest.cc index d3f4d99..d914b76 100644 --- a/chrome/browser/sync/sessions/sync_session_unittest.cc +++ b/chrome/browser/sync/sessions/sync_session_unittest.cc @@ -82,7 +82,7 @@ class SyncSessionTest : public testing::Test, out->swap(routes_); } - StatusController* status() { return session_->status_controller(); } + StatusController* status() { return session_->mutable_status_controller(); } protected: void FailControllerInvocationIfDisabled(const std::string& msg) { if (!controller_invocations_allowed_) diff --git a/chrome/browser/sync/sessions/test_util.cc b/chrome/browser/sync/sessions/test_util.cc index 65d307b..7c0eca9 100644 --- a/chrome/browser/sync/sessions/test_util.cc +++ b/chrome/browser/sync/sessions/test_util.cc @@ -10,7 +10,7 @@ namespace test_util { void SimulateHasMoreToSync(sessions::SyncSession* session, SyncerStep begin, SyncerStep end) { - session->status_controller()->update_conflicts_resolved(true); + session->mutable_status_controller()->update_conflicts_resolved(true); ASSERT_TRUE(session->HasMoreToSync()); } @@ -19,7 +19,7 @@ void SimulateDownloadUpdatesFailed(sessions::SyncSession* session, // Note that a non-zero value of changes_remaining once a session has // completed implies that the Syncer was unable to exhaust this count during // the GetUpdates cycle. This is an indication that an error occurred. - session->status_controller()->set_num_server_changes_remaining(1); + session->mutable_status_controller()->set_num_server_changes_remaining(1); } void SimulateCommitFailed(sessions::SyncSession* session, @@ -30,7 +30,7 @@ void SimulateCommitFailed(sessions::SyncSession* session, // See implementation of SyncSession::HasMoreToSync. std::vector<int64> handles; handles.push_back(1); - session->status_controller()->set_unsynced_handles(handles); + session->mutable_status_controller()->set_unsynced_handles(handles); } void SimulateSuccess(sessions::SyncSession* session, @@ -38,8 +38,8 @@ void SimulateSuccess(sessions::SyncSession* session, if (session->HasMoreToSync()) { ADD_FAILURE() << "Shouldn't have more to sync"; } - ASSERT_EQ(0U, session->status_controller()->num_server_changes_remaining()); - ASSERT_EQ(0U, session->status_controller()->unsynced_handles().size()); + ASSERT_EQ(0U, session->status_controller().num_server_changes_remaining()); + ASSERT_EQ(0U, session->status_controller().unsynced_handles().size()); } void SimulateThrottledImpl(sessions::SyncSession* session, |