diff options
Diffstat (limited to 'sync')
-rw-r--r-- | sync/engine/sync_scheduler_impl.cc | 154 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_impl.h | 24 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_unittest.cc | 9 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_whitebox_unittest.cc | 10 | ||||
-rw-r--r-- | sync/engine/sync_session_job.cc | 40 | ||||
-rw-r--r-- | sync/engine/sync_session_job.h | 21 | ||||
-rw-r--r-- | sync/engine/sync_session_job_unittest.cc | 117 | ||||
-rw-r--r-- | sync/engine/syncer_unittest.cc | 6 | ||||
-rw-r--r-- | sync/internal_api/debug_info_event_listener.cc | 20 | ||||
-rw-r--r-- | sync/internal_api/js_sync_manager_observer_unittest.cc | 1 | ||||
-rw-r--r-- | sync/internal_api/public/sessions/sync_session_snapshot.cc | 14 | ||||
-rw-r--r-- | sync/internal_api/public/sessions/sync_session_snapshot.h | 3 | ||||
-rw-r--r-- | sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc | 9 | ||||
-rw-r--r-- | sync/sessions/sync_session.cc | 8 | ||||
-rw-r--r-- | sync/sessions/sync_session.h | 8 | ||||
-rw-r--r-- | sync/sessions/sync_session_unittest.cc | 20 |
16 files changed, 169 insertions, 295 deletions
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index 5bfcbcc..7fd9ccd 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -217,11 +217,7 @@ void SyncSchedulerImpl::OnServerConnectionErrorFixed() { scoped_ptr<SyncSessionJob> pending(TakePendingJobForCurrentMode()); if (!pending.get()) return; - - PostTask(FROM_HERE, "DoCanaryJob", - base::Bind(&SyncSchedulerImpl::DoCanaryJob, - weak_ptr_factory_.GetWeakPtr(), - base::Passed(&pending))); + DoCanaryJob(pending.Pass()); } void SyncSchedulerImpl::Start(Mode mode) { @@ -310,17 +306,13 @@ bool SyncSchedulerImpl::ScheduleConfiguration( // Only reconfigure if we have types to download. if (!params.types_to_download.Empty()) { DCHECK(!restricted_routes.empty()); - scoped_ptr<SyncSession> session(new SyncSession( - session_context_, - this, - SyncSourceInfo(params.source, - ModelSafeRoutingInfoToInvalidationMap( - restricted_routes, - std::string())))); scoped_ptr<SyncSessionJob> job(new SyncSessionJob( SyncSessionJob::CONFIGURATION, TimeTicks::Now(), - session.Pass(), + SyncSourceInfo(params.source, + ModelSafeRoutingInfoToInvalidationMap( + restricted_routes, + std::string())), params)); bool succeeded = DoSyncSessionJob(job.Pass(), NORMAL_PRIORITY); @@ -389,11 +381,10 @@ SyncSchedulerImpl::JobProcessDecision SyncSchedulerImpl::DecideOnJob( ModelTypeSet throttled_types = session_context_->throttled_data_type_tracker()->GetThrottledTypes(); if (job.purpose() == SyncSessionJob::NUDGE && - job.session()->source().updates_source == GetUpdatesCallerInfo::LOCAL) { + job.source_info().updates_source == GetUpdatesCallerInfo::LOCAL) { ModelTypeSet requested_types; for (ModelTypeInvalidationMap::const_iterator i = - job.session()->source().types.begin(); - i != job.session()->source().types.end(); + job.source_info().types.begin(); i != job.source_info().types.end(); ++i) { requested_types.Put(i->first); } @@ -466,12 +457,11 @@ 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()->CoalesceSources( - job->session()->source()); + pending_nudge_->CoalesceSources(job->source_info()); return; } - scoped_ptr<SyncSessionJob> job_to_save = job->CloneAndAbandon(); + scoped_ptr<SyncSessionJob> job_to_save = job->Clone(); if (wait_interval_.get() && !wait_interval_->pending_configure_job) { // This job should be made the new canary. if (is_nudge) { @@ -486,7 +476,7 @@ void SyncSchedulerImpl::HandleSaveJobDecision(scoped_ptr<SyncSessionJob> job) { DCHECK(!unscheduled_nudge_storage_.get()); if (pending_nudge_) { // Pre-empt the nudge canary and abandon the old nudge (owned by task). - unscheduled_nudge_storage_ = pending_nudge_->CloneAndAbandon(); + unscheduled_nudge_storage_ = pending_nudge_->Clone(); pending_nudge_ = unscheduled_nudge_storage_.get(); } wait_interval_->pending_configure_job = job_to_save.get(); @@ -589,12 +579,11 @@ void SyncSchedulerImpl::ScheduleNudgeImpl( scoped_ptr<SyncSessionJob> job(new SyncSessionJob( SyncSessionJob::NUDGE, TimeTicks::Now() + delay, - CreateSyncSession(info).Pass(), + info, ConfigurationParams())); JobProcessDecision decision = DecideOnJob(*job, NORMAL_PRIORITY); SDVLOG(2) << "Should run " << SyncSessionJob::GetPurposeString(job->purpose()) - << " job " << job->session() << " in mode " << GetModeString(mode_) << ": " << GetDecisionString(decision); if (decision != CONTINUE) { @@ -609,8 +598,7 @@ void SyncSchedulerImpl::ScheduleNudgeImpl( if (pending_nudge_) { SDVLOG(2) << "Rescheduling pending nudge"; - pending_nudge_->mutable_session()->CoalesceSources( - job->session()->source()); + pending_nudge_->CoalesceSources(job->source_info()); // 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 @@ -621,7 +609,7 @@ void SyncSchedulerImpl::ScheduleNudgeImpl( // It's possible that by "rescheduling" we're actually taking a job that // was previously unscheduled and giving it wings, so take care to reset // unscheduled nudge storage. - job = pending_nudge_->CloneAndAbandon(); + job = pending_nudge_->Clone(); pending_nudge_ = NULL; unscheduled_nudge_storage_.reset(); // It's also possible we took a canary job, since we allow one nudge @@ -663,18 +651,6 @@ const char* SyncSchedulerImpl::GetDecisionString( return ""; } -void SyncSchedulerImpl::PostTask( - const tracked_objects::Location& from_here, - const char* name, const base::Closure& task) { - SDVLOG_LOC(from_here, 3) << "Posting " << name << " task"; - DCHECK_EQ(MessageLoop::current(), sync_loop_); - if (!started_) { - SDVLOG(1) << "Not posting task as scheduler is stopped."; - return; - } - sync_loop_->PostTask(from_here, task); -} - void SyncSchedulerImpl::PostDelayedTask( const tracked_objects::Location& from_here, const char* name, const base::Closure& task, base::TimeDelta delay) { @@ -685,35 +661,24 @@ void SyncSchedulerImpl::PostDelayedTask( SDVLOG(1) << "Not posting task as scheduler is stopped."; return; } - sync_loop_->PostDelayedTask(from_here, task, delay); + // This cancels the previous task, if one existed. + pending_wakeup_.Reset(task); + sync_loop_->PostDelayedTask(from_here, pending_wakeup_.callback(), delay); } bool SyncSchedulerImpl::DoSyncSessionJob(scoped_ptr<SyncSessionJob> job, JobPriority priority) { DCHECK_EQ(MessageLoop::current(), sync_loop_); if (job->purpose() == SyncSessionJob::NUDGE) { - if (pending_nudge_ == NULL || - pending_nudge_->session() != job->session()) { - // |job| is abandoned. - SDVLOG(2) << "Dropping a nudge in " - << "DoSyncSessionJob because another nudge was scheduled"; - return false; - } pending_nudge_ = NULL; } - if (!job->session()) { - SDVLOG(2) << "Dropping abandoned job"; - return false; // Fix for crbug.com/190085. - } - base::AutoReset<bool> protector(&no_scheduling_allowed_, true); JobProcessDecision decision = DecideOnJob(*job, priority); SDVLOG(2) << "Should run " << SyncSessionJob::GetPurposeString(job->purpose()) - << " job " << job->session() << " in mode " << GetModeString(mode_) - << " with source " << job->session()->source().updates_source + << " with source " << job->source_info().updates_source << ": " << GetDecisionString(decision); if (decision != CONTINUE) { if (decision == SAVE) { @@ -724,14 +689,18 @@ bool SyncSchedulerImpl::DoSyncSessionJob(scoped_ptr<SyncSessionJob> job, return false; } - SDVLOG(2) << "Calling SyncShare with " - << SyncSessionJob::GetPurposeString(job->purpose()) << " job"; - bool premature_exit = !syncer_->SyncShare(job->mutable_session(), + DVLOG(2) << "Creating sync session with routes " + << ModelSafeRoutingInfoToString(session_context_->routing_info()) + << "and purpose " << job->purpose(); + SyncSession session(session_context_, this, job->source_info()); + bool premature_exit = !syncer_->SyncShare(&session, job->start_step(), job->end_step()); SDVLOG(2) << "Done SyncShare, returned: " << premature_exit; - bool success = FinishSyncSessionJob(job.get(), premature_exit); + bool success = FinishSyncSessionJob(job.get(), + premature_exit, + &session); if (IsSyncingCurrentlySilenced()) { SDVLOG(2) << "We are currently throttled; scheduling Unthrottle."; @@ -751,7 +720,7 @@ bool SyncSchedulerImpl::DoSyncSessionJob(scoped_ptr<SyncSessionJob> job, } if (!success) - ScheduleNextSync(job.Pass()); + ScheduleNextSync(job.Pass(), &session); return success; } @@ -786,14 +755,15 @@ void SyncSchedulerImpl::DoPollSyncSessionJob(scoped_ptr<SyncSessionJob> job) { if (!ShouldPoll()) return; - SDVLOG(2) << "Calling SyncShare with " - << SyncSessionJob::GetPurposeString(job->purpose()) << " job"; - bool premature_exit = !syncer_->SyncShare(job->mutable_session(), + DVLOG(2) << "Polling with routes " + << ModelSafeRoutingInfoToString(session_context_->routing_info()); + SyncSession session(session_context_, this, job->source_info()); + bool premature_exit = !syncer_->SyncShare(&session, job->start_step(), job->end_step()); SDVLOG(2) << "Done SyncShare, returned: " << premature_exit; - FinishSyncSessionJob(job.get(), premature_exit); + FinishSyncSessionJob(job.get(), premature_exit, &session); if (IsSyncingCurrentlySilenced()) { // This will start the countdown to unthrottle. Other kinds of jobs would @@ -831,14 +801,15 @@ void SyncSchedulerImpl::UpdateNudgeTimeRecords(const SyncSourceInfo& info) { } bool SyncSchedulerImpl::FinishSyncSessionJob(SyncSessionJob* job, - bool exited_prematurely) { + bool exited_prematurely, + SyncSession* session) { DCHECK_EQ(MessageLoop::current(), sync_loop_); // Let job know that we're through syncing (calling SyncShare) at this point. bool succeeded = false; { base::AutoReset<bool> protector(&no_scheduling_allowed_, true); - succeeded = job->Finish(exited_prematurely); + succeeded = job->Finish(exited_prematurely, session); } SDVLOG(2) << "Updating the next polling time after SyncMain"; @@ -858,7 +829,8 @@ bool SyncSchedulerImpl::FinishSyncSessionJob(SyncSessionJob* job, } void SyncSchedulerImpl::ScheduleNextSync( - scoped_ptr<SyncSessionJob> finished_job) { + scoped_ptr<SyncSessionJob> finished_job, + SyncSession* session) { DCHECK_EQ(MessageLoop::current(), sync_loop_); DCHECK(finished_job->purpose() == SyncSessionJob::CONFIGURATION || finished_job->purpose() == SyncSessionJob::NUDGE); @@ -891,7 +863,7 @@ void SyncSchedulerImpl::ScheduleNextSync( // Either this is the first failure or a consecutive failure after our // backoff timer expired. We handle it the same way in either case. SDVLOG(2) << "Non-'backoff nudge' SyncShare job failed"; - HandleContinuationError(finished_job.Pass()); + HandleContinuationError(finished_job.Pass(), session); } } @@ -921,26 +893,28 @@ void SyncSchedulerImpl::RestartWaiting(scoped_ptr<SyncSessionJob> job) { wait_interval_->timer.Stop(); DCHECK(wait_interval_->length >= TimeDelta::FromSeconds(0)); if (wait_interval_->mode == WaitInterval::THROTTLED) { - wait_interval_->timer.Start(FROM_HERE, wait_interval_->length, - base::Bind(&SyncSchedulerImpl::Unthrottle, - weak_ptr_factory_.GetWeakPtr(), - base::Passed(&job))); + pending_wakeup_.Reset(base::Bind(&SyncSchedulerImpl::Unthrottle, + weak_ptr_factory_.GetWeakPtr(), + base::Passed(&job))); + } else { - wait_interval_->timer.Start(FROM_HERE, wait_interval_->length, - base::Bind(&SyncSchedulerImpl::DoCanaryJob, - weak_ptr_factory_.GetWeakPtr(), - base::Passed(&job))); + pending_wakeup_.Reset(base::Bind(&SyncSchedulerImpl::DoCanaryJob, + weak_ptr_factory_.GetWeakPtr(), + base::Passed(&job))); } + wait_interval_->timer.Start(FROM_HERE, wait_interval_->length, + pending_wakeup_.callback()); } void SyncSchedulerImpl::HandleContinuationError( - scoped_ptr<SyncSessionJob> old_job) { + scoped_ptr<SyncSessionJob> old_job, + SyncSession* session) { DCHECK_EQ(MessageLoop::current(), sync_loop_); TimeDelta length = delay_provider_->GetDelay( IsBackingOff() ? wait_interval_->length : delay_provider_->GetInitialDelay( - old_job->session()->status_controller().model_neutral_state())); + session->status_controller().model_neutral_state())); SDVLOG(2) << "In handle continuation error with " << SyncSessionJob::GetPurposeString(old_job->purpose()) @@ -989,6 +963,7 @@ void SyncSchedulerImpl::StopImpl(const base::Closure& callback) { poll_timer_.Stop(); pending_nudge_ = NULL; unscheduled_nudge_storage_.reset(); + pending_wakeup_.Cancel(); if (started_) { started_ = false; } @@ -1000,18 +975,6 @@ void SyncSchedulerImpl::DoCanaryJob(scoped_ptr<SyncSessionJob> to_be_canary) { DCHECK_EQ(MessageLoop::current(), sync_loop_); SDVLOG(2) << "Do canary job"; - if (to_be_canary->purpose() == SyncSessionJob::NUDGE) { - // TODO(tim): Bug 158313. Remove this check. - if (pending_nudge_ == NULL || - pending_nudge_->session() != to_be_canary->session()) { - // |job| is abandoned. - SDVLOG(2) << "Dropping a nudge in " - << "DoCanaryJob because another nudge was scheduled"; - return; - } - DCHECK_EQ(pending_nudge_->session(), to_be_canary->session()); - } - // This is the only place where we invoke DoSyncSessionJob with canary // privileges. Everyone else should use NORMAL_PRIORITY. DoSyncSessionJob(to_be_canary.Pass(), CANARY_PRIORITY); @@ -1026,11 +989,11 @@ scoped_ptr<SyncSessionJob> SyncSchedulerImpl::TakePendingJobForCurrentMode() { && wait_interval_->pending_configure_job) { SDVLOG(2) << "Found pending configure job"; candidate = - wait_interval_->pending_configure_job->CloneAndAbandon().Pass(); + wait_interval_->pending_configure_job->Clone().Pass(); wait_interval_->pending_configure_job = candidate.get(); } else if (mode_ == NORMAL_MODE && pending_nudge_) { SDVLOG(2) << "Found pending nudge job"; - candidate = pending_nudge_->CloneAndAbandon(); + candidate = pending_nudge_->Clone(); pending_nudge_ = candidate.get(); unscheduled_nudge_storage_.reset(); } @@ -1040,26 +1003,15 @@ scoped_ptr<SyncSessionJob> SyncSchedulerImpl::TakePendingJobForCurrentMode() { return candidate.Pass(); } -scoped_ptr<SyncSession> SyncSchedulerImpl::CreateSyncSession( - const SyncSourceInfo& source) { - DCHECK_EQ(MessageLoop::current(), sync_loop_); - DVLOG(2) << "Creating sync session with routes " - << ModelSafeRoutingInfoToString(session_context_->routing_info()); - - SyncSourceInfo info(source); - return scoped_ptr<SyncSession>(new SyncSession(session_context_, this, info)); -} - void SyncSchedulerImpl::PollTimerCallback() { DCHECK_EQ(MessageLoop::current(), sync_loop_); ModelSafeRoutingInfo r; ModelTypeInvalidationMap invalidation_map = ModelSafeRoutingInfoToInvalidationMap(r, std::string()); SyncSourceInfo info(GetUpdatesCallerInfo::PERIODIC, invalidation_map); - scoped_ptr<SyncSession> s(CreateSyncSession(info)); scoped_ptr<SyncSessionJob> job(new SyncSessionJob(SyncSessionJob::POLL, TimeTicks::Now(), - s.Pass(), + info, ConfigurationParams())); if (no_scheduling_allowed_) { // The no_scheduling_allowed_ flag is set by a function-scoped AutoReset in diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h index 61351b7..4fc8c17 100644 --- a/sync/engine/sync_scheduler_impl.h +++ b/sync/engine/sync_scheduler_impl.h @@ -9,6 +9,7 @@ #include <string> #include "base/callback.h" +#include "base/cancelable_callback.h" #include "base/compiler_specific.h" #include "base/gtest_prod_util.h" #include "base/memory/linked_ptr.h" @@ -159,11 +160,8 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : public SyncScheduler { static const char* GetDecisionString(JobProcessDecision decision); - // Helpers that log before posting to |sync_loop_|. These will only post - // the task in between calls to Start/Stop. - void PostTask(const tracked_objects::Location& from_here, - const char* name, - const base::Closure& task); + // Helper to cancel any existing delayed task and replace it with a new one. + // It will not post any tasks if the scheduler is in a "stopped" state. void PostDelayedTask(const tracked_objects::Location& from_here, const char* name, const base::Closure& task, @@ -184,10 +182,12 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : public SyncScheduler { // cycle from job.start_step() to job.end_step(), likely because the // scheduler was forced to quit the job mid-way through. bool FinishSyncSessionJob(SyncSessionJob* job, - bool exited_prematurely); + bool exited_prematurely, + sessions::SyncSession* session); // Helper to schedule retries of a failed configure or nudge job. - void ScheduleNextSync(scoped_ptr<SyncSessionJob> finished_job); + void ScheduleNextSync(scoped_ptr<SyncSessionJob> finished_job, + sessions::SyncSession* session); // Helper to configure polling intervals. Used by Start and ScheduleNextSync. void AdjustPolling(const SyncSessionJob* old_job); @@ -196,7 +196,8 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : public SyncScheduler { void RestartWaiting(scoped_ptr<SyncSessionJob> job); // Helper to ScheduleNextSync in case of consecutive sync errors. - void HandleContinuationError(scoped_ptr<SyncSessionJob> old_job); + void HandleContinuationError(scoped_ptr<SyncSessionJob> old_job, + sessions::SyncSession* session); // Decide whether we should CONTINUE, SAVE or DROP the job. JobProcessDecision DecideOnJob(const SyncSessionJob& job, @@ -254,9 +255,6 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : public SyncScheduler { // Called when the root cause of the current connection error is fixed. void OnServerConnectionErrorFixed(); - scoped_ptr<sessions::SyncSession> CreateSyncSession( - const sessions::SyncSourceInfo& info); - // Creates a session for a poll and performs the sync. void PollTimerCallback(); @@ -324,6 +322,10 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : public SyncScheduler { scoped_ptr<BackoffDelayProvider> delay_provider_; + // We allow at most one PostedTask to be pending at one time. This is it. + // We will cancel this task before starting a new one. + base::CancelableClosure pending_wakeup_; + // Invoked to run through the sync cycle. scoped_ptr<Syncer> syncer_; diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc index 9b3d7da..406be4e 100644 --- a/sync/engine/sync_scheduler_unittest.cc +++ b/sync/engine/sync_scheduler_unittest.cc @@ -1184,7 +1184,7 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) { .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConnectionFailure), Return(true))) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess), - QuitLoopNowAction())); + Return(true))); StartSyncScheduler(SyncScheduler::NORMAL_MODE); scheduler()->ScheduleNudgeAsync( @@ -1211,7 +1211,7 @@ TEST_F(SyncSchedulerTest, ServerConnectionChangeDuringBackoff) { .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConnectionFailure), Return(true))) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess), - QuitLoopNowAction())); + Return(true))); scheduler()->ScheduleNudgeAsync( zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(BOOKMARKS), FROM_HERE); @@ -1226,6 +1226,9 @@ TEST_F(SyncSchedulerTest, ServerConnectionChangeDuringBackoff) { MessageLoop::current()->RunUntilIdle(); } +// This was supposed to test the scenario where we receive a nudge while a +// connection change canary is scheduled, but has not run yet. Since we've made +// the connection change canary synchronous, this is no longer possible. TEST_F(SyncSchedulerTest, ConnectionChangeCanaryPreemptedByNudge) { UseMockDelayProvider(); EXPECT_CALL(*delay(), GetDelay(_)) @@ -1239,6 +1242,8 @@ TEST_F(SyncSchedulerTest, ConnectionChangeCanaryPreemptedByNudge) { .WillOnce(DoAll(Invoke(sessions::test_util::SimulateConnectionFailure), Return(true))) .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess), + Return(true))) + .WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess), QuitLoopNowAction())); scheduler()->ScheduleNudgeAsync( diff --git a/sync/engine/sync_scheduler_whitebox_unittest.cc b/sync/engine/sync_scheduler_whitebox_unittest.cc index 77bb11d..9aceada 100644 --- a/sync/engine/sync_scheduler_whitebox_unittest.cc +++ b/sync/engine/sync_scheduler_whitebox_unittest.cc @@ -105,9 +105,8 @@ class SyncSchedulerWhiteboxTest : public testing::Test { SyncSchedulerImpl::JobProcessDecision CreateAndDecideJob( SyncSessionJob::Purpose purpose) { - scoped_ptr<SyncSession> s(scheduler_->CreateSyncSession(SyncSourceInfo())); - SyncSessionJob job(purpose, TimeTicks::Now(), s.Pass(), - ConfigurationParams()); + SyncSessionJob job(purpose, TimeTicks::Now(), + SyncSourceInfo(), ConfigurationParams()); return DecideOnJob(job, SyncSchedulerImpl::NORMAL_PRIORITY); } @@ -156,12 +155,11 @@ TEST_F(SyncSchedulerWhiteboxTest, SaveNudgeWhileTypeThrottled) { ModelTypeSetToInvalidationMap(types, std::string()); SyncSourceInfo info(GetUpdatesCallerInfo::LOCAL, invalidation_map); - scoped_ptr<SyncSession> s(scheduler_->CreateSyncSession(info)); // Now schedule a nudge with just bookmarks and the change is local. SyncSessionJob job(SyncSessionJob::NUDGE, TimeTicks::Now(), - s.Pass(), + info, ConfigurationParams()); SyncSchedulerImpl::JobProcessDecision decision = DecideOnJob(job, SyncSchedulerImpl::NORMAL_PRIORITY); @@ -264,7 +262,7 @@ TEST_F(SyncSchedulerWhiteboxTest, ContinueCanaryJobConfig) { SetWaitIntervalToExponentialBackoff(); SyncSessionJob job(SyncSessionJob::CONFIGURATION, - TimeTicks::Now(), scoped_ptr<SyncSession>(), + TimeTicks::Now(), SyncSourceInfo(), ConfigurationParams()); SyncSchedulerImpl::JobProcessDecision decision = diff --git a/sync/engine/sync_session_job.cc b/sync/engine/sync_session_job.cc index f0d0d33..0195f80 100644 --- a/sync/engine/sync_session_job.cc +++ b/sync/engine/sync_session_job.cc @@ -13,11 +13,11 @@ SyncSessionJob::~SyncSessionJob() { SyncSessionJob::SyncSessionJob( Purpose purpose, base::TimeTicks start, - scoped_ptr<sessions::SyncSession> session, + const sessions::SyncSourceInfo& source_info, const ConfigurationParams& config_params) : purpose_(purpose), + source_info_(source_info), scheduled_start_(start), - session_(session.Pass()), config_params_(config_params), finished_(NOT_FINISHED) { } @@ -35,7 +35,7 @@ const char* SyncSessionJob::GetPurposeString(SyncSessionJob::Purpose purpose) { } #undef ENUM_CASE -bool SyncSessionJob::Finish(bool early_exit) { +bool SyncSessionJob::Finish(bool early_exit, sessions::SyncSession* session) { DCHECK_EQ(finished_, NOT_FINISHED); // Did we run through all SyncerSteps from start_step() to end_step() // until the SyncSession returned !HasMoreToSync()? @@ -50,12 +50,12 @@ bool SyncSessionJob::Finish(bool early_exit) { // Did we hit any errors along the way? if (sessions::HasSyncerError( - session_->status_controller().model_neutral_state())) { + session->status_controller().model_neutral_state())) { return false; } const sessions::ModelNeutralState& state( - session_->status_controller().model_neutral_state()); + session->status_controller().model_neutral_state()); switch (purpose_) { case POLL: case NUDGE: @@ -75,31 +75,25 @@ bool SyncSessionJob::Finish(bool early_exit) { return true; } -scoped_ptr<SyncSessionJob> SyncSessionJob::CloneAndAbandon() { - DCHECK_EQ(finished_, NOT_FINISHED); - // Clone |this|, and abandon it by NULL-ing session_. - return scoped_ptr<SyncSessionJob> (new SyncSessionJob( - purpose_, scheduled_start_, session_.Pass(), - config_params_)); -} - scoped_ptr<SyncSessionJob> SyncSessionJob::Clone() const { - DCHECK_GT(finished_, NOT_FINISHED); return scoped_ptr<SyncSessionJob>(new SyncSessionJob( - purpose_, scheduled_start_, CloneSession().Pass(), + purpose_, scheduled_start_, source_info_, config_params_)); } -scoped_ptr<sessions::SyncSession> SyncSessionJob::CloneSession() const { - return scoped_ptr<sessions::SyncSession>( - new sessions::SyncSession(session_->context(), - session_->delegate(), session_->source())); +void SyncSessionJob::CoalesceSources(const sessions::SyncSourceInfo& source) { + CoalesceStates(source.types, &source_info_.types); + source_info_.updates_source = source.updates_source; } SyncSessionJob::Purpose SyncSessionJob::purpose() const { return purpose_; } +const sessions::SyncSourceInfo& SyncSessionJob::source_info() const { + return source_info_; +} + base::TimeTicks SyncSessionJob::scheduled_start() const { return scheduled_start_; } @@ -108,14 +102,6 @@ void SyncSessionJob::set_scheduled_start(base::TimeTicks start) { scheduled_start_ = start; }; -const sessions::SyncSession* SyncSessionJob::session() const { - return session_.get(); -} - -sessions::SyncSession* SyncSessionJob::mutable_session() { - return session_.get(); -} - ConfigurationParams SyncSessionJob::config_params() const { return config_params_; } diff --git a/sync/engine/sync_session_job.h b/sync/engine/sync_session_job.h index 33d2171..feb7405 100644 --- a/sync/engine/sync_session_job.h +++ b/sync/engine/sync_session_job.h @@ -33,19 +33,17 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { SyncSessionJob(Purpose purpose, base::TimeTicks start, - scoped_ptr<sessions::SyncSession> session, + const sessions::SyncSourceInfo& source_info, const ConfigurationParams& config_params); ~SyncSessionJob(); // Returns a new clone of the job, with a cloned SyncSession ready to be - // retried / rescheduled. A job can only be cloned once it has finished, to - // prevent bugs where multiple jobs are scheduled with the same session. Use - // CloneAndAbandon if you want to clone before finishing. + // retried / rescheduled. scoped_ptr<SyncSessionJob> Clone() const; - // Same as Clone() above, but also ejects the SyncSession from this job, - // preventing it from ever being used for a sync cycle. - scoped_ptr<SyncSessionJob> CloneAndAbandon(); + // Overwrite the sync update source with the most recent and merge the + // type/state map. + void CoalesceSources(const sessions::SyncSourceInfo& source); // Record that the scheduler has deemed the job as finished and give it a // chance to perform any remaining cleanup and/or notification completion @@ -61,7 +59,7 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { // timer has expired. Most importantly, the SyncScheduler should not assume // that the original action that triggered the sync cycle (ie. a nudge or a // notification) has been properly serviced. - bool Finish(bool early_exit); + bool Finish(bool early_exit, sessions::SyncSession* session); static const char* GetPurposeString(Purpose purpose); static void GetSyncerStepsForPurpose(Purpose purpose, @@ -69,10 +67,9 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { SyncerStep* end); Purpose purpose() const; + const sessions::SyncSourceInfo& source_info() const; base::TimeTicks scheduled_start() const; void set_scheduled_start(base::TimeTicks start); - const sessions::SyncSession* session() const; - sessions::SyncSession* mutable_session(); SyncerStep start_step() const; SyncerStep end_step() const; ConfigurationParams config_params() const; @@ -86,12 +83,10 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { FINISHED // Indicates a "clean" finish operation. }; - scoped_ptr<sessions::SyncSession> CloneSession() const; - const Purpose purpose_; + sessions::SyncSourceInfo source_info_; base::TimeTicks scheduled_start_; - scoped_ptr<sessions::SyncSession> session_; // Only used for purpose_ == CONFIGURATION. This, and different Finish() and // Succeeded() behavior may be arguments to subclass in the future. diff --git a/sync/engine/sync_session_job_unittest.cc b/sync/engine/sync_session_job_unittest.cc index 66026ce..4f3a67c 100644 --- a/sync/engine/sync_session_job_unittest.cc +++ b/sync/engine/sync_session_job_unittest.cc @@ -62,13 +62,28 @@ class SyncSessionJobTest : public testing::Test { context_->set_routing_info(routes_); } - scoped_ptr<SyncSession> NewLocalSession() { - sessions::SyncSourceInfo info( - sync_pb::GetUpdatesCallerInfo::LOCAL, ModelTypeInvalidationMap()); + scoped_ptr<SyncSession> MakeSession() { + sessions::SyncSourceInfo info(MakeSourceInfo()); + std::vector<sessions::SyncSourceInfo> sources_list; return scoped_ptr<SyncSession>( new SyncSession(context_.get(), &delegate_, info)); } + sessions::SyncSourceInfo MakeSourceInfo() { + sessions::SyncSourceInfo info(sync_pb::GetUpdatesCallerInfo::LOCAL, + ModelTypeInvalidationMap()); + return info; + } + + ModelTypeSet ParamsMeaningAllEnabledTypes() { + ModelTypeSet request_params(BOOKMARKS, AUTOFILL); + return request_params; + } + + ModelTypeSet ParamsMeaningJustOneEnabledType() { + return ModelTypeSet(AUTOFILL); + } + void GetWorkers(std::vector<ModelSafeWorker*>* out) const { out->clear(); for (std::vector<scoped_refptr<ModelSafeWorker> >::const_iterator it = @@ -89,10 +104,8 @@ class SyncSessionJobTest : public testing::Test { SyncSession::Delegate* delegate() { return &delegate_; } const ModelSafeRoutingInfo& routes() { return routes_; } - // Checks that the two jobs are "clones" as defined by SyncSessionJob, - // minus location and SyncSession checking, for reuse in different - // scenarios. - void ExpectClonesBase(SyncSessionJob* job, SyncSessionJob* clone) { + // Checks that the two jobs are "clones" as defined by SyncSessionJob. + void ExpectClones(SyncSessionJob* job, SyncSessionJob* clone) { EXPECT_EQ(job->purpose(), clone->purpose()); EXPECT_EQ(job->scheduled_start(), clone->scheduled_start()); EXPECT_EQ(job->start_step(), clone->start_step()); @@ -109,78 +122,60 @@ class SyncSessionJobTest : public testing::Test { TEST_F(SyncSessionJobTest, Clone) { SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - NewLocalSession().Pass(), ConfigurationParams()); + MakeSourceInfo(), ConfigurationParams()); - sessions::test_util::SimulateSuccess(job1.mutable_session(), + scoped_ptr<SyncSession> session1 = MakeSession().Pass(); + sessions::test_util::SimulateSuccess(session1.get(), job1.start_step(), job1.end_step()); - job1.Finish(false); + job1.Finish(false, session1.get()); ModelSafeRoutingInfo new_routes; new_routes[AUTOFILL] = GROUP_PASSIVE; context()->set_routing_info(new_routes); scoped_ptr<SyncSessionJob> clone1 = job1.Clone(); - ExpectClonesBase(&job1, clone1.get()); - EXPECT_NE(job1.session(), clone1->session()); + ExpectClones(&job1, clone1.get()); context()->set_routing_info(routes()); - sessions::test_util::SimulateSuccess(clone1->mutable_session(), + scoped_ptr<SyncSession> session2 = MakeSession().Pass(); + sessions::test_util::SimulateSuccess(session2.get(), clone1->start_step(), clone1->end_step()); - clone1->Finish(false); + clone1->Finish(false, session2.get()); scoped_ptr<SyncSessionJob> clone2 = clone1->Clone(); - ExpectClonesBase(clone1.get(), clone2.get()); - EXPECT_NE(clone1->session(), clone2->session()); - EXPECT_NE(clone1->session(), clone2->session()); + ExpectClones(clone1.get(), clone2.get()); clone1.reset(); - ExpectClonesBase(&job1, clone2.get()); - EXPECT_NE(job1.session(), clone2->session()); + ExpectClones(&job1, clone2.get()); } TEST_F(SyncSessionJobTest, CloneAfterEarlyExit) { + scoped_ptr<SyncSession> session = MakeSession().Pass(); SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - NewLocalSession().Pass(), ConfigurationParams()); - job1.Finish(true); + MakeSourceInfo(), ConfigurationParams()); + job1.Finish(true, session.get()); scoped_ptr<SyncSessionJob> job2 = job1.Clone(); - ExpectClonesBase(&job1, job2.get()); -} - -TEST_F(SyncSessionJobTest, CloneAndAbandon) { - scoped_ptr<SyncSession> session = NewLocalSession(); - SyncSession* session_ptr = session.get(); - - SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - session.Pass(), ConfigurationParams()); - ModelSafeRoutingInfo new_routes; - new_routes[AUTOFILL] = GROUP_PASSIVE; - context()->set_routing_info(new_routes); - - scoped_ptr<SyncSessionJob> clone1 = job1.CloneAndAbandon(); - ExpectClonesBase(&job1, clone1.get()); - EXPECT_FALSE(job1.session()); - EXPECT_EQ(session_ptr, clone1->session()); + ExpectClones(&job1, job2.get()); } // Tests interaction between Finish and sync cycle success / failure. TEST_F(SyncSessionJobTest, Finish) { SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - NewLocalSession().Pass(), ConfigurationParams()); + MakeSourceInfo(), ConfigurationParams()); - sessions::test_util::SimulateSuccess(job1.mutable_session(), + scoped_ptr<SyncSession> session1 = MakeSession().Pass(); + sessions::test_util::SimulateSuccess(session1.get(), job1.start_step(), job1.end_step()); - EXPECT_TRUE(job1.Finish(false /* early_exit */)); + EXPECT_TRUE(job1.Finish(false /* early_exit */, session1.get())); scoped_ptr<SyncSessionJob> job2 = job1.Clone(); - sessions::test_util::SimulateConnectionFailure(job2->mutable_session(), + scoped_ptr<SyncSession> session2 = MakeSession().Pass(); + sessions::test_util::SimulateConnectionFailure(session2.get(), job2->start_step(), job2->end_step()); - EXPECT_FALSE(job2->Finish(false)); - - scoped_ptr<SyncSessionJob> job3 = job2->Clone(); - EXPECT_FALSE(job3->Finish(true)); + EXPECT_FALSE(job2->Finish(false, session2.get())); } TEST_F(SyncSessionJobTest, FinishCallsReadyTask) { @@ -195,13 +190,35 @@ TEST_F(SyncSessionJobTest, FinishCallsReadyTask) { scoped_ptr<SyncSession> session( new SyncSession(context(), delegate(), info)); - SyncSessionJob job1(SyncSessionJob::CONFIGURATION, TimeTicks::Now(), - session.Pass(), params); - sessions::test_util::SimulateSuccess(job1.mutable_session(), + SyncSessionJob job1( + SyncSessionJob::CONFIGURATION, TimeTicks::Now(), info, params); + sessions::test_util::SimulateSuccess(session.get(), job1.start_step(), job1.end_step()); - job1.Finish(false); + job1.Finish(false, session.get()); EXPECT_TRUE(config_params_callback_invoked()); } +TEST_F(SyncSessionJobTest, CoalesceSources) { + ModelTypeInvalidationMap one_type = + ModelTypeSetToInvalidationMap( + ParamsMeaningJustOneEnabledType(), + std::string()); + ModelTypeInvalidationMap all_types = + ModelTypeSetToInvalidationMap( + ParamsMeaningAllEnabledTypes(), + std::string()); + sessions::SyncSourceInfo source_one( + sync_pb::GetUpdatesCallerInfo::PERIODIC, one_type); + sessions::SyncSourceInfo source_two( + sync_pb::GetUpdatesCallerInfo::LOCAL, all_types); + + SyncSessionJob job(SyncSessionJob::NUDGE, TimeTicks::Now(), + source_one, ConfigurationParams()); + + job.CoalesceSources(source_two); + + EXPECT_EQ(source_two.updates_source, job.source_info().updates_source); +} + } // namespace syncer diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index 1bdc13a..44b452f 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -176,9 +176,9 @@ class SyncerTest : public testing::Test, GetModelSafeRoutingInfo(&info); ModelTypeInvalidationMap invalidation_map = ModelSafeRoutingInfoToInvalidationMap(info, std::string()); - return new SyncSession(context_.get(), this, - sessions::SyncSourceInfo(sync_pb::GetUpdatesCallerInfo::UNKNOWN, - invalidation_map)); + sessions::SyncSourceInfo source_info(sync_pb::GetUpdatesCallerInfo::UNKNOWN, + invalidation_map); + return new SyncSession(context_.get(), this, source_info); } diff --git a/sync/internal_api/debug_info_event_listener.cc b/sync/internal_api/debug_info_event_listener.cc index 107b10e..f9c0d89 100644 --- a/sync/internal_api/debug_info_event_listener.cc +++ b/sync/internal_api/debug_info_event_listener.cc @@ -43,26 +43,6 @@ void DebugInfoEventListener::OnSyncCycleCompleted( sync_completed_event_info->mutable_caller_info()->set_notifications_enabled( snapshot.notifications_enabled()); - // Log the sources and per-type payloads coalesced into this session. - const std::vector<sessions::SyncSourceInfo>& snap_sources = - snapshot.debug_info_sources_list(); - for (std::vector<sessions::SyncSourceInfo>::const_iterator source_iter = - snap_sources.begin(); source_iter != snap_sources.end(); ++source_iter) { - sync_pb::SourceInfo* pb_source_info = - sync_completed_event_info->add_source_info(); - - pb_source_info->set_source(source_iter->updates_source); - - for (ModelTypeInvalidationMap::const_iterator type_iter = - source_iter->types.begin(); - type_iter != source_iter->types.end(); ++type_iter) { - sync_pb::TypeHint* pb_type_hint = pb_source_info->add_type_hint(); - pb_type_hint->set_data_type_id( - GetSpecificsFieldNumberFromModelType(type_iter->first)); - pb_type_hint->set_has_valid_hint(!type_iter->second.payload.empty()); - } - } - AddEventToQueue(event_info); } diff --git a/sync/internal_api/js_sync_manager_observer_unittest.cc b/sync/internal_api/js_sync_manager_observer_unittest.cc index ce09797..0084a4b 100644 --- a/sync/internal_api/js_sync_manager_observer_unittest.cc +++ b/sync/internal_api/js_sync_manager_observer_unittest.cc @@ -83,7 +83,6 @@ TEST_F(JsSyncManagerObserverTest, OnSyncCycleCompleted) { 2, 7, sessions::SyncSourceInfo(), - std::vector<sessions::SyncSourceInfo>(), false, 0, base::Time::Now(), diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.cc b/sync/internal_api/public/sessions/sync_session_snapshot.cc index b0e9fac..8c0c2b9 100644 --- a/sync/internal_api/public/sessions/sync_session_snapshot.cc +++ b/sync/internal_api/public/sessions/sync_session_snapshot.cc @@ -31,7 +31,6 @@ SyncSessionSnapshot::SyncSessionSnapshot( int num_hierarchy_conflicts, int num_server_conflicts, const SyncSourceInfo& source, - const std::vector<SyncSourceInfo>& debug_info_sources_list, bool notifications_enabled, size_t num_entries, base::Time sync_start_time, @@ -44,7 +43,6 @@ SyncSessionSnapshot::SyncSessionSnapshot( num_hierarchy_conflicts_(num_hierarchy_conflicts), num_server_conflicts_(num_server_conflicts), source_(source), - debug_info_sources_list_(debug_info_sources_list), notifications_enabled_(notifications_enabled), num_entries_(num_entries), sync_start_time_(sync_start_time), @@ -86,13 +84,6 @@ DictionaryValue* SyncSessionSnapshot::ToValue() const { num_server_conflicts_); value->SetInteger("numEntries", num_entries_); value->Set("source", source_.ToValue()); - scoped_ptr<ListValue> sources_list(new ListValue()); - for (std::vector<SyncSourceInfo>::const_iterator i = - debug_info_sources_list_.begin(); - i != debug_info_sources_list_.end(); ++i) { - sources_list->Append(i->ToValue()); - } - value->Set("sourcesList", sources_list.release()); value->SetBoolean("notificationsEnabled", notifications_enabled_); scoped_ptr<DictionaryValue> counter_entries(new DictionaryValue()); @@ -147,11 +138,6 @@ SyncSourceInfo SyncSessionSnapshot::source() const { return source_; } -const std::vector<SyncSourceInfo>& -SyncSessionSnapshot::debug_info_sources_list() const { - return debug_info_sources_list_; -} - bool SyncSessionSnapshot::notifications_enabled() const { return notifications_enabled_; } diff --git a/sync/internal_api/public/sessions/sync_session_snapshot.h b/sync/internal_api/public/sessions/sync_session_snapshot.h index be29d35..3e1c20e 100644 --- a/sync/internal_api/public/sessions/sync_session_snapshot.h +++ b/sync/internal_api/public/sessions/sync_session_snapshot.h @@ -38,7 +38,6 @@ class SYNC_EXPORT SyncSessionSnapshot { int num_hierarchy_conflicts, int num_server_conflicts, const SyncSourceInfo& source, - const std::vector<SyncSourceInfo>& debug_info_sources_list, bool notifications_enabled, size_t num_entries, base::Time sync_start_time, @@ -61,7 +60,6 @@ class SYNC_EXPORT SyncSessionSnapshot { int num_hierarchy_conflicts() const; int num_server_conflicts() const; SyncSourceInfo source() const; - const std::vector<SyncSourceInfo>& debug_info_sources_list() const; bool notifications_enabled() const; size_t num_entries() const; base::Time sync_start_time() const; @@ -79,7 +77,6 @@ class SYNC_EXPORT SyncSessionSnapshot { int num_hierarchy_conflicts_; int num_server_conflicts_; SyncSourceInfo source_; - std::vector<SyncSourceInfo> debug_info_sources_list_; bool notifications_enabled_; size_t num_entries_; base::Time sync_start_time_; diff --git a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc index 9301ffd..1d9f198 100644 --- a/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc +++ b/sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc @@ -47,11 +47,6 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) { SyncSourceInfo source; scoped_ptr<DictionaryValue> expected_source_value(source.ToValue()); - std::vector<SyncSourceInfo> debug_info_sources_list; - debug_info_sources_list.push_back(source); - scoped_ptr<ListValue> expected_sources_list_value(new ListValue()); - expected_sources_list_value->Append(source.ToValue()); - SyncSessionSnapshot snapshot(model_neutral, download_progress_markers, kIsSilenced, @@ -59,14 +54,13 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) { kNumHierarchyConflicts, kNumServerConflicts, source, - debug_info_sources_list, false, 0, base::Time::Now(), std::vector<int>(MODEL_TYPE_COUNT,0), std::vector<int>(MODEL_TYPE_COUNT, 0)); scoped_ptr<DictionaryValue> value(snapshot.ToValue()); - EXPECT_EQ(18u, value->size()); + EXPECT_EQ(17u, value->size()); ExpectDictIntegerValue(model_neutral.num_successful_commits, *value, "numSuccessfulCommits"); ExpectDictIntegerValue(model_neutral.num_successful_bookmark_commits, @@ -93,7 +87,6 @@ TEST_F(SyncSessionSnapshotTest, SyncSessionSnapshotToValue) { ExpectDictIntegerValue(kNumServerConflicts, *value, "numServerConflicts"); ExpectDictDictionaryValue(*expected_source_value, *value, "source"); - ExpectDictListValue(*expected_sources_list_value, *value, "sourcesList"); ExpectDictBooleanValue(false, *value, "notificationsEnabled"); } diff --git a/sync/sessions/sync_session.cc b/sync/sessions/sync_session.cc index 58cf11a..2e82dae 100644 --- a/sync/sessions/sync_session.cc +++ b/sync/sessions/sync_session.cc @@ -23,17 +23,10 @@ SyncSession::SyncSession( source_(source), delegate_(delegate) { status_controller_.reset(new StatusController()); - debug_info_sources_list_.push_back(source_); } SyncSession::~SyncSession() {} -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 { syncable::Directory* dir = context_->directory(); @@ -56,7 +49,6 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const { status_controller_->num_hierarchy_conflicts(), status_controller_->num_server_conflicts(), source_, - debug_info_sources_list_, context_->notifications_enabled(), dir->GetEntriesCount(), status_controller_->sync_start_time(), diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h index 126d57f..763bd3b 100644 --- a/sync/sessions/sync_session.h +++ b/sync/sessions/sync_session.h @@ -101,10 +101,6 @@ class SYNC_EXPORT_PRIVATE SyncSession { // Builds and sends a snapshot to the session context's listeners. void SendEventNotification(SyncEngineEvent::EventCause cause); - // 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_; } Delegate* delegate() const { return delegate_; } @@ -124,10 +120,6 @@ class SYNC_EXPORT_PRIVATE SyncSession { // The source for initiating this sync session. SyncSourceInfo source_; - // A list of sources for sessions that have been merged with this one. - // Currently used only for logging. - std::vector<SyncSourceInfo> debug_info_sources_list_; - // The delegate for this session, must never be NULL. Delegate* const delegate_; diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc index c3c4d69..118e64b 100644 --- a/sync/sessions/sync_session_unittest.cc +++ b/sync/sessions/sync_session_unittest.cc @@ -172,26 +172,6 @@ TEST_F(SyncSessionTest, MoreToDownloadIfGotNoChangesRemaining) { EXPECT_TRUE(status()->download_updates_succeeded()); } -TEST_F(SyncSessionTest, CoalesceSources) { - 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); - - session.CoalesceSources(source_two); - - 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"; |