Move download filename determination into a separate class.

Move download filename determination into a separate class.

[Relanding. It was reverted due to a memory leak.]

* Extracts download filename determination into
DownloadTargetDeterminer. The new class maintains state and observes
the download. Doing so eliminates the need to pass state around as
bound arguments.

* Guarantees that the completion callback to
DownloadManagerDelegate::DetermineDownloadTarget is always invoked,
even if the download is interrupted on initialization. This is
required for reliably resuming downloads.

* The DownloadFilePicker always returns the virtual path for downloads
to Drive. ChromeDownloadManagerDelegate can use it to keep track of
the correct last selected directory for 'Save As' downloads.

* Since no path subsubstituion is necessary during prompting,
DownloadFilePickerChromeOS is no longer necessary.

* Re-orders the sequence of events so that the user is prompted as early
as possible. Expensive history database lookups won't introduce
unnecssary jank.

* History database lookups are only performed if the results of the
lookup are necessary.

* Downloads to Drive don't need the temporary local path to be
determined twice when the default downloads directory is Drive.

BUG=151618
BUG=104335
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=197518
Reverted: https://src.chromium.org/viewvc/chrome?view=rev&revision=197578

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

[asanka, 2013-05-01 18:30:24]

Could you take a look? The only change is in tests.

Originally https://codereview.chromium.org/12850002/.

Patchset 1 is the landed CL.

[Randy Smith (Not in Mondays), 2013-05-01 18:37:20]

What was being leaked?  Got a pointer to the error?

[benjhayden, 2013-05-01 18:38:58]

lgtm

[asanka, 2013-05-01 18:55:59]

On 2013/05/01 18:37:20, rdsmith wrote:
> What was being leaked?  Got a pointer to the error?

http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28v...

During the TargetDeterminer_VisitedReferrer test, DTD would bounce off-of the
HistoryService which has it's own worker thread. During this time the 
ChromeRenderViewHostTestHarnes' message_loop_ can become idle causing the test
to exit early before the final callback is invoked and the self owned
DownloadTargetDeterminer object is deleted.

This apparently only happened on Linux and Chrome OS.

[Randy Smith (Not in Mondays), 2013-05-01 19:42:12]

lgtm

[asanka, 2013-05-01 21:39:09]

Thanks!

[commit-bot: I haz the power, 2013-05-02 03:15:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asanka@chromium.org/14773004/16001

[commit-bot: I haz the power, 2013-05-02 03:49:24]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-02 12:46:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asanka@chromium.org/14773004/38001

[commit-bot: I haz the power, 2013-05-02 13:27:54]

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-02 13:34:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asanka@chromium.org/14773004/38001

[commit-bot: I haz the power, 2013-05-02 14:57:07]

Change committed as 197920

[Nico, 2014-01-15 18:01:07]

https://chromiumcodereview.appspot.com/14773004/diff/38001/chrome/browser/dow...
File chrome/browser/download/download_target_determiner_unittest.cc (right):

https://chromiumcodereview.appspot.com/14773004/diff/38001/chrome/browser/dow...
chrome/browser/download/download_target_determiner_unittest.cc:114: const
base::FilePath::CharType* expected_virtual_path;
This appears unused. I'll send out a CL to remove it.