drive: Rename JobScheduler::QueueEntry to JobEntry

drive: Rename JobScheduler::QueueEntry to JobEntry

Stop passing scoped_ptr<QueueEntry> around.
JobEntry lives in job_map_, while queue_ holds job IDs.
JobInfo is stored in JobEntry.

BUG=230308
TEST=unit_tests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=199335

[hidehiko, 2013-05-09 08:25:23]

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
File chrome/browser/chromeos/drive/job_scheduler.cc (right):

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/job_scheduler.cc:46: bool
JobScheduler::JobEntry::Compare(const JobEntry& left,
Less may be more descriptive?

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/job_scheduler.cc:404: JobID
JobScheduler::StartNewJob(scoped_ptr<JobEntry> job) {
nit: How about add DCHECK for |job| just in case?

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/job_scheduler.cc:429: if
(JobEntry::Compare(*job_entry, *job_entry2))
If I understand correctly, we should put the job after job_entry2 here?
Otherwise, the later job would be taken earlier?
(Though, previous code doesn't look taking care about it...)

[hashimoto, 2013-05-09 08:53:18]

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
File chrome/browser/chromeos/drive/job_scheduler.cc (right):

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/job_scheduler.cc:46: bool
JobScheduler::JobEntry::Compare(const JobEntry& left,
On 2013/05/09 08:25:23, hidehiko wrote:
> Less may be more descriptive?

Done.

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/job_scheduler.cc:404: JobID
JobScheduler::StartNewJob(scoped_ptr<JobEntry> job) {
On 2013/05/09 08:25:23, hidehiko wrote:
> nit: How about add DCHECK for |job| just in case?

Done.

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/job_scheduler.cc:429: if
(JobEntry::Compare(*job_entry, *job_entry2))
On 2013/05/09 08:25:23, hidehiko wrote:
> If I understand correctly, we should put the job after job_entry2 here?
> Otherwise, the later job would be taken earlier?
> (Though, previous code doesn't look taking care about it...)

This code puts the new job at just before the first job which is in lower
priority than the new job. (i.e. Highest priority and first added job is
processed first.)
JobSchedulerTest.GetResourceEntryPriority is a test to verify this.

[hidehiko, 2013-05-09 08:56:36]

lgtm

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
File chrome/browser/chromeos/drive/job_scheduler.cc (right):

https://codereview.chromium.org/14634002/diff/7001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/job_scheduler.cc:429: if
(JobEntry::Compare(*job_entry, *job_entry2))
On 2013/05/09 08:53:18, hashimoto wrote:
> On 2013/05/09 08:25:23, hidehiko wrote:
> > If I understand correctly, we should put the job after job_entry2 here?
> > Otherwise, the later job would be taken earlier?
> > (Though, previous code doesn't look taking care about it...)
> 
> This code puts the new job at just before the first job which is in lower
> priority than the new job. (i.e. Highest priority and first added job is
> processed first.)
> JobSchedulerTest.GetResourceEntryPriority is a test to verify this.

Oops. Sorry, I misread. Thank you for your explanation.

[commit-bot: I haz the power, 2013-05-09 11:46:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/14634002/12001

[commit-bot: I haz the power, 2013-05-09 12:02:32]

Retried try job too often on ios_dbg_simulator for step(s) googleurl_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-05-09 14:52:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/14634002/12001

[commit-bot: I haz the power, 2013-05-09 14:59:53]

Retried try job too often on ios_dbg_simulator for step(s) googleurl_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-05-10 02:26:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/14634002/12001

[commit-bot: I haz the power, 2013-05-10 02:31:44]

Change committed as 199335