Update base/timer.h code to pass through Location from call sites.

Currently, base/timer.cc calls PostTask with FROM_HERE as the Location, so the original code that created the delayed callback is lost.

This change adds a tracked_objects::Location parameter to the APIs in base/timer.h so we can trace the PostTask callbacks. The other files are touched to add the FROM_HERE Location parameter.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99284

[jbates, 2011-08-31 20:59:53]

This is related to http://codereview.chromium.org/7778033/ (but not dependent on
it).

[darin (slow to review), 2011-08-31 21:27:38]

http://codereview.chromium.org/7812036/diff/1/base/timer.h
File base/timer.h (right):

http://codereview.chromium.org/7812036/diff/1/base/timer.h#newcode124
base/timer.h:124: void Start(TimeDelta delay,
consistency nit:  it'd be nice to place the Location parameter first, like it is
for PostTask.  that way whenever we have a parameter like this, people can just
remember that they should always type FROM_HERE first, and then list the other
parameters.

[darin (slow to review), 2011-08-31 21:28:26]

... apologies, i know my suggestion amounts to a lot of busy work :-(

[jbates, 2011-08-31 21:33:58]

On 2011/08/31 21:28:26, darin wrote:
> ... apologies, i know my suggestion amounts to a lot of busy work :-(

It's right though, I should have looked at PostTask first!

[jbates, 2011-08-31 23:01:59]

http://codereview.chromium.org/7812036/diff/1/base/timer.h
File base/timer.h (right):

http://codereview.chromium.org/7812036/diff/1/base/timer.h#newcode124
base/timer.h:124: void Start(TimeDelta delay,
On 2011/08/31 21:27:38, darin wrote:
> consistency nit:  it'd be nice to place the Location parameter first, like it
is
> for PostTask.  that way whenever we have a parameter like this, people can
just
> remember that they should always type FROM_HERE first, and then list the other
> parameters.

Done.

[darin (slow to review), 2011-08-31 23:28:46]

This change LGTM except for this one issue:

http://codereview.chromium.org/7812036/diff/3002/base/timer.h
File base/timer.h (right):

http://codereview.chromium.org/7812036/diff/3002/base/timer.h#newcode97
base/timer.h:97: tracked_objects::Location posted_from_;
Since Task extends from Tracked, and Tracked contains a Location, perhaps you
should just be calling SetBirthPlace?

however, it looks like MessageLoop::PostTask no longer uses SetBirthPlace.  it
instead stores the Location on the PendingTask object.  i think there may be
plans to make Task not inherit from Tracked.  you should probably talk to ajwong
about this.

[darin (slow to review), 2011-08-31 23:29:55]

Also, you might want to add some TRACK_ALL_TASK_OBJECTS guards since PostTask
will just drop the from_here on the floor when that is not defined.

[jbates, 2011-09-01 00:26:58]

I think we're good then, assuming tracked_objects::Tracked is deleted (filed
http://code.google.com/p/chromium/issues/detail?id=95001).

It looks like TRACK_ALL_TASK_OBJECTS isn't necessary. If I'm reading PostTask
code correctly, it always passes the posted_from parameter through to the
PendingTask constructor. In the other CL where I enable tracing of all tasks
(http://codereview.chromium.org/7778033/) I change PendingTask to always store
posted_from.

http://codereview.chromium.org/7812036/diff/3002/base/timer.h
File base/timer.h (right):

http://codereview.chromium.org/7812036/diff/3002/base/timer.h#newcode97
base/timer.h:97: tracked_objects::Location posted_from_;
On 2011/08/31 23:28:46, darin wrote:
> Since Task extends from Tracked, and Tracked contains a Location, perhaps you
> should just be calling SetBirthPlace?
> 
> however, it looks like MessageLoop::PostTask no longer uses SetBirthPlace.  it
> instead stores the Location on the PendingTask object.  i think there may be
> plans to make Task not inherit from Tracked.  you should probably talk to
ajwong
> about this.

As discussed, I'll assume Tracked will be removed and leave the posted_from_
member here.

[awong, 2011-09-01 00:37:04]

LGTM w/ nit

http://codereview.chromium.org/7812036/diff/3002/base/timer.h
File base/timer.h (right):

http://codereview.chromium.org/7812036/diff/3002/base/timer.h#newcode90
base/timer.h:90: explicit TimerTask(const tracked_objects::Location&
posted_from,
No need for explicit if you have multiple arguments.

http://codereview.chromium.org/7812036/diff/3002/base/timer.h#newcode97
base/timer.h:97: tracked_objects::Location posted_from_;
On 2011/08/31 23:28:46, darin wrote:
> Since Task extends from Tracked, and Tracked contains a Location, perhaps you
> should just be calling SetBirthPlace?
> 
> however, it looks like MessageLoop::PostTask no longer uses SetBirthPlace.  it
> instead stores the Location on the PendingTask object.  i think there may be
> plans to make Task not inherit from Tracked.  you should probably talk to
ajwong
> about this.

Yes...please don't depend on Task inheriting from Tracked.

[darin (slow to review), 2011-09-01 05:52:09]

LGTM2