base: Refactor PendingTask out of MessageLoop.

base: Refactor PendingTask out of MessageLoop.

Also removes copy/pasted instances of this class.

BUG=none
TEST=none

R=willchan@chromium.org

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

[James Hawkins, 2011-11-14 21:16:24]

cc: ajwong

[willchan no longer on Chromium, 2011-11-14 21:22:00]

Are you missing the new files?

[James Hawkins, 2011-11-14 21:27:27]

Oops, done.

[awong, 2011-11-14 21:39:31]

http://codereview.chromium.org/8565024/diff/2003/base/pending_task.h
File base/pending_task.h (right):

http://codereview.chromium.org/8565024/diff/2003/base/pending_task.h#newcode52
base/pending_task.h:52: class TaskQueue : public std::queue<PendingTask> {
I don't think you can inherit from queue<>.  The destructor is non-virtual.

what's wrong with just using the default queue.swap()?

[James Hawkins, 2011-11-14 21:45:22]

http://codereview.chromium.org/8565024/diff/2003/base/pending_task.h
File base/pending_task.h (right):

http://codereview.chromium.org/8565024/diff/2003/base/pending_task.h#newcode52
base/pending_task.h:52: class TaskQueue : public std::queue<PendingTask> {
On 2011/11/14 21:39:31, awong wrote:
> I don't think you can inherit from queue<>.  The destructor is non-virtual.
> 
> what's wrong with just using the default queue.swap()?

Caveat: This code is directly cut-pasted from MessageLoop, so the best I can
answer about inheriting from std::queue is that I've read that the fact it has a
protected section implies that inheritance is not off the table as far as the
designers of the class are concerned.

std::queue doesn't have a swap method, which is the purpose of the wrapper.

From the c++ 4.4 stl implementation:

/**
       *  'c' is the underlying container.  Maintainers wondering why
       *  this isn't uglified as per style guidelines should note that
       *  this name is specified in the standard, [23.2.3.1].  (Why?
       *  Presumably for the same reason that it's protected instead
       *  of private: to allow derivation.  But none of the other
       *  containers allow for derivation.  Odd.)
       */

[willchan no longer on Chromium, 2011-11-14 22:05:55]

http://codereview.chromium.org/8565024/diff/2004/base/pending_task.h
File base/pending_task.h (right):

http://codereview.chromium.org/8565024/diff/2004/base/pending_task.h#newcode20
base/pending_task.h:20: struct PendingTask : public base::TrackingInfo {
base:: is unnecessary throughout this file.

http://codereview.chromium.org/8565024/diff/2004/base/pending_task.h#newcode52
base/pending_task.h:52: typedef std::priority_queue<PendingTask>
DelayedTaskQueue;
Can you rename this to OrderedTaskQueue or something? DelayedTaskQueue is
specific to MessageLoop's use. Actually, perhaps you should just put this
directly in MessageLoop, since it's not used anywhere else.

[James Hawkins, 2011-11-15 22:02:48]

http://codereview.chromium.org/8565024/diff/2004/base/pending_task.h
File base/pending_task.h (right):

http://codereview.chromium.org/8565024/diff/2004/base/pending_task.h#newcode20
base/pending_task.h:20: struct PendingTask : public base::TrackingInfo {
On 2011/11/14 22:05:55, willchan wrote:
> base:: is unnecessary throughout this file.

Done.

http://codereview.chromium.org/8565024/diff/2004/base/pending_task.h#newcode52
base/pending_task.h:52: typedef std::priority_queue<PendingTask>
DelayedTaskQueue;
On 2011/11/14 22:05:55, willchan wrote:
> Can you rename this to OrderedTaskQueue or something? DelayedTaskQueue is
> specific to MessageLoop's use. Actually, perhaps you should just put this
> directly in MessageLoop, since it's not used anywhere else.

Done.

[willchan no longer on Chromium, 2011-11-15 22:08:01]

lgtm

[awong, 2011-11-15 22:27:34]

LGTM