diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-02 20:50:23 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-02 20:50:23 +0000 |
commit | fa6496c11220e64cedef64285a186d1e22ccdb51 (patch) | |
tree | 7d2983d822fb2bef3fc7351e29b07820721597c3 /sync/engine | |
parent | 49cc02d62108097cb6415b8f41327e14b74d5355 (diff) | |
download | chromium_src-fa6496c11220e64cedef64285a186d1e22ccdb51.zip chromium_src-fa6496c11220e64cedef64285a186d1e22ccdb51.tar.gz chromium_src-fa6496c11220e64cedef64285a186d1e22ccdb51.tar.bz2 |
Remove SyncSessionJob's location member
Surprisingly, this has no noticeable impact. Not even our debug logs
are affected. The only location where the value was accessed was in
ScheduleSyncSessionJob, and we can achieve the same effect by passing
the location to that function directly. The only change is that the
poll jobs are now reported as being scheduled from a location two lines
later than previously.
BUG=175024
Review URL: https://chromiumcodereview.appspot.com/12320027
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@185774 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/engine')
-rw-r--r-- | sync/engine/sync_scheduler_impl.cc | 17 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_impl.h | 3 | ||||
-rw-r--r-- | sync/engine/sync_scheduler_whitebox_unittest.cc | 7 | ||||
-rw-r--r-- | sync/engine/sync_session_job.cc | 22 | ||||
-rw-r--r-- | sync/engine/sync_session_job.h | 12 | ||||
-rw-r--r-- | sync/engine/sync_session_job_unittest.cc | 28 |
6 files changed, 22 insertions, 67 deletions
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc index c0014f3..1345e67 100644 --- a/sync/engine/sync_scheduler_impl.cc +++ b/sync/engine/sync_scheduler_impl.cc @@ -347,8 +347,7 @@ bool SyncSchedulerImpl::ScheduleConfiguration( SyncSessionJob::CONFIGURATION, TimeTicks::Now(), session.Pass(), - params, - FROM_HERE)); + params)); job->set_destruction_observer(weak_ptr_factory_.GetWeakPtr()); bool succeeded = DoSyncSessionJob(job.Pass()); @@ -619,8 +618,7 @@ void SyncSchedulerImpl::ScheduleNudgeImpl( SyncSessionJob::NUDGE, TimeTicks::Now() + delay, CreateSyncSession(info).Pass(), - ConfigurationParams(), - nudge_location)); + ConfigurationParams())); job->set_destruction_observer(weak_ptr_factory_.GetWeakPtr()); JobProcessDecision decision = DecideOnJob(*job); SDVLOG(2) << "Should run " @@ -662,7 +660,7 @@ void SyncSchedulerImpl::ScheduleNudgeImpl( // TODO(zea): Consider adding separate throttling/backoff for datatype // refresh requests. - ScheduleSyncSessionJob(job.Pass()); + ScheduleSyncSessionJob(nudge_location, job.Pass()); } const char* SyncSchedulerImpl::GetModeString(SyncScheduler::Mode mode) { @@ -709,6 +707,7 @@ void SyncSchedulerImpl::PostDelayedTask( } void SyncSchedulerImpl::ScheduleSyncSessionJob( + const tracked_objects::Location& loc, scoped_ptr<SyncSessionJob> job) { DCHECK_EQ(MessageLoop::current(), sync_loop_); if (no_scheduling_allowed_) { @@ -717,7 +716,6 @@ void SyncSchedulerImpl::ScheduleSyncSessionJob( } TimeDelta delay = job->scheduled_start() - TimeTicks::Now(); - tracked_objects::Location loc(job->from_location()); if (delay < TimeDelta::FromMilliseconds(0)) delay = TimeDelta::FromMilliseconds(0); SDVLOG_LOC(loc, 2) @@ -967,7 +965,7 @@ void SyncSchedulerImpl::HandleContinuationError( wait_interval_.reset(new WaitInterval(WaitInterval::EXPONENTIAL_BACKOFF, length)); NotifyRetryTime(base::Time::Now() + length); - scoped_ptr<SyncSessionJob> new_job(old_job->CloneFromLocation(FROM_HERE)); + scoped_ptr<SyncSessionJob> new_job(old_job->Clone()); new_job->set_scheduled_start(TimeTicks::Now() + length); if (old_job->purpose() == SyncSessionJob::CONFIGURATION) { SDVLOG(2) << "Configuration did not succeed, scheduling retry."; @@ -1079,10 +1077,9 @@ void SyncSchedulerImpl::PollTimerCallback() { scoped_ptr<SyncSessionJob> job(new SyncSessionJob(SyncSessionJob::POLL, TimeTicks::Now(), s.Pass(), - ConfigurationParams(), - FROM_HERE)); + ConfigurationParams())); job->set_destruction_observer(weak_ptr_factory_.GetWeakPtr()); - ScheduleSyncSessionJob(job.Pass()); + ScheduleSyncSessionJob(FROM_HERE, job.Pass()); } void SyncSchedulerImpl::Unthrottle(scoped_ptr<SyncSessionJob> to_be_canary) { diff --git a/sync/engine/sync_scheduler_impl.h b/sync/engine/sync_scheduler_impl.h index 8f03e89..f909920 100644 --- a/sync/engine/sync_scheduler_impl.h +++ b/sync/engine/sync_scheduler_impl.h @@ -168,7 +168,8 @@ class SYNC_EXPORT_PRIVATE SyncSchedulerImpl : base::TimeDelta delay); // Helper to assemble a job and post a delayed task to sync. - void ScheduleSyncSessionJob(scoped_ptr<SyncSessionJob> job); + void ScheduleSyncSessionJob(const tracked_objects::Location& loc, + scoped_ptr<SyncSessionJob> job); // Invoke the Syncer to perform a sync. bool DoSyncSessionJob(scoped_ptr<SyncSessionJob> job); diff --git a/sync/engine/sync_scheduler_whitebox_unittest.cc b/sync/engine/sync_scheduler_whitebox_unittest.cc index 8b0f7ef..335caff 100644 --- a/sync/engine/sync_scheduler_whitebox_unittest.cc +++ b/sync/engine/sync_scheduler_whitebox_unittest.cc @@ -106,7 +106,7 @@ class SyncSchedulerWhiteboxTest : public testing::Test { SyncSessionJob::Purpose purpose) { scoped_ptr<SyncSession> s(scheduler_->CreateSyncSession(SyncSourceInfo())); SyncSessionJob job(purpose, TimeTicks::Now(), s.Pass(), - ConfigurationParams(), FROM_HERE); + ConfigurationParams()); return DecideOnJob(job); } @@ -157,8 +157,7 @@ TEST_F(SyncSchedulerWhiteboxTest, SaveNudgeWhileTypeThrottled) { SyncSessionJob job(SyncSessionJob::NUDGE, TimeTicks::Now(), s.Pass(), - ConfigurationParams(), - FROM_HERE); + ConfigurationParams()); SyncSchedulerImpl::JobProcessDecision decision = DecideOnJob(job); // TODO(tim): This shouldn't drop. Bug 177659. EXPECT_EQ(decision, SyncSchedulerImpl::DROP); @@ -256,7 +255,7 @@ TEST_F(SyncSchedulerWhiteboxTest, ContinueCanaryJobConfig) { SyncSessionJob job(SyncSessionJob::CONFIGURATION, TimeTicks::Now(), scoped_ptr<SyncSession>(), - ConfigurationParams(), FROM_HERE); + ConfigurationParams()); job.GrantCanaryPrivilege(); SyncSchedulerImpl::JobProcessDecision decision = DecideOnJob(job); diff --git a/sync/engine/sync_session_job.cc b/sync/engine/sync_session_job.cc index f980a5c..adf898e 100644 --- a/sync/engine/sync_session_job.cc +++ b/sync/engine/sync_session_job.cc @@ -16,15 +16,13 @@ SyncSessionJob::SyncSessionJob( Purpose purpose, base::TimeTicks start, scoped_ptr<sessions::SyncSession> session, - const ConfigurationParams& config_params, - const tracked_objects::Location& from_location) + const ConfigurationParams& config_params) : purpose_(purpose), scheduled_start_(start), session_(session.Pass()), is_canary_(false), config_params_(config_params), - finished_(NOT_FINISHED), - from_location_(from_location) { + finished_(NOT_FINISHED) { } void SyncSessionJob::set_destruction_observer( @@ -90,22 +88,14 @@ scoped_ptr<SyncSessionJob> SyncSessionJob::CloneAndAbandon() { // Clone |this|, and abandon it by NULL-ing session_. return scoped_ptr<SyncSessionJob> (new SyncSessionJob( purpose_, scheduled_start_, session_.Pass(), - config_params_, from_location_)); + config_params_)); } scoped_ptr<SyncSessionJob> SyncSessionJob::Clone() const { DCHECK_GT(finished_, NOT_FINISHED); return scoped_ptr<SyncSessionJob>(new SyncSessionJob( purpose_, scheduled_start_, CloneSession().Pass(), - config_params_, from_location_)); -} - -scoped_ptr<SyncSessionJob> SyncSessionJob::CloneFromLocation( - const tracked_objects::Location& from_here) const { - DCHECK_GT(finished_, NOT_FINISHED); - return scoped_ptr<SyncSessionJob>(new SyncSessionJob( - purpose_, scheduled_start_, CloneSession().Pass(), - config_params_, from_here)); + config_params_)); } scoped_ptr<sessions::SyncSession> SyncSessionJob::CloneSession() const { @@ -138,10 +128,6 @@ sessions::SyncSession* SyncSessionJob::mutable_session() { return session_.get(); } -const tracked_objects::Location& SyncSessionJob::from_location() const { - return from_location_; -} - 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 9fe0361..64ad1c6 100644 --- a/sync/engine/sync_session_job.h +++ b/sync/engine/sync_session_job.h @@ -8,7 +8,6 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/time.h" -#include "base/tracked_objects.h" #include "sync/base/sync_export.h" #include "sync/engine/sync_scheduler.h" #include "sync/engine/syncer.h" @@ -42,8 +41,7 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { SyncSessionJob(Purpose purpose, base::TimeTicks start, scoped_ptr<sessions::SyncSession> session, - const ConfigurationParams& config_params, - const tracked_objects::Location& nudge_location); + const ConfigurationParams& config_params); ~SyncSessionJob(); // Returns a new clone of the job, with a cloned SyncSession ready to be @@ -52,8 +50,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { // to prevent bugs where multiple jobs are scheduled with the same session. // Use CloneAndAbandon if you want to clone before finishing. scoped_ptr<SyncSessionJob> Clone() const; - scoped_ptr<SyncSessionJob> CloneFromLocation( - const tracked_objects::Location& from_here) const; // Same as Clone() above, but also ejects the SyncSession from this job, // preventing it from ever being used for a sync cycle. @@ -89,7 +85,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { void set_scheduled_start(base::TimeTicks start); const sessions::SyncSession* session() const; sessions::SyncSession* mutable_session(); - const tracked_objects::Location& from_location() const; SyncerStep start_step() const; SyncerStep end_step() const; ConfigurationParams config_params() const; @@ -122,11 +117,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionJob { // all requisite steps given |purpose_| without being preempted. FinishedState finished_; - // This is the location the job came from. Used for debugging. - // In case of multiple nudges getting coalesced this stores the - // first location that came in. - tracked_objects::Location from_location_; - base::WeakPtr<DestructionObserver> destruction_observer_; DISALLOW_COPY_AND_ASSIGN(SyncSessionJob); diff --git a/sync/engine/sync_session_job_unittest.cc b/sync/engine/sync_session_job_unittest.cc index 7a3f2ba..b55cc3d 100644 --- a/sync/engine/sync_session_job_unittest.cc +++ b/sync/engine/sync_session_job_unittest.cc @@ -110,7 +110,7 @@ class SyncSessionJobTest : public testing::Test { TEST_F(SyncSessionJobTest, Clone) { SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - NewLocalSession().Pass(), ConfigurationParams(), FROM_HERE); + NewLocalSession().Pass(), ConfigurationParams()); sessions::test_util::SimulateSuccess(job1.mutable_session(), job1.start_step(), @@ -119,17 +119,10 @@ TEST_F(SyncSessionJobTest, Clone) { ModelSafeRoutingInfo new_routes; new_routes[AUTOFILL] = GROUP_PASSIVE; context()->set_routing_info(new_routes); - const tracked_objects::Location from_here1(FROM_HERE); scoped_ptr<SyncSessionJob> clone1 = job1.Clone(); - scoped_ptr<SyncSessionJob> clone1_loc = job1.CloneFromLocation(from_here1); ExpectClonesBase(&job1, clone1.get()); - ExpectClonesBase(&job1, clone1_loc.get()); EXPECT_NE(job1.session(), clone1->session()); - EXPECT_EQ(job1.from_location().ToString(), - clone1->from_location().ToString()); - EXPECT_NE(job1.session(), clone1_loc->session()); - EXPECT_EQ(from_here1.ToString(), clone1_loc->from_location().ToString()); context()->set_routing_info(routes()); clone1->GrantCanaryPrivilege(); @@ -137,34 +130,23 @@ TEST_F(SyncSessionJobTest, Clone) { clone1->start_step(), clone1->end_step()); clone1->Finish(false); - const tracked_objects::Location from_here2(FROM_HERE); scoped_ptr<SyncSessionJob> clone2 = clone1->Clone(); - scoped_ptr<SyncSessionJob> clone2_loc(clone1->CloneFromLocation(from_here2)); ExpectClonesBase(clone1.get(), clone2.get()); - ExpectClonesBase(clone1.get(), clone2_loc.get()); EXPECT_NE(clone1->session(), clone2->session()); - EXPECT_EQ(clone1->from_location().ToString(), - clone2->from_location().ToString()); EXPECT_NE(clone1->session(), clone2->session()); - 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.from_location().ToString(), - clone2->from_location().ToString()); } TEST_F(SyncSessionJobTest, CloneAfterEarlyExit) { SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - NewLocalSession().Pass(), ConfigurationParams(), FROM_HERE); + NewLocalSession().Pass(), ConfigurationParams()); job1.Finish(true); scoped_ptr<SyncSessionJob> job2 = job1.Clone(); - scoped_ptr<SyncSessionJob> job2_loc = job1.CloneFromLocation(FROM_HERE); ExpectClonesBase(&job1, job2.get()); - ExpectClonesBase(&job1, job2_loc.get()); } TEST_F(SyncSessionJobTest, CloneAndAbandon) { @@ -172,7 +154,7 @@ TEST_F(SyncSessionJobTest, CloneAndAbandon) { SyncSession* session_ptr = session.get(); SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - session.Pass(), ConfigurationParams(), FROM_HERE); + session.Pass(), ConfigurationParams()); ModelSafeRoutingInfo new_routes; new_routes[AUTOFILL] = GROUP_PASSIVE; context()->set_routing_info(new_routes); @@ -186,7 +168,7 @@ TEST_F(SyncSessionJobTest, CloneAndAbandon) { // Tests interaction between Finish and sync cycle success / failure. TEST_F(SyncSessionJobTest, Finish) { SyncSessionJob job1(SyncSessionJob::NUDGE, TimeTicks::Now(), - NewLocalSession().Pass(), ConfigurationParams(), FROM_HERE); + NewLocalSession().Pass(), ConfigurationParams()); sessions::test_util::SimulateSuccess(job1.mutable_session(), job1.start_step(), @@ -216,7 +198,7 @@ TEST_F(SyncSessionJobTest, FinishCallsReadyTask) { new SyncSession(context(), delegate(), info)); SyncSessionJob job1(SyncSessionJob::CONFIGURATION, TimeTicks::Now(), - session.Pass(), params, FROM_HERE); + session.Pass(), params); sessions::test_util::SimulateSuccess(job1.mutable_session(), job1.start_step(), job1.end_step()); |