Remove SyncSessionJob's location member

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
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=185774

[rlarocque, 2013-02-21 17:43:26]

Please review.

[tim (not reviewing), 2013-02-21 18:06:36]

On 2013/02/21 17:43:26, rlarocque wrote:
> Please review.

I disagree that this has no impact. First, it's very useful information to have
when debugging. Second, I think the reason this has little impact is because our
scheduler logging and visibility is poor. It doesn't surprise me that there are
more things we could take advantage of (e.g from about:sync). Just removing them
seems like the wrong direction.

[rlarocque, 2013-02-25 20:46:48]

On 2013/02/21 18:06:36, timsteele wrote:
> On 2013/02/21 17:43:26, rlarocque wrote:
> > Please review.
> 
> I disagree that this has no impact. First, it's very useful information to
have
> when debugging. Second, I think the reason this has little impact is because
our
> scheduler logging and visibility is poor. It doesn't surprise me that there
are
> more things we could take advantage of (e.g from about:sync). Just removing
them
> seems like the wrong direction.

Are you sure?  No information is being discarded here.  It still prints the
location that triggered the nudge, as before.  It just doesn't need to save this
information in the job.

[rlarocque, 2013-02-28 20:58:35]

On 2013/02/25 20:46:48, rlarocque wrote:
> On 2013/02/21 18:06:36, timsteele wrote:
> > On 2013/02/21 17:43:26, rlarocque wrote:
> > > Please review.
> > 
> > I disagree that this has no impact. First, it's very useful information to
> have
> > when debugging. Second, I think the reason this has little impact is because
> our
> > scheduler logging and visibility is poor. It doesn't surprise me that there
> are
> > more things we could take advantage of (e.g from about:sync). Just removing
> them
> > seems like the wrong direction.
> 
> Are you sure?  No information is being discarded here.  It still prints the
> location that triggered the nudge, as before.  It just doesn't need to save
this
> information in the job.

ping.

[tim (not reviewing), 2013-03-01 07:55:41]

lgtm

[commit-bot: I haz the power, 2013-03-01 17:16:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/12320027/1

[commit-bot: I haz the power, 2013-03-01 17:16:13]

Failed to apply patch for sync/engine/sync_scheduler_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file sync/engine/sync_scheduler_impl.cc
  Hunk #6 FAILED at 967.
  Hunk #7 succeeded at 1077 (offset -1 lines).
  1 out of 7 hunks FAILED -- saving rejects to file
sync/engine/sync_scheduler_impl.cc.rej

Patch:       sync/engine/sync_scheduler_impl.cc
Index: sync/engine/sync_scheduler_impl.cc
diff --git a/sync/engine/sync_scheduler_impl.cc
b/sync/engine/sync_scheduler_impl.cc
index
ef6ae6d58de7a92d159f569285a6122d499c8043..b02557056286ea0dcd99a65f2dfda27fc5c930ad
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)
@@ -969,7 +967,7 @@ void SyncSchedulerImpl::HandleContinuationError(
   // This will reset the had_nudge variable as well.
   wait_interval_.reset(new WaitInterval(WaitInterval::EXPONENTIAL_BACKOFF,
                                         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.";
@@ -1080,10 +1078,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) {

[commit-bot: I haz the power, 2013-03-01 18:38:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/12320027/15001

[commit-bot: I haz the power, 2013-03-02 16:09:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/12320027/15001

[commit-bot: I haz the power, 2013-03-02 20:50:24]

Change committed as 185774