summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-29 22:33:34 +0000
committerrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-29 22:33:34 +0000
commit66565693eca820d5b8e6510a02b7330a2fc2a2f5 (patch)
treecb5aedec5132f93c02bcef5e5e0f8d677c8dd8e0
parent0e534402bcd0586a7d85cde515de4631b441f0f2 (diff)
downloadchromium_src-66565693eca820d5b8e6510a02b7330a2fc2a2f5.zip
chromium_src-66565693eca820d5b8e6510a02b7330a2fc2a2f5.tar.gz
chromium_src-66565693eca820d5b8e6510a02b7330a2fc2a2f5.tar.bz2
sync: Remove session member from SyncSessionJob
This change removes the SyncSession member of SyncSessionJob. The session had two very important responsibilities: 1. Tracking the source and notification hints for the current job. 2. Indicating whether or not a job had been abandoned. We now track the source of a job directly in SyncSessionJob. The debug_info_sources_list has been removed entirely. The investigation that triggered its addition has been put on hold for now and it is unliekly we will need this information in the future. Tracking abandoned jobs is a bit more difficult. The scheduler used to keep track of SyncSessions very carefully, since only a valid session would allow a job to actually proceed. This patch introduces a new approach for limiting the number of pending jobs. The scheduler now maintains a single scoped_ptr to a CancelableClosure. All scheduled tasks will be wrapped in a CancelableClosure that will be owned by this scoped_ptr. If a new task pre-empts an existing task, it will overwrite the scoped_ptr, which will prevent the old task from running. Therefore, only one task may be pending at any given time. This should help prevent issues like crbug.com/139723, though it also introduces new failure modes. BUG=175024 Review URL: https://chromiumcodereview.appspot.com/12942005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191458 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--sync/engine/sync_scheduler_impl.cc154
-rw-r--r--sync/engine/sync_scheduler_impl.h24
-rw-r--r--sync/engine/sync_scheduler_unittest.cc9
-rw-r--r--sync/engine/sync_scheduler_whitebox_unittest.cc10
-rw-r--r--sync/engine/sync_session_job.cc40
-rw-r--r--sync/engine/sync_session_job.h21
-rw-r--r--sync/engine/sync_session_job_unittest.cc117
-rw-r--r--sync/engine/syncer_unittest.cc6
-rw-r--r--sync/internal_api/debug_info_event_listener.cc20
-rw-r--r--sync/internal_api/js_sync_manager_observer_unittest.cc1
-rw-r--r--sync/internal_api/public/sessions/sync_session_snapshot.cc14
-rw-r--r--sync/internal_api/public/sessions/sync_session_snapshot.h3
-rw-r--r--sync/internal_api/public/sessions/sync_session_snapshot_unittest.cc9
-rw-r--r--sync/sessions/sync_session.cc8
-rw-r--r--sync/sessions/sync_session.h8
-rw-r--r--sync/sessions/sync_session_unittest.cc20
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";