diff options
23 files changed, 80 insertions, 464 deletions
diff --git a/sync/engine/apply_updates_and_resolve_conflicts_command.cc b/sync/engine/apply_updates_and_resolve_conflicts_command.cc index 214fb39..54b0b38 100644 --- a/sync/engine/apply_updates_and_resolve_conflicts_command.cc +++ b/sync/engine/apply_updates_and_resolve_conflicts_command.cc @@ -37,7 +37,7 @@ ApplyUpdatesAndResolveConflictsCommand::GetGroupsToChange( for (FullModelTypeSet::Iterator it = server_types_with_unapplied_updates.First(); it.Good(); it.Inc()) { groups_with_unapplied_updates.insert( - GetGroupForModelType(it.Get(), session.routing_info())); + GetGroupForModelType(it.Get(), session.context()->routing_info())); } return groups_with_unapplied_updates; @@ -56,7 +56,7 @@ SyncerError ApplyUpdatesAndResolveConflictsCommand::ModelChangingExecuteImpl( FullModelTypeSet server_type_restriction; for (FullModelTypeSet::Iterator it = server_types_with_unapplied_updates.First(); it.Good(); it.Inc()) { - if (GetGroupForModelType(it.Get(), session->routing_info()) == + if (GetGroupForModelType(it.Get(), session->context()->routing_info()) == status->group_restriction()) { server_type_restriction.Put(it.Get()); } @@ -73,7 +73,7 @@ SyncerError ApplyUpdatesAndResolveConflictsCommand::ModelChangingExecuteImpl( // First set of update application passes. UpdateApplicator applicator( dir->GetCryptographer(&trans), - session->routing_info(), + session->context()->routing_info(), status->group_restriction()); applicator.AttemptApplications(&trans, handles); status->increment_num_updates_applied_by(applicator.updates_applied()); @@ -97,7 +97,7 @@ SyncerError ApplyUpdatesAndResolveConflictsCommand::ModelChangingExecuteImpl( UpdateApplicator conflict_applicator( dir->GetCryptographer(&trans), - session->routing_info(), + session->context()->routing_info(), status->group_restriction()); conflict_applicator.AttemptApplications(&trans, handles); diff --git a/sync/engine/build_commit_command.cc b/sync/engine/build_commit_command.cc index de1dae5..fb49553 100644 --- a/sync/engine/build_commit_command.cc +++ b/sync/engine/build_commit_command.cc @@ -99,7 +99,7 @@ void BuildCommitCommand::AddExtensionsActivityToMessage( void BuildCommitCommand::AddClientConfigParamsToMessage( SyncSession* session, sync_pb::CommitMessage* message) { - const ModelSafeRoutingInfo& routing_info = session->routing_info(); + const ModelSafeRoutingInfo& routing_info = session->context()->routing_info(); sync_pb::ClientConfigParams* config_params = message->mutable_config_params(); for (std::map<ModelType, ModelSafeGroup>::const_iterator iter = routing_info.begin(); iter != routing_info.end(); ++iter) { @@ -147,7 +147,9 @@ SyncerError BuildCommitCommand::ExecuteImpl(SyncSession* session) { syncable::GET_BY_ID, id); CHECK(meta_entry.good()); - DCHECK(0 != session->routing_info().count(meta_entry.GetModelType())) + DCHECK_NE(0UL, + session->context()->routing_info().count( + meta_entry.GetModelType())) << "Committing change to datatype that's not actively enabled."; string name = meta_entry.Get(syncable::NON_UNIQUE_NAME); diff --git a/sync/engine/commit.cc b/sync/engine/commit.cc index 59dbc2d..f2728cd 100644 --- a/sync/engine/commit.cc +++ b/sync/engine/commit.cc @@ -144,7 +144,7 @@ SyncerError BuildAndPostCommitsImpl(Syncer* syncer, SyncerError BuildAndPostCommits(Syncer* syncer, sessions::SyncSession* session) { - sessions::OrderedCommitSet commit_set(session->routing_info()); + sessions::OrderedCommitSet commit_set(session->context()->routing_info()); SyncerError result = BuildAndPostCommitsImpl(syncer, session, &commit_set); if (result != SYNCER_OK) { ClearSyncingBits(session->context()->directory(), commit_set); diff --git a/sync/engine/download_updates_command.cc b/sync/engine/download_updates_command.cc index 906a6ab..329fab1 100644 --- a/sync/engine/download_updates_command.cc +++ b/sync/engine/download_updates_command.cc @@ -68,7 +68,7 @@ SyncerError DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) { // Request updates for all enabled types. const ModelTypeSet enabled_types = - GetRoutingInfoTypes(session->routing_info()); + GetRoutingInfoTypes(session->context()->routing_info()); DVLOG(1) << "Getting updates for types " << ModelTypeSetToString(enabled_types); DCHECK(!enabled_types.Empty()); diff --git a/sync/engine/get_commit_ids_command.cc b/sync/engine/get_commit_ids_command.cc index b701ea5..a84024a 100644 --- a/sync/engine/get_commit_ids_command.cc +++ b/sync/engine/get_commit_ids_command.cc @@ -68,7 +68,7 @@ SyncerError GetCommitIdsCommand::ExecuteImpl(SyncSession* session) { &ready_unsynced_set); BuildCommitIds(session->write_transaction(), - session->routing_info(), + session->context()->routing_info(), ready_unsynced_set); const vector<syncable::Id>& verified_commit_ids = diff --git a/sync/engine/model_changing_syncer_command.cc b/sync/engine/model_changing_syncer_command.cc index 3fe6b2d..c2c2750 100644 --- a/sync/engine/model_changing_syncer_command.cc +++ b/sync/engine/model_changing_syncer_command.cc @@ -19,8 +19,8 @@ SyncerError ModelChangingSyncerCommand::ExecuteImpl( const std::set<ModelSafeGroup>& groups_to_change = GetGroupsToChange(*work_session_); - for (size_t i = 0; i < session->workers().size(); ++i) { - ModelSafeWorker* worker = work_session_->workers()[i]; + for (size_t i = 0; i < session->context()->workers().size(); ++i) { + ModelSafeWorker* worker = session->context()->workers()[i]; ModelSafeGroup group = worker->GetModelSafeGroup(); // Skip workers whose group isn't active. if (groups_to_change.count(group) == 0u) { diff --git a/sync/engine/model_changing_syncer_command_unittest.cc b/sync/engine/model_changing_syncer_command_unittest.cc index 867f93e..71345ef 100644 --- a/sync/engine/model_changing_syncer_command_unittest.cc +++ b/sync/engine/model_changing_syncer_command_unittest.cc @@ -28,7 +28,18 @@ class FakeModelChangingSyncerCommand : public ModelChangingSyncerCommand { protected: virtual std::set<ModelSafeGroup> GetGroupsToChange( const sessions::SyncSession& session) const OVERRIDE { - return session.GetEnabledGroups(); + // This command doesn't actually make changes, so the empty set might be an + // appropriate response. That would not be very interesting, so instead we + // return the set of groups with active types. + std::set<ModelSafeGroup> enabled_groups; + const ModelSafeRoutingInfo& routing_info = + session.context()->routing_info(); + for (ModelSafeRoutingInfo::const_iterator it = routing_info.begin(); + it != routing_info.end(); ++it) { + enabled_groups.insert(it->second); + } + enabled_groups.insert(GROUP_PASSIVE); + return enabled_groups; } virtual SyncerError ModelChangingExecuteImpl( diff --git a/sync/engine/process_commit_response_command.cc b/sync/engine/process_commit_response_command.cc index b5b1b81..49b1a4d 100644 --- a/sync/engine/process_commit_response_command.cc +++ b/sync/engine/process_commit_response_command.cc @@ -75,7 +75,7 @@ std::set<ModelSafeGroup> ProcessCommitResponseCommand::GetGroupsToChange( for (size_t i = 0; i < commit_set_.Size(); ++i) { groups_with_commits.insert( GetGroupForModelType(commit_set_.GetModelTypeAt(i), - session.routing_info())); + session.context()->routing_info())); } return groups_with_commits; diff --git a/sync/engine/process_commit_response_command_unittest.cc b/sync/engine/process_commit_response_command_unittest.cc index 40bd61b..3fb7017 100644 --- a/sync/engine/process_commit_response_command_unittest.cc +++ b/sync/engine/process_commit_response_command_unittest.cc @@ -166,7 +166,7 @@ class ProcessCommitResponseCommandTest : public SyncerCommandTest { }; TEST_F(ProcessCommitResponseCommandTest, MultipleCommitIdProjections) { - sessions::OrderedCommitSet commit_set(session()->routing_info()); + sessions::OrderedCommitSet commit_set(session()->context()->routing_info()); sync_pb::ClientToServerMessage request; sync_pb::ClientToServerResponse response; @@ -244,7 +244,7 @@ TEST_F(ProcessCommitResponseCommandTest, MultipleCommitIdProjections) { // how this scenario used to fail, reversing the order for the second half // of the children. TEST_F(ProcessCommitResponseCommandTest, NewFolderCommitKeepsChildOrder) { - sessions::OrderedCommitSet commit_set(session()->routing_info()); + sessions::OrderedCommitSet commit_set(session()->context()->routing_info()); sync_pb::ClientToServerMessage request; sync_pb::ClientToServerResponse response; @@ -375,7 +375,7 @@ INSTANTIATE_TEST_CASE_P(ProcessCommitResponse, // happens to the extensions activity records. Commits could fail or succeed, // depending on the test parameter. TEST_P(MixedResult, ExtensionActivity) { - sessions::OrderedCommitSet commit_set(session()->routing_info()); + sessions::OrderedCommitSet commit_set(session()->context()->routing_info()); sync_pb::ClientToServerMessage request; sync_pb::ClientToServerResponse response; diff --git a/sync/engine/process_updates_command.cc b/sync/engine/process_updates_command.cc index 87533c5..0789958 100644 --- a/sync/engine/process_updates_command.cc +++ b/sync/engine/process_updates_command.cc @@ -45,7 +45,7 @@ std::set<ModelSafeGroup> ProcessUpdatesCommand::GetGroupsToChange( for (int i = 0; i < updates.entries().size(); i++) { groups_with_updates.insert( GetGroupForModelType(GetModelType(updates.entries(i)), - session.routing_info())); + session.context()->routing_info())); } return groups_with_updates; @@ -119,7 +119,7 @@ SyncerError ProcessUpdatesCommand::ModelChangingExecuteImpl( int update_count = updates.entries().size(); ModelTypeSet requested_types = GetRoutingInfoTypes( - session->routing_info()); + session->context()->routing_info()); DVLOG(1) << update_count << " entries to verify"; for (int i = 0; i < update_count; i++) { @@ -133,13 +133,12 @@ SyncerError ProcessUpdatesCommand::ModelChangingExecuteImpl( // TODO(tim): Don't allow access to objects in other ModelSafeGroups. // See crbug.com/121521 . ModelSafeGroup g = GetGroupForModelType(GetModelType(update), - session->routing_info()); + session->context()->routing_info()); if (g != status->group_restriction()) continue; - VerifyResult verify_result = VerifyUpdate(&trans, update, - requested_types, - session->routing_info()); + VerifyResult verify_result = VerifyUpdate( + &trans, update, requested_types, session->context()->routing_info()); status->increment_num_updates_downloaded_by(1); if (!UpdateContainsNewVersion(&trans, update)) status->increment_num_reflected_updates_downloaded_by(1); diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 64d79d7..ef6ae6d 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -279,12 +279,6 @@ void SyncSchedulerImpl::Start(Mode mode) { scoped_ptr<SyncSessionJob> pending(TakePendingJobForCurrentMode()); if (pending.get()) { - // TODO(tim): We should be able to remove this... - scoped_ptr<SyncSession> session(CreateSyncSession( - pending->session()->source())); - // Also the routing info might have been changed since we cached the - // pending nudge. Update it by coalescing to the latest. - pending->mutable_session()->Coalesce(*session); SDVLOG(2) << "Executing pending job. Good luck!"; DoSyncSessionJob(pending.Pass()); } @@ -293,9 +287,8 @@ void SyncSchedulerImpl::Start(Mode mode) { void SyncSchedulerImpl::SendInitialSnapshot() { DCHECK_EQ(MessageLoop::current(), sync_loop_); - scoped_ptr<SyncSession> dummy(new SyncSession(session_context_, this, - SyncSourceInfo(), ModelSafeRoutingInfo(), - std::vector<ModelSafeWorker*>())); + scoped_ptr<SyncSession> dummy(new SyncSession( + session_context_, this, SyncSourceInfo())); SyncEngineEvent event(SyncEngineEvent::STATUS_CHANGED); event.snapshot = dummy->TakeSnapshot(); session_context_->NotifyListeners(event); @@ -304,7 +297,7 @@ void SyncSchedulerImpl::SendInitialSnapshot() { namespace { // Helper to extract the routing info corresponding to types in -// |types| from |current_routes|. +// |types_to_download| from |current_routes|. void BuildModelSafeParams( ModelTypeSet types_to_download, const ModelSafeRoutingInfo& current_routes, @@ -338,7 +331,7 @@ bool SyncSchedulerImpl::ScheduleConfiguration( BuildModelSafeParams(params.types_to_download, params.routing_info, &restricted_routes); - session_context_->set_routing_info(params.routing_info); + session_context_->set_routing_info(restricted_routes); // Only reconfigure if we have types to download. if (!params.types_to_download.Empty()) { @@ -349,9 +342,7 @@ bool SyncSchedulerImpl::ScheduleConfiguration( SyncSourceInfo(params.source, ModelSafeRoutingInfoToInvalidationMap( restricted_routes, - std::string())), - restricted_routes, - session_context_->workers())); + std::string())))); scoped_ptr<SyncSessionJob> job(new SyncSessionJob( SyncSessionJob::CONFIGURATION, TimeTicks::Now(), @@ -512,7 +503,8 @@ void SyncSchedulerImpl::HandleSaveJobDecision(scoped_ptr<SyncSessionJob> job) { // logic in ScheduleNudgeImpl that takes the min of the two nudge start // times, because we're calling this function first. Pull this out // into a function to coalesce + set start times and reuse. - pending_nudge_->mutable_session()->Coalesce(*(job->session())); + pending_nudge_->mutable_session()->CoalesceSources( + job->session()->source()); return; } @@ -648,7 +640,8 @@ void SyncSchedulerImpl::ScheduleNudgeImpl( if (pending_nudge_) { SDVLOG(2) << "Rescheduling pending nudge"; - pending_nudge_->mutable_session()->Coalesce(*(job->session())); + pending_nudge_->mutable_session()->CoalesceSources( + job->session()->source()); // Choose the start time as the earliest of the 2. Note that this means // if a nudge arrives with delay (e.g. kDefaultSessionsCommitDelaySeconds) // but a nudge is already scheduled to go out, we'll send the (tab) commit @@ -759,11 +752,6 @@ bool SyncSchedulerImpl::DoSyncSessionJob(scoped_ptr<SyncSessionJob> job) { return false; } pending_nudge_ = NULL; - - // Rebase the session with the latest model safe table and use it to purge - // and update any disabled or modified entries in the job. - job->mutable_session()->RebaseRoutingInfoWithLatest( - session_context_->routing_info(), session_context_->workers()); } base::AutoReset<bool> protector(&no_scheduling_allowed_, true); @@ -1045,12 +1033,6 @@ void SyncSchedulerImpl::DoCanaryJob(scoped_ptr<SyncSessionJob> to_be_canary) { return; } DCHECK_EQ(pending_nudge_->session(), to_be_canary->session()); - // TODO(tim): We should be able to remove this... - scoped_ptr<SyncSession> temp = CreateSyncSession( - to_be_canary->session()->source()).Pass(); - // The routing info might have been changed since we cached the - // pending nudge. Update it by coalescing to the latest. - to_be_canary->mutable_session()->Coalesce(*(temp)); } DoSyncSessionJob(to_be_canary.Pass()); } @@ -1085,8 +1067,7 @@ scoped_ptr<SyncSession> SyncSchedulerImpl::CreateSyncSession( << ModelSafeRoutingInfoToString(session_context_->routing_info()); SyncSourceInfo info(source); - return scoped_ptr<SyncSession>(new SyncSession(session_context_, this, info, - session_context_->routing_info(), session_context_->workers())); + return scoped_ptr<SyncSession>(new SyncSession(session_context_, this, info)); } void SyncSchedulerImpl::PollTimerCallback() { diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index 4f7783a..4718952 100644 --- a/sync/engine/sync_scheduler_unittest.cc +++ b/sync/engine/sync_scheduler_unittest.cc @@ -581,8 +581,8 @@ TEST_F(SyncSchedulerTest, NudgeWithStatesCoalescing) { ASSERT_EQ(1U, r.snapshots.size()); EXPECT_GE(r.times[0], optimal_time); ModelTypeInvalidationMap coalesced_types; - CoalesceStates(&coalesced_types, types1); - CoalesceStates(&coalesced_types, types2); + CoalesceStates(types1, &coalesced_types); + CoalesceStates(types2, &coalesced_types); EXPECT_THAT(coalesced_types, Eq(r.snapshots[0].source().types)); EXPECT_EQ(GetUpdatesCallerInfo::LOCAL, r.snapshots[0].source().updates_source); diff --git a/sync/engine/sync_session_job.cc b/sync/engine/sync_session_job.cc index 72a7741..f980a5c 100644 --- a/sync/engine/sync_session_job.cc +++ b/sync/engine/sync_session_job.cc @@ -111,8 +111,7 @@ scoped_ptr<SyncSessionJob> SyncSessionJob::CloneFromLocation( scoped_ptr<sessions::SyncSession> SyncSessionJob::CloneSession() const { return scoped_ptr<sessions::SyncSession>( new sessions::SyncSession(session_->context(), - session_->delegate(), session_->source(), session_->routing_info(), - session_->workers())); + session_->delegate(), session_->source())); } bool SyncSessionJob::is_canary() const { diff --git a/sync/engine/sync_session_job_unittest.cc b/sync/engine/sync_session_job_unittest.cc index 466dffa..f19ade8 100644 --- a/sync/engine/sync_session_job_unittest.cc +++ b/sync/engine/sync_session_job_unittest.cc @@ -64,9 +64,8 @@ class SyncSessionJobTest : public testing::Test { scoped_ptr<SyncSession> NewLocalSession() { sessions::SyncSourceInfo info( sync_pb::GetUpdatesCallerInfo::LOCAL, ModelTypeInvalidationMap()); - return scoped_ptr<SyncSession>(new SyncSession(context_.get(), - &delegate_, info, context_->routing_info(), - context_->workers())); + return scoped_ptr<SyncSession>( + new SyncSession(context_.get(), &delegate_, info)); } void GetWorkers(std::vector<ModelSafeWorker*>* out) const { @@ -126,13 +125,9 @@ TEST_F(SyncSessionJobTest, Clone) { ExpectClonesBase(&job1, clone1.get()); ExpectClonesBase(&job1, clone1_loc.get()); EXPECT_NE(job1.session(), clone1->session()); - EXPECT_EQ(job1.session()->routing_info(), - clone1->session()->routing_info()); EXPECT_EQ(job1.from_location().ToString(), clone1->from_location().ToString()); EXPECT_NE(job1.session(), clone1_loc->session()); - EXPECT_EQ(job1.session()->routing_info(), - clone1_loc->session()->routing_info()); EXPECT_EQ(from_here1.ToString(), clone1_loc->from_location().ToString()); context()->set_routing_info(routes()); @@ -148,21 +143,15 @@ TEST_F(SyncSessionJobTest, Clone) { ExpectClonesBase(clone1.get(), clone2.get()); ExpectClonesBase(clone1.get(), clone2_loc.get()); EXPECT_NE(clone1->session(), clone2->session()); - EXPECT_EQ(clone1->session()->routing_info(), - clone2->session()->routing_info()); EXPECT_EQ(clone1->from_location().ToString(), clone2->from_location().ToString()); EXPECT_NE(clone1->session(), clone2->session()); - EXPECT_EQ(clone1->session()->routing_info(), - clone2->session()->routing_info()); EXPECT_EQ(from_here2.ToString(), clone2_loc->from_location().ToString()); clone1.reset(); clone1_loc.reset(); ExpectClonesBase(&job1, clone2.get()); EXPECT_NE(job1.session(), clone2->session()); - EXPECT_EQ(job1.session()->routing_info(), - clone2->session()->routing_info()); EXPECT_EQ(job1.from_location().ToString(), clone2->from_location().ToString()); } @@ -191,7 +180,6 @@ TEST_F(SyncSessionJobTest, CloneAndAbandon) { ExpectClonesBase(&job1, clone1.get()); EXPECT_FALSE(job1.session()); EXPECT_EQ(session_ptr, clone1->session()); - EXPECT_EQ(session_ptr->routing_info(), clone1->session()->routing_info()); } // Tests interaction between Finish and sync cycle success / failure. @@ -223,9 +211,8 @@ TEST_F(SyncSessionJobTest, FinishCallsReadyTask) { sessions::SyncSourceInfo info( sync_pb::GetUpdatesCallerInfo::RECONFIGURATION, ModelTypeInvalidationMap()); - scoped_ptr<SyncSession> session(new SyncSession(context(), - delegate(), info, context()->routing_info(), - context()->workers())); + scoped_ptr<SyncSession> session( + new SyncSession(context(), delegate(), info)); SyncSessionJob job1(SyncSessionJob::CONFIGURATION, TimeTicks::Now(), session.Pass(), params, FROM_HERE); diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index 0224709..37d127e 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -174,15 +174,12 @@ class SyncerTest : public testing::Test, SyncSession* MakeSession() { ModelSafeRoutingInfo info; - std::vector<ModelSafeWorker*> workers; GetModelSafeRoutingInfo(&info); - GetWorkers(&workers); ModelTypeInvalidationMap invalidation_map = ModelSafeRoutingInfoToInvalidationMap(info, std::string()); return new SyncSession(context_.get(), this, sessions::SyncSourceInfo(sync_pb::GetUpdatesCallerInfo::UNKNOWN, - invalidation_map), - info, workers); + invalidation_map)); } diff --git a/sync/internal_api/public/base/model_type_invalidation_map.cc b/sync/internal_api/public/base/model_type_invalidation_map.cc index 1084947..65a125f 100644 --- a/sync/internal_api/public/base/model_type_invalidation_map.cc +++ b/sync/internal_api/public/base/model_type_invalidation_map.cc @@ -56,8 +56,8 @@ DictionaryValue* ModelTypeInvalidationMapToValue( return value; } -void CoalesceStates(ModelTypeInvalidationMap* original, - const ModelTypeInvalidationMap& update) { +void CoalesceStates(const ModelTypeInvalidationMap& update, + ModelTypeInvalidationMap* original) { // TODO(dcheng): Where is this called? Do we need to add more clever logic for // handling ack_handle? We probably want to always use the "latest" // ack_handle, which might imply always using the one in update? diff --git a/sync/internal_api/public/base/model_type_invalidation_map.h b/sync/internal_api/public/base/model_type_invalidation_map.h index 9216562..462e17e 100644 --- a/sync/internal_api/public/base/model_type_invalidation_map.h +++ b/sync/internal_api/public/base/model_type_invalidation_map.h @@ -45,8 +45,8 @@ SYNC_EXPORT_PRIVATE base::DictionaryValue* ModelTypeInvalidationMapToValue( // Coalesce |update| into |original|, overwriting only when |update| has // a non-empty payload. -SYNC_EXPORT_PRIVATE void CoalesceStates(ModelTypeInvalidationMap* original, - const ModelTypeInvalidationMap& update); +SYNC_EXPORT_PRIVATE void CoalesceStates( + const ModelTypeInvalidationMap& update, ModelTypeInvalidationMap* original); } // namespace syncer diff --git a/sync/internal_api/public/base/model_type_invalidation_map_unittest.cc b/sync/internal_api/public/base/model_type_invalidation_map_unittest.cc index 2b1049b..f051479 100644 --- a/sync/internal_api/public/base/model_type_invalidation_map_unittest.cc +++ b/sync/internal_api/public/base/model_type_invalidation_map_unittest.cc @@ -57,7 +57,7 @@ TEST_F(ModelTypeInvalidationMapTest, CoalesceStates) { update[SESSIONS].payload = payload2; // New. // Themes untouched. - CoalesceStates(&original, update); + CoalesceStates(update, &original); ASSERT_EQ(5U, original.size()); EXPECT_EQ(empty_payload, original[BOOKMARKS].payload); EXPECT_EQ(payload1, original[PASSWORDS].payload); diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc index f9241d9..11c95eb 100644 --- a/sync/sessions/sync_session.cc +++ b/sync/sessions/sync_session.cc @@ -15,136 +15,24 @@ namespace syncer { namespace sessions { -namespace { - -std::set<ModelSafeGroup> ComputeEnabledGroups( - const ModelSafeRoutingInfo& routing_info, - const std::vector<ModelSafeWorker*>& workers) { - std::set<ModelSafeGroup> enabled_groups; - // Project the list of enabled types (i.e., types in the routing - // info) to a list of enabled groups. - for (ModelSafeRoutingInfo::const_iterator it = routing_info.begin(); - it != routing_info.end(); ++it) { - enabled_groups.insert(it->second); - } - // GROUP_PASSIVE is always enabled, since that's the group that - // top-level folders map to. - enabled_groups.insert(GROUP_PASSIVE); - if (DCHECK_IS_ON()) { - // We sometimes create dummy SyncSession objects (see - // SyncScheduler::InitialSnapshot) so don't check in that case. - if (!routing_info.empty() || !workers.empty()) { - std::set<ModelSafeGroup> groups_with_workers; - for (std::vector<ModelSafeWorker*>::const_iterator it = workers.begin(); - it != workers.end(); ++it) { - groups_with_workers.insert((*it)->GetModelSafeGroup()); - } - // All enabled groups should have a corresponding worker. - DCHECK(std::includes( - groups_with_workers.begin(), groups_with_workers.end(), - enabled_groups.begin(), enabled_groups.end())); - } - } - return enabled_groups; -} - -void PurgeStaleStates(ModelTypeInvalidationMap* original, - const ModelSafeRoutingInfo& routing_info) { - std::vector<ModelTypeInvalidationMap::iterator> iterators_to_delete; - for (ModelTypeInvalidationMap::iterator i = original->begin(); - i != original->end(); ++i) { - if (routing_info.end() == routing_info.find(i->first)) { - iterators_to_delete.push_back(i); - } - } - - for (std::vector<ModelTypeInvalidationMap::iterator>::iterator - it = iterators_to_delete.begin(); it != iterators_to_delete.end(); - ++it) { - original->erase(*it); - } -} - -} // namesepace - -SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate, - const SyncSourceInfo& source, - const ModelSafeRoutingInfo& routing_info, - const std::vector<ModelSafeWorker*>& workers) +SyncSession::SyncSession( + SyncSessionContext* context, + Delegate* delegate, + const SyncSourceInfo& source) : context_(context), source_(source), write_transaction_(NULL), - delegate_(delegate), - workers_(workers), - routing_info_(routing_info), - enabled_groups_(ComputeEnabledGroups(routing_info_, workers_)) { - status_controller_.reset(new StatusController(routing_info_)); - std::sort(workers_.begin(), workers_.end()); + delegate_(delegate) { + status_controller_.reset(new StatusController(context_->routing_info())); debug_info_sources_list_.push_back(source_); } SyncSession::~SyncSession() {} -void SyncSession::Coalesce(const SyncSession& session) { - if (context_ != session.context() || delegate_ != session.delegate_) { - NOTREACHED(); - return; - } - - // When we coalesce sessions, the sync update source gets overwritten with the - // most recent, while the type/state map gets merged. - debug_info_sources_list_.push_back(session.source_); - CoalesceStates(&source_.types, session.source_.types); - source_.updates_source = session.source_.updates_source; - - std::vector<ModelSafeWorker*> temp; - std::set_union(workers_.begin(), workers_.end(), - session.workers_.begin(), session.workers_.end(), - std::back_inserter(temp)); - workers_.swap(temp); - - // We have to update the model safe routing info to the union. In case the - // same key is present in both pick the one from session. - for (ModelSafeRoutingInfo::const_iterator it = - session.routing_info_.begin(); - it != session.routing_info_.end(); - ++it) { - routing_info_[it->first] = it->second; - } - - // Now update enabled groups. - enabled_groups_ = ComputeEnabledGroups(routing_info_, workers_); -} - -void SyncSession::RebaseRoutingInfoWithLatest( - const ModelSafeRoutingInfo& routing_info, - const std::vector<ModelSafeWorker*>& workers) { - ModelSafeRoutingInfo temp_routing_info; - - // Take the intersection and also set the routing info(it->second) from the - // passed in session. - for (ModelSafeRoutingInfo::const_iterator it = - routing_info.begin(); it != routing_info.end(); - ++it) { - if (routing_info_.find(it->first) != routing_info_.end()) { - temp_routing_info[it->first] = it->second; - } - } - routing_info_.swap(temp_routing_info); - - PurgeStaleStates(&source_.types, routing_info); - - // Now update the workers. - std::vector<ModelSafeWorker*> temp; - std::vector<ModelSafeWorker*> sorted_workers = workers; - std::sort(sorted_workers.begin(), sorted_workers.end()); - std::set_intersection(workers_.begin(), workers_.end(), - sorted_workers.begin(), sorted_workers.end(), - std::back_inserter(temp)); - workers_.swap(temp); - - // Now update enabled groups. - enabled_groups_ = ComputeEnabledGroups(routing_info_, workers_); +void SyncSession::CoalesceSources(const SyncSourceInfo& source) { + debug_info_sources_list_.push_back(source); + CoalesceStates(source.types, &source_.types); + source_.updates_source = source.updates_source; } SyncSessionSnapshot SyncSession::TakeSnapshot() const { @@ -187,10 +75,6 @@ void SyncSession::SendEventNotification(SyncEngineEvent::EventCause cause) { context()->NotifyListeners(event); } -const std::set<ModelSafeGroup>& SyncSession::GetEnabledGroups() const { - return enabled_groups_; -} - bool SyncSession::DidReachServer() const { const ModelNeutralState& state = status_controller_->model_neutral_state(); return state.last_get_key_result >= FIRST_SERVER_RETURN_VALUE || diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h index 901e3b5..7271b5f 100644 --- a/sync/sessions/sync_session.h +++ b/sync/sessions/sync_session.h @@ -97,9 +97,7 @@ class SYNC_EXPORT_PRIVATE SyncSession { SyncSession(SyncSessionContext* context, Delegate* delegate, - const SyncSourceInfo& source, - const ModelSafeRoutingInfo& routing_info, - const std::vector<ModelSafeWorker*>& workers); + const SyncSourceInfo& source); ~SyncSession(); // Builds a thread-safe and read-only copy of the current session state. @@ -114,20 +112,9 @@ class SYNC_EXPORT_PRIVATE SyncSession { // See SERVER_RETURN_* in the SyncerError enum for values. bool DidReachServer() const; - // Collects all state pertaining to how and why |s| originated and unions it - // with corresponding state in |this|, leaving |s| unchanged. Allows |this| - // to take on the responsibilities |s| had (e.g. certain data types) in the - // next SyncShare operation using |this|, rather than needed two separate - // sessions. - void Coalesce(const SyncSession& session); - - // Compares the routing_info_, workers and payload map with those passed in. - // Purges types from the above 3 which are not present in latest. Useful - // to update the sync session when the user has disabled some types from - // syncing. - void RebaseRoutingInfoWithLatest( - const ModelSafeRoutingInfo& routing_info, - const std::vector<ModelSafeWorker*>& workers); + // Overwrite the sync update source with the most recent and merge the + // type/state map. + void CoalesceSources(const SyncSourceInfo& source); // TODO(akalin): Split this into context() and mutable_context(). SyncSessionContext* context() const { return context_; } @@ -147,13 +134,8 @@ class SYNC_EXPORT_PRIVATE SyncSession { return &extensions_activity_; } - const std::vector<ModelSafeWorker*>& workers() const { return workers_; } - const ModelSafeRoutingInfo& routing_info() const { return routing_info_; } const SyncSourceInfo& source() const { return source_; } - // Returns the set of groups which have enabled types. - const std::set<ModelSafeGroup>& GetEnabledGroups() const; - private: // Extend the encapsulation boundary to utilities for internal member // assignments. This way, the scope of these actions is explicit, they can't @@ -182,19 +164,6 @@ class SYNC_EXPORT_PRIVATE SyncSession { // Our controller for various status and error counters. scoped_ptr<StatusController> status_controller_; - // The set of active ModelSafeWorkers for the duration of this session. - // This can change if this session is Coalesce()'d with another. - std::vector<ModelSafeWorker*> workers_; - - // The routing info for the duration of this session, dictating which - // datatypes should be synced and which workers should be used when working - // on those datatypes. - ModelSafeRoutingInfo routing_info_; - - // The set of groups with enabled types. Computed from - // |routing_info_|. - std::set<ModelSafeGroup> enabled_groups_; - DISALLOW_COPY_AND_ASSIGN(SyncSession); }; diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc index 8044113..008e893 100644 --- a/sync/sessions/sync_session_unittest.cc +++ b/sync/sessions/sync_session_unittest.cc @@ -33,10 +33,7 @@ class SyncSessionTest : public testing::Test, SyncSessionTest() : controller_invocations_allowed_(false) {} SyncSession* MakeSession() { - std::vector<ModelSafeWorker*> workers; - GetWorkers(&workers); - return new SyncSession(context_.get(), this, SyncSourceInfo(), - routes_, workers); + return new SyncSession(context_.get(), this, SyncSourceInfo()); } virtual void SetUp() { @@ -142,24 +139,6 @@ class SyncSessionTest : public testing::Test, scoped_ptr<ThrottledDataTypeTracker> throttled_data_type_tracker_; }; -TEST_F(SyncSessionTest, EnabledGroupsEmpty) { - routes_.clear(); - workers_.clear(); - scoped_ptr<SyncSession> session(MakeSession()); - std::set<ModelSafeGroup> expected_enabled_groups; - expected_enabled_groups.insert(GROUP_PASSIVE); - EXPECT_EQ(expected_enabled_groups, session->GetEnabledGroups()); -} - -TEST_F(SyncSessionTest, EnabledGroups) { - scoped_ptr<SyncSession> session(MakeSession()); - std::set<ModelSafeGroup> expected_enabled_groups; - expected_enabled_groups.insert(GROUP_PASSIVE); - expected_enabled_groups.insert(GROUP_UI); - expected_enabled_groups.insert(GROUP_DB); - EXPECT_EQ(expected_enabled_groups, session->GetEnabledGroups()); -} - TEST_F(SyncSessionTest, SetWriteTransaction) { TestDirectorySetterUpper dir_maker; dir_maker.SetUp(); @@ -206,9 +185,7 @@ TEST_F(SyncSessionTest, MoreToDownloadIfGotNoChangesRemaining) { EXPECT_TRUE(status()->download_updates_succeeded()); } -TEST_F(SyncSessionTest, Coalesce) { - std::vector<ModelSafeWorker*> workers_one, workers_two; - ModelSafeRoutingInfo routes_one, routes_two; +TEST_F(SyncSessionTest, CoalesceSources) { ModelTypeInvalidationMap one_type = ModelTypeSetToInvalidationMap( ParamsMeaningJustOneEnabledType(), @@ -220,204 +197,14 @@ TEST_F(SyncSessionTest, Coalesce) { SyncSourceInfo source_one(sync_pb::GetUpdatesCallerInfo::PERIODIC, one_type); SyncSourceInfo source_two(sync_pb::GetUpdatesCallerInfo::LOCAL, all_types); - scoped_refptr<ModelSafeWorker> passive_worker( - new FakeModelWorker(GROUP_PASSIVE)); - scoped_refptr<ModelSafeWorker> db_worker(new FakeModelWorker(GROUP_DB)); - scoped_refptr<ModelSafeWorker> ui_worker(new FakeModelWorker(GROUP_UI)); - workers_one.push_back(passive_worker); - workers_one.push_back(db_worker); - workers_two.push_back(passive_worker); - workers_two.push_back(db_worker); - workers_two.push_back(ui_worker); - routes_one[AUTOFILL] = GROUP_DB; - routes_two[AUTOFILL] = GROUP_DB; - routes_two[BOOKMARKS] = GROUP_UI; - SyncSession one(context_.get(), this, source_one, routes_one, workers_one); - SyncSession two(context_.get(), this, source_two, routes_two, workers_two); - - std::set<ModelSafeGroup> expected_enabled_groups_one; - expected_enabled_groups_one.insert(GROUP_PASSIVE); - expected_enabled_groups_one.insert(GROUP_DB); - - std::set<ModelSafeGroup> expected_enabled_groups_two; - expected_enabled_groups_two.insert(GROUP_PASSIVE); - expected_enabled_groups_two.insert(GROUP_DB); - expected_enabled_groups_two.insert(GROUP_UI); - - EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups()); - EXPECT_EQ(expected_enabled_groups_two, two.GetEnabledGroups()); - - one.Coalesce(two); - - EXPECT_EQ(expected_enabled_groups_two, one.GetEnabledGroups()); - EXPECT_EQ(expected_enabled_groups_two, two.GetEnabledGroups()); - - EXPECT_EQ(two.source().updates_source, one.source().updates_source); - EXPECT_THAT(all_types, Eq(one.source().types)); - std::vector<ModelSafeWorker*>::const_iterator it_db = - std::find(one.workers().begin(), one.workers().end(), db_worker); - std::vector<ModelSafeWorker*>::const_iterator it_ui = - std::find(one.workers().begin(), one.workers().end(), ui_worker); - EXPECT_NE(it_db, one.workers().end()); - EXPECT_NE(it_ui, one.workers().end()); - EXPECT_EQ(routes_two, one.routing_info()); -} - -TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestRemoveOneType) { - std::vector<ModelSafeWorker*> workers_one, workers_two; - ModelSafeRoutingInfo routes_one, routes_two; - ModelTypeInvalidationMap one_type = - ModelTypeSetToInvalidationMap( - ParamsMeaningJustOneEnabledType(), - std::string()); - ModelTypeInvalidationMap all_types = - ModelTypeSetToInvalidationMap( - ParamsMeaningAllEnabledTypes(), - std::string()); - SyncSourceInfo source_one(sync_pb::GetUpdatesCallerInfo::PERIODIC, one_type); - SyncSourceInfo source_two(sync_pb::GetUpdatesCallerInfo::LOCAL, all_types); + SyncSession session(context_.get(), this, source_one); - scoped_refptr<ModelSafeWorker> passive_worker( - new FakeModelWorker(GROUP_PASSIVE)); - scoped_refptr<ModelSafeWorker> db_worker(new FakeModelWorker(GROUP_DB)); - scoped_refptr<ModelSafeWorker> ui_worker(new FakeModelWorker(GROUP_UI)); - workers_one.push_back(passive_worker); - workers_one.push_back(db_worker); - workers_two.push_back(passive_worker); - workers_two.push_back(db_worker); - workers_two.push_back(ui_worker); - routes_one[AUTOFILL] = GROUP_DB; - routes_two[AUTOFILL] = GROUP_DB; - routes_two[BOOKMARKS] = GROUP_UI; - SyncSession one(context_.get(), this, source_one, routes_one, workers_one); - SyncSession two(context_.get(), this, source_two, routes_two, workers_two); - - std::set<ModelSafeGroup> expected_enabled_groups_one; - expected_enabled_groups_one.insert(GROUP_PASSIVE); - expected_enabled_groups_one.insert(GROUP_DB); - - std::set<ModelSafeGroup> expected_enabled_groups_two; - expected_enabled_groups_two.insert(GROUP_PASSIVE); - expected_enabled_groups_two.insert(GROUP_DB); - expected_enabled_groups_two.insert(GROUP_UI); - - EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups()); - EXPECT_EQ(expected_enabled_groups_two, two.GetEnabledGroups()); - - two.RebaseRoutingInfoWithLatest(one.routing_info(), one.workers()); - - EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups()); - EXPECT_EQ(expected_enabled_groups_one, two.GetEnabledGroups()); - - // Make sure the source has not been touched. - EXPECT_EQ(two.source().updates_source, - sync_pb::GetUpdatesCallerInfo::LOCAL); - - // Make sure the payload is reduced to one. - EXPECT_THAT(one_type, Eq(two.source().types)); - - // Make sure the workers are udpated. - std::vector<ModelSafeWorker*>::const_iterator it_db = - std::find(two.workers().begin(), two.workers().end(), db_worker); - std::vector<ModelSafeWorker*>::const_iterator it_ui = - std::find(two.workers().begin(), two.workers().end(), ui_worker); - EXPECT_NE(it_db, two.workers().end()); - EXPECT_EQ(it_ui, two.workers().end()); - EXPECT_EQ(two.workers().size(), 2U); - - // Make sure the model safe routing info is reduced to one type. - ModelSafeRoutingInfo::const_iterator it = - two.routing_info().find(AUTOFILL); - // Note that attempting to use EXPECT_NE would fail for an Android build due - // to seeming incompatibility with gtest and stlport. - EXPECT_TRUE(it != two.routing_info().end()); - EXPECT_EQ(it->second, GROUP_DB); - EXPECT_EQ(two.routing_info().size(), 1U); -} + session.CoalesceSources(source_two); -TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestWithSameType) { - std::vector<ModelSafeWorker*> workers_first, workers_second; - ModelSafeRoutingInfo routes_first, routes_second; - ModelTypeInvalidationMap all_types = - ModelTypeSetToInvalidationMap( - ParamsMeaningAllEnabledTypes(), - std::string()); - SyncSourceInfo source_first(sync_pb::GetUpdatesCallerInfo::PERIODIC, - all_types); - SyncSourceInfo source_second(sync_pb::GetUpdatesCallerInfo::LOCAL, - all_types); - - scoped_refptr<ModelSafeWorker> passive_worker( - new FakeModelWorker(GROUP_PASSIVE)); - scoped_refptr<FakeModelWorker> db_worker(new FakeModelWorker(GROUP_DB)); - scoped_refptr<FakeModelWorker> ui_worker(new FakeModelWorker(GROUP_UI)); - workers_first.push_back(passive_worker); - workers_first.push_back(db_worker); - workers_first.push_back(ui_worker); - workers_second.push_back(passive_worker); - workers_second.push_back(db_worker); - workers_second.push_back(ui_worker); - routes_first[AUTOFILL] = GROUP_DB; - routes_first[BOOKMARKS] = GROUP_UI; - routes_second[AUTOFILL] = GROUP_DB; - routes_second[BOOKMARKS] = GROUP_UI; - SyncSession first(context_.get(), this, source_first, routes_first, - workers_first); - SyncSession second(context_.get(), this, source_second, routes_second, - workers_second); - - std::set<ModelSafeGroup> expected_enabled_groups; - expected_enabled_groups.insert(GROUP_PASSIVE); - expected_enabled_groups.insert(GROUP_DB); - expected_enabled_groups.insert(GROUP_UI); - - EXPECT_EQ(expected_enabled_groups, first.GetEnabledGroups()); - EXPECT_EQ(expected_enabled_groups, second.GetEnabledGroups()); - - second.RebaseRoutingInfoWithLatest(first.routing_info(), first.workers()); - - EXPECT_EQ(expected_enabled_groups, first.GetEnabledGroups()); - EXPECT_EQ(expected_enabled_groups, second.GetEnabledGroups()); - - // Make sure the source has not been touched. - EXPECT_EQ(second.source().updates_source, - sync_pb::GetUpdatesCallerInfo::LOCAL); - - // Make sure our payload is still the same. - EXPECT_THAT(all_types, Eq(second.source().types)); - - // Make sure the workers are still the same. - std::vector<ModelSafeWorker*>::const_iterator it_passive = - std::find(second.workers().begin(), second.workers().end(), - passive_worker); - std::vector<ModelSafeWorker*>::const_iterator it_db = - std::find(second.workers().begin(), second.workers().end(), db_worker); - std::vector<ModelSafeWorker*>::const_iterator it_ui = - std::find(second.workers().begin(), second.workers().end(), ui_worker); - EXPECT_NE(it_passive, second.workers().end()); - EXPECT_NE(it_db, second.workers().end()); - EXPECT_NE(it_ui, second.workers().end()); - EXPECT_EQ(second.workers().size(), 3U); - - // Make sure the model safe routing info is reduced to first type. - ModelSafeRoutingInfo::const_iterator it1 = - second.routing_info().find(AUTOFILL); - ModelSafeRoutingInfo::const_iterator it2 = - second.routing_info().find(BOOKMARKS); - - // Note that attempting to use EXPECT_NE would fail for an Android build due - // to seeming incompatibility with gtest and stlport. - EXPECT_TRUE(it1 != second.routing_info().end()); - EXPECT_EQ(it1->second, GROUP_DB); - - // Note that attempting to use EXPECT_NE would fail for an Android build due - // to seeming incompatibility with gtest and stlport. - EXPECT_TRUE(it2 != second.routing_info().end()); - EXPECT_EQ(it2->second, GROUP_UI); - EXPECT_EQ(second.routing_info().size(), 2U); + EXPECT_EQ(source_two.updates_source, session.source().updates_source); + EXPECT_THAT(all_types, Eq(session.source().types)); } - TEST_F(SyncSessionTest, MakeTypeInvalidationMapFromBitSet) { ModelTypeSet types; std::string payload = "test"; diff --git a/sync/test/engine/syncer_command_test.cc b/sync/test/engine/syncer_command_test.cc index 2439c1b..43d3b54 100644 --- a/sync/test/engine/syncer_command_test.cc +++ b/sync/test/engine/syncer_command_test.cc @@ -17,10 +17,10 @@ SyncerCommandTestBase::~SyncerCommandTestBase() { } void SyncerCommandTestBase::SetUp() { - ResetContext(); // The session always expects there to be a passive worker. workers()->push_back( make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE))); + ResetContext(); } void SyncerCommandTestBase::TearDown() { diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h index e177a87..d3870bb 100644 --- a/sync/test/engine/syncer_command_test.h +++ b/sync/test/engine/syncer_command_test.h @@ -109,8 +109,7 @@ class SyncerCommandTestBase : public testing::Test, sessions::SyncSession* session(const sessions::SyncSourceInfo& source) { if (!session_.get()) { std::vector<ModelSafeWorker*> workers = GetWorkers(); - session_.reset(new sessions::SyncSession(context(), delegate(), source, - routing_info_, workers)); + session_.reset(new sessions::SyncSession(context(), delegate(), source)); } return session_.get(); } @@ -129,6 +128,7 @@ class SyncerCommandTestBase : public testing::Test, &mock_debug_info_getter_, &traffic_recorder_, true /* enable keystore encryption*/ )); + context_->set_routing_info(routing_info_); context_->set_account_name(directory()->name()); ClearSession(); } |