Download code cleanup patch of death:

Download code cleanup patch of death:

Separate all interactions with HistoryService out of DownloadManager
to new class DownloadHistory owned by the DownloadManager.

The goal is to create more smaller classes with clearly defined
responsibilities.

TEST=unit_tests, browser_tests, ui_tests
BUG=48913
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54205

[Paweł Hajdan Jr., 2010-07-28 00:45:35]

Please review:

ahendrickson: download code
brettw: interaction with HistoryService
darin: download code
nirnimesh: AutomationProvider changes that affect pyauto

[Nirnimesh, 2010-07-28 01:07:04]

AutomationProvider changes LGTM

[brettw, 2010-07-28 16:48:40]

LGTM

http://codereview.chromium.org/3071005/diff/1/3
File chrome/browser/automation/automation_provider_observers.h (right):

http://codereview.chromium.org/3071005/diff/1/3#newcode663
chrome/browser/automation/automation_provider_observers.h:663: : public
PasswordStoreConsumer {
Nit: this should be indented 4 spaces.

http://codereview.chromium.org/3071005/diff/1/8
File chrome/browser/download/download_history.h (right):

http://codereview.chromium.org/3071005/diff/1/8#newcode66
chrome/browser/download/download_history.h:66: void RemoveEntriesBetween(const
base::Time& remove_begin,
I often tell people to just pass time by value. Since it's just a 64-bit int,
there's no penalty, and might even be faster than passing by ref. Not a big deal
though.

[ahendrickson, 2010-07-28 21:20:58]

LGTM, with a minor nit.

http://codereview.chromium.org/3071005/diff/8001/4007
File chrome/browser/download/download_history.cc (right):

http://codereview.chromium.org/3071005/diff/8001/4007#newcode13
chrome/browser/download/download_history.cc:13:
DownloadHistory::DownloadItemMapper::~DownloadItemMapper() {
Do you need this?

[Paweł Hajdan Jr., 2010-07-29 17:25:20]

Darin: ping (no problem if the review is longer due to complicated patch; just
making sure it's in your queue)

[darin (slow to review), 2010-07-29 21:46:17]

LGTM, nice refactoring!

http://codereview.chromium.org/3071005/diff/8001/4008
File chrome/browser/download/download_history.h (right):

http://codereview.chromium.org/3071005/diff/8001/4008#newcode28
chrome/browser/download/download_history.h:28: virtual ~DownloadItemMapper();
nit: i don't think there's any value in having this destructor implemented in
the .cc file.  this abstract class has no members.  the dtor is a no-op.  just
declare inline here.