GTTF: Clean up DownloadFileManager

GTTF: Clean up DownloadFileManager

This removes a lot of duplication, locking, and thread jumping.

Most of the operations run on the FILE thread, and we do not duplicate
so much information. Each DownloadFile keeps track of its DownloadManager
(each Profile has its own DownloadManager). This allows us to remove
many maps from DownloadFileManager that were duplicating that information.

There is still SaveFileManager, but hopefully I will be able
to merge those two in small steps.

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

[darin (slow to review), 2010-08-28 06:13:32]

LGTM

http://codereview.chromium.org/3245005/diff/1/7
File chrome/browser/download/download_file_manager.cc (right):

http://codereview.chromium.org/3245005/diff/1/7#newcode38
chrome/browser/download/download_file_manager.cc:38: DownloadManager*
DownloadManagerFromRenderIds(int render_process_id,
nit: Ids -> IDs

you might also name this function DownloadManagerForRenderViewHost

[ahendrickson, 2010-08-28 17:37:35]

LGTM.

I'd like to see a comment to the effect that download managers are now tracked
by the base file instead of a map in the download file manager.

On 2010/08/28 06:13:32, darin wrote:
> LGTM
> 
> http://codereview.chromium.org/3245005/diff/1/7
> File chrome/browser/download/download_file_manager.cc (right):
> 
> http://codereview.chromium.org/3245005/diff/1/7#newcode38
> chrome/browser/download/download_file_manager.cc:38: DownloadManager*
> DownloadManagerFromRenderIds(int render_process_id,
> nit: Ids -> IDs
> 
> you might also name this function DownloadManagerForRenderViewHost

[Bernhard Bauer, 2010-08-30 12:25:07]

some smallish things below, otherwise LGTM.

http://codereview.chromium.org/3245005/diff/1/7
File chrome/browser/download/download_file_manager.cc (right):

http://codereview.chromium.org/3245005/diff/1/7#newcode81
chrome/browser/download/download_file_manager.cc:81: DownloadFile* download_file
= new DownloadFile(info, download_manager);
Could you use a scoped_ptr here...

http://codereview.chromium.org/3245005/diff/1/7#newcode95
chrome/browser/download/download_file_manager.cc:95:
downloads_[info->download_id] = download_file;
...and call release() on it here?

http://codereview.chromium.org/3245005/diff/1/7#newcode203
chrome/browser/download/download_file_manager.cc:203: ChromeThread::UI,
FROM_HERE,
Nit: I think there was a thread on chromium-dev recently about using separate
lines for function arguments, but personally I don't mind this style. Use your
judgement.